secret rotate: match the other credential-store URL predicates on which lines count as blank #214

Closed
opened 2026-09-13 03:58:22 +01:00 by allod-agent · 1 comment
Member

Bring isCredentialStoreURLSource's blank-line test up to the same space-and-tab-only standard the other two credential-store URL predicates use, so all three consumers accept exactly the same set of templates.

Primary goals:

  • One blank-line class — the nonblank-line filter drops a line only when it is empty or holds spaces and tabs, matching allod/archetypes and allod/nexus, instead of Go's strings.TrimSpace, which also strips a carriage return, a vertical tab, a form feed, and the Unicode spaces.
  • Agree with the parser that consumes the file — the deployed netrc activation splits with awk 'NF { print }', whose default field splitting separates on space, tab and newline alone, so a carriage-return-only line survives there and must not be dropped when validating.
  • A witness per class — a test that pins a template ending in a carriage-return-only line as refused, beside the space-only line that stays accepted.

Current state and specifics

cmd/allod/secret_rotate.go:213-219 builds its nonblank-line list with strings.TrimSpace(line) != "". strings.TrimSpace trims every rune unicode.IsSpace accepts, which includes \r, \v, \f, U+0085 and U+00A0. The other two predicates use a space-and-tab-only test: allod/archetypes flake.nix:780-784 filters on builtins.match "^[ \t]*$", and allod/nexus scripts/refresh-local-auth filters on test("^[ \t]*$") in its jq program. Both carry a comment naming the awk parser as the reason.

The divergence is observable. For the template https://alice:{secret}@forge.test followed by a newline and a lone carriage return, the three predicates split:

Template tail allod/tools allod/archetypes allod/nexus
A carriage-return-only line accepts refuses refuses
A vertical-tab-only line accepts refuses refuses
A space-only line accepts accepts accepts

archetypes/modules/netrc.nix:25 is the parser that settles which reading is right: it runs awk 'NF { print }' over the deployed credential file, gawk's default field splitting separates on space, tab and newline alone, so a carriage-return-only line has NF == 1 and is kept. A template allod secret rotate accepts today would therefore deploy a two-line credential file, and activation refuses it. The strict reading is the correct one, and allod/tools is the loose consumer.

Nothing can reach a deployment through this gap: the archetypes credential-profiles check refuses such a template as a refresh source before a registry lands, so this is a consistency defect between validators rather than a live exposure. It surfaced in review of the allod/nexus change for allod/nexus#56, where a comment claiming all three predicates match character for character had to be narrowed to archetypes alone.

Default: replace the strings.TrimSpace(line) != "" test with an explicit strings.Trim(line, " \t") != "", and add the carriage-return case to the existing predicate tests in cmd/allod/secret_value_test.go. Validation: go test ./... in allod/tools.

Scope

In: the blank-line test inside isCredentialStoreURLSource and its tests, in allod/tools. Out: the regexp that validates the surviving line and the {secret} placeholder count, which already agree across all three; and the allod/nexus and allod/archetypes predicates, which are the standard being matched.

Refs allod/nexus#56.

Bring `isCredentialStoreURLSource`'s blank-line test up to the same space-and-tab-only standard the other two credential-store URL predicates use, so all three consumers accept exactly the same set of templates. Primary goals: - **One blank-line class** — the nonblank-line filter drops a line only when it is empty or holds spaces and tabs, matching allod/archetypes and allod/nexus, instead of Go's `strings.TrimSpace`, which also strips a carriage return, a vertical tab, a form feed, and the Unicode spaces. - **Agree with the parser that consumes the file** — the deployed netrc activation splits with `awk 'NF { print }'`, whose default field splitting separates on space, tab and newline alone, so a carriage-return-only line survives there and must not be dropped when validating. - **A witness per class** — a test that pins a template ending in a carriage-return-only line as refused, beside the space-only line that stays accepted. ### Current state and specifics `cmd/allod/secret_rotate.go:213-219` builds its nonblank-line list with `strings.TrimSpace(line) != ""`. `strings.TrimSpace` trims every rune `unicode.IsSpace` accepts, which includes `\r`, `\v`, `\f`, U+0085 and U+00A0. The other two predicates use a space-and-tab-only test: allod/archetypes `flake.nix:780-784` filters on `builtins.match "^[ \t]*$"`, and allod/nexus `scripts/refresh-local-auth` filters on `test("^[ \t]*$")` in its jq program. Both carry a comment naming the awk parser as the reason. The divergence is observable. For the template `https://alice:{secret}@forge.test` followed by a newline and a lone carriage return, the three predicates split: | Template tail | allod/tools | allod/archetypes | allod/nexus | |---|---|---|---| | A carriage-return-only line | accepts | refuses | refuses | | A vertical-tab-only line | accepts | refuses | refuses | | A space-only line | accepts | accepts | accepts | `archetypes/modules/netrc.nix:25` is the parser that settles which reading is right: it runs `awk 'NF { print }'` over the deployed credential file, gawk's default field splitting separates on space, tab and newline alone, so a carriage-return-only line has `NF == 1` and is kept. A template `allod secret rotate` accepts today would therefore deploy a two-line credential file, and activation refuses it. The strict reading is the correct one, and `allod/tools` is the loose consumer. Nothing can reach a deployment through this gap: the archetypes `credential-profiles` check refuses such a template as a refresh source before a registry lands, so this is a consistency defect between validators rather than a live exposure. It surfaced in review of the allod/nexus change for allod/nexus#56, where a comment claiming all three predicates match character for character had to be narrowed to archetypes alone. Default: replace the `strings.TrimSpace(line) != ""` test with an explicit `strings.Trim(line, " \t") != ""`, and add the carriage-return case to the existing predicate tests in `cmd/allod/secret_value_test.go`. Validation: `go test ./...` in allod/tools. ### Scope In: the blank-line test inside `isCredentialStoreURLSource` and its tests, in allod/tools. Out: the regexp that validates the surviving line and the `{secret}` placeholder count, which already agree across all three; and the allod/nexus and allod/archetypes predicates, which are the standard being matched. Refs allod/nexus#56.
Author
Member

Context from a wider look at this predicate, filed as allod/secrets#29: the same https://user:token@host grammar is written out five times in four languages across allod/archetypes, allod/nexus and allod/tools, and the blank-line rule this issue is about is written separately alongside each one. That issue proposes one exported definition the consumers read, so a divergence like this one becomes impossible rather than merely caught.

This issue should still land first, and allod/secrets#29 says so in its scope. Two reasons. It is a one-line behaviour fix against an inconsistency that exists today, whereas the unification is unscheduled work. And unifying three implementations that disagree would force a decision about which behaviour wins in the middle of a structural change, which is exactly the kind of mixing that makes a refactor hard to review. Getting all three to agree first means the cutover can assert equivalence instead of choosing.

One part of this issue is affected. The fix here says to add the carriage-return case to the predicate tests in cmd/allod/secret_value_test.go. If the shared accept-and-reject vector table in allod/secrets#29 lands, that case belongs in the shared table instead, so it runs against every implementation rather than only this one. That is a question of where the test lives, not whether it is written, and it does not block anything here.

Context from a wider look at this predicate, filed as allod/secrets#29: the same `https://user:token@host` grammar is written out five times in four languages across allod/archetypes, allod/nexus and allod/tools, and the blank-line rule this issue is about is written separately alongside each one. That issue proposes one exported definition the consumers read, so a divergence like this one becomes impossible rather than merely caught. This issue should still land first, and allod/secrets#29 says so in its scope. Two reasons. It is a one-line behaviour fix against an inconsistency that exists today, whereas the unification is unscheduled work. And unifying three implementations that disagree would force a decision about which behaviour wins in the middle of a structural change, which is exactly the kind of mixing that makes a refactor hard to review. Getting all three to agree first means the cutover can assert equivalence instead of choosing. One part of this issue is affected. The fix here says to add the carriage-return case to the predicate tests in `cmd/allod/secret_value_test.go`. If the shared accept-and-reject vector table in allod/secrets#29 lands, that case belongs in the shared table instead, so it runs against every implementation rather than only this one. That is a question of where the test lives, not whether it is written, and it does not block anything here.
vnprc closed this issue 2026-09-18 22:29:36 +01:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
allod/tools#214
No description provided.