[pithead-developer]

What's the problem

Closes #629

The revision published beside the masked feed was a bare SHA-256 of the unmasked canonical writable config. A reader holding the masked feed could substitute password guesses into the public canonicalization and compare digests, which gave them an offline verifier for weak pool passwords.

How I fixed it

The revision is now SHA-256 over a persistent local key plus the canonical config. The key is 32 random bytes in .rigforge-revision-key (mode 0600, git-ignored). util/revision-key.sh creates it on first use with a symlink-safe temp file and an atomic ln, and sets the owner to $REAL_USER when root creates it. A symlinked key is refused and an empty one is replaced. The key is never served by the API and is not in backups or support bundles. While the key persists, unchanged content keeps its revision and a credential rotation changes it.

Upgrade re-base, which the owner should accept explicitly: the first feed refresh after upgrading creates the key, so every rig's revision moves once with no new last_change_id. The same happens when the key is deleted. Pithead's note_worker_revision treats such a move as a hand-edit. It writes one permanent rig-drift audit row per rig and shows Inspect drift until the next recorded change. This is disclosed in docs/pithead-integration.md. Suppressing it needs a Pithead-side signal and is filed as p2pool-starter-stack/pithead#3915. It cannot be avoided in RigForge alone: carrying the old unkeyed revision forward would keep the offline verifier this issue removes.

Shortcut: a key that cannot be read or created makes the feed publish an empty revision (without an ERR-trap message), never an unkeyed hash. Upgrade when a consumer needs an explicit null. The rigforge.sh file-budget ceiling is unchanged because the logic lives in util/.

Here is the proof

• bash tests/test-revision-key.sh (new, added to make test-suite) passes 10 checks. An offline guess of the true password does not reproduce the revision. The key is mode 600. The revision is stable across processes. Rotation changes it, a different key changes it, and restoring the key restores it. An empty key file self-heals. A symlinked key file is refused and not written through.
• Feed fixture: the sister-API fixture test writes a fixed key (RIGFORGE_REVISION_KEY), and tests/contract/v1/feed.json pins the revision that key yields. This fixed the earlier Test suite and Coverage reds.
• Round-2 findings:
  • The stale comments at rigforge.sh:5258-5259 and tests/run.sh:9514-9516 are removed.
  • _writable_config_hash ends in || :, so an unreadable key no longer fires the ERR trap.
  • The empty-key race (two callers on a crashed empty key) costs one extra re-base. It is recorded and not changed.
• FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh and make lint-topology pass. make lint shows a shfmt diff in tests/run.sh that is already on develop.
• New tier4 row in tests/e2e-pithead.sh (network phase): the live :8081 feed serves a 16-hex config_meta.revision, the key file is 0600, and the key never appears in the served body.
• rigforge tier4-e2e job 3405@5c7f9da4de05bad1a8bc0476a4074a26b764c1e9 (phases connect,network,dashboard) is pending; its evidence block is added when it completes.
• Security review found a predictable temp path, an empty-key lockout and a root-versus-operator ownership race. All three are fixed.
• Ponytail pass: nothing cut.

🤖 Generated with Claude Code

[pithead-reviewer]

FAIL

What I investigated

I reviewed PR #647 at head b3e48cf against #629. The fix does what it says: the revision is now SHA-256 over a 0600 local key plus the canonical config, the new test fails with the fix reverted and passes at head, and every GitHub check is green. I'm returning it for three reasons. First, every rig's revision changes once on upgrade with no change_id, and Pithead records that as a hand-edit, a permanent rig-drift audit row per rig. The PR doesn't disclose this and it isn't handled. Second, this change to what Pithead observes has no rigforge tier4-e2e job. Third, explanatory comments remain inside the edited hunks, one of them now wrong.

• gh pr view 647: head b3e48cf, base develop, author app/pithead-developer, 3 commits, 0 comments, 0 reviews. The body quotes no bench-ci job and says no bench tier is needed.

• gh issue view 629: the acceptance criteria are (a) the revision can't be reproduced from the masked feed plus candidate passwords without a local secret, (b) the revision is stable for unchanged content, including across restarts, and changes on credential rotation, (c) a regression test runs the dictionary comparison and checks stability and rotation. The issue's only comments are the operator's claim and release notices.

• gh pr checks 647 --json: all 10 checks pass, none skipped: gitleaks, Coverage (kcov + diff-cover), yamllint, markdownlint, zizmor, shellcheck+shfmt, Test suite, Stock home setup (historical and current), End-to-end (Docker).

• gh api actions/jobs/125766406274 (Test suite): head_sha b3e48cf…, conclusion success. In the log, line 2847 shows '✓ sister-API feed shape matches the fixture (Cross-repo wire contracts have no automated drift guard against pithead's fixtures #351)' and lines 2891-2901 show all 10 rows of tests/test-revision-key.sh as 'ok'. No failing assertion: the 6 lines containing 'FAIL' are test names.

• bash tests/test-revision-key.sh at head: 10 ok, rc=0.

• Scratch worktree at head with rigforge.sh and util/ reset to origin/develop (81ec495) and util/revision-key.sh removed, same test: 4 FAIL ('offline guess … reproduced', 'key file private' got [], 'different local key' got same, 'symlinked key file is refused' got 047761faac8e9bca), rc=1. The test fails without the fix.

• FILE_BUDGET_REQUIRE_BASE=1 bash scripts/lint-file-budget.sh: 'File budget OK', rc=0.

• make lint-topology: 'topology classes OK', rc=0.

• shellcheck and shfmt -d -i 4 on util/revision-key.sh and tests/test-revision-key.sh: rc=0 and rc=0.

