feat: add repository scoped tokens - #1549
Open
brendan-kellam wants to merge 5 commits into
Open
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
This comment has been minimized.
This comment has been minimized.
Contributor
WalkthroughScoped access tokens now support hashed storage, one-hour expiration, API-key-only creation and revocation, bearer authentication, repository-scoped search, Prisma filtering, and public API documentation. ChangesScoped access tokens
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web/src/middleware/withAuth.test.ts (1)
304-312: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a negative test for a scoped token presented with the DPoP scheme.
getAuthenticatedUserrejects non-Bearerschemes at line 319 ofpackages/web/src/middleware/withAuth.ts, before the scoped-token branch at line 323. No test pins that ordering. A later refactor that moves the scoped-token branch above the scheme check would let a scoped token authenticate over DPoP without proof verification.Add a test that sends the token with the DPoP scheme and asserts
undefined.🧪 Proposed test
test('should return undefined for a token without a secret', async () => { setMockHeaders(new Headers({ 'Authorization': 'Bearer sbst_' })); const result = await getAuthenticatedUser(); expect(result).toBeUndefined(); expect(prisma.scopedAccessToken.findUnique).not.toHaveBeenCalled(); expect(prisma.apiKey.findUnique).not.toHaveBeenCalled(); }); + + test('should return undefined when a scoped token is presented with the DPoP scheme', async () => { + setMockHeaders(new Headers({ 'Authorization': 'DPoP sbst_scopedtoken' })); + + const result = await getAuthenticatedUser(); + + expect(result).toBeUndefined(); + expect(prisma.scopedAccessToken.findUnique).not.toHaveBeenCalled(); + });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/middleware/withAuth.test.ts` around lines 304 - 312, Add a negative test alongside the existing scoped-token tests that passes a scoped token using the DPoP authorization scheme, calls getAuthenticatedUser, and asserts it returns undefined. Also verify scoped-token and API-key lookups are not called, preserving rejection before the scoped-token branch.packages/web/src/ee/features/scopedAccessTokens/api.ts (1)
11-13: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueConsider bounding the
reposarray length.
reposaccepts an unbounded array. Each entry expands theINlist of therepo.findManyquery and the nestedcreatelist. Add a.max(...)bound to keep the request cost predictable.♻️ Proposed bound
export const createScopedAccessTokenRequestSchema = z.object({ - repos: z.array(z.string().min(1)).min(1), + repos: z.array(z.string().min(1)).min(1).max(100), }).strict();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/web/src/ee/features/scopedAccessTokens/api.ts` around lines 11 - 13, Add a finite .max(...) constraint to the repos array in createScopedAccessTokenRequestSchema, preserving the existing non-empty string validation and strict object behavior. Use the project’s established maximum collection-size constant if one exists; otherwise choose an appropriate bound that keeps the downstream repo.findMany query and nested create list predictable.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/web/src/openapi/publicApiDocument.ts`:
- Around line 432-491: Document the EE license requirement for scoped access
tokens: in packages/web/src/openapi/publicApiDocument.ts lines 432-491, add the
x-mint metadata using EE_LICENSE_KEY_NOTE to both createScopedAccessToken and
revokeScopedAccessToken registrations, then regenerate the OpenAPI spec; in
docs/docs/api-reference/authentication.mdx lines 36-38, add a Note at the
beginning of the scoped access token section linking to
/docs/activating-a-subscription.
---
Nitpick comments:
In `@packages/web/src/ee/features/scopedAccessTokens/api.ts`:
- Around line 11-13: Add a finite .max(...) constraint to the repos array in
createScopedAccessTokenRequestSchema, preserving the existing non-empty string
validation and strict object behavior. Use the project’s established maximum
collection-size constant if one exists; otherwise choose an appropriate bound
that keeps the downstream repo.findMany query and nested create list
predictable.
In `@packages/web/src/middleware/withAuth.test.ts`:
- Around line 304-312: Add a negative test alongside the existing scoped-token
tests that passes a scoped token using the DPoP authorization scheme, calls
getAuthenticatedUser, and asserts it returns undefined. Also verify scoped-token
and API-key lookups are not called, preserving rejection before the scoped-token
branch.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a200573c-d681-4fde-ad8f-91059c43107e
📒 Files selected for processing (23)
CHANGELOG.mddocs/api-reference/sourcebot-public.openapi.jsondocs/docs.jsondocs/docs/api-reference/authentication.mdxpackages/db/prisma/migrations/20260806033656_add_scoped_access_tokens/migration.sqlpackages/db/prisma/schema.prismapackages/shared/src/constants.tspackages/shared/src/crypto.tspackages/shared/src/index.server.tspackages/web/src/app/api/(server)/ee/scoped_access_token/[id]/route.tspackages/web/src/app/api/(server)/ee/scoped_access_token/route.tspackages/web/src/ee/features/scopedAccessTokens/api.test.tspackages/web/src/ee/features/scopedAccessTokens/api.tspackages/web/src/features/search/searchApi.test.tspackages/web/src/features/search/searchApi.tspackages/web/src/features/search/zoektSearcher.tspackages/web/src/lib/errorCodes.tspackages/web/src/middleware/withAuth.test.tspackages/web/src/middleware/withAuth.tspackages/web/src/openapi/publicApiDocument.tspackages/web/src/openapi/publicApiSchemas.tspackages/web/src/prisma.test.tspackages/web/src/prisma.ts
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort 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 e0b2171. Configure here.
Contributor
Author
|
Companion Lighthouse entitlement registry PR: https://github.com/sourcebot-dev/lighthouse/pull/52. Deploy the Lighthouse change first so signed license assertions can grant scoped-access-tokens before this PR begins enforcing it. |
brendan-kellam
changed the title
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.
Summary
Validation
Linear: SOU-1870
Note
High Risk
New authentication path and repository scoping touch auth middleware, Prisma query filtering, and search—security-sensitive surfaces where mis-scoping could leak repository access.
Overview
Adds one-hour opaque bearer tokens (
sbst_) limited to explicit repository IDs, mintable and revocable only with a Sourcebot API key under the newscoped-access-tokensentitlement.Mint/revoke:
POST /api/ee/scoped_access_tokenvalidatesrepoIdsagainst the API-key owner’s org repos, stores hashed secrets with repo join rows, and returns the plaintext token once;DELETE /api/ee/scoped_access_token/{id}removes tokens owned by that user in the org.Auth: Bearer
sbst_tokens resolve to the creating user and anAuthPrincipalcarryingrepositoryIds;withAuthsupportsrequiredAuthSource: 'api_key'so scoped tokens cannot mint or revoke. The user-scoped Prisma extension intersects token repo IDs with permission-sync filters when enabled.Search: Blocking and streaming search always constrain Zoekt to repos visible through the scoped client for scoped tokens (including empty scope), instead of treating them as unrestricted like API keys when permission syncing is off.
Reviewed by Cursor Bugbot for commit c9b3cbd. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit