allod secret declare: write a credential's non-secret half in canonical shape #206

Merged
vnprc merged 5 commits from agent/secret-declare into master 2026-09-11 21:34:19 +01:00
Member

allod secret declare <name> ... writes a new credential's non-secret half into a secrets checkout in the canonical shape: the credentials.nix entry in rotation_state = "pending", the secrets.nix recipient line, and the rotation registry group. It exists on every machine, dev VMs included, because it writes no ciphertext and reads no identity; create and rekey stay behind the secret tag and remain unknown words untagged. The command never edits an existing declaration: any match for the name refuses with what was found. Nothing changes for a running machine until an allod-tools pin moves. The proof is a fixture-checkout test suite pinning the exact inserted text and every refusal. If the output is wrong, the diff it produces is reviewed in a PR before any ciphertext exists, and reverting that PR undoes it.

What changes

  • The secret namespace dispatcher moves to an untagged file, following the site pattern: declare is registered untagged, create and rekey register themselves from the tagged file. Untagged builds list only declare; the absent-tag test now pins that shape. The flake comment about the tag is corrected.
  • allod secret declare <name> [<checkout>] --kind <kind> --owner <owner> --to <machine>[,<machine>...] --format <format> --deployed-path <path> --verify <probe-type> [--verify-repo-url <url>] [--verify-context <text>] [--verify-user <user>] [--service none|forgejo] [--account <a>] [--ui-token-name <n>] [--strategy overlap|in-place]. --to is comma-separated and repeatable. Every enum is validated against the lists rotate-token's validator accepts; forgejo requires the account and UI token name and none refuses them. --kind accepts only user, agent, and service: machine-host and forge-git entries are generated by the template and carry a public key and consumers this shape cannot express. git-ls-remote requires --verify-repo-url and forge-token-verify requires --verify-user, because those are the fields rotate-token's printed verification commands substitute.
  • Each target's kind is derived, not typed: nix eval of the inventory flake's machines.<name>.type, mapped dev→dev-vm, privacy→privacy-vm, hypervisor→nixos-host, service→service-vm. A machine the inventory does not know, or an unmapped type, is refused. The inventory checkout comes from the repository registry, or INVENTORY.
  • The three writes are textual insertions, one block per file, so the reviewed diff is three hunks: the entry before the attrset's closing brace in the agent-pr-token layout, the recipient line [ hostKey ] ++ vmKeys "a" ++ vmKeys "b" after the last such line, and the JSON group before the final brace with a comma added. The built text is validated before writing (brace balance for Nix, json.Valid for JSON); each file is replaced by temp-file-and-rename; a file that changed since it was read is refused, and files already replaced are restored on a later failure.
  • Values that reach Nix or JSON text are validated first: the name is ^[a-z][a-z0-9-]*$ (an unquoted Nix attribute and a filename), identifiers ^[A-Za-z0-9][A-Za-z0-9_.@-]*$, the deployed path absolute with no whitespace, control character, quote, backslash, or $; JSON strings go through the encoder.
  • Collision checks: the credentials.nix attribute and the secrets.nix path as text, matched across whitespace, line breaks, and line comments; a registry group keyed by the name, and any group whose credentials[] names the credential or its secret path; and the evaluated credential inventory (nix eval of the checkout's lib.credentials), because the template's credentials.nix merges generated entries the text scan cannot see. A failed evaluation refuses rather than trusting the text checks alone. Nix keywords are refused as names.
  • Docs: a declare section in docs/allod-secret.md ahead of the landing half, describing the agent's step before the human runs create.

Risk

R2. Writes three files in a checkout, never commits, and produces a diff the inventory check validates before any ciphertext exists. Worst credible failure after the tests pass is inserted text the template's flake rejects, which fails the PR's checks rather than landing. Rollback is a revert.

Validation

  • gofmt -l . empty; go vet and go test ./... green untagged and with site, secret, and site,secret, run by the overseer in the branch worktree after the final commit.
  • Tests: golden output for one and two machines and for none and forgejo groups; every collision variant, including declarations split across lines, refuses and leaves all three files byte-identical; each missing or invalid flag names the flag or accepted values; injection attempts through every string flag refuse; a leading-digit name refuses; an unknown machine and an unmapped type refuse; the sabotaged registry refuses and restores; a file edited underneath refuses and restores the file already replaced; atomic replacement is unit-tested.
  • Cross-family review, two passes. Two reviewers on the first two commits found seven defects (unescaped values, the name pattern, both collision regexes, truncating writes, lost concurrent edits, docs claiming a re-read), fixed in the third commit. A second pass found five more (forge-git and machine-host accepted as kinds, verify fields missing for the printed probes, generated inventory entries invisible to the text scan, Nix keywords as names, comments defeating the collision regexes), fixed in the fourth commit and read by the overseer. Two race findings between the re-read and the rename were discarded as inherent without file locking, and one finding that the JSON guard test does not run through the command was discarded because a registry that parses cannot become invalid by insertion after its last token; the end-to-end sabotage test covers the command path.
  • The packaged checks.x86_64-linux.go-checks derivation builds green on the final commit.

Refs allod/tools#182
Closes allod/tools#196

`allod secret declare <name> ...` writes a new credential's non-secret half into a secrets checkout in the canonical shape: the `credentials.nix` entry in `rotation_state = "pending"`, the `secrets.nix` recipient line, and the rotation registry group. It exists on every machine, dev VMs included, because it writes no ciphertext and reads no identity; `create` and `rekey` stay behind the `secret` tag and remain unknown words untagged. The command never edits an existing declaration: any match for the name refuses with what was found. Nothing changes for a running machine until an `allod-tools` pin moves. The proof is a fixture-checkout test suite pinning the exact inserted text and every refusal. If the output is wrong, the diff it produces is reviewed in a PR before any ciphertext exists, and reverting that PR undoes it. ## What changes - The `secret` namespace dispatcher moves to an untagged file, following the `site` pattern: `declare` is registered untagged, `create` and `rekey` register themselves from the tagged file. Untagged builds list only `declare`; the absent-tag test now pins that shape. The flake comment about the tag is corrected. - `allod secret declare <name> [<checkout>] --kind <kind> --owner <owner> --to <machine>[,<machine>...] --format <format> --deployed-path <path> --verify <probe-type> [--verify-repo-url <url>] [--verify-context <text>] [--verify-user <user>] [--service none|forgejo] [--account <a>] [--ui-token-name <n>] [--strategy overlap|in-place]`. `--to` is comma-separated and repeatable. Every enum is validated against the lists `rotate-token`'s validator accepts; `forgejo` requires the account and UI token name and `none` refuses them. `--kind` accepts only `user`, `agent`, and `service`: `machine-host` and `forge-git` entries are generated by the template and carry a public key and consumers this shape cannot express. `git-ls-remote` requires `--verify-repo-url` and `forge-token-verify` requires `--verify-user`, because those are the fields `rotate-token`'s printed verification commands substitute. - Each target's `kind` is derived, not typed: `nix eval` of the inventory flake's `machines.<name>.type`, mapped `dev`→`dev-vm`, `privacy`→`privacy-vm`, `hypervisor`→`nixos-host`, `service`→`service-vm`. A machine the inventory does not know, or an unmapped type, is refused. The inventory checkout comes from the repository registry, or `INVENTORY`. - The three writes are textual insertions, one block per file, so the reviewed diff is three hunks: the entry before the attrset's closing brace in the `agent-pr-token` layout, the recipient line `[ hostKey ] ++ vmKeys "a" ++ vmKeys "b"` after the last such line, and the JSON group before the final brace with a comma added. The built text is validated before writing (brace balance for Nix, `json.Valid` for JSON); each file is replaced by temp-file-and-rename; a file that changed since it was read is refused, and files already replaced are restored on a later failure. - Values that reach Nix or JSON text are validated first: the name is `^[a-z][a-z0-9-]*$` (an unquoted Nix attribute and a filename), identifiers `^[A-Za-z0-9][A-Za-z0-9_.@-]*$`, the deployed path absolute with no whitespace, control character, quote, backslash, or `$`; JSON strings go through the encoder. - Collision checks: the `credentials.nix` attribute and the `secrets.nix` path as text, matched across whitespace, line breaks, and line comments; a registry group keyed by the name, and any group whose `credentials[]` names the credential or its secret path; and the evaluated credential inventory (`nix eval` of the checkout's `lib.credentials`), because the template's credentials.nix merges generated entries the text scan cannot see. A failed evaluation refuses rather than trusting the text checks alone. Nix keywords are refused as names. - Docs: a `declare` section in `docs/allod-secret.md` ahead of the landing half, describing the agent's step before the human runs `create`. ## Risk R2. Writes three files in a checkout, never commits, and produces a diff the inventory check validates before any ciphertext exists. Worst credible failure after the tests pass is inserted text the template's flake rejects, which fails the PR's checks rather than landing. Rollback is a revert. ## Validation - `gofmt -l .` empty; `go vet` and `go test ./...` green untagged and with `site`, `secret`, and `site,secret`, run by the overseer in the branch worktree after the final commit. - Tests: golden output for one and two machines and for `none` and `forgejo` groups; every collision variant, including declarations split across lines, refuses and leaves all three files byte-identical; each missing or invalid flag names the flag or accepted values; injection attempts through every string flag refuse; a leading-digit name refuses; an unknown machine and an unmapped type refuse; the sabotaged registry refuses and restores; a file edited underneath refuses and restores the file already replaced; atomic replacement is unit-tested. - Cross-family review, two passes. Two reviewers on the first two commits found seven defects (unescaped values, the name pattern, both collision regexes, truncating writes, lost concurrent edits, docs claiming a re-read), fixed in the third commit. A second pass found five more (`forge-git` and `machine-host` accepted as kinds, verify fields missing for the printed probes, generated inventory entries invisible to the text scan, Nix keywords as names, comments defeating the collision regexes), fixed in the fourth commit and read by the overseer. Two race findings between the re-read and the rename were discarded as inherent without file locking, and one finding that the JSON guard test does not run through the command was discarded because a registry that parses cannot become invalid by insertion after its last token; the end-to-end sabotage test covers the command path. - The packaged `checks.x86_64-linux.go-checks` derivation builds green on the final commit. Refs allod/tools#182 Closes allod/tools#196
Give agents a command that generates the credentials.nix entry, the
secrets.nix recipient line, and the forgejo-token-groups.json rotation
registry group for a new credential, so that PR is generated rather than
hand-edited across three files from the secrets template's README. declare
writes no ciphertext, reads no identity, and never commits: it checks all
three files for a name collision before touching any of them, writes each
insertion as one contiguous text block, and re-reads what it wrote to
confirm it still parses, restoring on the first failure.

The command is untagged, unlike create and rekey, so it exists on every
machine that can check out the secrets repository. The namespace is
restructured the way site_common.go/site_preview.go/site.go already split:
secret_common.go carries the shared command table, dispatcher, and checkout
resolution (moved out of secret.go); secret_declare.go registers the
namespace and the untagged declare command; secret.go, behind the secret
tag, now only adds create and rekey to the same table.

Target machine kind (nixos-host/dev-vm/privacy-vm) is derived per --to
machine rather than typed, to keep it out of an agent's hands. nexus is
the host and gets nixos-host; every other name is looked up in the
inventory's vm-specs.json. That file's guest entries carry no type field in
the current public template (it is filtered out on the way from the
inventory flake to JSON), so declare falls back to the one remaining
distinguishing signal: a non-empty repos list means dev-vm, an empty one
means privacy-vm. This is a real limitation of the committed template, not
a design choice; the worker report on the corresponding issue has the
detail for the overseer to weigh.

Refs allod/tools#196
Replace the vm-specs.json repos-count heuristic with the real signal: nix
eval machines.<name>.type against the inventory checkout, the same 'machines'
output archetypes consumes, and map dev/privacy/hypervisor/service to
dev-vm/privacy-vm/nixos-host/service-vm. Drop the literal nexus special
case now that nexus reaches nixos-host through its own type (hypervisor)
like every other machine; the secrets.nix vmKeys-skip for a host target now
reads each target's derived kind instead of a literal name match.

nixEvalJSON moves from secret.go into the untagged secret_common.go so
declare can call it too, alongside a new inventoryCheckout()/
inventoryCheckoutRelative() pair that resolves the inventory checkout
through the repository registry the way resolveSecretsCheckout resolves
the secrets one, with INVENTORY still the override.

Tests replace the declareLoadVMSpecs fixture with a fake 'nix' on PATH
(the TestRcloneSiteRemoteCheck pattern) that answers by machine name and
fails for an unknown one; a new test covers a type outside the four the
inventory flake enforces.

Refs allod/tools#196
Cross-family review findings, all fixed:

1. Every flag value that lands in generated nix or JSON text is validated
   as a shape before it reaches any template. --owner, --kind, --account,
   and --ui-token-name must match ^[A-Za-z0-9][A-Za-z0-9_.@-]*$;
   --deployed-path must be an absolute path with no whitespace, control
   character, quote, backslash, or $. JSON content already went through
   encoding/json rather than raw formatting; this closes the one field
   that did not, --owner, interpolated into credentials.nix.

2. The name pattern allowed a leading digit, but the credentials.nix
   attribute is unquoted nix, so 1token = { ... } would not evaluate. Now
   ^[a-z][a-z0-9-]*$.

3. The credentials.nix and secrets.nix collision checks only matched their
   name/line on one line; a declaration split across lines slipped past
   and would have been duplicated. Both now match across whitespace
   including newlines.

4 & 5. Writes are now atomic (temp file, then rename, so a failed write
   never leaves a file truncated) and guarded against a concurrent edit:
   immediately before each file is replaced, declare re-reads it and
   refuses, restoring every file already replaced, if the bytes on disk no
   longer match what was read at the start.

6. The reparse-guard unit test is kept (it is the only way to exercise
   declareInsertRegistryGroup's post-build json.Valid check at all — for
   any JSON that parses to begin with, appending at the point this
   function finds can never produce invalid JSON, so no CLI-reachable
   fixture trips that specific guard). The existing end-to-end test that
   drives secretDeclare against the sabotage registry and asserts the
   refusal and byte-identical files is what review finding 6 asked for.

7. Usage text and docs/allod-secret.md now describe what the code actually
   does: validate-then-build in memory, atomic per-file replacement,
   refusal on a file changed underneath, and restoring every file already
   replaced on a later failure.

A test was added for each behavior change.

Refs allod/tools#196
credential inventory for collisions, refuse nix keywords, and span
comments in the collision regexes

Second review pass, all five verified findings fixed:

1. --kind now accepts only user, agent, service. forge-git and
   machine-host are real credential-inventory kinds, but the shape
   declare writes (public_key null, one agenix consumer) is exactly what
   the inventory check rejects for them; the refusal names the three
   accepted kinds and says the other two are generated by the template.

2. Added --verify-repo-url, --verify-context, and --verify-user.
   rotate-token's printed verification (print_group_verification,
   scripts/rotate-token) reads .verify.repo_url and
   .verify.credential_context for git-ls-remote (printing the literal
   command 'git ls-remote - HEAD' without them) and .user for
   forge-token-verify on a nixos-host target ('sudo -u <user> forge token
   verify'). --verify-repo-url and --verify-user are required with their
   respective probe and refused with any other; --verify-context is
   optional but still refused outside git-ls-remote. All three are
   validated before use and emitted through the JSON encoder.

3. Before writing, declare now also evaluates the secrets checkout's own
   credential inventory (nix eval --json <checkout>#lib.credentials,
   confirmed against README.md and flake.nix in the secrets repository)
   and refuses a name found there, naming the evaluated source.
   credentials.nix is 'hostEntries // activeEntries // stagedEntries //
   forgeGitEntries // { ...literal...; }', so a name one of the generated
   attrsets already produces is invisible to the textual scan and the
   appended literal entry would silently shadow it. The textual checks
   stay, since they also catch a name on a branch that does not evaluate
   yet; a failed evaluation refuses outright rather than falling through
   to the text checks alone.

4. The name pattern now refuses nix keywords (let, in, with, rec, assert,
   if, then, else, inherit, or), which match the pattern but are not
   usable as unquoted attribute names.

5. The collision regexes now allow a nix line comment interleaved with
   whitespace between the name and '=', not just whitespace alone.

A test was added for each behavior change, including one whose fixture
credentials.nix text names only one credential while the fake evaluated
inventory also knows a second, isolating the evaluated-only collision
path, and one where the evaluated-inventory check itself fails.

Refs allod/tools#196
Three review findings on the branch, each a value that reached a consumer
in a shape the consumer could not use.

--to appended every comma-separated machine name to the target list
without validating it, while --owner, --kind, --account, --ui-token-name,
and --verify-user all went through validateIdentifier. A machine name is
interpolated into the nix eval attribute path 'machines.<name>.type', into
the secrets.nix recipient line, and into the JSON group, so it reaches the
same generated text those flags do. --to '"dev-a"' evaluated successfully
and wrote a broken declaration. Every name now goes through
validateIdentifier("--to", machine) at parse time, so the usage error
names the flag and the accepted pattern like the others.

validateVerifyRepoURL checked only the 'https://' prefix, so the bare
string 'https://' was accepted and rotate-token's printed verification
became 'git ls-remote https:// HEAD'. The first character after the scheme
must now be a letter or digit: refusing only a leading '/' would still let
'https://?query' through, which leaves the same command with no repository
in it. The unsafe-character check is unchanged.

inventoryCheckout trimmed trailing slashes off $INVENTORY, which turns the
root into the empty string; nix would then have run in the process working
directory instead of /. The trim stays and an empty result becomes "/".
filepath.Clean would have done both in one call but resolves '..'
lexically, so INVENTORY=/base/link/../inventory, with link a symlink
elsewhere, would name a different directory than the one the OS reaches.

Tests: three --to cases (a quoted name, a nix interpolation, whitespace)
and three --verify-repo-url cases (no host, a host starting with '/', a
host starting with '?') join the existing injection table, each asserting
exit 1, the flag named on stderr, and all three fixture files untouched; a
unit test pins inventoryCheckout for INVENTORY=/, /tmp/x/, and /tmp/x. All
seven fail against the unfixed code. The two docs sentences that stated
the old --verify-repo-url shape and omitted --to from the identifier list
are corrected.

Refs allod/tools#196
vnprc approved these changes 2026-09-11 21:34:02 +01:00
vnprc force-pushed agent/secret-declare from 2eb07b4baa to 551163f0a3 2026-09-11 21:34:09 +01:00 Compare
vnprc merged commit 551163f0a3 into master 2026-09-11 21:34:19 +01:00
vnprc deleted branch agent/secret-declare 2026-09-11 21:34:19 +01:00
Sign in to join this conversation.
No description provided.