Skip to content

fix(orb-broker): validate App credentials only after enrollment is proven valid - #2733

Merged
JSONbored merged 1 commit into
mainfrom
fix/orb-broker-auth-ordering
Jul 3, 2026
Merged

fix(orb-broker): validate App credentials only after enrollment is proven valid#2733
JSONbored merged 1 commit into
mainfrom
fix/orb-broker-auth-ordering

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • brokerOrbToken checked ORB_GITHUB_APP_ID/ORB_GITHUB_APP_PRIVATE_KEY before looking up the enrollment secret, so an unauthenticated caller with any bad/unknown secret could distinguish "broker misconfigured" (503) from "invalid secret" (401) via the response code alone — leaking the server's deployment-config state without ever proving a valid enrollment. Moves the credential check to immediately before the mint call (the only place it's actually needed — a cache hit never needs it), so an invalid/unknown secret always yields invalid_enrollment regardless of server configuration.
  • Follow-up to fix(orb-broker): improve error handling for token exchange failures #2710, which merged before this ordering issue (flagged during that PR's own review) was addressed.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed. (No pre-existing issue; a single-function ordering fix with full test coverage, discovered via the review gate on fix(orb-broker): improve error handling for token exchange failures #2710.)

Validation

  • git diff --check
  • npm run typecheck
  • npm run test:coverage locally (scoped to src/orb/broker.ts — 100% patch coverage on the changed lines/branches)
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. (Regression test asserts an invalid secret always returns invalid_enrollment, even with zero App credentials configured.)
  • API/OpenAPI/MCP behavior is updated and tested where needed. (No public API contract change — same result shapes/status codes, only the internal check order changed.)
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. (N/A — no UI changes.)
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. (N/A — internal broker ordering fix, no public-facing doc change.)

UI Evidence

N/A — backend-only change, no UI.

Notes

  • The eligibility check (installation_not_eligible) still runs before the credential check, so a revoked/suspended install never leaks config state either.
  • A still-fresh cached token is served without ever touching the credential check, since readCachedOrbToken only depends on TOKEN_ENCRYPTION_SECRET.

…oven valid

brokerOrbToken checked ORB_GITHUB_APP_ID/ORB_GITHUB_APP_PRIVATE_KEY before
looking up the enrollment secret, so ANY caller — including one with no
valid secret at all — could distinguish "broker misconfigured" from
"invalid secret" via the response code, leaking the server's deployment
config state to unauthenticated callers. Move the credential check to
immediately before the mint call, the only place it's actually needed
(a cache hit never needs it), so an invalid/unknown secret always yields
invalid_enrollment regardless of server configuration.
@JSONbored JSONbored self-assigned this Jul 3, 2026
@JSONbored
JSONbored merged commit 3a3cd12 into main Jul 3, 2026
8 checks passed
@JSONbored
JSONbored deleted the fix/orb-broker-auth-ordering branch July 3, 2026 08:53
@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@86dde13). Learn more about missing BASE report.
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2733   +/-   ##
=======================================
  Coverage        ?   96.02%           
=======================================
  Files           ?      244           
  Lines           ?    27300           
  Branches        ?     9919           
=======================================
  Hits            ?    26214           
  Misses          ?      460           
  Partials        ?      626           
Files with missing lines Coverage Δ
src/orb/broker.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant