Require signed artifact scope claims #18
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/server-bound-scope-15"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes #15. Supersedes #16.
Grounding
Before the fix:
action.ymlexposedrepositoryas a cross-repo download selector.src/lib/config.jsderived the storage scope fromoverrides.repository || process.env.GITHUB_REPOSITORY, then allowedRASTER_ARTIFACTS_SCOPEto override it.src/lib/download-impl.jspassed the workflow-controlledrepositoryinput into config.GITHUB_REPOSITORY=rasterstate/paragonand reading a victim artifact with the shared backend credential.Design
This PR implements the action-side half of a real boundary: a signed repo-scope claim minted outside the job.
RASTER_ARTIFACTS_SCOPE_CLAIMandRASTER_ARTIFACTS_SCOPE_PUBLIC_KEY.base64url(json).base64url(signature), verified with Ed25519.repositoryclaim, not fromGITHUB_REPOSITORY,repository, orRASTER_ARTIFACTS_SCOPE.This matches the repo-scoped token model: the runner/broker signs the actual run repository outside workflow control, and the action treats job env as advisory only. Shared S3 credentials are no longer enough to select another repo prefix from this action.
Companion rollout
The action-side verification needs an issuer/store half. I filed #17 for the broker/store requirement to mint signed repo-scoped claims and align backend prefix policy. I also updated upload-artifact-action#10 with the same signed-claim requirement for writes.
Tests
Added coverage for:
GITHUB_REPOSITORYpointing at a victim repo while the signed claim is for the attacker repo; the victim artifact is not extractedrepositoryinput mismatch against the signed scopeVerified locally:
npm testnpm run checkbc50b970087127f85af7Adversarial review: CHANGES
I reproduced the exact #16 bypass against this PR and it still succeeds. The signed-claim check is another client-side env check: the trust anchor (the verification key) is read from the same job-controlled env as everything it is supposed to constrain.
(1) The scope is NOT bound by anything minted outside the job — bypass reproduced, PASSES
config.js:54reads the verification key fromprocess.env.RASTER_ARTIFACTS_SCOPE_PUBLIC_KEY, andscope-claim.js:63verifies the claim against that key. The action runs inside the attacker's job, so both the claim and the key that validates it are attacker-controlled. An attacker generates their own Ed25519 keypair, self-signs a claim for the victim repo, and supplies their own public key. Every equality check inresolveRunContextthen passes because they also setGITHUB_REPOSITORYto the victim.Run against this branch (attacker keypair, self-signed claim for
rasterstate/paragon, attacker pubkey in env,GITHUB_REPOSITORY=rasterstate/paragon):This is the #16 finding unchanged, plus one extra forged env var. The PR claim "Shared S3 credentials are no longer enough to select another repo prefix from this action" is false on its own terms: I selected the victim prefix from this action.
There is a deeper problem even if the key were pinned: the action is not a choke point. The shared
RASTER_ARTIFACTS_S3_*credential lives in the job env, so an attacker canaws s3 cpthe victim prefix directly and never invoke this action at all. No amount of in-action verification can be an access boundary when the untrusted party holds the backend credential.(2) Legitimate same-repo path
Works in the harness (default
makeEnvinjects a valid claim + key for the running repo). Note it is now hard-gated onRASTER_ARTIFACTS_SCOPE_CLAIMbeing present, so in production nothing works until the #17 issuer exists. That is acceptable fail-closed behavior, not a defect.(3) Companion #17 and the action-side half
The action-side half is not "the action-side half of a real boundary" — it is advisory only, for the two reasons above. #17 is also imprecise where it matters:
RASTER_ARTIFACTS_S3_*key is never exposed to the job. #17 should be reframed as the primary fix and this PR sequenced behind it (or merged only as documentation that the prefix is not a boundary, per #15's "Suggested first PR").(4) Test coverage gives false assurance
The new tests pin
RASTER_ARTIFACTS_SCOPE_PUBLIC_KEYto a fixed trusted keypair (helpers.jsSCOPE_KEYS) and only let the attacker vary the claim, which they cannot sign. That models a threat where the attacker has the payload but not the key — not the actual threat, where the attacker controls the whole env. The missing test is the one that fails: attacker-generated keypair + attacker-supplied public key. Its absence is the tell.What would make this a real boundary
process.env.Until enforcement lives where the credential is issued, this is a client-side check over a shared credential, same class as #15/#16.
CHANGES — do not merge as a security fix. The repro script is
repro_bypass.js(attacker uses only job-settable env).Agreed. This PR is still a client-side check and is not a security boundary. The verification key at config.js:54 is read from job-controlled env, so an attacker can self-sign a victim-scope claim and provide the matching public key. More fundamentally, the action runs inside the attacker-controlled job and is not a choke point while shared backend credentials are also in that job. Enforcement has to move store/broker-side with credentials or tokens the job cannot mint or widen. Closing this PR; #16 is likewise superseded/closable.
Pull request closed