• bash probe of set -Eeuo pipefail with an ERR trap: inside a command substitution, a failing r=$(false | cat) fires the ERR trap and continues with r="". So the claimed empty-revision fallback holds, but in production the ERR trap prints its message too.

• rigforge.sh 2129-2168 (upgrade()): no apply, no _stamp_config_meta, so an upgrade leaves last_change_id unchanged while the revision moves.

• pithead origin/develop 2b8a778, worker_config_store.py:311 note_worker_revision: a revision that moves with the same last_change_id returns a drift. worker_change_audit.py:236-243 records audit event 'rig-drift' … 'revision X -> Y with no new change_id'. client/rig_config_meta.py treats revision as an opaque token and never recomputes it, so the wire shape still works.

• rigforge.sh 3638-3672 and 3754-3814: backup copies config.json and the watchdog files; support-bundle copies redacted configs and units. The key is in neither. util/api-server.py has no SCRIPT_DIR reference.

• private/detect-topology.py over the PR body, the PR comments (none), the PR review comments (none), and the issue body and comments: 'clean', rc=0.

• util/revision-key.sh:4, util/revision-key.sh:14: When the key is a symlink or unreadable, the script exits 1. rigforge.sh sets the ERR trap with set -E, so on_err fires inside the command substitution, and every api-refresh writes '[ERROR] rigforge aborted while …' to the journal even though the feed carries on with an empty revision. That message misleads anyone diagnosing the documented shortcut.

• util/revision-key.sh:5-11: When the key file is empty, two concurrent callers can both pass [ ! -s ]. The second caller's rm -f then deletes the key the first just linked, which costs one extra re-base. This needs a crashed empty key plus a race; recording it only.

• ponytail pass over the diff: Nothing to cut. The separate util/revision-key.sh keeps the rigforge.sh budget flat. RIGFORGE_REVISION_KEY is needed by the tests. Prefixing the key into SHA-256 instead of using HMAC is adequate for a 64-bit opaque identifier the rig never verifies, and it avoids an openssl dependency.

Reviewed head: b3e48cf0465e9c86e9cbc549f59eeaa069c683e2.

What are the gaps

• rigforge.sh:5260: Every rig's revision changes on the first feed refresh after upgrading to this build: the old value was unkeyed, the new one is keyed. upgrade() (rigforge.sh:2129-2168) stamps no change_id. Pithead's note_worker_revision (pithead dashboard/mining_dashboard/service/workers/worker_config_store.py:311, at origin/develop 2b8a778) treats a revision that moves while last_change_id stays the same as a hand-edit underneath RigForge. worker_change_audit.py:236-243 then writes a permanent 'rig-drift … with no new change_id' audit row naming the rig, and the worker Inspect view shows revision drift. A fleet upgrade therefore falsely accuses every rig, the outcome Pithead's own docstring calls worse than a missed detection. I found this by reading the code; I did not run it. The PR body and docs/pithead-integration.md:86-90 mention only the re-base when the key is deleted, and still say the revision 'changes iff that config changes'. Either handle it (a Pithead companion that ignores a drift across a RigForge version change, or RigForge recording the re-key as a change with its own change_id), or disclose it in the PR body under How I fixed it and in docs/pithead-integration.md so the owner can accept it explicitly.
• PR body, 'No bench tier is needed': This changes how config_meta.revision is derived on every rig, a field Pithead polls and stores per worker. The documented contract section (docs/pithead-integration.md:86) is edited in this diff. prompts/rigforge.md requires a rigforge tier4-e2e job when the worker-to-Pithead contract changes. A 16-hex shape that Pithead still parses doesn't show that the re-base above is harmless against the bench's stack. Quote a tier4-e2e job (targeted or matrix) on the head with bench-ci evidence <id>@<sha>.
• rigforge.sh:5258-5259, tests/run.sh:9514-9516: Explanatory comments remain inside the hunks this PR edits. Under the comment rule they are deleted. rigforge.sh:5258-5259 also still describes the function as 'a short content hash of the canonical writable config', the unkeyed description this issue exists to retire. tests/run.sh:9514-9516 (the CONFIG_META_FILE override explanation) is in the same hunk as the new RIGFORGE_REVISION_KEY line.

[pithead-developer]

Round-2 findings addressed at 5c7f9da: the upgrade re-base is disclosed (docs and PR body; Pithead-side handling is filed as p2pool-starter-stack/pithead#3915), the stale comments are removed, the ERR trap is quiet on an unreadable key, and a tier4 feed row is added. rigforge tier4-e2e job 3405 is pending.

[VijitSingh97]

Reviewed by Hermes agent (operator's delegate) against origin/develop b82758b: full diff read, make test-suite run per PR, body claims reproduced. Not a substitute for the owner's approval; merge stays with the operator.

Verdict: PASS (non-blocking notes)

• Verified independently: contract pin tests/contract/v1/feed.json revision 6bd0e04fa9f6609a recomputed via the real _writable_config_canonical + sha256(key||canonical)[:16] and matched; tests/test-revision-key.sh 10/10; key created 0600 via umask-077 mktemp+hardlink, symlink refused (util/revision-key.sh:4-8), owner-chowned, gitignored, absent from the support bundle; suite rc=0.
• Note (docs/adr/0001-writable-worker-config-control-path.md:94): the ADR still asserts "the hash is one-way, so no secret leaks" — the exact assumption #629 disproved. docs/pithead-integration.md was updated; the ADR rationale was not.
• Note (rigforge.sh:5258): _writable_config_hash ends || :, so a refused/failed key helper yields an empty revision on the feed — availability-only degradation under tampering, but worth a comment.