Make rotate-token's test runner enforce every assertion #49

Merged

vnprc merged 2 commits from agent/rotate-token-enforcing-runner into master

2026-09-10 21:57:02 +01:00

allod-agent commented

2026-09-10 19:28:42 +01:00

Member

The rotate-token test suite now enforces every assertion in every test. Until now the runner called each test inside an if, and bash suspends stop-on-error inside an if condition, so a failed check in the middle of a test was ignored and only the last line of each test decided its result. Nothing changes for a running machine: scripts/rotate-token is untouched and only tests/rotate-token.sh changes. Turning enforcement on exposed six tests whose middle assertions were wrong, all on the test side: two matched a command name against the script's explanatory prose instead of the command line, two shared a fixture secret path across two groups so the script refused the ambiguity before reaching the branch under test, and two ran without the hostname a later fixture change had made necessary. Each is corrected with a comment saying why. Assertions on prompt text were removed, since read -p prints nothing when stdin is a pipe, so they could never fail; the one that carried a real property was reshaped to observe the effect instead. The suite passes, the provisioning-contract flake check passes, and sabotaging one middle assertion now fails the suite, which it could not before. If this is wrong it shows as the suite going red on behaviour that is in fact correct; each correction is one hunk to revert.

Closes allod/nexus#48. Closes allod/nexus#46.

What changed

run_test runs the test in a subshell that sets -e itself and reads the status as a plain statement, outside any if or ||.
Six tests corrected: the git push step anchor in the two continuity tests; a dedicated fixture secret for the mixed-format group so secrets/git-a.age belongs to one group again (this also restored the allow-single-secret dry-run test); ROTATE_TOKEN_HOSTNAME and a local-auth root for the two malformed-plaintext tests whose fixtures had gained a refresh entry.
Thirteen tests lose an unobservable Paste new assertion; the shared-group test instead feeds a second stdin line and asserts it never lands in a rewritten secret, which is the property the prompt-count assertion was after.
The six tests added in allod/nexus#47 keep their || return 1 guards; they are redundant under the new runner and harmless.

No defect in scripts/rotate-token was found; every failure traced to the test or fixture side.

Risk

R1. A corrected assertion could now pin something narrower than the original intent; each correction carries its reasoning in a comment for review. The script's behaviour is unchanged, so no host rotates differently.

Validation

bash tests/rotate-token.sh: 36 tests pass, re-run by the integrator on the final commit.
nix build .#checks.x86_64-linux.provisioning-contract passes; shellcheck -x on the test file is clean.
Sabotage: changing one non-final expected string in the group dry-run test fails the suite at that test; reverted.
Reviewed by codex (gpt-5.6-sol) and pi (gpt-oss-120b). Codex notes the stdin canary in the shared-group test catches a second prompt that consumes and uses input, not one that reads and discards it; the prompt-count assertion it replaces could observe nothing at all through a pipe, so this is a gain, not a loss, and is left as is. Pi's pipefail point is wrong (the subshell inherits the file's set -euo pipefail), and its command-substitution point is the standing bash exemption documented in shell.md, unchanged by this PR.

Labels are unavailable to this token; none is warranted.

