refresh-local-auth: accept only a template as the local auth refresh source #57

Merged

vnprc merged 1 commit from agent/refresh-local-auth-template-only into master

2026-09-13 04:09:44 +01:00

allod-agent commented

2026-09-13 04:01:43 +01:00

Member

refresh-local-auth was the last consumer that still accepted the retired format = "credential-store-url" credential shape as a source for the local auth refresh. It now accepts only a value.template, the same shape allod/archetypes and allod secret already require. Nothing changes for a running machine: no registry anywhere holds a format entry, so the branch this deletes was unreachable, and the netrc bundle, its destinations, modes and ownership are untouched. What proves it is the script's own test suite, 17 tests green, including a new case that refuses a credential carrying format, plus nix flake check. If this is wrong the symptom is a refresh refusing a group it used to install, with source credential '<name>' is not a credential-store URL source; the fix is a straight revert of this commit.

Closes allod/nexus#56

What changed

scripts/refresh-local-auth: is_credential_store_url_source now has one branch. A credential is a source only when it carries no format key and its value.template renders exactly one credential-store URL line. The refusal message is unchanged.
scripts/refresh-local-auth: the comment above validate_refresh_metadata claimed the predicate mirrored the allod/archetypes and allod/tools predicates "character for character". That was false and is now accurate per consumer, with the one real divergence named. The "wholly legacy or wholly new" paragraph is retired.
tests/refresh-local-auth.sh: the legacy.git fixture group, its secret, legacy_source_installs_bundle_and_is_idempotent, and the cross-shape comparison in new_shape_source_installs_the_same_bundle are gone. One test, new_shape_source_installs_bundle_and_is_idempotent, now carries the idempotence, bundle-contents, mode, metadata and no-secret-in-output assertions on new.git. The unused bundle_digests helper goes with them.
tests/refresh-local-auth.sh: sabotage.both-shapes is renamed sabotage.format, since there are no longer two shapes for it to straddle. A new sabotage.carriage-return case is added beside it, explained under Review below.

Review

One cold cross-family reviewer, gpt-5.5 via codex, read the diff and the two reference predicates. It returned one finding, which was real and is fixed here.

The finding: the replacement comment still claimed parity with allod/tools, and the two predicates genuinely disagree. isCredentialStoreURLSource in allod/tools picks nonblank lines with Go's strings.TrimSpace, which strips a carriage return, a vertical tab and the Unicode spaces; allod/nexus and allod/archetypes use a space-and-tab-only test, deliberately, because the deployed file's parser is awk 'NF { print }' and gawk's default field splitting separates on space, tab and newline alone. Measured on the three predicates:

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

allod/nexus is the strict and correct side: a template allod/tools accepts there would deploy a two-line credential file that activation refuses. Nothing can reach a deployment through the gap, because the archetypes credential-profiles check refuses such a template as a refresh source before a registry lands. The in-scope fix was to make the comment say what is true, which it now does; allod/tools#214 tracks closing the gap in allod/tools, and this PR does not touch it.

The finding also exposed that the claim was untested here, so sabotage.carriage-return now pins it: a template whose second line holds only a carriage return must be refused.

The reviewer cleared the rest explicitly: no jq evaluation error on any non-object value, format alongside a valid template correctly refused, and the deleted legacy coverage genuinely preserved on the surviving test.

Risk

R2. The change only narrows what a predicate accepts, in a direction that fails closed, on a path whose whole output is a local netrc bundle. The worst credible failure after the checks pass is a refresh refusing a group it used to install; the blast radius is one machine's local credential files, the refusal happens before anything is decrypted or written, and recovery is a revert. It is not R1 because the path is credential handling, which earns the walk and a reviewer.

Validation

All run in the worktree at commit 4fe01da.

bash tests/refresh-local-auth.sh — 17 tests passed, including new-shape refusals install nothing, which now covers sabotage.format and sabotage.carriage-return.
nix flake check — all checks passed.
Negative control on the format clause: deleting has("format") | not from the predicate and rerunning the suite fails new-shape refusals install nothing, so sabotage.format is a real witness rather than a case that passes for another reason.
Negative control on the blank-line class: the shipped predicate refuses the sabotage.carriage-return template, and the same predicate with test("^[[:space:]]*$") substituted accepts it, so that fixture discriminates exactly the clause its comment claims.
The predicate was also exercised directly against 13 hand-written credential shapes — value absent, a string, a bad encode, zero or two {secret} placeholders, http, a path, a second line, format with and without a value — with the expected verdict in each case and no jq error.

