Skip to content

fix(oauth): route empty-scope initiates through consent screen (closes #28)#32

Merged
stackbilt-admin merged 1 commit intomainfrom
fix/oauth-empty-scope-consent
Apr 11, 2026
Merged

fix(oauth): route empty-scope initiates through consent screen (closes #28)#32
stackbilt-admin merged 1 commit intomainfrom
fix/oauth-empty-scope-consent

Conversation

@stackbilt-admin
Copy link
Copy Markdown
Member

Summary

Closes #28 — the grant-creation side of the OAuth scope cluster. Implements Option C from the issue body (consent screen with default scopes) and explicitly rejects Option A (client allowlist) in the commit message so it can't be resurrected later.

The bug

C-1a remediation (256ba06, 2026-04-10) correctly removed the hardcoded ['generate','read'] grant from completeAuthorization() calls. Before C-1a, any OAuth-authed caller silently inherited full access regardless of their token claim — the C-1 privilege escalation.

Post-C-1a, scopes come from oauthReqInfo.scope. The gap: MCP clients (Claude Code, Claude.ai) don't include scope in their OAuth initiate, so the resulting grants are empty at every level (grant.scope AND encryptedProps.scopes). Every subsequent tool call fails with (none) scopes. That's the failure mode #28, #30, and this morning's dogfood session all ran into.

The fix (Option C — consent screen with defaults)

  1. New constant DEFAULT_CONSENT_SCOPES = ['read', 'generate'] with a long comment explaining why it exists and why it's not silently applied.
  2. handleGetAuthorize auto-approve branch: when oauthReqInfo.scope is empty AND the identity token is valid, render the (previously unreachable) renderConsentPage with DEFAULT_CONSENT_SCOPES. User sees the scopes and explicitly clicks approve.
  3. handlePostAuthorize approve branch: substitute DEFAULT_CONSENT_SCOPES when the POSTed scope is empty — matches exactly what the consent page showed. Non-empty client-requested scope is preserved verbatim.

Why this shape

  • Preserves C-1a's invariant — the user explicitly consents to every scope that enters the grant. Nothing is silent.
  • Preserves the zero-latency auto-approve path for clients that DO request scope (Claude Code callback timeout argument from the original auto-approve comment still holds — we only add the consent hop for the edge case of empty-scope initiates).
  • Reuses dead UI infrastructurerenderConsentPage was defined but never called anywhere. This PR makes it reachable for the exact failure mode it was designed to handle.

Rejected alternatives (explicit in commit message)

  • Option A — client allowlist keyed on clientId: REJECTED. clientId is attacker-controlled at OAuth client registration. Anyone can register a client with clientId: "claude-code" and inherit the allowlist's scopes. Same class as C-1, reskinned.
  • Silent default injection at auto-approve: REJECTED. Loses the "user explicitly consented" property that C-1a exists to preserve. One extra hop for this edge case is the right tradeoff.

What this does NOT fix

Test plan

  • 5 new test cases in test/oauth-handler.test.ts under OAuth handler — #28 empty-scope consent flow:
    • Renders consent page when scope is empty (asserts completeAuthorization was NOT called)
    • Auto-approves when scope is non-empty (regression guard for the zero-latency path)
    • handlePostAuthorize injects DEFAULT_CONSENT_SCOPES when approving an empty-scope grant
    • handlePostAuthorize preserves client-requested scopes verbatim (regression guard)
    • Deny action still works for empty-scope initiates (no grant minted)
  • npm run test125/125 green (up from 120)
  • npx -y tsc --noEmit — clean
  • Post-merge smoke: deploy, fresh Claude Code OAuth initiate, verify consent screen renders + tool calls succeed with ['read','generate'] scopes after approval

Related

…efaults (#28)

The C-1a remediation (commit 256ba06, 2026-04-10) correctly removed the
hardcoded ['generate','read'] grant from completeAuthorization() calls —
before C-1a, any OAuth-authed caller silently inherited full access
regardless of what their token claimed, which was the C-1 privilege
escalation bug. Post-C-1a, grant scopes come from oauthReqInfo.scope.

The gap: MCP clients like Claude Code and Claude.ai don't include scope
in their OAuth initiate. Post-fix, those initiates produced grants with
empty scope at every level (top-level grant.scope AND encryptedProps.scopes),
silently blocking every subsequent tool call with a "(none)" scopes error.
#28/#30 reported the symptom; this commit closes #28's creation-side bug.

Design (Option C from the #28 issue body):

- Add DEFAULT_CONSENT_SCOPES = ['read', 'generate'] as an explicit constant.
- In handleGetAuthorize auto-approve branch: when oauthReqInfo.scope is
  empty AND the identity token is valid, DO NOT auto-approve. Render the
  existing renderConsentPage (which was previously defined but never
  reachable) with DEFAULT_CONSENT_SCOPES, so the user explicitly approves.
- In handlePostAuthorize approve branch: substitute DEFAULT_CONSENT_SCOPES
  when the POSTed oauthReqInfo.scope is empty, matching what the consent
  page just showed the user. Non-empty scopes are preserved verbatim.

Rejected alternatives (explicit so they don't come back):

- Client allowlist keyed on clientId (Option A from #28): REJECTED.
  clientId is attacker-controlled at OAuth client registration. An
  attacker registers a client with clientId: "claude-code" and inherits
  the allowlist's hardcoded scopes — same class as C-1, reskinned.
- Silent injection of default scopes at auto-approve time: REJECTED.
  Loses the "user explicitly consented" property that was the whole
  point of C-1a. One extra hop for empty-scope initiates is an
  acceptable tradeoff.

What this does NOT fix:

- Legacy grants with stale encryptedProps.scopes from pre-2026-04-10
  code paths (where the old hardcoded path wrote correct top-level
  grant.scope but no props.scopes). That's #29's read-side fallback
  in gateway.ts resolveAuth, which lands in a separate PR.
- Static-bearer (ea_*) API key path — covered by the separate
  fix(auth): accept ea_* prefix PR, which is the Phase 0 unblock.

Tests (5 new cases, 125/125 total):

- Renders consent page when oauthReqInfo.scope is empty (no
  completeAuthorization call fires)
- Auto-approves when scope is non-empty (regression guard for the
  zero-latency path Claude Code depends on)
- handlePostAuthorize injects DEFAULT_CONSENT_SCOPES when approving
  an empty-scope grant
- handlePostAuthorize preserves client-requested scopes verbatim
  (regression guard)
- Deny action still works for empty-scope initiates (no grant minted)

Related: #28, #29, #30 on this repo; PR for the ea_* patch lands
separately as a Phase 0 unblock.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@stackbilt-admin stackbilt-admin merged commit 2dd29e5 into main Apr 11, 2026
1 check passed
@stackbilt-admin stackbilt-admin deleted the fix/oauth-empty-scope-consent branch April 11, 2026 20:33
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