Skip to content

ci(rescue): mint the read token with only the permissions the App has - #14627

Merged
teamleaderleo merged 2 commits into
mainfrom
ci/rescue-read-token-perms
Sep 25, 2026
Merged

teamleaderleo merged 2 commits into
mainfrom
ci/rescue-read-token-perms

Conversation

@teamleaderleo

@teamleaderleo teamleaderleo commented Sep 25, 2026 •

Copy link
Copy Markdown
Collaborator

#14499 moved the owned-pool watch's reads to a manaflow-glaeda-route App token, but every mint since then has failed:

The permissions requested are not granted to this installation. (422)

The step asks for contents: read, and the installation has no contents grant (hq#639 lists actions, pull_requests and administration). The step is continue-on-error, so the watch kept working but read on GITHUB_TOKEN, and the move saved nothing. Example: run 36138293052.

Changes

  • The mint asks only for actions: read and pull-requests: read. owned_pool_rescue.py only reads /actions/* and /pulls/{n}.
  • If a read the App may not make returns 403, only that one request retries on GITHUB_TOKEN. A 401 (expired token) still switches every later read back for the rest of the watch.
  • New test Tokens.test_a_read_the_app_may_not_make_uses_github_token_once. The mint-permissions test is updated. 94 tests pass, and actionlint is clean.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.


Summary by cubic

Fixes the rescue watch's App token mint, which was failing with 422 because it asked for a contents: read permission the installation doesn't have.

  • The mint now requests only actions: read and pull-requests: read, matching the endpoints owned_pool_rescue.py reads.
  • A 403 on a read returns only that request to GITHUB_TOKEN; a 401 (expired token) still switches all later reads back to GITHUB_TOKEN for the rest of the watch.
  • The one contents read (branch_head on /branches) is the fallback that triggers.

Written for commit 935a947. Summary will update on new commits.

Review in cubic

Summary by CodeRabbit

  • Bug Fixes
    • Improved CI recovery when a GitHub App read request is denied, allowing that request to retry with an alternate token without changing the token used for later reads.
  • Security
    • Reduced the permissions requested for the GitHub App token; workflow-level permissions remain unchanged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

Copy link
Copy Markdown
Contributor

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository: manaflow-ai/cmux/.coderabbit.yaml

Review profile: ASSERTIVE

Plan: Advanced

Run ID: 4bc1c23e-9f7d-4e72-bfe7-794e41efae0a

📥 Commits

Reviewing files that changed from the base of the PR and between c055747 and 935a947.

📒 Files selected for processing (3)
  • .github/workflows/ci-owned-pool-rescue.yml
  • scripts/ci/owned_pool_rescue.py
  • tests/test_ci_owned_pool_rescue.py
💤 Files with no reviewable changes (1)
  • .github/workflows/ci-owned-pool-rescue.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The rescue script now retries a read with GITHUB_TOKEN after a 403 from the App read token, without changing the token used for later reads. A 401 still switches later reads to GITHUB_TOKEN. The workflow removes contents: read from the minted App token request.

Changes

Owned pool rescue token handling

Layer / File(s) Summary
Read-token fallback and permissions
.github/workflows/ci-owned-pool-rescue.yml, scripts/ci/owned_pool_rescue.py, tests/test_ci_owned_pool_rescue.py
GitHub.request supports explicit primary-token reads. A 403 from the read token retries only the current request with GITHUB_TOKEN; a 401 changes the token used for later reads. Tests cover the 403 fallback and later App-token use. The workflow and test expectation remove contents: read from the minted App token.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant GitHubRequest
  participant GitHubAPI
  participant AppToken
  participant GITHUB_TOKEN
  GitHubRequest->>GitHubAPI: Read using AppToken
  GitHubAPI-->>GitHubRequest: Return 403
  GitHubRequest->>GitHubAPI: Retry current read using GITHUB_TOKEN
  GitHubRequest->>GitHubAPI: Send next read using AppToken
Loading

Merge Risk: ⚪ Minimal · up to 935a9

The App token no longer requests the unavailable contents permission, and the workflow’s contents-enabled job token handles the branch lookup fallback. The 403 retry is limited to that request; no concrete merge-blocking regression is evident.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 935a9

The change narrows the requested App permissions and keeps denied-read retries within the existing rescue workflow. No new security finding was established, but token fallback warrants review because it changes which credential makes a read.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The credential change affects reads made by this repository’s rescue job, including concurrent sweep watches, rather than granting a new credential to watched runs.

Trust Boundaries and Controls

  • observed — An App-token 403 now crosses to the workflow token for that GET. The inspected callers construct repository API paths inside the script, and the workflow token remains subject to its GitHub permissions; no new attacker-controlled request destination was established.

Resilience and Maintainability Implications

  • observed — The 403 retry leaves the App token selected for the next read, whereas a 401 changes later reads to the workflow token. Errors on the workflow-token retry propagate.
🚥 Pre-merge checks | ✅ 24 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 8 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (24 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: limiting the rescue watch App token to the permissions granted to the installation.
Description check ✅ Passed The description clearly explains the 422 failure, the permission change, the 403 and 401 fallback behavior, and the reported test and actionlint results. It does not use the template headings or inclu…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Cmux Cloud Persistent Session And Early Input ✅ Passed PASS. The reviewed diff changes only the owned-pool rescue workflow, GitHub API token selection, and related tests. It removes an App-token permission and adds 401/403 read fallback behavior. It does …
Cmux Swift Actor Isolation ✅ Passed PASS: The pull request changes only one YAML workflow, one Python production script, and Python tests. The authoritative diff contains no Swift files or Swift actor-isolation changes, so this custom c…
Cmux Swift Blocking Runtime ✅ Passed The pull request changes only a GitHub Actions workflow, Python code, and Python tests. The authoritative diff contains no Swift files, so it does not introduce or expand blocking or timing-based sync…
Cmux Browser Automation Off-Main ✅ Passed The PR changes only the rescue workflow, scripts/ci/owned_pool_rescue.py, and its Python tests. It does not change Sources/TerminalController.swift, ControlCommandExecutionPolicy.swift, or any `…
Cmux Expensive Synchronous Load ✅ Passed PASS: The pull request changes only a GitHub Actions workflow, Python code, and Python tests. The authoritative diff contains no Swift files, so it cannot introduce or move an expensive synchronous Sw…
Cmux Cache Substitution Correctness ✅ Passed PASS: The pull request changes only a GitHub Actions workflow, Python code, and Python tests. It contains no production Swift, TypeScript, or JavaScript changes, so the cache-substitution correctness …
Cmux No Hacky Sleeps ✅ Passed PASS: The PR adds no fixed sleep, timer, delayed dispatch, polling change, or wall-clock wait. The production change only retries one 403 request immediately with GITHUB_TOKEN; it does not use elaps…
Cmux Algorithmic Complexity ✅ Passed The pull request does not introduce an algorithmic-complexity violation. The production change only adds a bounded retry path in GitHub.request; it performs at most one extra request for a 403 and a…
Cmux Swift Concurrency ✅ Passed The pull request changes only one YAML workflow, one Python script, and one Python test. The authoritative diff contains no Swift files and introduces no Swift concurrency patterns. The custom check i…
Cmux Swift @Concurrent ✅ Passed The pull request changes only a GitHub Actions workflow, Python code, and Python tests. The authoritative diff contains no Swift files or Swift isolation changes, so the @concurrent check is not app…
Cmux Swift Package Boundaries ✅ Passed PASS: The pull request changes only one GitHub Actions workflow, one Python CI script, and Python tests. The authoritative diff contains no Swift or SwiftPM files, so it does not introduce a Swift app…
Cmux Swiftpm Lockfiles ✅ Passed PASS. The PR changes one GitHub Actions permission, Python token handling, and tests. It does not change a SwiftPM package, Package.swift dependency, .gitignore, Xcode package reference, or any Packag…
Cmux Swift Logging ✅ Passed PASS: The reviewed range changes only .github/workflows/ci-owned-pool-rescue.yml, scripts/ci/owned_pool_rescue.py, and tests/test_ci_owned_pool_rescue.py. It contains no Swift changes and adds n…
Cmux User-Facing Error Privacy ✅ Passed PASS. The diff changes an internal GitHub Actions rescue workflow and its CI helper. It removes an App permission and adds token fallback logic; it does not add or materially change cmux user-facing e…
Cmux Full Internationalization ✅ Passed The PR changes only CI workflow permissions, operational Python token handling, and tests. Added text is limited to developer comments/docstrings and test assertions; no user-facing Swift, web, metada…
Cmux Swiftui State Layout ✅ Passed The pull request changes only a GitHub Actions workflow, Python code, and Python tests. The authoritative diff contains no SwiftUI files or SwiftUI state/layout changes. The SwiftUI-specific failure c…
Cmux Architecture Rethink ✅ Passed PASS: The pull request changes only a GitHub Actions workflow, Python code, and Python tests. The authoritative diff contains no Swift, Xcode project, or Swift workspace changes. Therefore the Swift a…
Cmux Swift Auxiliary Window Close Shortcuts ✅ Passed PASS: The pull request changes only a GitHub Actions workflow, a Python CI script, and Python tests. The authoritative diff contains no Swift or standalone window changes, so the auxiliary-window clos…
Cmux Source Artifacts ✅ Passed The PR changes only three intentional source-control paths: a workflow configuration, the hand-written rescue script, and its test file. The diff adds no logs, screenshots, recordings, caches, build o…
Cmux No Test Or Debug Seam In Production Source ✅ Passed PASS: The pull request changes only one workflow file, one Python production script, and one Python test file. It changes no Swift file under a production Sources/ path, so the specified no-test/deb…
  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@teamleaderleo
teamleaderleo merged commit 805a699 into main Sep 25, 2026
51 checks passed
@teamleaderleo
teamleaderleo deleted the ci/rescue-read-token-perms branch September 25, 2026 17:41
@github-actions

Copy link
Copy Markdown
Contributor

Merge receipt for 935a947f76: every check was green at merge (8 verified; 11 skipped by policy). Full suite runs on main after merge.

rustybret pushed a commit to rustybret/bmux that referenced this pull request Sep 25, 2026
bd2d34e test: expect split zoom to survive closing one tab of a zoomed pane (manaflow-ai#14664)
f8857c5 fix(scripts): append, not prepend, the cargo fallback PATH in build-cmux-cua.sh (manaflow-ai#14665)
56a3e4c fix: make the event-stream reconnect decision a value, not a static namespace (manaflow-ai#14661)
06e064d ci: pin cla.yml and claude.yml to a GitHub-hosted runner (manaflow-ai#14668)
2509187 ci: restore the git object seed before checkout in E2E and iOS macOS jobs (manaflow-ai#14669)
402d0ad docs: move team-internal fleet and session rules out of CLAUDE.md (manaflow-ai#14595)
3bfe0b6 pull_request_template: drop the commented @codex review trigger block (manaflow-ai#14599)
4f0ac55 fix(control): honor color/icon keys and validate hex in workspace.group.set_color/set_icon (manaflow-ai#13877)
805a699 ci(rescue): mint the read token with only the permissions the App has (manaflow-ai#14627)
7e662c0 Tell the user why a file upload failed (manaflow-ai#11476)
f04320e test: yield to the main queue while the Files tree catches up after its menu closes (manaflow-ai#14660)
a5f705b A mirrored tmux window with one pane shows two tab bars (manaflow-ai#11248)
74917a9 iOS: remove unshipped push reconnect banner

# Conflicts:
#	.github/workflows/ci-owned-pool-rescue.yml
#	.github/workflows/cla.yml
#	.github/workflows/claude.yml
#	.github/workflows/test-e2e.yml
#	.github/workflows/test-ios.yml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant