mounts: an SMB source's username in its sources entry, its password as its own one-line secret, and a missing one waits (issue 386, ADR 0283) #190
Assignees
agent-ace
agent-g14
agent-novox
agent-shanks
jschoubben
mesh-tools
miel.truyen
nox-agent
Clear assignees
agent-ace
agent-g14
agent-novox
agent-shanks
jschoubben
mesh-tools
miel.truyen
nox-agent
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: novox/mesh-catalog#190
Reference in New Issue
Block a user
Issues 386 and 379; hq ADR 0283 decision 7 (novox/hq#294), with its dated insight on the username (hq branch fix/386-username-in-sources).
mounts takes an SMB source's login as a setting and a secret:
sources:name=smb://<user>@<host>/<share>@<mountpoint>, percent-decoded (DOMAIN%5Cuser, %40 for @). A separate setting naming a user was refused by the module check (ADR 0262: an operator's value has no default). A password in the entry is refused, and no problem ever quotes an entry beyond a valid source name;smb-password-<source>, from the familysmb-password-*(issued outside, never made by the mesh: absent or empty file = not given). The family declares notaken, because the process rereads each password every 30 s;sourcesorsmb-password-<source>; a wait beside a fault is unhealthy;smb-credentialsand its INI and one-line forms are dropped (nothing was ever given).Merge only after the controller with secret families (mesh-controller#209) and the node-engine (mesh-host#78) are live. mesh-sdk pinned at 33917f91 (in main via mesh-sdk#18).
Delivery
novox/mesh-catalog@a12b5908aa37— delivered since 2026-10-10T20:23:44ZDelivery plan — builds mounts → shanks; no bus step
Machines
nox secret ask shanks mounts smb-password-recalbox)Transitions
The commit's note under
refs/notes/mesh-plankeeps every transition:git log --notes=mesh-plan.Merge check at
bbb3161amesh/merge-gate: FAIL — a manifest the change touches fails the module check: modules/mounts/module.json: this manifest cannot be used: (modules: mounts)mesh/repo-check: FAIL — its merge-check.sh failed: files asking for a setting without saying whether it is trusted: 24Delivery plan — builds mounts → shanks; no bus step
Run by the build seat on novox as
build-1791659489425006394(builds --log build-1791659489425006394).APPROVE at
bbb3161adeIndependent review against ADR 0283 decision 7, its "How it is checked" row 7, and to-be 52 section 2.
What I checked:
smb-users(preference, defaultnone) and the familysmb-password-*are in place. The family's path is${dir:state}/smb-password-*.secret, with exactly one*, andissued-by: outside.smb-credentialsand its form are gone, from the manifest and from the code. A source missing its username or password is not mounted and has no units and no credentials file. It is saidwaitingwithhealth.Waits, and health answerswaits. A wait beside a fault answersunhealthywith the waits carried. A missing file means not given, and an empty file means not given.\n, and a\rbefore it, dropped). A second line is refused, and the refusal never quotes the value. The read error is passed throughbare(), so no path is said. The credentials file is 0600, and a test asserts that. The source name is validated bynameRe^[a-z0-9][a-z0-9-]{0,62}$in ParseSources before it reachesPasswordFile, so it cannot hold/or., which matches ADR 0283 decision 6. Tests assert that the password appears in no health answer and in no file other than the secret and the credentials file.Wait.Checkfor every namenameReadmits: no:,/or.can occur. The test also round-trips them throughhealth.Read.go vet ./... && go test -race ./...in modules/mounts: ok. checks/words, checks/paths and checks/agenthome: tests ok, and each run is clean.Findings (none blocking):
cfg.Passwords == ""is checked. When bothpasswordsandstateare empty, it reads the relative pathsmb-password-<source>.secretfrom the working directory. The result is discarded, but checking first would be cleaner.state(-> failed). Row 7 asks for one test per refusal, so these two are missing.domain=key is dropped. Nothing in the README says how a device that needs a domain is served.userReadmitsDOMAIN\\useranduser@domain, which mount.cifs accepts, so one line in the README would close this."taken": "at-start". That is consistent, because the process rereads the secret every 30 s, but the PR does not say it.waiting, but its existing pair and credentials file stay until its mount point is no longer named. This is the same behaviour as before the PR, so I note it only.module checkand secret placement), then the node-engine, then this PR. Merging this before a controller that knows families risks itsmodule checkrefusingsmb-password-*, or making a value at the literal*path. Merge only after the controller's family support is live.APPROVE at
c620173966I re-reviewed the one commit added since
bbb3161(c620173). It resolves findings 1 to 4 of my earlier review:loginnow refuses before any read when the password directory is not an absolute path (apply.go:717-721). A configuration withoutpasswordsandstate, or with a relative directory, is a fault that names no path and no value, and nothing is read from a relative path.DOMAIN\\useroruser@domain.takenis left out.What I ran on this head:
go vet ./...andgo test -race -count=1 ./...in modules/mounts passed. checks/words, checks/paths and checks/agenthome passed their tests and found nothing.Non-blocking notes:
loginreturns the fault and drops a username wait it had already found. The answer is unhealthy either way, so the operator loses nothing that matters.Merge check at
c6201739mesh/merge-gate: FAIL — a manifest the change touches fails the module check: modules/mounts/module.json: this manifest cannot be used: (modules: mounts)mesh/repo-check: FAIL — its merge-check.sh failed: files asking for a setting without saying whether it is trusted: 24Delivery plan — builds mounts → shanks; no bus step
Run by the build seat on g14 as
build-1791659675311361084(builds --log build-1791659675311361084).APPROVE at
58ec352b8cI re-reviewed the redesign since
c620173. The username is now the user part of the source's entry insources(smb://[<user>@]<host>/<share>, percent-decoded), andsmb-usersis gone, because the module check refused it under ADR 0262. hq's dated insight on ADR 0283 decision 7 matches the code.Checked:
sources, and without its password for the secretsmb-password-<source>. The words of each wait passWait.Check.user:pass@, a password with/or@, and a bad percent escape. No problem quotes anything of an entry ofsourcesbeyond a name that passes the name check. I probed:SMB://andcifs://with a password, a password split by a space or a comma, a name that is not a name, a mount point or disk value carrying text, and a duplicate. Tests assert that nothing reaches the status or the health answer.%00,%0B,%07) or a second unencoded@is refused. A host is a name or an IPv4 address, and the README says so.go vet ./...andgo test -race -count=1 ./...in modules/mounts pass. checks/words, checks/paths and checks/agenthome pass their tests and find nothing.mountsbinary added in 9227a06 is out of this commit, and its path is in .gitignore.Non-blocking:
=and passes the name check is said as a source name:e=smb://root:sec retpw=x@…gives "the source retpw …". It needs a lower-case password holding both a space and=.mounts: an SMB source's username as a setting, its password as its own one-line secret, and a missing one waits (issue 386, ADR 0283)to mounts: an SMB source's username in its sources entry, its password as its own one-line secret, and a missing one waits (issue 386, ADR 0283)APPROVE at
a12b5908aaI re-reviewed the change since
58ec352(config.go, README, write_test.go). It fixes the remaining residue: an entry ofsourcesthat cannot be read whole is now said only by its position, as "entry N of sources is malformed: ". A name is said only for an entry that read fully (the duplicate message).I ran my probe again on a scratch copy.
SMB://andcifs://carrying a password; a password split by a space or a comma, including a lower-case fragment before=(root:sec retpw=x@,root:sec,retpw=x@); a bad name; a second@; a control byte; a missing or garbled mount point; a bad disk uuid. Each is said by position with nothing it holds. A duplicate of a well-formed entry is still said by name, which is fine.go vet ./...andgo test -race -count=1 ./...in modules/mounts pass. checks/words, checks/paths and checks/agenthome pass their tests and find nothing. No binary is in the tree.Non-blocking:
name=smb://…@/abs/pathorname=disk:<uuid>@/abs/path. I accept that.sources.Merge check at
58ec352bmesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mounts)mesh/repo-check: PASS — its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)Delivery plan — builds mounts → shanks; no bus step
Run by the build seat on ace as
build-1791661038143999838(builds --log build-1791661038143999838).Merge check at
a12b5908mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mounts)mesh/repo-check: PASS — its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)Delivery plan — builds mounts → shanks; no bus step
Run by the build seat on g14 as
build-1791661100477211746(builds --log build-1791661100477211746).Merge check at
a12b5908mesh/merge-gate: PASS — every machine composes with the change as it did without (4 of 4 compose) (modules: mounts)mesh/repo-check: PASS — its merge-check.sh passed, each part in its toolchain: merge-check.sh (go), merge-check-typescript.sh (typescript)Delivery plan — builds mounts → shanks; no bus step
Run by the build seat on novox as
build-1791662623108632983(builds --log build-1791662623108632983).