`refresh-local-auth` was the last consumer that still accepted the retired `format = "credential-store-url"` credential shape as a source for the local auth refresh. It now accepts only a `value.template`, the same shape allod/archetypes and `allod secret` already require. Nothing changes for a running machine: no registry anywhere holds a `format` entry, so the branch this deletes was unreachable, and the netrc bundle, its destinations, modes and ownership are untouched. What proves it is the script's own test suite, 17 tests green, including a new case that refuses a credential carrying `format`, plus `nix flake check`. If this is wrong the symptom is a refresh refusing a group it used to install, with `source credential '<name>' is not a credential-store URL source`; the fix is a straight revert of this commit. Closes allod/nexus#56 ## What changed - `scripts/refresh-local-auth`: `is_credential_store_url_source` now has one branch. A credential is a source only when it carries no `format` key and its `value.template` renders exactly one credential-store URL line. The refusal message is unchanged. - `scripts/refresh-local-auth`: the comment above `validate_refresh_metadata` claimed the predicate mirrored the allod/archetypes and allod/tools predicates "character for character". That was false and is now accurate per consumer, with the one real divergence named. The "wholly legacy or wholly new" paragraph is retired. - `tests/refresh-local-auth.sh`: the `legacy.git` fixture group, its secret, `legacy_source_installs_bundle_and_is_idempotent`, and the cross-shape comparison in `new_shape_source_installs_the_same_bundle` are gone. One test, `new_shape_source_installs_bundle_and_is_idempotent`, now carries the idempotence, bundle-contents, mode, metadata and no-secret-in-output assertions on `new.git`. The unused `bundle_digests` helper goes with them. - `tests/refresh-local-auth.sh`: `sabotage.both-shapes` is renamed `sabotage.format`, since there are no longer two shapes for it to straddle. A new `sabotage.carriage-return` case is added beside it, explained under Review below. ## Review One cold cross-family reviewer, gpt-5.5 via codex, read the diff and the two reference predicates. It returned one finding, which was real and is fixed here. The finding: the replacement comment still claimed parity with allod/tools, and the two predicates genuinely disagree. `isCredentialStoreURLSource` in allod/tools picks nonblank lines with Go's `strings.TrimSpace`, which strips a carriage return, a vertical tab and the Unicode spaces; allod/nexus and allod/archetypes use a space-and-tab-only test, deliberately, because the deployed file's parser is `awk 'NF { print }'` and gawk's default field splitting separates on space, tab and newline alone. Measured on the three predicates: | 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 | allod/nexus is the strict and correct side: a template allod/tools accepts there would deploy a two-line credential file that activation refuses. Nothing can reach a deployment through the gap, because the archetypes `credential-profiles` check refuses such a template as a refresh source before a registry lands. The in-scope fix was to make the comment say what is true, which it now does; allod/tools#214 tracks closing the gap in allod/tools, and this PR does not touch it. The finding also exposed that the claim was untested here, so `sabotage.carriage-return` now pins it: a template whose second line holds only a carriage return must be refused. The reviewer cleared the rest explicitly: no jq evaluation error on any non-object `value`, `format` alongside a valid template correctly refused, and the deleted legacy coverage genuinely preserved on the surviving test. ## Risk R2. The change only narrows what a predicate accepts, in a direction that fails closed, on a path whose whole output is a local netrc bundle. The worst credible failure after the checks pass is a refresh refusing a group it used to install; the blast radius is one machine's local credential files, the refusal happens before anything is decrypted or written, and recovery is a revert. It is not R1 because the path is credential handling, which earns the walk and a reviewer. ## Validation All run in the worktree at commit 4fe01da. - `bash tests/refresh-local-auth.sh` — 17 tests passed, including `new-shape refusals install nothing`, which now covers `sabotage.format` and `sabotage.carriage-return`. - `nix flake check` — all checks passed. - Negative control on the `format` clause: deleting `has("format") | not` from the predicate and rerunning the suite fails `new-shape refusals install nothing`, so `sabotage.format` is a real witness rather than a case that passes for another reason. - Negative control on the blank-line class: the shipped predicate refuses the `sabotage.carriage-return` template, and the same predicate with `test("^[[:space:]]*$")` substituted accepts it, so that fixture discriminates exactly the clause its comment claims. - The predicate was also exercised directly against 13 hand-written credential shapes — `value` absent, a string, a bad `encode`, zero or two `{secret}` placeholders, `http`, a path, a second line, `format` with and without a `value` — with the expected verdict in each case and no jq error.

allod-agent added 1 commit

2026-09-13 04:01:43 +01:00

refresh-local-auth: accept only a template as the local auth refresh source 4fe01daaed

Narrow is_credential_store_url_source to a value.template that renders one
credential-store line, and refuse any credential carrying a format key at
all. The branch accepting format = "credential-store-url" with no value is
gone, along with the legacy.git fixtures and the cross-shape comparison
test that existed to witness it; the idempotence, bundle-contents, mode and
metadata assertions those carried now run on the new.git group.

The comment claiming parity with allod/archetypes and allod/tools character
for character now states what is actually true of each: archetypes matches
clause for clause, while allod/tools drops blank lines with Go's TrimSpace
and so accepts a template ending in a carriage-return-only line that this
refuses. A new sabotage.carriage-return case pins that class here.

Refs allod/nexus#56
Refs allod/tools#214