Require signed artifact scope claims #18

Closed
stephen wants to merge 1 commit from fix/server-bound-scope-15 into main
Owner

Fixes #15. Supersedes #16.

Grounding

Before the fix:

  • action.yml exposed repository as a cross-repo download selector.
  • src/lib/config.js derived the storage scope from overrides.repository || process.env.GITHUB_REPOSITORY, then allowed RASTER_ARTIFACTS_SCOPE to override it.
  • src/lib/download-impl.js passed the workflow-controlled repository input into config.
  • #16 only compared workflow-controlled values to other workflow-controlled values. The CHANGES review reproduced the bypass by setting GITHUB_REPOSITORY=rasterstate/paragon and 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.

  • The action now requires RASTER_ARTIFACTS_SCOPE_CLAIM and RASTER_ARTIFACTS_SCOPE_PUBLIC_KEY.
  • The claim is base64url(json).base64url(signature), verified with Ed25519.
  • The storage scope comes from the signed repository claim, not from GITHUB_REPOSITORY, repository, or RASTER_ARTIFACTS_SCOPE.
  • If those job-controlled values disagree with the signed claim, the action fails before backend access.

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:

  • forged GITHUB_REPOSITORY pointing at a victim repo while the signed claim is for the attacker repo; the victim artifact is not extracted
  • repository input mismatch against the signed scope
  • invalid claim signature refusal before backend access

Verified locally:

  • npm test
  • npm run check
Fixes #15. Supersedes #16. # Grounding Before the fix: - `action.yml` exposed `repository` as a cross-repo download selector. - `src/lib/config.js` derived the storage scope from `overrides.repository || process.env.GITHUB_REPOSITORY`, then allowed `RASTER_ARTIFACTS_SCOPE` to override it. - `src/lib/download-impl.js` passed the workflow-controlled `repository` input into config. - #16 only compared workflow-controlled values to other workflow-controlled values. The CHANGES review reproduced the bypass by setting `GITHUB_REPOSITORY=rasterstate/paragon` and 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. - The action now requires `RASTER_ARTIFACTS_SCOPE_CLAIM` and `RASTER_ARTIFACTS_SCOPE_PUBLIC_KEY`. - The claim is `base64url(json).base64url(signature)`, verified with Ed25519. - The storage scope comes from the signed `repository` claim, not from `GITHUB_REPOSITORY`, `repository`, or `RASTER_ARTIFACTS_SCOPE`. - If those job-controlled values disagree with the signed claim, the action fails before backend access. 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: - forged `GITHUB_REPOSITORY` pointing at a victim repo while the signed claim is for the attacker repo; the victim artifact is not extracted - `repository` input mismatch against the signed scope - invalid claim signature refusal before backend access Verified locally: - `npm test` - `npm run check`
Require signed artifact scope claims
Some checks failed
test / unit (pull_request) Successful in 6s
ci / test (pull_request) Successful in 7s
test / e2e (pull_request) Failing after 5s
bc50b97008
stephen force-pushed fix/server-bound-scope-15 from bc50b97008
Some checks failed
test / unit (pull_request) Successful in 6s
ci / test (pull_request) Successful in 7s
test / e2e (pull_request) Failing after 5s
to 7127f85af7
All checks were successful
test / e2e (pull_request) Successful in 5s
test / unit (pull_request) Successful in 6s
ci / test (pull_request) Successful in 6s
2026-07-05 20:47:02 +00:00
Compare
Author
Owner

Adversarial 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:54 reads the verification key from process.env.RASTER_ARTIFACTS_SCOPE_PUBLIC_KEY, and scope-claim.js:63 verifies 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 in resolveRunContext then passes because they also set GITHUB_REPOSITORY to the victim.

Run against this branch (attacker keypair, self-signed claim for rasterstate/paragon, attacker pubkey in env, GITHUB_REPOSITORY=rasterstate/paragon):

Artifact backend: local:/tmp/repro-store-...
Repository: rasterstate/paragon  run: 999
Verified artifact "dist" sha256 digest.
Downloaded artifact "dist" to /tmp/repro-dest-...

*** BYPASS SUCCEEDED — victim secret extracted: "VICTIM-SECRET"

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 can aws s3 cp the 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 makeEnv injects a valid claim + key for the running repo). Note it is now hard-gated on RASTER_ARTIFACTS_SCOPE_CLAIM being 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:

  • It says "Jobs receive only the claim and the public verification key." Handing the job the verification key is exactly the flaw: a job that can set the key can forge the whole chain. The verifier's trust anchor must be outside job control.
  • It frames the store/credential change as "narrow store credentials to the same repository prefix where possible" — a secondary nice-to-have. That server-side change is the only real boundary. The primary fix is: the broker validates the run's identity (signed claim or Forgejo OIDC) and hands back per-repo, prefix-scoped, short-lived credentials, and the shared long-lived 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_KEY to a fixed trusted keypair (helpers.js SCOPE_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

  1. Move enforcement server-side: broker/store validates run identity and issues per-repo scoped, short-lived credentials; the shared S3 key never reaches the job. (This is #17, promoted to primary.)
  2. If any in-action check remains, its verification key must be a non-job-overridable trust anchor (pinned in the action release or fetched from a trusted endpoint), never process.env.
  3. Add the attacker-keypair test above and assert it is rejected.

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).

## Adversarial 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:54` reads the verification key from `process.env.RASTER_ARTIFACTS_SCOPE_PUBLIC_KEY`, and `scope-claim.js:63` verifies 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 in `resolveRunContext` then passes because they also set `GITHUB_REPOSITORY` to the victim. Run against this branch (attacker keypair, self-signed claim for `rasterstate/paragon`, attacker pubkey in env, `GITHUB_REPOSITORY=rasterstate/paragon`): ``` Artifact backend: local:/tmp/repro-store-... Repository: rasterstate/paragon run: 999 Verified artifact "dist" sha256 digest. Downloaded artifact "dist" to /tmp/repro-dest-... *** BYPASS SUCCEEDED — victim secret extracted: "VICTIM-SECRET" ``` 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 can `aws s3 cp` the 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 `makeEnv` injects a valid claim + key for the running repo). Note it is now hard-gated on `RASTER_ARTIFACTS_SCOPE_CLAIM` being 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: - It says "Jobs receive only the claim and the public verification key." Handing the job the verification key is exactly the flaw: a job that can set the key can forge the whole chain. The verifier's trust anchor must be outside job control. - It frames the store/credential change as "narrow store credentials to the same repository prefix **where possible**" — a secondary nice-to-have. That server-side change is the *only* real boundary. The primary fix is: the broker validates the run's identity (signed claim or Forgejo OIDC) and hands back **per-repo, prefix-scoped, short-lived** credentials, and the shared long-lived `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_KEY` to a fixed trusted keypair (`helpers.js` `SCOPE_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 1. Move enforcement server-side: broker/store validates run identity and issues per-repo scoped, short-lived credentials; the shared S3 key never reaches the job. (This is #17, promoted to primary.) 2. If any in-action check remains, its verification key must be a non-job-overridable trust anchor (pinned in the action release or fetched from a trusted endpoint), never `process.env`. 3. Add the attacker-keypair test above and assert it is rejected. 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).
Author
Owner

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.

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.
stephen closed this pull request 2026-07-05 20:57:07 +00:00
All checks were successful
test / e2e (pull_request) Successful in 5s
test / unit (pull_request) Successful in 6s
ci / test (pull_request) Successful in 6s

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
rasterstate/download-artifact-action!18
No description provided.