Skip to content

feat(oauth): Store scopes Sentry actually granted as upstreamScopes - #1561

Merged
skaasten merged 1 commit into
mainfrom
feat/oauth-store-upstream-scopes
Oct 21, 2026
Merged

skaasten merged 1 commit into
mainfrom
feat/oauth-store-upstream-scopes

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Oct 20, 2026

Copy link
Copy Markdown
Contributor

Why

The OAuth callback stores grantedScopes on grant props. The name suggests "scopes Sentry granted", but the value is computed locally by getScopesForSkills(selectedSkills). It does not reflect what the stored Sentry token can actually do.

Today the upstream authorize request asks for every scope in SCOPES (org:read project:write team:write event:write alerts:write), whatever skills are selected. So an Inspect-only grant can have read-only grantedScopes while the Sentry token holds write scopes.

Sentry's token response already reports the token's real scopes in scope (Sentry builds it from token.get_scopes(), and TokenResponseSchema already requires the field). The callback parsed that value and then discarded it.

What

  • callback.ts: persist the token response scope on grant props as upstreamScopes. The list is split on whitespace, de-duplicated and sorted. If Sentry omits scope, the field is left undefined so a missing value isn't mistaken for "no scopes".
  • types.ts: add upstreamScopes?: string[] to WorkerProps, with a doc comment contrasting it with grantedScopes.
  • helpers.ts: carry upstreamScopes through token-exchange props explicitly, next to grantedScopes.
  • Docs: docs/operations/security.md and docs/cloudflare/oauth-architecture.md explain the difference.

grantedScopes is unchanged. Nothing reads upstreamScopes yet.

Why stop here

This PR records the data, which helps decide whether to drop the misleadingly named grantedScopes. Current runtime authorization uses grantedSkills, and the MCP handler doesn't read grantedScopes.

Notes:

  • Upstream token refresh: the toolkit doesn't call Sentry's refresh grant, and Sentry ignores scope on refresh anyway. The stored value therefore stays accurate for the life of the grant.
  • Grants created before this change won't have upstreamScopes until users re-authorize.
  • This does not narrow what is requested from Sentry. That would be a separate change.

Tests

  • New: the callback stores the parsed, sorted upstreamScopes from the token response, separate from skill-derived grantedScopes. Confirmed it fails without the change.
  • New: upstreamScopes is omitted when the token response has no scope.
  • @sentry/mcp-cloudflare: 435/435 pass. Typecheck, oxlint and oxfmt are clean.

via shaun.kaasten.

--

View Junior Session [Sentry]

Sentry's OAuth token response includes the access token's real scopes in
`scope`. The callback parsed it via TokenResponseSchema but discarded it,
and only stored `grantedScopes`, which is derived from the selected skills
and does not reflect what the upstream token can do.

Persist the token response scope on the grant props as `upstreamScopes`
(sorted, de-duplicated). Leave it undefined when Sentry omits the field so
an absent value is not mistaken for "no scopes". `grantedScopes` is
unchanged; this gives us data to decide whether to drop it.

Co-Authored-By: shaun.kaasten <shaun.kaasten@sentry.io>
@vercel

vercel Bot commented Oct 20, 2026 •

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
sentry-local Ready Ready Preview Oct 20, 2026 5:41pm UTC

Request Review

@BYK
BYK marked this pull request as ready for review October 21, 2026 12:52
@github-actions github-actions Bot added the risk: low PR risk score: low label Oct 21, 2026
@skaasten
skaasten merged commit e4a9da6 into main Oct 21, 2026
28 checks passed
@skaasten
skaasten deleted the feat/oauth-store-upstream-scopes branch October 21, 2026 14:07

This branch was successfully deployed

1 active deployment
Preview — af6dd2f2 Deployed Oct 20, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

risk: low PR risk score: low

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants