Skip to content

ci: add protected coordinator token rotation - #1216

Merged
vincentkoc merged 1 commit into
mainfrom
fix/coordinator-admin-token-rotation
Aug 1, 2026
Merged

ci: add protected coordinator token rotation#1216
vincentkoc merged 1 commit into
mainfrom
fix/coordinator-admin-token-rotation

Conversation

@vincentkoc

Copy link
Copy Markdown
Member

Related: #1208

What Problem This Solves

Resolves an operational problem where the coordinator admin token could not be rotated from the protected GitHub environment, leaving brokered image publication blocked when the existing token was unavailable locally.

Why This Change Was Made

Adds a manual, protected-default-branch workflow that reads a one-time next token only inside the coordinator environment and sends it to Wrangler over stdin. The workflow requires exact protected-source identity, keeps credentials out of checkout and dependency-install steps, and never places the token on argv.

User Impact

Maintainers can rotate the production coordinator admin token without local Cloudflare credentials or exposing the token in workflow inputs. The staging secret can be deleted immediately after rotation.

Evidence

  • actionlint .github/workflows/coordinator-admin-token-rotate.yml
  • node --test scripts/coordinator-admin-token-rotation-workflow.test.js scripts/workflow-action-pins.test.js — 3 passed
  • git diff --check
  • autoreview was not run because its secret-path guard refuses credential-rotation workflow filenames; this change is workflow metadata plus its static contract test

@vincentkoc
vincentkoc marked this pull request as ready for review August 1, 2026 07:41
@vincentkoc
vincentkoc enabled auto-merge (rebase) August 1, 2026 07:41
@clawsweeper clawsweeper Bot added rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Aug 1, 2026
@clawsweeper

clawsweeper Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed August 1, 2026, 3:45 AM ET / 07:45 UTC.

ClawSweeper review

What this changes

The PR adds a manually dispatched GitHub Actions workflow that rotates the Cloudflare coordinator’s CRABBOX_ADMIN_TOKEN from a protected GitHub environment secret through Wrangler stdin, plus static contract coverage.

Merge readiness

⚠️ Ready for maintainer review - 5 items remain

Keep this member-authored PR open for normal maintainer review. The source-level workflow is narrowly scoped and follows the repository’s existing protected-workflow pattern, but the production coordinator environment’s live approval and secret-access policy must be confirmed before a new credential-rotation path is accepted; also remove the release-owned changelog entry from the branch.

Priority: P2
Reviewed head: d1abe6937b917a521f6095d6395509cf9dd0cef2
Owner decision: Required. See Decision needed.

Review scores

