[CIP-4057] Add federated Claude PR review

[tobyhede]

Summary

• add advisory Claude review for trusted, same-repository pull requests
• authenticate through Anthropic workload identity federation with no API key
• restrict permissions and Claude tools to repository reads and PR comments
• add focused workflow, fork-boundary, and OIDC classification guards
• document the non-publishing OIDC exchange and add the required changeset

Validation

• pnpm test:scripts — 922 passed
• pnpm --dir e2e exec vitest run tests/supply-chain.e2e.test.ts — 26 passed
• pnpm exec turbo run typecheck — 16 tasks passed
• actionlint .github/workflows/claude-review.yml
• focused Biome checks and pnpm exec changeset status

The complete pnpm test run reached the package suites but the CLI live-test
collector could not resolve the locally unavailable
@cipherstash/eql-upgrade-baseline/sql; 90 other CLI test files passed.

Fixes CIP-4057

[changeset-bot]

🦋 Changeset detected

Latest commit: 7d54c27

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
stash	Patch
@cipherstash/wizard	Patch
@cipherstash/basic-example	Patch
@cipherstash/e2e	Patch
@cipherstash/stack-drizzle	Patch
@cipherstash/stack-prisma	Patch
@cipherstash/stack-supabase	Patch
@cipherstash/stack	Patch
@cipherstash/bench	Patch
@cipherstash/prisma-example	Patch
@cipherstash/test-kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[freshtonic]

Approving. The security posture is right and the guards are real, not decorative. Three follow-ups below, none blocking.

What I checked and agree with

• Fork boundary. pull_request rather than pull_request_target, plus an explicit head.repo.full_name == github.repository job condition, and the job is registered in EXPECTED_FORK_GUARDED_JOBS. The trigger set is pinned by an equality (Object.keys(triggers)), so the dead github.event_name != 'pull_request' disjunct cannot quietly become a bypass when somebody adds a second trigger — that test fails first. Worth keeping the equality in mind if the disjunct ever tempts anyone to delete it.
• No static credentials. anthropic_api_key and claude_code_oauth_token are asserted absent, not merely omitted. The action is SHA-pinned and the SHA is pinned again in the test. persist-credentials: false on the checkout.
• Least privilege. contents: read at workflow level, the three scopes granted per job, and pull-requests: write justified by the sticky comment and the inline-comment MCP tool.
• Prompt construction. No user-controlled string is interpolated into the prompt — only repository, PR number and head SHA. That closes the script-injection path that usually sinks these workflows, and it is the part most PRs of this kind get wrong. The tool denylist covers the exfiltration and mutation routes.
• Housekeeping. Changeset present and correctly scoped to stash and @cipherstash/wizard, since skills/ ships in both tarballs. stash-supply-chain-security is the right skill to have touched.

Follow-up 1 — the check is green and the review did not run

On this PR the action skipped:

Skipping action due to workflow validation: Workflow validation failed. The workflow file must exist and have identical content to the version on the repository's default branch.

The step then reported outcome=success and the review check passed. The vendor says this is expected when the workflow file itself is in the diff, and it is — nothing to fix here, and nothing gates on it today, since main carries no branch protection.

The durable problem is the shape. Any future PR that edits claude-review.yml gets a green review check that reviewed nothing, and nothing says so. This repository argues against precisely that elsewhere — workflow-publish-permissions.test.mjs pins the OIDC holder set because "a scan that finds none of them passes having verified nothing", and integration-workflow-paths.test.mjs carries a required floor for the same reason.

I do not think it blocks the merge, because the behaviour is the vendor's and the first real run only happens after this lands. Worth a follow-up that either fails the job on the validation skip, or records the behaviour so the next person does not read a green tick as a review.

Follow-up 2 — the classification exempts the file, not just the job

NON_PUBLISH_OIDC_JOBS correctly keeps the Claude job out of the publisher equality. But it also drops the whole file out of the "leaves the non-publishing jobs of a publishing workflow read-only" scan, because that filter now requires an OIDC holder that is not classified.

Nothing is lost today: the workflow has one job, and claude-review-workflow.test.mjs pins its permissions exactly. The gap is a second job added later with, say, contents: write and no id-token — the equality on OIDC holders would not see it, the sibling scan no longer covers the file, and no test pins the job set.

Cheapest fix is an equality on Object.keys(workflow.jobs) in the dedicated test. Alternatively keep the file inside the sibling scan and exempt only the publisher-specific assertions. Either way the reasoning in the new doc comment stays correct.

Follow-up 3 (documentation — for a separate PR)

SECURITY.md now says the test holds publishers and named non-publishing exchanges as "separate equalities". There is one equality, and it is over the union: holders.sort() against OIDC_JOBS.sort(). The two lists are separate lists feeding separate predicates, and REPO_WRITE_JOBS is a spread of the union rather than an independent equality. The distinction matters in this file specifically, because it is what a security reporter reads to work out which guard would have caught a given mistake.

Everything else in the SECURITY.md and skill wording reads accurately, including "keep its static credential inputs absent", which the test does enforce.

Note, not a finding

The four federation identifiers are echoed into a public build log by the preflight step. That is inherent to holding them in vars rather than secrets, and it is the right call — the trust sits in the OIDC token's repository and workflow claims, not in the identifiers. Flagging only so the choice is on the record as deliberate.