From 6869a4a974e10d37724c01c2b1c950c5ae33acf4 Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 15:21:39 -0700 Subject: [PATCH 1/4] fix(worker): extend permission-sync fail-closed to HTTP 410 PR #1215 added a fail-closed branch in the account-driven permission syncer that clears an account's AccountToRepoPermission rows when the upstream call throws 401, 403, or a token-refresh error. The predicate set is too narrow for endpoint deprecations: Bitbucket Cloud's CHANGE-2770 removed GET /2.0/user/permissions/repositories and the endpoint now returns HTTP 410 Gone for every caller. With the existing predicate, the syncer aborts without clearing the account's rows, so stale permissions persist indefinitely. Add `isGone` (status 410) alongside `isUnauthorized` (401) and `isForbidden` (403), and include it in the catch in accountPermissionSyncer.runJob. The cleanup is the same scorched-earth deleteMany used by the existing predicates. Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 1 + .../backend/src/ee/accountPermissionSyncer.ts | 10 ++-- packages/backend/src/errors.test.ts | 52 ++++++++++++++++++- packages/backend/src/errors.ts | 1 + 4 files changed, 59 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 55683269b..299c4e0a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed issue where repo permissions could go stale when authentication or token refresh related errors occured. [#1215](https://github.com/sourcebot-dev/sourcebot/pull/1215) +- [EE] Account-driven permission syncer now also cleans up an account's permission rows when an upstream endpoint returns HTTP 410 Gone, so permissions don't get stuck after an API is removed (e.g. Bitbucket Cloud's CHANGE-2770). ## [4.17.2] - 2026-05-16 diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index 5f524d829..7d2c289e0 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -17,7 +17,7 @@ import { import { createBitbucketCloudClient, createBitbucketServerClient, getReposForAuthenticatedBitbucketCloudUser, getReposForAuthenticatedBitbucketServerUser } from "../bitbucket.js"; import { Settings } from "../types.js"; import { setIntervalAsync } from "../utils.js"; -import { isUnauthorized, isForbidden } from "../errors.js"; +import { isUnauthorized, isForbidden, isGone } from "../errors.js"; const LOG_TAG = 'user-permission-syncer'; const logger = createLogger(LOG_TAG); @@ -191,12 +191,14 @@ export class AccountPermissionSyncer { } catch (error) { // Fail-closed: when the code-host layer signals that the upstream // account is permanently unauthorized (token revoked, user - // deprovisioned, OAuth grant dead), clear the account's existing - // permission rows so the read-side filter stops matching through - // them. + // deprovisioned, OAuth grant dead) or that the endpoint we depend + // on is gone (e.g. Bitbucket Cloud's CHANGE-2770), clear the + // account's existing permission rows so the read-side filter stops + // matching through them. if ( isUnauthorized(error) || isForbidden(error) || + isGone(error) || error instanceof RefreshTokenError ) { await this.db.account.update({ diff --git a/packages/backend/src/errors.test.ts b/packages/backend/src/errors.test.ts index ac6a00dd4..c96b4f694 100644 --- a/packages/backend/src/errors.test.ts +++ b/packages/backend/src/errors.test.ts @@ -1,7 +1,7 @@ import { describe, expect, test } from 'vitest'; import { RequestError } from '@octokit/request-error'; import { GitbeakerRequestError } from '@gitbeaker/requester-utils'; -import { isForbidden, isUnauthorized } from './errors'; +import { isForbidden, isGone, isUnauthorized } from './errors'; import { throwOnHttpError } from './bitbucket'; // Helper: invoke the openapi-fetch middleware against a synthetic Response and @@ -148,6 +148,56 @@ describe('isForbidden', () => { }); }); +describe('isGone', () => { + test('Octokit RequestError with status 410', () => { + const err = new RequestError('Gone', 410, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isGone(err)).toBe(true); + }); + + test('Octokit RequestError with status 401 is NOT gone', () => { + const err = new RequestError('Unauthorized', 401, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isGone(err)).toBe(false); + }); + + test('Bitbucket middleware throws an isGone error on 410 Response', async () => { + // Real-world case: Bitbucket Cloud's CHANGE-2770 removed + // /2.0/user/permissions/repositories and now returns 410 Gone. + const err = await invokeMiddleware(new Response('CHANGE-2770 - Functionality has been deprecated', { status: 410 })); + expect(err).toBeInstanceOf(Error); + expect(isGone(err)).toBe(true); + }); + + test('real GitbeakerRequestError with response status 410', () => { + const err = new GitbeakerRequestError('Gone', { + cause: { + description: 'Gone', + request: new Request('https://gitlab.com/api/v4/projects'), + response: new Response(null, { status: 410 }), + }, + }); + expect(isGone(err)).toBe(true); + }); + + test('plain Error without status is NOT gone', () => { + expect(isGone(new Error('Missing required scope'))).toBe(false); + }); + + test('null is NOT gone', () => { + expect(isGone(null)).toBe(false); + }); + + test('Octokit RequestError with status 500 is NOT gone', () => { + const err = new RequestError('Internal Server Error', 500, { + request: { method: 'GET', url: 'https://api.github.com/user/repos', headers: {} }, + }); + expect(isGone(err)).toBe(false); + }); +}); + describe('throwOnHttpError middleware contract', () => { test('does not throw on 2xx Response', async () => { const err = await invokeMiddleware(new Response('ok', { status: 200 })); diff --git a/packages/backend/src/errors.ts b/packages/backend/src/errors.ts index 341c76736..d14760a49 100644 --- a/packages/backend/src/errors.ts +++ b/packages/backend/src/errors.ts @@ -27,3 +27,4 @@ const getStatus = (err: unknown): number | null => { export const isUnauthorized = (err: unknown): boolean => getStatus(err) === 401; export const isForbidden = (err: unknown): boolean => getStatus(err) === 403; +export const isGone = (err: unknown): boolean => getStatus(err) === 410; From fd242f7278be047a4b9061125c85152cacc1bf71 Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 15:22:17 -0700 Subject: [PATCH 2/4] chore: link CHANGELOG entry to #1216 Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 299c4e0a5..f770d7242 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed issue where repo permissions could go stale when authentication or token refresh related errors occured. [#1215](https://github.com/sourcebot-dev/sourcebot/pull/1215) -- [EE] Account-driven permission syncer now also cleans up an account's permission rows when an upstream endpoint returns HTTP 410 Gone, so permissions don't get stuck after an API is removed (e.g. Bitbucket Cloud's CHANGE-2770). +- [EE] Account-driven permission syncer now also cleans up an account's permission rows when an upstream endpoint returns HTTP 410 Gone, so permissions don't get stuck after an API is removed (e.g. Bitbucket Cloud's CHANGE-2770). [#1216](https://github.com/sourcebot-dev/sourcebot/pull/1216) ## [4.17.2] - 2026-05-16 From 333a1aa68ed71f1b002b4347e4647e9c64967450 Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 15:26:02 -0700 Subject: [PATCH 3/4] fix(worker): log fail-closed permission cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the account-driven permission syncer's catch fires (401/403/410/ token-refresh), it silently wipes the account's AccountToRepoPermission rows before re-throwing. Operators can see the upstream error in three sinks already, but there's no signal that the *cleanup* itself ran — forcing a before/after row count to verify behavior. Switch the cleanup to a direct `accountToRepoPermission.deleteMany` so we get the deleted row count, and emit a warn log with the account id, user email, which predicate matched, the count, and the original error message. Behavior is equivalent to the prior `account.update` form (same DELETE WHERE accountId=...). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../backend/src/ee/accountPermissionSyncer.ts | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/packages/backend/src/ee/accountPermissionSyncer.ts b/packages/backend/src/ee/accountPermissionSyncer.ts index 7d2c289e0..031105dbc 100644 --- a/packages/backend/src/ee/accountPermissionSyncer.ts +++ b/packages/backend/src/ee/accountPermissionSyncer.ts @@ -195,20 +195,19 @@ export class AccountPermissionSyncer { // on is gone (e.g. Bitbucket Cloud's CHANGE-2770), clear the // account's existing permission rows so the read-side filter stops // matching through them. - if ( - isUnauthorized(error) || - isForbidden(error) || - isGone(error) || - error instanceof RefreshTokenError - ) { - await this.db.account.update({ - where: { id: account.id }, - data: { - accessibleRepos: { - deleteMany: {}, - }, - }, + const reason = + error instanceof RefreshTokenError ? 'token refresh failure' : + isUnauthorized(error) ? 'HTTP 401 Unauthorized' : + isForbidden(error) ? 'HTTP 403 Forbidden' : + isGone(error) ? 'HTTP 410 Gone' : + null; + + if (reason !== null) { + const { count } = await this.db.accountToRepoPermission.deleteMany({ + where: { accountId: account.id }, }); + const message = error instanceof Error ? error.message : String(error); + logger.warn(`Cleared ${count} permission row(s) for account ${account.id} (user ${account.user.email ?? 'unknown'}) — fail-closed cleanup triggered by ${reason}: ${message}`); } throw error; } From ee5b8ded57aa9700f27e36388f0aad1579728fca Mon Sep 17 00:00:00 2001 From: msukkari Date: Thu, 21 May 2026 15:28:41 -0700 Subject: [PATCH 4/4] docs: shorten CHANGELOG entry to one sentence Co-Authored-By: Claude Opus 4.7 (1M context) --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f770d7242..b70d11c4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed - Fixed issue where repo permissions could go stale when authentication or token refresh related errors occured. [#1215](https://github.com/sourcebot-dev/sourcebot/pull/1215) -- [EE] Account-driven permission syncer now also cleans up an account's permission rows when an upstream endpoint returns HTTP 410 Gone, so permissions don't get stuck after an API is removed (e.g. Bitbucket Cloud's CHANGE-2770). [#1216](https://github.com/sourcebot-dev/sourcebot/pull/1216) +- [EE] Fixed issue where repo permissions could go stale when an upstream endpoint returned HTTP 410 Gone (e.g. Bitbucket Cloud's CHANGE-2770). [#1216](https://github.com/sourcebot-dev/sourcebot/pull/1216) ## [4.17.2] - 2026-05-16