allod secret: count only spaces and tabs as a blank template line #215
No reviewers
Labels
No labels
blocked
bug
decision
duplicate
enhancement
help wanted
invalid
landed?
question
ready-to-merge
stale
wontfix
bug
duplicate
enhancement
help wanted
invalid
question
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
allod/tools!215
Loading…
Reference in a new issue
No description provided.
Delete branch "agent/credential-url-blank-line"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
allod secretaccepted a credential template that the other two validators refuse, and this makes all three agree. The difference was one Go call:strings.TrimSpacetreats a carriage return as blank, so a template ending in a carriage-return-only line passed here, while allod/archetypes and allod/nexus counted that line and refused it. The strict reading is the correct one, because the file such a template deploys has two lines and the netrc activation script refuses it. Nothing changes for a running machine: no registry holds such a template, and the archetypes check would have refused one before it could land, so this is a consistency fix rather than a repair of anything live. What proves it is the predicate's own test, which now pins the carriage-return and vertical-tab lines as refusals and a trailing space-and-tab line as still accepted, plusnix flake check. If this is wrong the symptom isallod secret rotaterefusing a credential it used to accept, naming it as not a credential-store URL source; the fix is a straight revert.Closes allod/tools#214
What changed
cmd/allod/secret_rotate.go: the blank-line filter inisCredentialStoreURLSourceisstrings.Trim(line, " \t")instead ofstrings.TrimSpace(line). Nothing else in the predicate moves.cmd/allod/secret_rotate.go: the comment claiming the old filter matched the runtime netrc parser is replaced. It now states the rule, names the parser behaviour it follows, and says which charactersTrimSpacewould have wrongly dropped.cmd/allod/secret_value_test.go: two refusals added, a template ending in a carriage-return-only line and one ending in a vertical-tab-only line. One acceptance added, a template ending in a line of spaces and one of tabs, so the narrowing is pinned from both sides.Why this direction
archetypes/modules/netrc.nix:25drops blank lines from the deployed credential file withawk 'NF { print }', and gawk's default field splitting separates on space, tab and newline alone. A line holding only a carriage return therefore survives there and is counted, so the file has two lines and activation exits 1. The validator has to count that line too, or it accepts a template that cannot deploy.archetypes/flake.nix:782andnexus/scripts/refresh-local-auth:146already make the space-and-tab-only test for this reason; this was the one consumer that did not.The change is strictly narrowing. The set of blank lines under
Trim(line, " \t")is a subset of the set underTrimSpace, so the set of non-blank lines only grows, and a template that was refused before cannot become accepted. There is no input that this newly lets through.Risk
R1. One predicate, one line, in a direction that only ever refuses more. It is caller-level tested with a negative control, it touches no secret material, no deployed state and no cross-repo ordering, and rollback is a straight revert of one commit. The credential subject matter is a reason to walk the failure rather than a score on its own: the worst credible outcome after the checks pass is a rotation refusing a valid credential at the point of use, before anything is written.
Validation
All run in the worktree at commit
6f6c825.nix flake check— all checks passed. This is the gate that matters, because it runsgo testunder thesecretandsite,secrettags, which is where this predicate's test lives.go test -tags secret ./...— every package ok.gofmt -l cmd/allod/— no output.strings.TrimSpaceand rerunningTestIsCredentialStoreURLSourcefails on exactly the two new refusal cases, so the added tests are real witnesses for this change rather than cases that pass for another reason.Note for anyone adding to this test in future:
cmd/allod/secret_value_test.gois behind a//go:build secrettag, so a plaingo test ./...reports the package ok without running any of it. Use the flake check, or pass the tag.Landed: commit
6f6c825with this exact change is already on master, so closing and deleting the branch.Pull request closed