-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
fix(vercel): tighten internal integration scopes to org:ci #113394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 The PR adds This causes a regression for new Vercel installations: Additionally affects SentryApp API creation validation
(Refers to lines 1910-1914) Was this helpful? React with 👍 or 👎 to provide feedback. |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -492,7 +492,7 @@ def post_install( | |||||||||||||||||
| is_internal=True, | ||||||||||||||||||
| verify_install=False, | ||||||||||||||||||
| overview=internal_integration_overview.strip(), | ||||||||||||||||||
| scopes=["project:releases", "project:read", "project:write"], | ||||||||||||||||||
| scopes=["org:ci"], | ||||||||||||||||||
|
sentry[bot] marked this conversation as resolved.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the internal integrations can always have their perms raised by the orgs themselves if they go to the edit page. this is how they currently appear after install:
We don't surface sentry/static/app/views/settings/organizationDeveloperSettings/sentryApplicationDetails.tsx Lines 117 to 124 in 6c394f9
If there's no issue in publicizing it, we can add |
||||||||||||||||||
| ).run(user=user) | ||||||||||||||||||
| sentry_app_installation = SentryAppInstallation.objects.get(sentry_app=sentry_app) | ||||||||||||||||||
| SentryAppInstallationForProvider.objects.create( | ||||||||||||||||||
|
|
||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2123,6 +2123,74 @@ def test_org_auth_token(self) -> None: | |
| assert org_token.date_last_used is not None | ||
| assert org_token.project_last_used_id == project1.id | ||
|
|
||
| def test_sentry_app_installation_token_with_org_ci_scope(self) -> None: | ||
| """ | ||
| We switched to org:ci for Vercel SentryApp in PR #113394 but the | ||
| existing webhook tests didn't test for scopes specifically. | ||
| """ | ||
| user = self.create_user(is_staff=False, is_superuser=False) | ||
| org = self.create_organization() | ||
| org.flags.allow_joinleave = False | ||
| org.save() | ||
|
|
||
| team = self.create_team(organization=org) | ||
| project = self.create_project(teams=[team], organization=org) | ||
|
|
||
| url = reverse( | ||
| "sentry-api-0-organization-releases", | ||
| kwargs={"organization_id_or_slug": org.slug}, | ||
| ) | ||
|
|
||
| sentry_app = self.create_internal_integration( | ||
| name="Test Vercel Internal Integration", | ||
| organization=org, | ||
| user=user, | ||
| scopes=["org:ci"], | ||
| ) | ||
| token = self.create_internal_integration_token(user=user, internal_integration=sentry_app) | ||
|
|
||
| with outbox_runner(): | ||
| response = self.client.post( | ||
| url, | ||
| data={"version": "1.2.1", "projects": [project.slug]}, | ||
| HTTP_AUTHORIZATION=f"Bearer {token.token}", | ||
| ) | ||
| assert response.status_code == 201, response.content | ||
| assert Release.objects.filter(organization_id=org.id, version="1.2.1").exists() | ||
|
|
||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this portion of the test doesn't seem necessary |
||
| app_no_scope = self.create_internal_integration( | ||
| name="Test No Scope Integration", | ||
| organization=org, | ||
| user=user, | ||
| scopes=[], | ||
| ) | ||
| token_no_scope = self.create_internal_integration_token( | ||
| user=user, internal_integration=app_no_scope | ||
| ) | ||
| response = self.client.post( | ||
| url, | ||
| data={"version": "1.2.2", "projects": [project.slug]}, | ||
| HTTP_AUTHORIZATION=f"Bearer {token_no_scope.token}", | ||
| ) | ||
| assert response.status_code == 403 | ||
|
|
||
| other_org = self.create_organization() | ||
| foreign_app = self.create_internal_integration( | ||
| name="Foreign Org Integration", | ||
| organization=other_org, | ||
| user=user, | ||
| scopes=["org:ci"], | ||
| ) | ||
| foreign_token = self.create_internal_integration_token( | ||
| user=user, internal_integration=foreign_app | ||
| ) | ||
| response = self.client.post( | ||
| url, | ||
| data={"version": "1.2.3", "projects": [project.slug]}, | ||
| HTTP_AUTHORIZATION=f"Bearer {foreign_token.token}", | ||
| ) | ||
| assert response.status_code == 403 | ||
|
|
||
| @patch("sentry.tasks.commits.fetch_commits") | ||
| def test_api_token(self, mock_fetch_commits: MagicMock) -> None: | ||
| user = self.create_user(is_staff=False, is_superuser=False) | ||
|
|
||

There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NON_ROLE_TOKEN_SCOPES bypasses role restrictions for all members
Medium Severity
_intersect_member_and_token_scopesgrantsNON_ROLE_TOKEN_SCOPES(org:ci,project:distribution) to any member who requests them via a token, regardless of their organization role. Previously, effective scopes were always the intersection of token scopes and member role scopes, meaning a low-privilege "member" role user couldn't exceed their role's capabilities via a token. Now, any member can create a user auth token withorg:ciand gain access to release creation, code mappings, and integration listing — capabilities their role doesn't grant. This broadens access for all token types across the platform, not just Vercel internal integration tokens.Additional Locations (1)
src/sentry/auth/access.py#L45-L48Reviewed by Cursor Bugbot for commit e807a5f. Configure here.