secret rotate: match the other credential-store URL predicates on which lines count as blank #214
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
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
allod/tools#214
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
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?
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:
strings.TrimSpace, which also strips a carriage return, a vertical tab, a form feed, and the Unicode spaces.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.Current state and specifics
cmd/allod/secret_rotate.go:213-219builds its nonblank-line list withstrings.TrimSpace(line) != "".strings.TrimSpacetrims every runeunicode.IsSpaceaccepts, which includes\r,\v,\f, U+0085 and U+00A0. The other two predicates use a space-and-tab-only test: allod/archetypesflake.nix:780-784filters onbuiltins.match "^[ \t]*$", and allod/nexusscripts/refresh-local-authfilters ontest("^[ \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.testfollowed by a newline and a lone carriage return, the three predicates split:archetypes/modules/netrc.nix:25is the parser that settles which reading is right: it runsawk '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 hasNF == 1and is kept. A templateallod secret rotateaccepts today would therefore deploy a two-line credential file, and activation refuses it. The strict reading is the correct one, andallod/toolsis the loose consumer.Nothing can reach a deployment through this gap: the archetypes
credential-profilescheck 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 explicitstrings.Trim(line, " \t") != "", and add the carriage-return case to the existing predicate tests incmd/allod/secret_value_test.go. Validation:go test ./...in allod/tools.Scope
In: the blank-line test inside
isCredentialStoreURLSourceand 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.
Context from a wider look at this predicate, filed as allod/secrets#29: the same
https://user:token@hostgrammar 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.