From ec1313c8f8333292506f05a355bb72d308a455d8 Mon Sep 17 00:00:00 2001 From: Sebastian Pugliese <73218979+RealDoc06@users.noreply.github.com> Date: Thu, 15 Oct 2026 03:15:53 +0200 Subject: [PATCH] fix(server): use workspace-scoped Bitbucket permissions API (#10984) Co-authored-by: RealDoc06 <73218979+RealDoc06@users.noreply.github.com> Co-authored-by: Theo Browne Co-Authored-By: Claude Opus 5.5 --- .../server/BitbucketPullRequestApi.test.ts | 63 +++++++++++-------- .../src/server/BitbucketPullRequestApi.ts | 46 +++++++------- .../src/server/bitbucketPullRequestJson.ts | 5 +- 3 files changed, 62 insertions(+), 52 deletions(-) diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts index 8d8feaba81503f75921a123f24c8f6510edf83e0..adc004bb84816ea78c06786ba61d463776bb49fb 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts @@ -998,11 +998,29 @@ layer("BitbucketPullRequestApi.layer", (it) => { assert.isFalse(yield* api.getRepositoryPermission({ repository: "acme/web" })); - expect(callAt(0).url).toContain("/user/permissions/repositories"); + expect(new URL(callAt(0).url, "https://api.bitbucket.org").pathname).toBe( + "/user/workspaces/acme/permissions/repositories", + ); assert.strictEqual(filterOfCall(0), 'repository.full_name="acme/web"'); }), ); + it.effect("matches repository permissions after normalizing whitespace around segments", () => + Effect.gen(function* () { + mockedRequest.mockImplementation((input) => { + const url = new URL(input.url, "https://api.bitbucket.org"); + const matches = + url.pathname === "/user/workspaces/acme/permissions/repositories" && + url.searchParams.get("q") === 'repository.full_name="acme/web"'; + return Effect.succeed(response(valuePage(matches ? [{ permission: "read" }] : []))); + }); + const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; + + // An unmatched, empty permission page is treated as unknown and would allow this attempt. + assert.isFalse(yield* api.getRepositoryPermission({ repository: " acme / web " })); + }), + ); + it.effect("escapes a repository name before it goes inside a filter literal", () => Effect.gen(function* () { mockedRequest.mockReturnValue(Effect.succeed(response(JSON.stringify({ values: [] })))); @@ -1015,43 +1033,36 @@ layer("BitbucketPullRequestApi.layer", (it) => { }), ); - it.effect( - "reads a removed permissions endpoint as granted rather than failing the merge on it", - () => + const permissionFailure = (status: number) => + Effect.fail( + new BitbucketApi.BitbucketResponseError({ + operation: "request", + status, + responseBodyLength: 0, + }), + ); + + it.effect.each([403, 404, 410])( + "reads HTTP %s from the permissions endpoint as an unknown permission", + (status) => Effect.gen(function* () { - // Bitbucket retired /user/permissions/repositories under CHANGE-2770: every account now - // gets HTTP 410 here, whatever it may do. - mockedRequest.mockReturnValue( - Effect.fail( - new BitbucketApi.BitbucketResponseError({ - operation: "request", - status: 410, - responseBodyLength: 0, - }), - ), - ); + mockedRequest.mockReturnValue(permissionFailure(status)); const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; assert.isTrue(yield* api.getRepositoryPermission({ repository: "acme/web" })); }), ); - it.effect("still fails the permission read on a failure that is not the removed endpoint", () => + it.effect.each([401, 500])("still fails the permission read on HTTP %s", (status) => Effect.gen(function* () { - mockedRequest.mockReturnValue( - Effect.fail( - new BitbucketApi.BitbucketResponseError({ - operation: "request", - status: 401, - responseBodyLength: 0, - }), - ), - ); + mockedRequest.mockReturnValue(permissionFailure(status)); const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; const error = yield* Effect.flip(api.getRepositoryPermission({ repository: "acme/web" })); - assert.strictEqual(error._tag, "BitbucketResponseError"); + if (error._tag === "BitbucketResponseError") { + assert.strictEqual(error.status, status); + } }), ); diff --git a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts index 6dd04a4a07736546eb65d4c2f1d2981aeea97a6c..a15283f691cb173d234bf53f1b2b99afd718b661 100644 --- a/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts +++ b/packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts @@ -112,14 +112,14 @@ export type BitbucketPullRequestApiError = | BitbucketRepositoryUnsupportedError | BitbucketDiffCommitError; -/** - * `/user/permissions/repositories` answering CHANGE-2770's removal notice rather than a - * permission — Bitbucket sends this for every account now, not only ones it would have refused. - */ -function isRepositoryPermissionRemovedError( +/** The permissions endpoint refusing or missing for these credentials, rather than answering. */ +function isRepositoryPermissionUnavailableError( error: BitbucketPullRequestApiError, ): error is BitbucketApi.BitbucketResponseError { - return error._tag === "BitbucketResponseError" && error.status === 410; + return ( + error._tag === "BitbucketResponseError" && + (error.status === 403 || error.status === 404 || error.status === 410) + ); } /** @@ -412,16 +412,19 @@ export const make = Effect.gen(function* () { const bitbucket = yield* BitbucketApi.BitbucketApi; /** - * The repository's own path, and the workspace above it — which the people who may review are - * kept on rather than on the repository, so both are handed over at once. + * The repository's API path and normalized segments, used for workspace reads and filters. */ const withRepository = ( repository: string, - use: (path: string, workspace: string) => Effect.Effect, + use: ( + path: string, + workspace: string, + slug: string, + ) => Effect.Effect, ): Effect.Effect => { const segments = repositorySegments(repository); return Result.isSuccess(segments) - ? use(repositoryPathOf(segments.success), segments.success.workspace) + ? use(repositoryPathOf(segments.success), segments.success.workspace, segments.success.slug) : Effect.fail(segments.failure); }; @@ -656,26 +659,23 @@ export const make = Effect.gen(function* () { }), ), - // Nothing on the repository, the pull request or the workspace states what the credentials - // may do, so this endpoint is the one request Bitbucket makes unavoidable. It is asked - // alongside the reads the detail was already making, so it costs no round trip of its own. + // Ask for the caller's effective permission within this workspace. The full name identifies + // the repository by its slug, even when its display name differs. // - // Bitbucket permanently removed this endpoint (CHANGE-2770): every account now gets HTTP 410 - // in place of an answer, whatever it may do. That is the deprecated-endpoint signal, not a - // permission being refused, so it is read the same way an unreachable read already is - // elsewhere — as a permission that could not be learned, which grants rather than blocks, and - // leaves the actual merge or write to say why if the account may not do it. Any other failure - // (a bad token, a network fault, an unreadable body) still fails as it did before. + // Bitbucket has retired permission endpoints before (CHANGE-2770), and some credentials may + // not reach `/user` endpoints at all. A 403, 404 or 410 here is read as a permission that + // could not be learned, the same as an empty page: it grants, and the write itself says why + // if the account may not do it. Other failures (a bad token, a network fault) still fail. getRepositoryPermission: (input) => - withRepository(input.repository, () => + withRepository(input.repository, (_path, workspace, slug) => readPage({ operation: "getRepositoryPermission", - url: `/user/permissions/repositories?q=${encodeURIComponent( - `repository.full_name="${filterLiteral(input.repository.trim())}"`, + url: `/user/workspaces/${encodeURIComponent(workspace)}/permissions/repositories?q=${encodeURIComponent( + `repository.full_name="${filterLiteral(`${workspace}/${slug}`)}"`, )}`, decode: decodeRepositoryPermissionJson, }), - ).pipe(Effect.catchIf(isRepositoryPermissionRemovedError, () => Effect.succeed(true))), + ).pipe(Effect.catchIf(isRepositoryPermissionUnavailableError, () => Effect.succeed(true))), getPullRequestDiff: pullRequestDiff, diff --git a/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts b/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts index 9337e888c840968df6b5a425cfe0cf17e8f42f9e..b59ea1a770757a850c0f0e70c156605cf3152ddc 100644 --- a/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts +++ b/packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts @@ -163,9 +163,8 @@ const RawViewerSchema = Schema.Struct({ }); /** - * `/user/permissions/repositories` filtered to one repository, which is the only place Bitbucket - * states what the credentials may do with it: nothing on the repository, the pull request or the - * workspace carries it. One row, or none where Bitbucket names no permission for this account. + * The caller's workspace-scoped repository permissions, filtered to one repository. + * One row, or none where Bitbucket names no permission for this account. */ const RawRepositoryPermissionsSchema = Schema.Struct({ values: Schema.optional( -- 2.54.0