Skip to content

fix(oauth): degrade instead of dying when a scope predates its ceiling edit - #1060

Open
candidate2-vapi wants to merge 1 commit into
PostHog:mainfrom
candidate2-vapi:fix/oauth-invalid-scope-degrade
Open

fix(oauth): degrade instead of dying when a scope predates its ceiling edit#1060
candidate2-vapi wants to merge 1 commit into
PostHog:mainfrom
candidate2-vapi:fix/oauth-invalid-scope-degrade

Conversation

@candidate2-vapi

Copy link
Copy Markdown

Problem

Widening the wizard OAuth app's scope ceiling (OAuthApplication.scopes) is a manual Django-admin edit on both prod regions, so it lands separately from the code that starts requesting the scope. src/lib/constants.ts already documents the consequence:

requesting anything outside it fails the WHOLE authorize request with error=invalid_scope before the consent screen renders

So during that window, one not-yet-live scope doesn't degrade the step that needs it — it locks every user of that program out of login entirely. The current copy in oauth-errors.ts describes this accurately and offers no way forward: "Re-running the wizard will not help until the ceiling is updated."

This is not hypothetical. I hit it running wizard self-driving off #1055, which requests the net-new replay_scanner:read / replay_scanner:write. Login to us.posthog.com succeeded, then the callback came back http://localhost:8239/callback?error=invalid_scope and the run was dead. The only way to proceed was to locally comment the two scopes out of SELF_DRIVING_SCOPE_ADDITIONS — which is exactly what this PR does properly, and automatically.

The failure mode is generic: it recurs for every future net-new scope, and the blast radius is the whole program rather than one step. #1055's own sequencing notes ("make the prod OAuth-ceiling edit, then merge") are a process workaround for a robustness gap in the client.

Changes

  • PENDING_CEILING_SCOPES in program-scopes.ts — declares scopes that may not be live in the ceiling yet. Ships empty, since everything requested today is live; it's the checklist entry that pairs with the admin edit.
  • One-shot retry in performOAuthFlow — on invalid_scope, if the request contained pending scopes, drop them and re-enter the existing do/while with a reduced set. Bounded to a single extra attempt: the retry requests nothing pending, so the branch cannot match twice.
  • Reuses the loop already there for port conflicts; the guard after the port loop keeps a scope retry from falling into the port-conflict path.
  • The degradation is loud — dropped scopes are named in a log.warn and captured as oauth pending ceiling scopes dropped, so this never silently hides a real misconfiguration.

config.scopes becomes a local scopes so the retry, the failure message, and the analytics payload all report what was actually requested on the final attempt.

The design tradeoff, stated plainly

This trades "fail loudly, fix the ceiling" for "warn loudly, finish the run with less". I think that's right when the alternative is a total lockout on a manual-edit race, and the empty default means nothing changes until someone opts a scope in. If you'd rather the client stay strict, the alternative worth considering is a CI check asserting every requested scope appears in the README ceiling list — happy to switch. Two notes for reviewers:

  1. An entry that outlives its ceiling edit permits a degraded grant indefinitely. The doc comment says to remove it once confirmed live; a stronger version could expire entries by date.
  2. The spinner prints "Authorization failed." immediately before the retry warning. Slightly awkward, but I left the existing spinner handling alone rather than restructure it.

Suggested follow-up for #1055

That PR should add its two scopes to PENDING_CEILING_SCOPES in the same commit that adds them to SELF_DRIVING_SCOPE_ADDITIONS, then drop them once the ceiling edit is confirmed in both regions. That makes the ordering constraint in its "Sequencing" section non-fatal if the halves land out of order.

Test plan

  • pnpm exec vitest run1723 pass / 121 files, no failures.
  • Four new tests in src/utils/__tests__/oauth.test.ts: pending scopes are dropped with order preserved; an empty pending list is a no-op; a pending scope that wasn't requested does not shrink the set (so it can't arm the retry and loop); and no base WIZARD_OAUTH_SCOPES entry may be listed pending — degrading one of those would hand back a token that authenticates but can't run the wizard.
  • pnpm exec prettier --check and pnpm exec eslint clean on all three changed files.
  • pnpm typecheck reports 25 errors before and after this change — all pre-existing on main (ReadableStream.destroy, Set.slice, etc., seemingly a Node/TS lib mismatch under Node 26). None in the changed files.

Not verified end-to-end against production, and I want to be upfront about it: exercising the retry needs a scope that is genuinely outside the live ceiling, and PENDING_CEILING_SCOPES ships empty. The scope-drop logic and the loop-arming conditions are unit-tested, and the reduced-set path is the same code that runs today for a normal first attempt, but the real invalid_scope → retry → consent round trip hasn't been observed. The clean way to confirm is to land #1055 with its two scopes listed pending, before the ceiling edit.

LLM context

Investigated and authored with Cursor (Opus 5). The invalid_scope failure was reproduced by hand while running wizard self-driving --local-mcp from #1055 against us.posthog.com; the fix, tests, and verification above were produced in that session.

Made with Cursor

…g edit

Widening the wizard OAuth app's scope ceiling (`OAuthApplication.scopes`) is
a manual Django-admin edit on both prod regions, so it inevitably lands
separately from the code that starts requesting the scope. In that window
PostHog rejects the authorize request with `invalid_scope` — and it rejects
the WHOLE request, before the consent screen renders. The result is that one
not-yet-live scope locks every user of that program out of login entirely,
rather than degrading only the step that needed it. The existing error copy
says as much ("Re-running the wizard will not help"), which is accurate and
also a dead end.

Declare those scopes in `PENDING_CEILING_SCOPES` and retry the flow once
without them, so the run continues with a reduced grant and only the
dependent step degrades. Bounded to a single extra attempt: the retry
requests nothing pending, so the branch cannot match twice. The dropped
scopes are named in a visible warning and captured for analytics, so this
stays a loud degradation rather than a silent one.

The list ships empty — every scope requested today is live. It is the
checklist entry that pairs with the admin edit: add a scope when you start
requesting it, remove it once the ceiling is confirmed in both regions.

Co-authored-by: Cursor <cursoragent@cursor.com>
@candidate2-vapi
candidate2-vapi requested a review from a team as a code owner August 4, 2026 20:20
@candidate2-vapi

Copy link
Copy Markdown
Author

This is me! ~@Twixes

@Twixes
Twixes self-requested a review August 5, 2026 03:21
@Twixes Twixes self-assigned this Aug 5, 2026
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.

2 participants