fix(worker): guard against anonymous Bitbucket Server token fallback in account permission sync#998
Merged
brendan-kellam merged 4 commits intomainfrom Mar 12, 2026
Conversation
…in account permission sync Bitbucket Server instances with anonymous access enabled silently treat expired/invalid OAuth tokens as anonymous rather than returning a 401. This caused account-driven permission syncing to receive an empty repo list (200 OK) and wipe all AccountToRepoPermission records. Added isBitbucketServerUserAuthenticated() which calls /rest/api/1.0/profile/recent/repos — an endpoint that always requires authentication even when anonymous access is enabled — to detect this condition before fetching repos. Also added explicit throws for unsupported provider/code host types instead of silently returning empty results. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
This comment has been minimized.
This comment has been minimized.
Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Contributor
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughAdds an explicit Bitbucket Server authentication check before fetching repositories and changes permission-sync workflows to throw errors for unsupported code host types; also documents a FIXED changelog entry for an OAuth-token expiration bug that could wipe Bitbucket Server repo permissions. Changes
Sequence Diagram(s)sequenceDiagram
participant Syncer as Account/RepoPermissionSyncer
participant Client as BitbucketClient
participant BBServer as Bitbucket Server API
participant DB as Permissions DB
rect rgba(0,128,255,0.5)
Syncer->>Client: isBitbucketServerUserAuthenticated()
end
rect rgba(0,200,83,0.5)
Client->>BBServer: GET /rest/api/1.0/profile/recent/repos
BBServer-->>Client: 200 / 401/403 / other
end
alt 200 OK
Client-->>Syncer: true
Syncer->>Client: getReposForAuthenticatedBitbucketServerUser()
Client->>BBServer: GET /rest/api/1.0/repos/...
BBServer-->>Client: repo list
Client-->>Syncer: repo IDs
Syncer->>DB: update permissions for repos
else 401 or 403
Client-->>Syncer: false
Syncer-->>Syncer: throw authentication error -> abort sync
else other error
Client-->>Syncer: propagate error -> abort sync
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/backend/src/bitbucket.ts`:
- Around line 789-794: The current isBitbucketServerUserAuthenticated conflates
any API error with “not authenticated”; update the implementation so it
distinguishes authentication failures from other errors by inspecting the API
error status (from the result or thrown exception from client.apiClient.GET).
Specifically, call client.apiClient.GET inside a try/catch, treat explicit auth
statuses (401 or 403) as authenticated=false (return false), rethrow or surface
any other errors so callers can handle service-side problems, and return true
only when the call succeeds; refer to isBitbucketServerUserAuthenticated and the
client.apiClient.GET(`/rest/api/1.0/profile/recent/repos`) call to locate where
to apply this change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5906002d-018b-4ac7-8c95-f05b8b375d07
📒 Files selected for processing (4)
CHANGELOG.mdpackages/backend/src/bitbucket.tspackages/backend/src/ee/accountPermissionSyncer.tspackages/backend/src/ee/repoPermissionSyncer.ts
brendan-kellam
deleted the
brendan/fix-bitbucket-server-permission-sync-SOU-635
branch
This was referenced Mar 13, 2026
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
feature.public.access) silently treat expired/invalid OAuth tokens as anonymous rather than returning a 401/rest/api/1.0/repos?permission=REPO_READ, which is guaranteed to return an empty list for anonymous requests, wiping allAccountToRepoPermissionrecordsisBitbucketServerUserAuthenticated()which calls/rest/api/1.0/profile/recent/repos— an endpoint that always requires authentication even with anonymous access enabled — to detect this condition and abort the sync job before any permissions are touchedTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit