Skip to content

Publish the hash list signed - #2

Open
MorningLightMountain713 wants to merge 17 commits into
masterfrom
feature/sign-hashlist
Open

Publish the hash list signed#2
MorningLightMountain713 wants to merge 17 commits into
masterfrom
feature/sign-hashlist

Conversation

@MorningLightMountain713

@MorningLightMountain713 MorningLightMountain713 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Background

fluxbench establishes that a node runs genuine FluxOS by checking its ZelBack tree hash against the list this repository serves. The list is unsigned today — a root operator can MITM hashes.runonflux.io and bless any tree — and it is written by flux CI with a contents-write PAT exposed to every branch build of the flux repo. This PR signs the list, and makes the signer the single writer of everything src/hashes/ serves, deriving every new entry itself.

Related PRs and merge order:

  1. This PR merges first and works standalone — the signer reads RunOnFlux/flux's refs directly, so publication continues even before the flux side changes.
  2. feat(ci): hash publication becomes a dispatch; flux CI loses write access to fluxhashes flux#1789 — flux CI's write path becomes a dispatch.
  3. feat(ci): gate release PRs on the signed hash list, and cut the tag after merge flux#1788 — the release gate consumes the signed document.

Once 1 and 2 are in, the master ruleset described below goes on and the old bot PAT (API_TOKEN_GITHUB) is revoked outright — a classic token with org-wide repo scope, so there is no narrower grant to leave in place. Nothing on the new branches references it.

The design

  • The signed document (src/hashes/hashlist-signed.json): Ed25519 over exact bytes {seq, issued_at, hashes}, verified against an any-of-N pinned key set. The sequence's high-water lives in the provenance record beside it, so a wiped document cannot restart the sequence.
  • The signer derives what it signs. Every run diffs git ls-remote of RunOnFlux/flux against the snapshot in its provenance record, fetches each new commit by SHA, computes the tree hash from the bytes it fetched, and emits hashes.js + signed document + provenance in one commit. A publication request carries pointers, never hash values. That bounds the credential; it does not vouch for the tree. GitHub shares one object store across a fork network, so every commit ever pushed to any public fork of this repository stays anonymously fetchable by SHA from the official remote, and a dispatched commit matching no ref is still derived — deliberately, since a branch can move past a commit before the signer looks. A dispatch is therefore trusted to name a commit reachable in flux's object store, forks included, which is a larger set than "a tree RunOnFlux authored". The credential lives in flux's repository secrets, reachable by anyone who can land a workflow change on any branch there — who can already get a hash listed by pushing a branch. It is the same trust boundary, not a defence against it. flux CI's own computation rides along as claimed_hash, a tripwire: a mismatch with the signer's derivation is a red run that publishes nothing, which also catches runner-environment drift. Stated plainly: a compromised dispatch credential can stall publication for as long as it keeps sending mismatched claims, since a mismatch aborts the whole run. That abort is deliberate — a mismatch is indistinguishable from signer-side drift, and skip-and-continue would let drift publish silently, signing hashes no node can match. The stall is availability-only, gains the attacker no list content, and self-heals on the next clean run.
  • Level-triggered. A request lost to the concurrency group's newest-pending-wins collapse is repaired by whichever run survives, since every run reconciles the full delta; a daily sweep bounds the tail and is the future home of the 60-day expiry.
  • Membership is monotonic. An entry leaves the regenerated list only via a cull in src/hashes/ledger.json — the one human-edited input, changed through reviewed PRs. validate fails any PR that edits the three generated outputs directly, and runs green on every signing commit as a live tripwire.
  • Key 1 was regenerated on 2026-08-24 when it moved into the environment — a secret's value cannot be moved between scopes, and nothing had yet consumed it, so regenerating was free. The pinned hex changed with it; SIGNING.md records both the new value and the reason.
  • Custody. Both secrets (signing seed, push deploy key) live in the hashlist-signing environment, restricted to master — a branch run is refused before its first step, so no collaborator's branch push can reach them. The signer pushes over SSH with the deploy key, the only bypass on the master ruleset (PRs + green validate required, admins included); its own run token is read-only.
  • Cutover is non-destructive. The first run grandfathers every existing entry as-is (derived: false), snapshots the refs, and signs the current list. Consumers see no change: hashes.js keeps its exact format, and the signed-document test vector is byte-identical to the fluxbench copy.

Repository configuration at activation (repo admin)

Environment hashlist-signing (deployment branch policy: master only) holding HASHLIST_SIGNING_SEED_B64 and HASHLIST_DEPLOY_KEY; the deploy key's public half as this repo's only write deploy key; a master ruleset requiring PRs and the validate check with a DeployKey bypass. The exact configuration was rehearsed and recorded.

Verification

  • A 37-case local harness drives the reconciler through bootstrap, derivation, claimed-hash mismatch, culls, duplicates, corruption, and old-format provenance compatibility.
  • A full sandbox rehearsal on real GitHub (2026-08-24), run on top of the sandbox's existing old-format state to mirror production cutover: the sequence continued without restart; the signer's derivation matched flux CI's claimed hash across two different runner environments; a cull PR worked end to end; a PR editing an output went red; direct pushes to master — repo admin included — were rejected; the signer never retriggered itself.
  • The GitHub platform behaviours this leans on (concurrency pending-collapse, anonymous fetch-by-SHA including orphaned commits, environment branch-policy gating, DeployKey ruleset bypass, the Actions-only dispatch credential's exact permissions) were each proven live the same day.
  • An independent implementation review found no defect above cosmetic; its notes are incorporated.

🤖 Generated with Claude Code

https://claude.ai/code/session_01SFrS7Q3JwuPj6Yr4vALnwp

@MorningLightMountain713
MorningLightMountain713 force-pushed the feature/sign-hashlist branch 4 times, most recently from fda9164 to cf1e4ff Compare August 14, 2026 11:03
Adds src/hashes/hashlist-signed.json: the same list, signed with Ed25519, so a
consumer can verify it came from us rather than trusting the transport or
whatever relayed the response. Published alongside src/hashes/hashes.js, not
instead of it -- both are served.

Signing lives here because the hashes already arrive here from flux CI, so it
needs no cross-repository dispatch and no additional token. It runs unattended.

The sequence lives inside the published document rather than a file beside it,
so nothing can drift out of step with what was actually signed. It advances
only when the list changes: re-signing an unchanged list would burn a sequence
and make every consumer re-fetch a document identical to the one it holds.

Signing verifies its own output against the published public keys before
publishing, so a mangled secret is a red run rather than a document that looks
published and satisfies nobody.

The /hashlist route is ordered before the existing catch-all, which would
otherwise answer that path with the unsigned array.

Key 1 is not generated yet -- it needs repository admin. SIGNING.md carries the
command. Until then only key 2 is pinned, so a premature run fails verification
instead of publishing something unverifiable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This repository had no CI at all. The list is served by requiring it, so a file
that does not load takes the endpoint down rather than merely publishing
something odd -- and while flux CI now checks its own edit before pushing, the
list is also edited by hand. The recent cull removed 1265 entries that way,
which is both the most likely source of a mistake and the least guarded.

Checks that the list loads, is non-empty, holds only lowercase md5s, and has no
duplicates. Then, if the signed copy exists, that it verifies against the pinned
keys and describes the list beside it: a signed document that no longer matches
what it claims to sign would be accepted by a consumer and then not contain what
that consumer came for.

Its absence is not a failure -- CI writes it, so it does not exist until the
first signing run.

Shape only. Whether a particular hash should be listed is not knowable from
here: removing one still in use looks identical to removing one that is
obsolete.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
git add followed by a bare git commit also commits anything else a previous
step left staged, and a bare git diff --cached would report those as a change
and publish a document that had not moved. Both are now scoped to the one path.

Found by running the workflow's steps against a local origin: with an unrelated
file staged, the publish swept it into the commit.

Verified end to end there afterwards. A new list signs, verifies and publishes;
an unchanged one reports changed=false and skips both later steps; a grown list
advances the sequence; a key that is not pinned fails verification and publishes
nothing; and a staged unrelated file stays out of the commit.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
apicache stores whatever the handler returned, including a 404. The signed
document does not exist until the first signing run, so any request arriving
before then pinned that 404 for the whole cache window -- and it kept being
served well after the document had been published.

Caught by starting the app and requesting the endpoint before and after
creating the file: the second request still answered 404.

The /hashlist route now uses an apicache instance scoped to 200 responses. The
catch-all is untouched; it only ever returns 200.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The new scripts carried 'use strict' and dynamic requires, both of which this
repository's config rejects. Nothing runs lint in CI, so this would have
surfaced as thirteen errors for whoever next ran npm run lint.

The dynamic requires were unnecessary: scripts/ sits at a fixed depth, so the
paths are static. The remaining requires are deliberately lazy and keep their
disables.

Verified clean afterwards, and the scripts still sign, verify and validate.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

@Cabecinha84 Cabecinha84 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: yes, safe to ack — with one correction and one confirmation before merge

What I checked, and what holds up

Workflow attack surface (the part that matters most here):

  • sign-hashlist.yml triggers only on push to master (path-filtered) and workflow_dispatch. It does not use
    pull_request_target or pull_request — which is exactly what would otherwise let a fork read HASHLIST_SIGNING_SEED_B64
    in a public repo. Correct.
  • permissions: contents: write is scoped to the signing job only; validate.yml is contents: read with no secrets.
  • validate.yml does run on fork PRs, but the scripts use only stdlib crypto/fs/path — there is no npm ci, so there's
    no supply-chain exposure and no useful token. Fine.
  • concurrency: sign-hashlist prevents two runs burning the same sequence.
  • Both the commit and the git diff --cached are scoped to the one path — the bug described in the third commit is
    genuinely fixed.

Cryptography — I ran it rather than just reading it:

  • The test vector (test/vectors/hashlist.json) verifies correctly under the committed test key. ✅
  • The test key really does derive from sha256('fluxos-hashlist-interop-vector-test-key-not-for-production') as
    documented — so it is not a leaked real key. ✅
  • Both pinned public keys are valid, importable Ed25519 points. ✅
  • The payload is signed and transmitted as exact bytes in base64, so verification doesn't depend on signer and
    verifier agreeing on JSON key order or whitespace. That's the right design call.
  • The /hashlist route takes no user input in the path (it's a constant) — no traversal. The apicache fix
    (newInstance({statusCodes:{include:[200]}})) is the correct API and does resolve the pinned-404 problem.

One real discrepancy worth raising before the ack

The PR body claims, about merge order:

▎ "If it happens the other way round the signer refuses to continue — it will not restart the sequence at 1"

The code doesn't do that. In scripts/sign-hashlist.js:

const seq = previous ? previous.seq + 1 : 1;

If flux CI wipes src/hashes/ — which is the current behaviour; I confirmed
cpina/github-action-push-to-another-repository with target-directory: src/hashes/ deletes the destination directory —
then previousDocument() returns null and the signer silently restarts at seq 1 and publishes, on a green run.
validate.js only checks seq >= 1, so it doesn't catch monotonicity either. The rollback that's advertised as "a noisy
red run" would in practice be a silent one.

Impact today is low (nothing consumes the document yet), but sequence monotonicity is the design's only anti-replay
control. I'd ask for either an explicit guard (e.g. read the last published seq from git history and refuse to go
below it) or, at minimum, a correction to the PR description so it doesn't promise a safeguard that isn't there.

One confirmation to request

Key 2 is the entire recovery story — key 1's private half deliberately exists nowhere outside the repository secret.
But nothing in the PR demonstrates that key 2 exists and is usable; only its public hex is present. Before merging,
I'd ask David for a one-off proof: sign a challenge offline with key 2 and confirm it verifies under the pinned hex.
If key 2 is lost or was transcribed wrong, better to find out now than on the day it's needed.

Minor notes (non-blocking)

  • Behaviour change: today the catch-all app.get('*') answers any path with the array, including /hashlist. After this
    PR, /hashlist returns 404 until the first signing run. Very unlikely anyone depends on that, but it's the one change
    to existing behaviour.
  • Merge order: flux#1785 is still OPEN. I checked flux's current workflow — the dependency is real, not theoretical.
    Merge this one after it.
  • The final git push has no rebase or retry: if flux CI pushes between checkout and push, you get a red run and the
    signed document lags until the next hashes.js push (or a manual workflow_dispatch). Noisy, not dangerous.
  • PR checks are green (validate + GitGuardian), no reviews yet.

Bottom line: the security design is sound, and the thing I'd worry about most in a public repo — the signing secret
reaching a fork — is properly closed off. I'd ack, noting the seq-1 restart and asking for proof of key 2.

@MorningLightMountain713

MorningLightMountain713 commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator Author

Answering the review properly — two of its findings were real, and both are now addressed in the PR:

  • The seq-1 restart. previous ? previous.seq + 1 : 1 meant a missing document silently restarted the sequence on a green run. The high-water now lives in src/hashes/provenance.json, committed in the same commit as the document, and the signer takes the next sequence from whichever of the two is higher. validate.js enforces it: a document below the recorded high-water — or a missing document while the record says one was signed — is a red run, not a skip.
  • The missing push retry. Sign, verify and publish now run as one step with three attempts, each re-syncing to origin/master and re-signing from scratch, so racing a push from flux CI no longer leaves the newest hash unsigned until the next list change.

On key 2: I have the key stored, and have validated it.

The /hashlist 404-until-first-signing behaviour is intended — the endpoint is new in this PR.

Both workflows have since been rehearsed end to end on real GitHub, in throwaway private copies of the two repositories using the committed interop test key (no production key material): publish → sign → sequence advances; the signed document deleted by commit → validate red, and the next signing continued the sequence rather than restarting; two publishes in rapid succession and a rejected push both recovered; a tag push annotated its provenance row exactly once. Details in the PR body.

The sequence was previous ? previous.seq + 1 : 1, so the value depended
entirely on reading the previous document, and absence read as "this is the
first one". Consumers hold the highest sequence they have accepted, so a
restart at 1 would not merely roll back -- it would wedge publication on a
green run, discovered whenever something next needed the newer document.

The signer now takes the next sequence from whichever is higher: the previous
document or the signed high-water in src/hashes/provenance.json, written by
the same run and committed alongside the document. Losing the document no
longer resets anything; a record that exists but does not parse stops the run,
because treating corruption as absence is exactly the restart this prevents.

The payload gains issued_at alongside seq, per the build spec -- a document's
age is actionable in a way a bare sequence number is not, and the shape has to
be right before the first document exists. Consumers read seq and ignore the
rest. Test vector regenerated accordingly under the committed test key.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
seq >= 1 passes a restart at 1, which is the one failure the sequence exists
to prevent. The document's sequence must now be exactly the provenance
high-water: below it is a restart, above it means the record missed a write.
A document without a recorded high-water, and a recorded high-water without a
document, both fail -- a wiped file becomes a red run on the next push rather
than silence.

Also shape-checks the provenance record itself, which has two writers and
would take the signing run down if either produced something malformed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The final git push had no retry: flux CI pushing between checkout and push
made a red run, and the newest hash stayed unsigned until the next change to
hashes.js -- which could be days. Same exposure as the flux-side publish, same
fix: three attempts, each re-syncing to origin/master and signing again from
scratch, because the list may have gained an entry between attempts.

Sign, verify and publish collapse into one step to make that loop possible.
Verification still runs against the published public keys on every attempt.

Verified against a local origin under Linux: the first run signs seq 1, an
unchanged list does not burn a sequence, a wiped document signs the next
sequence rather than 1, and a rejected push recovers on the second attempt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The publication pipeline inverts: flux CI no longer writes here, it
dispatches "commit X exists". The signer diffs the flux remote's refs
against its snapshot, fetches each new commit itself, hashes the tree
from the bytes it fetched, and emits hashes.js, the signed document and
provenance in one commit -- pushed with a deploy key held in the
master-gated hashlist-signing environment, through the ruleset's
DeployKey bypass.

A hash value can enter the list through no other path. flux CI's own
computation rides the dispatch as claimed_hash, a tripwire whose
mismatch is a red run: it is the only signal that catches environment
drift, whose silent form publishes a hash no node matches.

Membership is monotonic: an entry leaves the regenerated list only via
a cull in src/hashes/ledger.json, the one human-edited input, which the
signer reads and never writes -- so the ledger-filtered push trigger
cannot retrigger the signer, and validate fails any PR that edits the
three generated outputs directly.

Level-triggered: a dispatch lost to the concurrency group's
newest-pending-wins cancellation is repaired by whichever run survives
reconciling the full delta; the daily sweep bounds the tail. State
advances without membership changes commit provenance alone and burn no
sequence.

37-case local harness green; platform semantics proven in sandbox
2026-08-24 (concurrency collapse, anonymous fetch-by-SHA including
orphaned commits, environment branch-policy gating, DeployKey ruleset
bypass on an org repo, Actions-only dispatch credential).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFrS7Q3JwuPj6Yr4vALnwp
…, honest comments

Three notes from the independent implementation review, none above low
severity: a commit arriving as branch tip and tag in one run now keeps
both labels regardless of ref iteration order; the outputs guard's
comment stops claiming to be a security control (pull_request runs the
PR head's workflow copy -- required review is the control, the guard
catches mistakes); the monotonicity guard's comment says what it is,
defense-in-depth for a future regeneration refactor, mutation-tested.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFrS7Q3JwuPj6Yr4vALnwp
The event payload's base.sha is the base branch at the last synchronize
event -- stale on any PR whose base has moved since, and the signer
moves master daily. Diffing against it flagged every hash published
since the PR opened as if the PR had edited it, going red on this very
PR. HEAD^1 of the merge-ref checkout is the base as it stands now, so
the guard sees exactly what merging would change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFrS7Q3JwuPj6Yr4vALnwp
Key 1 was regenerated 2026-08-24 straight into the hashlist-signing
environment's secret: a secret's value cannot be moved from the
repository level into an environment, the environment scoping is the
custody hardening, and nothing had ever consumed the old key -- no
production signing run has happened and fluxbench has not shipped -- so
regeneration was free. Key 2 (cold) is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SFrS7Q3JwuPj6Yr4vALnwp
@MorningLightMountain713

Copy link
Copy Markdown
Collaborator Author

This PR was rewritten in place on 2026-08-24. The architecture you reviewed on the 14th is not the architecture here now — flagging that explicitly rather than letting a re-review start from the old reading. My earlier comment above answers the 14th's design and parts of it no longer describe the code; it stays as posted rather than being edited, but read it as history.

What changed. The old design had two writers: flux CI pushed hashes.js, and the signer signed whatever it found. Now there is one. The signer diffs git ls-remote of RunOnFlux/flux against a refs snapshot in its provenance record, fetches each new commit by SHA, derives the tree hash from the bytes it fetched, and emits list, signed document and provenance in a single deploy-key commit. flux CI no longer writes here at all — it only dispatches "commit X exists, I computed H", and that claimed_hash is a tripwire the signer checks against its own derivation, never a value that can enter the list.

Your two findings from the 14th both still hold, one for a different reason:

  • The seq-1 restart. Still fixed, same mechanism: the high-water is taken as max(document.seq, provenance.signed.seq) + 1, so a wiped document cannot restart the sequence, and validate refuses one below the recorded high-water.
  • The missing push retry. Still there — three attempts, each re-syncing to origin/master and reconciling from scratch. But the race it covers has changed: flux CI cannot push here any more, so what a retry now absorbs is a human merge landing between checkout and push (a ledger cull PR). My earlier comment describes it as covering a race with flux CI; that race no longer exists.

Two things that invalidate parts of your review, through no fault of it:

  • Key 1 was regenerated on 2026-08-24, so the pinned hex you verified has changed — 3023cb5e… became 14837066…. Worth redoing that check against the current verify-hashlist.js. The reason is in SIGNING.md: the key moved into a master-restricted environment, a secret's value cannot be moved between scopes, and nothing had yet consumed it, so regenerating was free rather than disruptive. Key 2 is untouched, and stands as answered.
  • The merge order inverted. Your note said flux#1785 was open and this should merge after it. #1785 is closed, superseded by flux#1789, and this PR now merges first — it works standalone, because the signer reads flux's refs directly rather than waiting to be told.

The body has been corrected. It carried a pre-merge instruction to remove the fluxhashes grant from the old bot PAT and keep its fluxjsdocs one. That is not an operation that exists — we fingerprinted the token and it is a classic PAT with org-wide scope, which is the same reason your trim-the-grant suggestion on #1789 didn't apply. The instruction is gone; the plan is full revocation once this PR and #1789 are in and the master ruleset is on. The key 1 regeneration is now disclosed in the body too.

Re-verification since. The rewritten design went through a full sandbox rehearsal on real GitHub on the 24th, run on top of the sandbox's existing old-format state so it mirrored a production cutover: the sequence continued without restart, the signer's derivation matched flux CI's claimed hash across two different runner environments, a cull PR worked end to end, a PR editing a generated output went red, and direct pushes to master — repo admin included — were rejected. A 37-case local harness drives the reconciler; an independent implementation review found nothing above cosmetic.

One bug did come out of the real PR that the sandbox structurally could not produce: the validate outputs guard anchored on pull_request.base.sha, which is the base as of the last synchronize event and therefore stale on any PR whose base has moved — and the signer moves master daily. It read every hash published since this PR opened as if the PR had edited outputs. It now diffs HEAD^1..HEAD on the merge ref, whose first parent is the current base tip by construction. Fixed in 28822679, and validate is green on this PR against the real list.

@Cabecinha84 Cabecinha84 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finding 1 — "a compromised dispatch credential gains the attacker no list content" is not true as written

Severity: medium. Correct the claim; ideally close the gap.

The PR body and the workflow comments state the property plainly: "There is no input through which a hash value
can enter the list — a publication request carries pointers." The reasoning is that the dispatch only names a
commit, and the signer derives the hash itself from the official repository.

The gap is that "fetchable from https://github.com/RunOnFlux/flux" is a much larger set than "a tree RunOnFlux
authored". GitHub's fork network shares one object store, so every commit ever pushed to any public fork of flux —
every fork PR head — is anonymously fetchable by SHA from the upstream repo, permanently.

I proved both halves separately:

$ git fetch --depth 1 https://github.com/RunOnFlux/flux 00050a0649281861f7b9eeb41adc803d8de50d84
From https://github.com/RunOnFlux/flux

  • branch 00050a06... -> FETCH_HEAD # exit 0 — this SHA exists only in ykris45's fork (PR #1174)

and in the sandbox, that the signer accepts a commit on no ref at all:

$ DISPATCH_COMMIT= node scripts/sign-hashlist.js
flux remote: 2 refs
derived b0c85bfcba835bd9775cde7cb82c2507 from 0454b3039 (unlabelled)
signed seq 3 over 179 hashes (1 added, 0 culled)
changed=signed

The relevant code is scripts/sign-hashlist.js:260-265 — the dispatched commit is added to the work set even when
it matches no ref in current, falling back to dispatchLabel(). claimed_hash is no obstacle: it's required: false,
and an attacker controls both values anyway.

So a holder of the dispatch credential can put the tree hash of an arbitrary attacker-authored FluxOS into the
signed list — not merely stall publication.

How much does this actually matter? Less than it sounds, and I want to be fair about that. FLUXHASHES_APP_KEY
lives in flux's repo secrets, reachable by anyone who can land a workflow change on any branch of flux (on: [push]
hands secrets to every branch build). Those same people can already inject a hash by pushing a branch — that's
the design's accepted trust boundary. So this is not a privilege escalation for the realistic attacker; it's a
defense-in-depth gap and a false statement of the trust model.

But the statement is load-bearing. flux#1789 pins actions/create-github-app-token@v1 (mutable tag) and keeps
cpina/github-action-push-to-another-repository@main (a branch). "Worst case the dispatch credential stalls us" is
exactly the reasoning that makes those pins feel acceptable. If the true worst case is arbitrary hash injection,
that risk acceptance should be re-taken consciously.

Ask: either

  • (a) correct the body and the sign-hashlist.yml header comment to say the credential is trusted to name commits
    reachable in the official repository's object store, forks included; or preferably
  • (b) close it: when the dispatched commit is on no current ref, prove ancestry before deriving — fetch the
    dispatched ref name from the remote and require git merge-base --is-ancestor FETCH_HEAD. That keeps the
    legitimate "the branch moved past it before I looked" case working (which is why the fallback exists — under
    today's on: [push] CI every intermediate commit gets a hash, and tips-only would lose those) while a fork PR
    head, which is an ancestor of nothing upstream, is refused.

Finding 2 — a commit with no ZelBack directory derives the md5 of nothing, silently

Severity: medium. Two-line fix.

TREE_HASH_PIPELINE (sign-hashlist.js:35) is a shell pipeline run without pipefail. find failing sets no exit
status the caller sees — the status is awk's — and md5sum of an empty stream is a perfectly well-formed 32-hex
value:

$ cd && bash -c "$TREE_HASH_PIPELINE"
find: './ZelBack': No such file or directory
d41d8cd98f00b204e9800998ecf8427e # passes the /^[0-9a-f]{32}$/ guard

Reproduced end to end — a branch with ZelBack removed goes straight into the signed list:

derived d41d8cd98f00b204e9800998ecf8427e from 22d11f93e (docsonly)
signed seq 4 over 180 hashes (1 added, 0 culled)
verified: seq 4, 180 hashes # green. validate: ok.

That hash means "a node whose ZelBack contains zero regular files is genuine FluxOS." It is monotonic once in —
removable only by someone noticing and opening a cull PR.

Two honest mitigations: the pipeline is inherited byte-for-byte from flux CI's existing Check Hash, so this is not
a regression; and I checked all 211 flux branches — they're all code branches, and d41d8… is not in today's
177-entry list, so nothing has tripped it yet. It's latent, reachable by a docs-only branch, a ZelBack rename, or
a new tag placed on pre-ZelBack history.

But this PR is what makes the derivation load-bearing and signed, and it widens the surface from "pushes" to
"every ref that moves, tags included."

Ask: set -o pipefail on the pipeline plus an explicit "at least one file was hashed" guard, and reject
d41d8cd98f00b204e9800998ecf8427e outright. Cheap, and it turns a silent poisoning into a skipped ref (or a red run
on the dispatch path).


Notes — none blocking

  1. Nothing in CI exercises the test vectors, and the 37-case harness isn't in the repo. npm test still exit 1s.
    The verification evidence is real but reviewer-unreproducible and won't guard regressions. I'd like the harness
    landed, or at minimum a vector check wired into validate.yml. This is my main process ask.
  2. Merging does not trigger the signer. Triggers are workflow_dispatch, pushes to ledger.json, and the 03:43 cron
    — a merge commit hits none. So /hashlist 404s until the cron fires or someone dispatches manually, and the
    environment + secrets + ruleset must all be in place before that first run or it goes red. Worth an explicit
    cutover step: land repo config, merge, dispatch, confirm seq 1.
  3. validate.yml runs on fork PRs and does require('../src/hashes/hashes') — arbitrary JS from an unauthenticated
    fork executing on a runner. Impact is genuinely negligible (no secrets, read-only token on a public repo, no
    npm ci), and "the list is served by requiring it" is the point of the check. Worth a permissions: {} and an
    awareness note, nothing more.
  4. The cutover signs 177 entries nobody derived. All grandfathered derived:false — deliberate and correct for a
    non-destructive cutover, but it means the signature attests provenance only for post-cutover entries.
    SIGNING.md should say that plainly so a consumer doesn't over-read it.
  5. issued_at only advances when membership changes. A list that sits unchanged never gets re-signed, so the
    planned 60-day expiry can't be satisfied without also re-signing on age. No format change needed later — just
    don't let #1788/fluxbench ship a freshness rule before mustSign learns about age.
  6. The GitHub host key is pinned to a single ed25519 line. Right call; add it to SIGNING.md's rotation touchpoints
    so a GitHub key rotation is diagnosed in seconds rather than hours.
  7. A bad claimed_hash aborts the entire run, including the unrelated legitimate refs in that sweep. Documented and
    deliberate, self-heals next run — flagging only so it isn't a surprise in the logs.

The derivation pipeline yields d41d8cd98f00b204e9800998ecf8427e -- the md5 of an
empty stream -- whenever nothing was hashed, and that value is a well-formed 32-hex
string, so the existing shape guard passed it straight through to the signature. It
means "a node whose ZelBack holds no regular files is genuine FluxOS", and membership
is monotonic, so one accident becomes a listed entry only a cull PR removes.

Two distinct ways in, and only one of them is a pipeline failure:

  ZelBack absent          find errors, but the pipeline's status is awk's, so without
                          pipefail the caller sees success
  ZelBack present, empty  find exits 0 and emits nothing -- pipefail cannot see this

Mutation-tested rather than assumed: with the empty-hash guard removed, the second
case lists the empty hash; with pipefail removed, the guard still catches both. So the
guard is what makes this correct and pipefail is defence in depth -- it makes the
absent case fail on find's own error rather than laundering it into a hash comparison.

pipefail is passed as a bash argument rather than folded into TREE_HASH_PIPELINE so
the constant stays byte-identical to the pipeline flux CI runs; that identity is what
makes claimed_hash comparable at all.

Failure routing needed no new code: throwing here already means a red run when the
commit was dispatched, and a logged skip that retains the ref's old snapshot entry
when it was found by the sweep. A permanently non-code branch therefore cannot wedge
publication.

Reported by Cabecinha84 on #2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQKpZbuxYqrWeErU7vKgoA
The header claimed "a hash value can enter the list through no other path". That is
false, and it was load-bearing: it is the reasoning behind accepting looser pins
elsewhere, so it needed correcting whether or not the gap gets closed.

A fork network shares one object store, so every commit ever pushed to any public fork
of flux stays anonymously fetchable by SHA from the official remote -- and a dispatched
commit matching no ref is derived anyway, on purpose, because a branch can move past a
commit before the signer looks. "Fetchable from RunOnFlux/flux" is therefore a much
larger set than "a tree RunOnFlux authored", and a dispatch credential can name anything
in it.

What is true is narrower: a dispatch carries pointers rather than hash values, so the
credential needs no write authority. That bounds the credential; it does not vouch for
the tree. And the credential lives in flux's repository secrets, reachable by anyone who
can land a workflow change on any branch there -- who can already get a hash listed by
pushing one. Same trust boundary, not a defence against it.

Wording only. The ancestry check that would close the gap is not taken: it would refuse
commits on no ref, which are legitimate after a force-push or a deleted branch and are
supported deliberately.

Reported by Cabecinha84 on #2.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQKpZbuxYqrWeErU7vKgoA
actions/checkout v4 -> v7, actions/setup-node v4 -> v7.

checkout v7 refuses to fetch fork pull request code under pull_request_target and
workflow_run, the configuration behind pwn request attacks. Neither workflow here uses
those triggers -- validate runs on plain pull_request, which GitHub already denies
secrets and write to -- so this changes nothing today. It is worth taking anyway: it
makes the dangerous configuration fail closed if one is ever added, rather than
depending on the next author knowing why not to.

setup-node's v5 automatic caching does not engage here: it keys off a packageManager
field in package.json, which this repository does not set, and validate installs
nothing regardless -- setup-node only provides the runtime.

Prompted by Cabecinha84's note on #2 about fork pull requests executing the list file.
That remains true and remains negligible: fork runs get no secrets, a read-only token
and an ephemeral runner. Its permissions: {} suggestion is not taken -- the workflow
already declares contents: read, which on a public repository grants only what is
already public.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQKpZbuxYqrWeErU7vKgoA
The 43 assertions that justified this PR lived in a session scratchpad and were never
committed, so npm test printed "No tests have been implemented" and exited 1. The
evidence was real but unreproducible by anyone else, and nothing guarded against
regressions.

The harness drives scripts/sign-hashlist.js against a git remote it builds itself:
bootstrap grandfathering, derivation and labelling, no-op and state-only runs, an
orphaned dispatched commit, claimed-hash mismatch, culls and their retained audit
rows, duplicates, tag annotation, corrupt provenance, corrupt ledger, a wiped
document, and v1 record compatibility. No network, no secrets, nothing outside a temp
dir -- so it runs on fork pull requests like the rest of validate.

Restructured to live in the repository: the working copy, fake remote and all scratch
output moved into mktemp with a trap, the source is taken from the repository root
rather than a sibling checkout, and .git and node_modules are excluded from the copy.
Verified it leaves the working tree clean.

Two cases are new, covering the defect this PR fixed one commit earlier. They are
split because the two ways in have different guards, and each asserts the message it
should die on -- asserting only "the run went red" passes for any reason and tests
nothing. Case 17 removes ZelBack and expects the pipeline's own failure; case 18 gives
ZelBack only symlinks, so find matches nothing and exits 0, which pipefail cannot see
and only the value check catches. Git cannot store an empty directory, so a symlink is
what "present but nothing to hash" actually looks like in a commit.

Mutation-tested, because coverage proves execution and not that an assertion would
notice. Deleting the empty-hash guard fails case 18 with "EMPTY HASH REACHED THE
LIST"; deleting pipefail fails case 17, which then observes the guard firing instead.
An earlier draft of both cases passed under both mutations -- it asserted redness
without asserting cause, and its "empty directory" fixture had silently collapsed into
the absent-directory case.

Raised by Cabecinha84 on #2 as his main process ask.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQKpZbuxYqrWeErU7vKgoA
…ouchpoint

Two review notes from Cabecinha84 on #2, both about a reader over-reading what is
there.

The cutover carries the existing entries across as derived: false, which is correct
for a non-destructive cutover but means the signature attests provenance only for what
the signer derived afterwards. Said plainly next to the row description, so nobody
reads a grandfathered row as "the signer fetched this commit and checked it".

The workflow pins GitHub's SSH host key rather than trusting ssh-keyscan at run time.
That is the right call and it is also a rotation touchpoint: if GitHub rotates that
key the push fails host verification and the run goes red looking exactly like a
credential fault. Recorded beside the deploy key rotation steps so it is diagnosed in
seconds.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HQKpZbuxYqrWeErU7vKgoA
@MorningLightMountain713

Copy link
Copy Markdown
Collaborator Author

Both findings were real. Serviced, plus five of the seven notes — tip is now 13732fed, five commits past the 9fda0cd76 you reviewed.

Finding 2, the empty-tree hash — fixed first, because it was the worse one (d8238412). You are right that it is latent rather than live, and I would add one thing in the design's favour that also makes it less urgent: the cutover snapshots existing refs without deriving them, so nothing currently published can trip it. Only a ref that moves after cutover can.

Your diagnosis of the two ways in is exactly right, and the second is the one that matters: ZelBack absent is a pipeline failure that pipefail catches, but ZelBack present holding no regular files is not — find exits 0 and emits nothing, and no shell option can see that. So the value check is what actually closes this and pipefail is defence in depth. I mutation-tested that rather than assuming it: with the value check removed, the empty hash reaches the list; with pipefail removed, the value check still catches both. pipefail earns its place by making the absent case die on find's own error instead of laundering it into a hash comparison.

pipefail is passed as a bash argument rather than folded into the constant, so TREE_HASH_PIPELINE stays byte-identical to the one flux CI runs — that identity is the whole basis of the claimed_hash comparison. The matching guard is on the flux side too (RunOnFlux/flux#1789, fd184c6af), so the failure lands in the repository that caused it rather than as a puzzling red run over here.

Finding 1, the fork object store — the claim is corrected (636fca10). You are right, and it was worse than one sentence: the false claim appeared in the script header, the workflow header, this PR's body, and both flux PRs. All now say a dispatch is trusted to name a commit reachable in flux's object store, forks included, and that this is the same trust boundary as pushing a branch rather than a defence against it.

I have not taken the ancestry check, and I want to be explicit rather than quietly skip it. It would refuse a commit that is an ancestor of nothing upstream — but that is also the shape of a legitimate commit whose branch was force-pushed or deleted after CI ran, which is exactly why the on-no-ref path exists and was proven deliberately on 2026-08-24. So the choice is defence-in-depth against an attacker who already holds a credential they could reach the same outcome with, paid for by dropping support for orphaned commits. Happy to revisit if you think orphaned commits are not worth keeping, but I did not want to trade a working case for a bound that does not move the realistic attacker.

Note 1, the harness — this was your best point, and it is done (1892bdf1). npm test now runs 43 assertions instead of printing "No tests have been implemented" and exiting 1, and validate runs them on every PR. It drives the reconciler against a git remote it builds itself in a temp dir — no network, no secrets, so it runs on fork PRs like everything else here.

Two cases are new and cover finding 2. Getting them right took a second attempt worth mentioning, because the first version was worse than useless: it asserted only that the run went red, which passes for any reason at all, and its "empty directory" fixture had silently collapsed into the absent-directory case — git cannot store an empty directory. The reachable shape is a ZelBack holding only symlinks, which find -type f skips. Both cases now assert the message they should die on, so each is sensitive to its own guard.

Note 2 — this one I think is wrong, and it changes the cutover, so it is worth settling. Merging does trigger the signer. The push trigger watches src/hashes/ledger.json, and this PR creates that file, so the merge commit touches exactly the watched path. It was observed twice on 2026-08-24: the sandbox setup push carried the commit that adds both sign-hashlist.yml and ledger.json, and fired run 32714659908 (event: push, workflow sign-hashlist) — a workflow introduced by the very push that triggers it. Rung 5 of the same rehearsal saw a cull PR's merge fire the signer independently.

Your underlying point stands regardless and is the useful half: the run starts the instant the merge lands, so the environment, secrets and deploy key must all be in place before merging, not after. They are — I verified the deploy key is the repository's only write key and both environment secrets are present. That same sandbox run is the evidence, incidentally: it went red because it fired before its deploy key was effective.

Note 3 — your premise is right and my first reading was wrong. I initially thought the outputs guard closed this, because it runs before anything loads the list. It does not: pull_request runs the PR head's copy of the workflow, so a hostile PR can weaken the guard it is judged by — which is precisely what the comment above that step already says.

I have not taken permissions: {}, for a different reason than disagreement. The workflow already declares contents: read, and on a public repository that grants only what is already public, so it removes nothing an attacker could use. What actually contains this is that fork runs get no secrets, a read-only token, an ephemeral runner, and — unusually — no dependency install. I did bump actions/checkout to v7 (f2df1298), which refuses to fetch fork PR code under pull_request_target and workflow_run. That changes nothing today, since neither trigger is used here; it makes the dangerous shape fail closed if someone adds one later.

Notes 4 and 6 — both applied (13732fed). SIGNING.md now says the signature attests provenance only for the entries the signer derived, so a grandfathered row is not read as "the signer fetched this commit and checked it"; and the pinned SSH host key is recorded as a rotation touchpoint next to the deploy key, so a GitHub key rotation is diagnosed in seconds rather than mistaken for a credential fault.

Note 5 — agreed, and noted where it will bite. issued_at only advances when membership changes, so an unchanged list is never re-signed and an age-based freshness rule cannot be satisfied until mustSign learns about age. Nothing to change here; the constraint is that #1788 and fluxbench must not ship a freshness rule before that lands.

Note 7 — correct, and deliberate. A bad claimed_hash aborts the whole sweep including the innocent refs in it. That is the same argument as last round: a mismatch is indistinguishable from signer-side drift, and drift that publishes "just the other commits" is drift publishing silently. It self-heals on the next clean run, and the availability cost is stated in the body.

Also in this range: actions/setup-node to v7 alongside checkout.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants