fix(ci): Give eval-crate its own Bedrock role instead of the reviewer's

[leongdl]

The first real eval-crate run failed at the assume-role step (run 32084700533): 12 STS retries, then Not authorized to perform sts:AssumeRoleWithWebIdentity. No retry fixes it, and the fix is not only an IAM change — so this PR makes the workflow inert and records why.

Why it failed

AWS_CLAUDE_PR_REVIEW_ROLE looks like it should work. Its trust policy admits repo:OpenJobDescription/*:*, which covers this repo. But the same condition block also requires:

token.actions.githubusercontent.com:job_workflow_ref LIKE
  aws-deadline/.github/.github/workflows/reusable_claude_pr_review.yml@*

Conditions in one block are ANDed, and job_workflow_ref names the workflow file the job was defined in — not the caller. Only a job running inside that reusable workflow can satisfy it. This job is defined in eval_crate.yml, so STS refuses.

That is the pin working as designed. It is mitigation M-19 of the Claude PR-review threat model, and its stated purpose is that a caller workflow in an allowed org cannot mint the Bedrock token by itself. This workflow is that caller. Widening the pin would remove the guarantee for the reviewer too.

What this PR does

Points at a dedicated secret, AWS_EVAL_CRATE_ROLE (naming follows the existing AWS_CLAUDE_PR_REVIEW_ROLE), and until it exists makes every trigger no-op with a notice instead of spending four minutes on a cargo build and then failing:

• plan gains a presence check on the secret and refuses the run when unset. secrets is not available in if:, so it has to be a step; it tests presence only and never echoes the value.
• A notice, not a failure. An unconfigured optional workflow should be inert, and a weekly red X trains people to ignore red. The notice names the missing secret, so "why hasn't the sweep run?" has a greppable answer.
• The header records the trust policy a new role would need, and the security decision that has to precede it (below).

Also corrects a comment: role-duration-seconds: 3600 is not merely "what the org uses", it is the ceiling — these roles carry a one-hour maxSessionDuration specifically to bound the token TTL, so a larger value is refused rather than honoured. The 55-minute job budget is a hard constraint.

This needs a security decision, not just a role

I went to write the CDK for a sibling role and read the threat model first. This workflow contradicts several of its load-bearing mitigations by construction:

mitigation	as written	this workflow
M-7 — no execute-arbitrary tools	allowlist is read-only: Read, Grep, Bash(cat:*), gh api	holds Bash(cargo …), Bash(python3:*), Write, Edit, subagents
M-20 — no contributor scripts run	agent reads PR-head files, never executes them	compiles and runs the crate, including build.rs, by design
M-25 / M-10 / M-17 — privileged stage runs the base-branch reusable workflow	two-stage pull_request + workflow_run so a PR cannot alter the privileged definition	PR mode runs this file as it exists on the PR branch, with credentials
M-3 — dedicated account, "no other roles"	one role, one purpose	proposes a second role in that account

Two consequences worth stating plainly. T-4 (token exfiltration via callback) is marked Mitigated because M-7 forbids outbound HTTP — Bash(python3:*) reinstates it, so for this workflow that threat is open, not mitigated. And T-16 (PR content executes) is mitigated there by never executing; here it is mitigated only by same-repo + maintainer-label gating, i.e. by trusting the contributor rather than by design.

I think that trade is defensible for a maintainer-triggered quality report on a same-repo branch — the token stays Bedrock-only and one hour, the IAM credentials are dropped before the agent runs, and fork PRs are refused. But it is a different trade from the one AppSec looked at, so it should be its own threat-model decision and plausibly its own AWS account, rather than inheriting a model written for a read-only reviewer. I have not written the IAM code, because writing it first would put a role in that account ahead of the decision that justifies it.

Owners for that call: BealineClientSoftwareCDK / BealineAccountsCDK (bindle BealineClientSoftware, team thinkbox-render-bender) for the role and account, and the threat model's author for the model delta.

Options, if the answer is "not worth it"

The gate makes all three cheap: leave it off indefinitely, revert the workflow and keep eval-crate a local skill, or do the account/threat-model work and set one secret. Nothing else in the repo depends on it.

Verification

• Workflow parses; 9 structural assertions pass (secret wired in both places, value never echoed, check ordered before the decision, no remaining use of the review role, duration and budget unchanged, trust conditions documented).
• The real plan script was extracted and run over 8 cases: all three triggers refuse when unconfigured, all three proceed when configured, and the fork-PR and unknown-crate refusals still hold. Throwaway harness, not checked in.
• Cannot be exercised end to end, which is the point of the gate.

[github-actions]

Ordering this check before the trigger dispatch makes the fork-PR skip reason unreachable, and replaces it with a misleading one.

Repository secrets are not exposed to workflows triggered by pull_request from a fork, so for a labelled fork PR secrets.AWS_EVAL_CRATE_BEDROCK_ROLE is always empty, configured=false, and this branch fires first. The notice then reads

AWS_EVAL_CRATE_BEDROCK_ROLE is not set; see the workflow header for the role this needs

even when the secret is configured. The reason=fork PR; eval-crate runs only on same-repo branches message at line 210 can never be emitted for an actual fork PR — the only event class it was written for. A maintainer who labels a fork PR is told the workflow is misconfigured rather than that fork PRs are deliberately out of scope, and (per the comment above) the notice is meant to be the greppable explanation for why a run did not happen.

Doing the same-repo test before the secret test fixes both: move the pull_request fork check ahead of the CONFIGURED gate, or fold the fork case in, e.g.

if [ "$EVENT" = pull_request ] && [ "$HEAD_REPO" != "$THIS_REPO" ]; then
  echo "run=false" >> "$GITHUB_OUTPUT"
  echo "reason=fork PR; eval-crate runs only on same-repo branches" >> "$GITHUB_OUTPUT"
  exit 0
fi
if [ "$CONFIGURED" != "true" ]; then
  ...

[github-actions]

The incident this step cites is not one a presence check can catch.

Not authorized to perform sts:AssumeRoleWithWebIdentity, preceded by STS retries, is what STS returns when the ARN was supplied and the role's trust policy refused the token — which is exactly the AWS_CLAUDE_PR_REVIEW_ROLE / job_workflow_ref mismatch the new header section documents at length. An absent secret fails differently and earlier: configure-aws-credentials gets an empty role-to-assume, never reaches AssumeRoleWithWebIdentity, and errors on missing credentials instead.

So the step's stated justification describes the one failure mode it does not prevent. Set-but-wrong ARN — the far more likely state once someone creates a role from the header's spec and gets a condition subtly wrong — still burns the full checkout + cargo build before failing, because configured=true is satisfied by any non-empty string.

The step is still worth having for the genuinely-unset case (that is what makes the workflow inert until the secret exists, per the header). The comment just needs to say that rather than attributing the STS-authorization incident to it, e.g. "the secret does not exist yet, so every trigger would otherwise reach the assume-role step and fail there after a full build; presence is all that can be checked from here — a wrong ARN still fails late."