fix(auth): bake public OAuth client ID as fallback in getClientId()#916
Merged
fix(auth): bake public OAuth client ID as fallback in getClientId()#916
Conversation
The OAuth client ID is a public value — Device Authorization Grant (RFC 8628) uses a public-client flow with no client secret. The value is already stored as a plain repo variable (vars.SENTRY_CLIENT_ID) in CI, not a secret. Committing it as DEFAULT_OAUTH_CLIENT_ID eliminates the .env.local requirement for local development against sentry.io. The priority chain is unchanged: SENTRY_CLIENT_ID env var → SENTRY_CLIENT_ID_BUILD (build-time) → committed default Self-hosted users still override via SENTRY_CLIENT_ID env var; production release builds still inject via SENTRY_CLIENT_ID_BUILD — no behaviour change for either path.
Contributor
|
Contributor
Codecov Results 📊✅ 6664 passed | Total: 6664 | Pass Rate: 100% | Execution Time: 0ms 📊 Comparison with Base Branch
✨ No test changes detected All tests are passing successfully. ✅ Patch coverage is 100.00%. Project has 13516 uncovered lines. Coverage diff@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 76.63% 76.63% —%
==========================================
Files 303 303 —
Lines 57823 57823 —
Branches 0 0 —
==========================================
+ Hits 44307 44307 —
- Misses 13516 13516 —
- Partials 0 0 —Generated by Codecov Action |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit d7e43e9. Configure here.
…lways returns a value DEFAULT_OAUTH_CLIENT_ID ensures getClientId() is never empty, making the ConfigError guards in requestDeviceCode() and refreshAccessToken() unreachable. Remove them and the now-unused ConfigError import.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Commit the sentry.io OAuth client ID directly as
DEFAULT_OAUTH_CLIENT_IDinoauth.ts, used as the final fallback ingetClientId().Why this is safe
The OAuth client ID is public by design. Device Authorization Grant (RFC 8628) is a public-client flow — there is no client secret involved. The value is already stored as a plain repo variable (
vars.SENTRY_CLIENT_ID) in CI, not a secret, and has always been readable by anyone with repo access.Why #911 was over-engineered
PR #911 addressed the same problem but introduced a separate "development" client ID, a new
RELEASE_BUILDguard, anensureSentryClientIdConfigured()function, asymmetric behaviour betweenbuild.tsandbundle.ts, and touched 10 files. The dev-vs-production distinction isn't meaningful when both IDs are equally public.What changes
The fallback chain in
getClientId()is unchanged in priority:.env.localneededSENTRY_CLIENT_IDenv varSENTRY_CLIENT_ID_BUILDfromSENTRY_CLIENT_IDenv var at build time — no behaviour changeCloses #911