The `rotate-token` test suite now enforces every assertion in every test. Until now the runner called each test inside an `if`, and bash suspends stop-on-error inside an `if` condition, so a failed check in the middle of a test was ignored and only the last line of each test decided its result. Nothing changes for a running machine: `scripts/rotate-token` is untouched and only `tests/rotate-token.sh` changes. Turning enforcement on exposed six tests whose middle assertions were wrong, all on the test side: two matched a command name against the script's explanatory prose instead of the command line, two shared a fixture secret path across two groups so the script refused the ambiguity before reaching the branch under test, and two ran without the hostname a later fixture change had made necessary. Each is corrected with a comment saying why. Assertions on prompt text were removed, since `read -p` prints nothing when stdin is a pipe, so they could never fail; the one that carried a real property was reshaped to observe the effect instead. The suite passes, the `provisioning-contract` flake check passes, and sabotaging one middle assertion now fails the suite, which it could not before. If this is wrong it shows as the suite going red on behaviour that is in fact correct; each correction is one hunk to revert. Closes allod/nexus#48. Closes allod/nexus#46. ## What changed - `run_test` runs the test in a subshell that sets `-e` itself and reads the status as a plain statement, outside any `if` or `||`. - Six tests corrected: the `git push` step anchor in the two continuity tests; a dedicated fixture secret for the mixed-format group so `secrets/git-a.age` belongs to one group again (this also restored the allow-single-secret dry-run test); `ROTATE_TOKEN_HOSTNAME` and a local-auth root for the two malformed-plaintext tests whose fixtures had gained a refresh entry. - Thirteen tests lose an unobservable `Paste new` assertion; the shared-group test instead feeds a second stdin line and asserts it never lands in a rewritten secret, which is the property the prompt-count assertion was after. - The six tests added in allod/nexus#47 keep their `|| return 1` guards; they are redundant under the new runner and harmless. No defect in `scripts/rotate-token` was found; every failure traced to the test or fixture side. ## Risk R1. A corrected assertion could now pin something narrower than the original intent; each correction carries its reasoning in a comment for review. The script's behaviour is unchanged, so no host rotates differently. ## Validation - `bash tests/rotate-token.sh`: 36 tests pass, re-run by the integrator on the final commit. - `nix build .#checks.x86_64-linux.provisioning-contract` passes; `shellcheck -x` on the test file is clean. - Sabotage: changing one non-final expected string in the group dry-run test fails the suite at that test; reverted. - Reviewed by codex (gpt-5.6-sol) and pi (gpt-oss-120b). Codex notes the stdin canary in the shared-group test catches a second prompt that consumes and uses input, not one that reads and discards it; the prompt-count assertion it replaces could observe nothing at all through a pipe, so this is a gain, not a loss, and is left as is. Pi's pipefail point is wrong (the subshell inherits the file's `set -euo pipefail`), and its command-substitution point is the standing bash exemption documented in shell.md, unchanged by this PR. Labels are unavailable to this token; none is warranted.

allod-agent added 2 commits

2026-09-10 19:28:42 +01:00

tests/rotate-token: make run_test enforce every assertion in a test body f9cc56719c

run_test invoked each test as 'if "$@"; then', and bash suspends -e for
the duration of an if condition and for any function called from it, so a
failed [[ ... ]] mid-test fell through silently and only the test's last
command decided pass or fail (allod/nexus#46). run_test now runs the test
in a subshell that sets its own -e and reads the exit status outside any
if or ||, so a failed assertion anywhere in the body now ends the test.

Refs allod/nexus#48

tests/rotate-token: fix the tests the enforcing runner exposed as vacuous cd46f77a93

Under the enforcing run_test, six of the suite's assertions no longer held.

- 'in-place continuity dry-run orders refresh before network steps' and
  'live continuity rotation skipped refresh prints required step' searched
  for the literal text 'git push' to locate the deployment step's push
  command, but the script also restates 'git push' in prose describing why
  the ordering matters, earlier in the same output; grep found the prose
  first. The actual command line is exactly three spaces then 'git push'
  with nothing after it, which the prose never is; both tests now search
  for that instead.

- 'multi-secret path mode is guarded' and 'allow-single-secret dry-run
  warns without revocation' both resolve a group from a bare secret path
  (secrets/git-a.age), which requires that path to belong to exactly one
  Forgejo token group. The mixed.site fixture (added later, for a
  different test) reused that same path for its second credential, making
  it ambiguous across two groups and changing the error both tests die
  on. mixed.site's non-stanza credential now has its own dedicated secret
  (secrets/mixed-git.age) instead of reusing git-a's.

- 'malformed credential-store fails before prompt' and 'multiline
  credential-store fails before prompt' run a live rotation with no
  ROTATE_TOKEN_HOSTNAME set. malformed.git and multiline.git both carry a
  local_auth_refresh entry targeting 'control-host' (added later, for
  refresh_rejects_malformed_plaintext_before_installing), so on the actual
  test host the run now fails preflight's deliberate wrong-host check
  before ever reaching the credential-store parse these tests exist to
  exercise. Both now set ROTATE_TOKEN_HOSTNAME=control-host and a
  local-auth root, matching the group's declared target, so the run
  reaches the parse failure again.

Also drops every 'read -p' prompt-text assertion outside the six tests
from allod/nexus#47: 'read -p' prints its prompt only to a terminal, so
that text is invisible through every piped fixture in this suite whether
or not the code actually reached the prompt -- an assertion on it can
never fail for a reason. Where a test's status and message already prove
the script stopped before prompting (or an unchanged checksum proves
nothing was written), the prompt assertion is simply removed. Where the
claim was really about consuming exactly one prompt for a whole group
('live shared group rotation rewrites all members'), it now feeds a
canary second line behind the real answer and checks the canary never
reaches a rewritten secret -- observing the prompt's effect instead of
its invisible text.

Refs allod/nexus#48