Skip to content

fix(orb): bind enrollment admin checks to immutable GitHub account ids#1393

Open
JSONbored wants to merge 2 commits into
mainfrom
codex/fix-oauth-self-enrollment-login-vulnerability
Open

fix(orb): bind enrollment admin checks to immutable GitHub account ids#1393
JSONbored wants to merge 2 commits into
mainfrom
codex/fix-oauth-self-enrollment-login-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent a privilege-escalation where OAuth self-enrollment authorized by a stored mutable account_login could be abused if a GitHub account/org renames or an attacker reclaims the old login.
  • Ensure enrollment authorization is tied to the immutable GitHub account id so a recycled login cannot enroll a victim installation_id.

Description

  • Add a migration migrations/0070_orb_github_installation_account_id.sql to persist account_id in orb_github_installations.
  • Persist account_id from webhook upserts and the App-installation backfill by wiring it through src/orb/installations.ts and src/orb/app-auth.ts.
  • Harden the admin gate in src/orb/oauth.ts by threading the OAuth user.id and stored account_id into verifyInstallationAdmin and requiring id-equality for User installs and matching organization.id for Org installs.
  • Read account_id in the OAuth enrollment flow and pass the authenticated user.id into the authorization check, and add a regression test that verifies stale-login reuse is refused; update related tests to exercise the new branches.

Testing

  • Ran npx vitest run test/integration/orb-oauth.test.ts test/integration/orb-installations.test.ts test/unit/orb-app-auth.test.ts and all specified tests passed.
  • Ran npm run db:migrations:check which passed and npm run typecheck which passed with no type errors.
  • Attempted the full gate via npm run test:ci but the run failed during coverage generation with TypeError: jsTokens is not a function from the coverage tooling (failure appears unrelated to the orb change and blocked full CI completion).
  • npm audit --audit-level=moderate could not complete in this environment (registry audit returned 403).

Codex Task

@dosubot dosubot Bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Jun 25, 2026
@superagent-security

Copy link
Copy Markdown

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.33%. Comparing base (6138b9e) to head (34e9f07).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1393      +/-   ##
==========================================
- Coverage   95.33%   95.33%   -0.01%     
==========================================
  Files         192      192              
  Lines       20750    20750              
  Branches     7500     7501       +1     
==========================================
- Hits        19783    19782       -1     
  Misses        383      383              
- Partials      584      585       +1     
Files with missing lines Coverage Δ
src/orb/app-auth.ts 100.00% <100.00%> (ø)
src/orb/installations.ts 100.00% <100.00%> (ø)
src/orb/oauth.ts 97.36% <100.00%> (-2.64%) ⬇️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 25, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui 34e9f07 Commit Preview URL

Branch Preview URL
Jun 25 2026, 11:05 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

aardvark codex size:M This PR changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant