allod secret: count only spaces and tabs as a blank template line #215

Closed
allod-agent wants to merge 0 commits from agent/credential-url-blank-line into master
Member

allod secret accepted a credential template that the other two validators refuse, and this makes all three agree. The difference was one Go call: strings.TrimSpace treats 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, plus nix flake check. If this is wrong the symptom is allod secret rotate refusing 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 in isCredentialStoreURLSource is strings.Trim(line, " \t") instead of strings.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 characters TrimSpace would 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:25 drops blank lines from the deployed credential file with awk '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:782 and nexus/scripts/refresh-local-auth:146 already 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 under TrimSpace, 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 runs go test under the secret and site,secret tags, which is where this predicate's test lives.
  • go test -tags secret ./... — every package ok.
  • gofmt -l cmd/allod/ — no output.
  • Negative control: restoring strings.TrimSpace and rerunning TestIsCredentialStoreURLSource fails 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.go is behind a //go:build secret tag, so a plain go test ./... reports the package ok without running any of it. Use the flake check, or pass the tag.

`allod secret` accepted a credential template that the other two validators refuse, and this makes all three agree. The difference was one Go call: `strings.TrimSpace` treats 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, plus `nix flake check`. If this is wrong the symptom is `allod secret rotate` refusing 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 in `isCredentialStoreURLSource` is `strings.Trim(line, " \t")` instead of `strings.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 characters `TrimSpace` would 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:25` drops blank lines from the deployed credential file with `awk '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:782` and `nexus/scripts/refresh-local-auth:146` already 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 under `TrimSpace`, 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 runs `go test` under the `secret` and `site,secret` tags, which is where this predicate's test lives. - `go test -tags secret ./...` — every package ok. - `gofmt -l cmd/allod/` — no output. - Negative control: restoring `strings.TrimSpace` and rerunning `TestIsCredentialStoreURLSource` fails 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.go` is behind a `//go:build secret` tag, so a plain `go test ./...` reports the package ok without running any of it. Use the flake check, or pass the tag.
isCredentialStoreURLSource dropped blank lines with strings.TrimSpace,
which also strips a carriage return, a vertical tab and the unicode
spaces. A template ending in a carriage-return-only line therefore passed
here while allod/archetypes and allod/nexus refused the same template,
and the credential file it would deploy has two lines, which the netrc
activation script refuses.

The blank-line test is now space and tab only. That matches the awk
default field splitting the deployed parser uses, and it matches the
other two predicates, so all three now accept the same set of templates.
The comment that claimed the old test matched the runtime parser is
replaced with what the code actually does and why.

The predicate test gains the carriage-return and vertical-tab lines as
refusals, and a trailing space-and-tab line as an acceptance so the
narrowing cannot quietly go too far.

Refs allod/tools#214
Refs allod/secrets#29
vnprc approved these changes 2026-09-18 22:29:30 +01:00
Author
Member

Landed: commit 6f6c825 with this exact change is already on master, so closing and deleting the branch.

Landed: commit 6f6c825 with this exact change is already on master, so closing and deleting the branch.
allod-agent closed this pull request 2026-09-18 23:45:26 +01:00

Pull request closed

Sign in to join this conversation.
No description provided.