mounts: name a malformed entry of sources by its position, never by its name

A password holding '=' and a space or comma splits the entry, and the tail
read as name=... was said as "the source <tail>", carrying part of the
secret into the health answer and the operator's conditions (review of
mesh-catalog#190, issue 386).

modules/mounts/README.md

@@ -70,8 +70,8 @@ Both are preferences whose default, `none`, names nothing (ADR 0262 allows no em
   source's username is in its entry: it is not a secret (novox/hq ADR 0283). It is percent-decoded, so a device that needs a domain takes it as
   `WORKGROUP%5Cuser` (for `WORKGROUP\user`), and an `@` in it is written `%40`; it holds no space, comma or
   `=`. A password never goes in the entry: `user:password@` is refused, saying so and never what was
-  written. An entry that cannot be read is said by its source's name, or by its position when even the name
-  cannot be read, and never with what it holds.
+  written. An entry that cannot be read whole is said as malformed by its position alone ("entry 2 of sources
+  is malformed: …"), never with its name or anything else it holds.
 
 A mount point is an absolute, plainly written path, not a directory of the system's own (`/etc`, `/usr`,
 `/run`, …). An entry that cannot be read is said, and only that entry is left out.

modules/mounts/cmd/mounts/config.go

@@ -163,24 +163,24 @@ func ParseSources(v string) ([]Source, []string) {
 	var out []Source
 	var problems []string
 	seen := map[string]bool{}
-	// **Nothing an entry holds is said back but a name that reads as one**: an entry may carry a password
-	// where the user goes, in any scheme, and a password with a space or a comma splits it in two. An entry
-	// is named by its position otherwise.
+	// **Nothing an entry holds is said back until the whole entry has read**: an entry may carry a password
+	// where the user goes, in any scheme, and a password with a space or a comma splits it in two, whose
+	// tail may itself look like name=…. A malformed entry is named by its position alone.
 	for i, e := range entries(v) {
 		name, rest, ok := strings.Cut(e, "=")
 		if !ok || !nameRe.MatchString(name) {
-			problems = append(problems, fmt.Sprintf("entry %d of sources is not name=<where>@<mountpoint>, its name lower-case letters, digits and dashes", i+1))
+			problems = append(problems, malformed(i, "it is not name=<where>@<mountpoint>, its name lower-case letters, digits and dashes"))
 			continue
 		}
 		at := strings.LastIndex(rest, "@")
 		if at < 0 {
-			problems = append(problems, fmt.Sprintf("the source %s names no mount point (…@<mountpoint>)", name))
+			problems = append(problems, malformed(i, "it names no mount point (…@<mountpoint>)"))
 			continue
 		}
 		where, pointRO := rest[:at], rest[at+1:]
 		point, ro := readOnly(pointRO)
 		if CheckMountpoint(point) != nil {
-			problems = append(problems, fmt.Sprintf("the source %s: its mount point is not an absolute, plainly written path outside the system's own directories", name))
+			problems = append(problems, malformed(i, "its mount point is not an absolute, plainly written path outside the system's own directories"))
 			continue
 		}
 		s := Source{Name: name, Mountpoint: point, ReadOnly: ro}
@@ -188,19 +188,19 @@ func ParseSources(v string) ([]Source, []string) {
 		case strings.HasPrefix(where, "smb://"):
 			user, host, share, problem := parseSMB(strings.TrimPrefix(where, "smb://"))
 			if problem != "" {
-				problems = append(problems, fmt.Sprintf("the source %s: %s", name, problem))
+				problems = append(problems, malformed(i, problem))
 				continue
 			}
 			s.Kind, s.User, s.Host, s.Share = "smb", user, host, share
 		case strings.HasPrefix(where, "disk:"):
 			uuid := strings.TrimPrefix(where, "disk:")
 			if !uuidRe.MatchString(uuid) {
-				problems = append(problems, fmt.Sprintf("the source %s is not disk:<filesystem uuid>", name))
+				problems = append(problems, malformed(i, "it is not disk:<filesystem uuid>"))
 				continue
 			}
 			s.Kind, s.UUID = "disk", uuid
 		default:
-			problems = append(problems, fmt.Sprintf("the source %s is neither smb://[<user>@]<host>/<share> nor disk:<uuid>", name))
+			problems = append(problems, malformed(i, "it is neither smb://[<user>@]<host>/<share> nor disk:<uuid>"))
 			continue
 		}
 		if seen[name] || seen["@"+point] {
@@ -213,6 +213,11 @@ func ParseSources(v string) ([]Source, []string) {
 	return out, problems
 }
 
+// malformed is a problem with the i-th entry of sources, by its position alone: nothing it holds is said.
+func malformed(i int, why string) string {
+	return fmt.Sprintf("entry %d of sources is malformed: %s", i+1, why)
+}
+
 // ParseConfig reads the rendered file: `key=value` lines.
 func ParseConfig(text string) Config {
 	kv := map[string]string{}
@@ -301,7 +306,7 @@ func parseSMB(body string) (user, host, share, problem string) {
 		body = body[at+1:]
 		switch {
 		case strings.Contains(raw, ":"):
-			return "", "", "", "its entry carries a password in the URL; a password is given as the secret smb-password-<source>, never in a setting"
+			return "", "", "", "it carries a password in the URL; a password is given as the secret smb-password-<source>, never in a setting"
 		case strings.Contains(raw, "@"):
 			return "", "", "", "its user holds an @; write it %40"
 		}

modules/mounts/cmd/mounts/write_test.go

@@ -490,10 +490,15 @@ func TestAnSMBSourcesUserIsInItsEntryAndAPasswordThereIsRefusedUnsaid(t *testing
 		"e=SMB://root:" + secret + "@nas/Media@/mnt/e",
 		"e=cifs://root:" + secret + "@nas/Media@/mnt/e",
 		"SECRET" + "PW=smb://nas/Media@/mnt/e",
+		// A password holding '=' and a space, or '=' and a comma: the tail reads like name=… and is still not said.
+		"e=smb://root:sec RETPW=x@nas/Media@/mnt/e",
+		"e=smb://root:sec,RETPW=x@nas/Media@/mnt/e",
+		"e=smb://root:SEC RETPW=smb://nas/Media@/mnt/e",
 	} {
 		_, problems := ParseSources(v)
 		said := strings.Join(problems, " ")
-		if len(problems) == 0 || strings.Contains(said, "RETPW") || strings.Contains(said, "SEC") {
+		if len(problems) == 0 || strings.Contains(strings.ToUpper(said), "RETPW") || strings.Contains(said, "SEC") ||
+			strings.Contains(said, "sec") && !strings.Contains(said, "secret smb-password") {
 			t.Errorf("%q says %q", strings.ReplaceAll(v, "RETPW", "<pw>"), said)
 		}
 		// And nothing of it reaches the status or the health answer.
@@ -502,7 +507,7 @@ func TestAnSMBSourcesUserIsInItsEntryAndAPasswordThereIsRefusedUnsaid(t *testing
 		now := time.Now()
 		f.applier(now).Look(context.Background())
 		raw, _ := json.Marshal(f.machine().Health(now))
-		if strings.Contains(string(raw), "RETPW") || strings.Contains(f.everything("mounts.conf"), "RETPW") {
+		if strings.Contains(strings.ToUpper(string(raw)), "RETPW") || strings.Contains(strings.ToUpper(f.everything("mounts.conf")), "RETPW") {
 			t.Errorf("%q leaked into the status or the health: %s", strings.ReplaceAll(v, "RETPW", "<pw>"), raw)
 		}
 	}