Measure Result What it means
Overall readiness 🐚 platinum hermit (4/6) The focused workflow follows established security controls and has static regression coverage, with a minor release-process correction and a live environment-policy check remaining.
Proof confidence 🌊 off-meta tidepool Not applicable: This member-authored operational workflow is not subject to the external-contributor real-behavior-proof gate; the PR supplies static validation, while any production rotation should be verified through a controlled, redacted maintainer run.
Patch quality 🐚 platinum hermit (4/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Not applicable Not applicable: This member-authored operational workflow is not subject to the external-contributor real-behavior-proof gate; the PR supplies static validation, while any production rotation should be verified through a controlled, redacted maintainer run.
Evidence reviewed 5 items Protected workflow implementation: The proposed workflow is manual-only, checks the default-branch workflow identity, requires a protected ref, checks that the workflow and run SHAs match, and checks out the workflow SHA without persisted credentials before using the environment secrets.
Existing protected-workflow pattern: Current main already uses the same protected-default-branch guard and workflow-SHA checkout pattern for image publication, so the new workflow aligns with an established repository control.
Coordinator authorization contract: The Worker grants admin access only when the bearer token matches CRABBOX_ADMIN_TOKEN, which is the secret updated by this workflow.
Findings 1 actionable finding [P3] Remove the release-owned changelog entry
Security None None.

How this fits together

Crabbox’s coordinator is a Cloudflare Worker control plane whose CRABBOX_ADMIN_TOKEN authorizes fleet-wide administrative routes. The new workflow takes a one-time GitHub environment secret, verifies it is running from protected main, and writes the replacement Worker secret through Wrangler.

flowchart LR
  A[Maintainer dispatch] --> B[Protected main workflow guard]
  B --> C[Coordinator GitHub environment]
  C --> D[One-time next-token secret]
  D --> E[Wrangler secret update]
  E --> F[Coordinator admin authentication]
Loading

Decision needed

Question Recommendation
Does the live coordinator GitHub environment enforce the intended approver and secret-access boundary for a workflow that can rotate the production coordinator admin token? Confirm environment controls: Merge after confirming that the coordinator environment limits approval and both rotation secrets to the intended production operators.

Why: The checkout proves the workflow’s source guards and environment reference, but it cannot establish the repository’s live GitHub environment protection configuration.

Before merge

  • Remove the release-owned changelog entry (P3) - CHANGELOG.md is release-owned under the review policy, so ordinary PRs should not add an Unreleased entry. Remove these two lines; the PR body and commit message can carry the needed user-visible context.
  • Resolve merge risk (P1) - Merging creates a protected GitHub Actions path that can replace the coordinator’s production admin credential; repository source cannot verify the live coordinator environment’s approvers or secret-access rules.
  • Resolve merge risk (P1) - The branch currently edits release-owned CHANGELOG.md, creating avoidable release-note ownership churn.
  • Complete next step (P2) - The release-note ownership correction is a narrow mechanical repair; the separate GitHub environment authorization confirmation remains a maintainer action.

Findings

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7-8
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Credential-rotation surface 1 workflow added, 1 contract test added, 1 release-owned file modified The small patch adds a production credential-update path while also including an unnecessary changelog modification.

Merge-risk options

Maintainer options:

  1. Confirm the production environment boundary (recommended)
    Verify the coordinator environment’s reviewers and secret-access scope before merging the new path that can replace the production admin token.
  2. Accept the existing environment policy
    Merge if maintainers have already independently verified that the environment’s protection policy is the intended authorization boundary.

Technical review

Best possible solution:

Remove the changelog addition, retain the guarded workflow and its static contract test, and merge only after maintainers confirm that the coordinator GitHub environment restricts approval and secret access to the intended production operators.

Do we have a high-confidence way to reproduce the issue?

Not applicable; this PR adds an operator workflow rather than repairing a user-observable existing-behavior failure. Its static workflow assertions are present in the added script test and are included in the repository’s scripts CI job.

Is this the best way to solve the issue?

Yes, subject to the live environment-policy confirmation: the workflow reuses the repository’s existing protected-source guard and avoids passing the replacement token on argv. Removing the release-owned changelog entry is the narrower release-process path.

Full review comments:

  • [P3] Remove the release-owned changelog entry — CHANGELOG.md:7-8
    CHANGELOG.md is release-owned under the review policy, so ordinary PRs should not add an Unreleased entry. Remove these two lines; the PR body and commit message can carry the needed user-visible context.
    Confidence: 0.98

Overall correctness: patch is correct
Overall confidence: 0.93

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against f793138a161e.

Labels

Label changes:

  • add P2: This is a bounded production-operational improvement whose merge safety depends on a maintainer-controlled authorization boundary.
  • add merge-risk: 🚨 security-boundary: The new workflow can replace the coordinator’s production admin credential using GitHub environment secrets.
  • add rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • add status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This member-authored operational workflow is not subject to the external-contributor real-behavior-proof gate; the PR supplies static validation, while any production rotation should be verified through a controlled, redacted maintainer run.

Label justifications:

  • P2: This is a bounded production-operational improvement whose merge safety depends on a maintainer-controlled authorization boundary.
  • merge-risk: 🚨 security-boundary: The new workflow can replace the coordinator’s production admin credential using GitHub environment secrets.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🌊 off-meta tidepool and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Not applicable: This member-authored operational workflow is not subject to the external-contributor real-behavior-proof gate; the PR supplies static validation, while any production rotation should be verified through a controlled, redacted maintainer run.

Evidence

Acceptance criteria:

  • [P1] node --test scripts/coordinator-admin-token-rotation-workflow.test.js scripts/workflow-action-pins.test.js.
  • [P1] git diff --check.

What I checked:

  • Protected workflow implementation: The proposed workflow is manual-only, checks the default-branch workflow identity, requires a protected ref, checks that the workflow and run SHAs match, and checks out the workflow SHA without persisted credentials before using the environment secrets. (.github/workflows/coordinator-admin-token-rotate.yml:19, d1abe6937b91)
  • Existing protected-workflow pattern: Current main already uses the same protected-default-branch guard and workflow-SHA checkout pattern for image publication, so the new workflow aligns with an established repository control. (.github/workflows/devtools-image-publish.yml:57, 26fe7e2f6edc)
  • Coordinator authorization contract: The Worker grants admin access only when the bearer token matches CRABBOX_ADMIN_TOKEN, which is the secret updated by this workflow. (worker/src/auth.ts:95, e31157d65ba5)
  • Regression coverage is in the CI script suite: The PR adds a static contract test for the manual trigger, protected ref, immutable action pins, environment scoping, and stdin secret handoff; current CI runs all scripts/*.test.js files. (.github/workflows/ci.yml:230, d1abe6937b91)
  • Release ownership: The branch also changes the unreleased changelog, but release-note ownership guidance says ordinary PRs should not edit that release-owned file. (CHANGELOG.md:7, d1abe6937b91)

Likely related people:

  • Vincent Koc: Authored the current protected image-publication workflow that establishes the same guard and immutable-source pattern used by this PR. (role: recent area contributor; confidence: high; commits: 26fe7e2f6edc; files: .github/workflows/devtools-image-publish.yml)
  • Peter Steinberger: Most recently maintained the Worker authentication path that evaluates CRABBOX_ADMIN_TOKEN for coordinator admin access. (role: recent authentication contributor; confidence: medium; commits: e31157d65ba5; files: worker/src/auth.ts, worker/src/index.ts)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Remove the CHANGELOG.md addition.
  • Confirm the coordinator environment’s approver and secret-access policy before merge.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@vincentkoc
vincentkoc merged commit 64a7f8f into main Aug 1, 2026
26 checks passed
@vincentkoc
vincentkoc deleted the fix/coordinator-admin-token-rotation branch August 1, 2026 07:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant