| author | |
| committer | |
| log | ec1313c8f8333292506f05a355bb72d308a455d8 |
| tree | a28518d652a039da21c461a27d8bca695986d8db |
| parent | ab3a3cd162d90fd9f01ac62efdb9d4ad55c33639 |
| signature | Signed by PGP key B5690EEEBB952194 |
Co-authored-by: RealDoc06 <73218979+RealDoc06@users.noreply.github.com>
Co-authored-by: Theo Browne <me@t3.gg>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>3 files changed, 62 insertions(+), 52 deletions(-)
packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.test.ts+37-26| ... | @@ -998,11 +998,29 @@ layer("BitbucketPullRequestApi.layer", (it) => { | ... | @@ -998,11 +998,29 @@ layer("BitbucketPullRequestApi.layer", (it) => { |
| 998 | 998 | ||
| 999 | assert.isFalse(yield* api.getRepositoryPermission({ repository: "acme/web" })); | 999 | assert.isFalse(yield* api.getRepositoryPermission({ repository: "acme/web" })); |
| 1000 | 1000 | ||
| 1001 | expect(callAt(0).url).toContain("/user/permissions/repositories"); | 1001 | expect(new URL(callAt(0).url, "https://api.bitbucket.org").pathname).toBe( |
| 1002 | "/user/workspaces/acme/permissions/repositories", | ||
| 1003 | ); | ||
| 1002 | assert.strictEqual(filterOfCall(0), 'repository.full_name="acme/web"'); | 1004 | assert.strictEqual(filterOfCall(0), 'repository.full_name="acme/web"'); |
| 1003 | }), | 1005 | }), |
| 1004 | ); | 1006 | ); |
| 1005 | 1007 | ||
| 1008 | it.effect("matches repository permissions after normalizing whitespace around segments", () => | ||
| 1009 | Effect.gen(function* () { | ||
| 1010 | mockedRequest.mockImplementation((input) => { | ||
| 1011 | const url = new URL(input.url, "https://api.bitbucket.org"); | ||
| 1012 | const matches = | ||
| 1013 | url.pathname === "/user/workspaces/acme/permissions/repositories" && | ||
| 1014 | url.searchParams.get("q") === 'repository.full_name="acme/web"'; | ||
| 1015 | return Effect.succeed(response(valuePage(matches ? [{ permission: "read" }] : []))); | ||
| 1016 | }); | ||
| 1017 | const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; | ||
| 1018 | |||
| 1019 | // An unmatched, empty permission page is treated as unknown and would allow this attempt. | ||
| 1020 | assert.isFalse(yield* api.getRepositoryPermission({ repository: " acme / web " })); | ||
| 1021 | }), | ||
| 1022 | ); | ||
| 1023 | |||
| 1006 | it.effect("escapes a repository name before it goes inside a filter literal", () => | 1024 | it.effect("escapes a repository name before it goes inside a filter literal", () => |
| 1007 | Effect.gen(function* () { | 1025 | Effect.gen(function* () { |
| 1008 | mockedRequest.mockReturnValue(Effect.succeed(response(JSON.stringify({ values: [] })))); | 1026 | mockedRequest.mockReturnValue(Effect.succeed(response(JSON.stringify({ values: [] })))); |
| ... | @@ -1015,43 +1033,36 @@ layer("BitbucketPullRequestApi.layer", (it) => { | ... | @@ -1015,43 +1033,36 @@ layer("BitbucketPullRequestApi.layer", (it) => { |
| 1015 | }), | 1033 | }), |
| 1016 | ); | 1034 | ); |
| 1017 | 1035 | ||
| 1018 | it.effect( | 1036 | const permissionFailure = (status: number) => |
| 1019 | "reads a removed permissions endpoint as granted rather than failing the merge on it", | 1037 | Effect.fail( |
| 1020 | () => | 1038 | new BitbucketApi.BitbucketResponseError({ |
| 1039 | operation: "request", | ||
| 1040 | status, | ||
| 1041 | responseBodyLength: 0, | ||
| 1042 | }), | ||
| 1043 | ); | ||
| 1044 | |||
| 1045 | it.effect.each([403, 404, 410])( | ||
| 1046 | "reads HTTP %s from the permissions endpoint as an unknown permission", | ||
| 1047 | (status) => | ||
| 1021 | Effect.gen(function* () { | 1048 | Effect.gen(function* () { |
| 1022 | // Bitbucket retired /user/permissions/repositories under CHANGE-2770: every account now | 1049 | mockedRequest.mockReturnValue(permissionFailure(status)); |
| 1023 | // gets HTTP 410 here, whatever it may do. | ||
| 1024 | mockedRequest.mockReturnValue( | ||
| 1025 | Effect.fail( | ||
| 1026 | new BitbucketApi.BitbucketResponseError({ | ||
| 1027 | operation: "request", | ||
| 1028 | status: 410, | ||
| 1029 | responseBodyLength: 0, | ||
| 1030 | }), | ||
| 1031 | ), | ||
| 1032 | ); | ||
| 1033 | const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; | 1050 | const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; |
| 1034 | 1051 | ||
| 1035 | assert.isTrue(yield* api.getRepositoryPermission({ repository: "acme/web" })); | 1052 | assert.isTrue(yield* api.getRepositoryPermission({ repository: "acme/web" })); |
| 1036 | }), | 1053 | }), |
| 1037 | ); | 1054 | ); |
| 1038 | 1055 | ||
| 1039 | it.effect("still fails the permission read on a failure that is not the removed endpoint", () => | 1056 | it.effect.each([401, 500])("still fails the permission read on HTTP %s", (status) => |
| 1040 | Effect.gen(function* () { | 1057 | Effect.gen(function* () { |
| 1041 | mockedRequest.mockReturnValue( | 1058 | mockedRequest.mockReturnValue(permissionFailure(status)); |
| 1042 | Effect.fail( | ||
| 1043 | new BitbucketApi.BitbucketResponseError({ | ||
| 1044 | operation: "request", | ||
| 1045 | status: 401, | ||
| 1046 | responseBodyLength: 0, | ||
| 1047 | }), | ||
| 1048 | ), | ||
| 1049 | ); | ||
| 1050 | const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; | 1059 | const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi; |
| 1051 | 1060 | ||
| 1052 | const error = yield* Effect.flip(api.getRepositoryPermission({ repository: "acme/web" })); | 1061 | const error = yield* Effect.flip(api.getRepositoryPermission({ repository: "acme/web" })); |
| 1053 | |||
| 1054 | assert.strictEqual(error._tag, "BitbucketResponseError"); | 1062 | assert.strictEqual(error._tag, "BitbucketResponseError"); |
| 1063 | if (error._tag === "BitbucketResponseError") { | ||
| 1064 | assert.strictEqual(error.status, status); | ||
| 1065 | } | ||
| 1055 | }), | 1066 | }), |
| 1056 | ); | 1067 | ); |
| 1057 | 1068 |
packages/source-control-bitbucket/src/server/BitbucketPullRequestApi.ts+23-23| ... | @@ -112,14 +112,14 @@ export type BitbucketPullRequestApiError = | ... | @@ -112,14 +112,14 @@ export type BitbucketPullRequestApiError = |
| 112 | | BitbucketRepositoryUnsupportedError | 112 | | BitbucketRepositoryUnsupportedError |
| 113 | | BitbucketDiffCommitError; | 113 | | BitbucketDiffCommitError; |
| 114 | 114 | ||
| 115 | /** | 115 | /** The permissions endpoint refusing or missing for these credentials, rather than answering. */ |
| 116 | * `/user/permissions/repositories` answering CHANGE-2770's removal notice rather than a | 116 | function isRepositoryPermissionUnavailableError( |
| 117 | * permission — Bitbucket sends this for every account now, not only ones it would have refused. | ||
| 118 | */ | ||
| 119 | function isRepositoryPermissionRemovedError( | ||
| 120 | error: BitbucketPullRequestApiError, | 117 | error: BitbucketPullRequestApiError, |
| 121 | ): error is BitbucketApi.BitbucketResponseError { | 118 | ): error is BitbucketApi.BitbucketResponseError { |
| 122 | return error._tag === "BitbucketResponseError" && error.status === 410; | 119 | return ( |
| 120 | error._tag === "BitbucketResponseError" && | ||
| 121 | (error.status === 403 || error.status === 404 || error.status === 410) | ||
| 122 | ); | ||
| 123 | } | 123 | } |
| 124 | 124 | ||
| 125 | /** | 125 | /** |
| ... | @@ -412,16 +412,19 @@ export const make = Effect.gen(function* () { | ... | @@ -412,16 +412,19 @@ export const make = Effect.gen(function* () { |
| 412 | const bitbucket = yield* BitbucketApi.BitbucketApi; | 412 | const bitbucket = yield* BitbucketApi.BitbucketApi; |
| 413 | 413 | ||
| 414 | /** | 414 | /** |
| 415 | * The repository's own path, and the workspace above it — which the people who may review are | 415 | * The repository's API path and normalized segments, used for workspace reads and filters. |
| 416 | * kept on rather than on the repository, so both are handed over at once. | ||
| 417 | */ | 416 | */ |
| 418 | const withRepository = <A>( | 417 | const withRepository = <A>( |
| 419 | repository: string, | 418 | repository: string, |
| 420 | use: (path: string, workspace: string) => Effect.Effect<A, BitbucketPullRequestApiError>, | 419 | use: ( |
| 420 | path: string, | ||
| 421 | workspace: string, | ||
| 422 | slug: string, | ||
| 423 | ) => Effect.Effect<A, BitbucketPullRequestApiError>, | ||
| 421 | ): Effect.Effect<A, BitbucketPullRequestApiError> => { | 424 | ): Effect.Effect<A, BitbucketPullRequestApiError> => { |
| 422 | const segments = repositorySegments(repository); | 425 | const segments = repositorySegments(repository); |
| 423 | return Result.isSuccess(segments) | 426 | return Result.isSuccess(segments) |
| 424 | ? use(repositoryPathOf(segments.success), segments.success.workspace) | 427 | ? use(repositoryPathOf(segments.success), segments.success.workspace, segments.success.slug) |
| 425 | : Effect.fail(segments.failure); | 428 | : Effect.fail(segments.failure); |
| 426 | }; | 429 | }; |
| 427 | 430 | ||
| ... | @@ -656,26 +659,23 @@ export const make = Effect.gen(function* () { | ... | @@ -656,26 +659,23 @@ export const make = Effect.gen(function* () { |
| 656 | }), | 659 | }), |
| 657 | ), | 660 | ), |
| 658 | 661 | ||
| 659 | // Nothing on the repository, the pull request or the workspace states what the credentials | 662 | // Ask for the caller's effective permission within this workspace. The full name identifies |
| 660 | // may do, so this endpoint is the one request Bitbucket makes unavoidable. It is asked | 663 | // the repository by its slug, even when its display name differs. |
| 661 | // alongside the reads the detail was already making, so it costs no round trip of its own. | ||
| 662 | // | 664 | // |
| 663 | // Bitbucket permanently removed this endpoint (CHANGE-2770): every account now gets HTTP 410 | 665 | // Bitbucket has retired permission endpoints before (CHANGE-2770), and some credentials may |
| 664 | // in place of an answer, whatever it may do. That is the deprecated-endpoint signal, not a | 666 | // not reach `/user` endpoints at all. A 403, 404 or 410 here is read as a permission that |
| 665 | // permission being refused, so it is read the same way an unreachable read already is | 667 | // could not be learned, the same as an empty page: it grants, and the write itself says why |
| 666 | // elsewhere — as a permission that could not be learned, which grants rather than blocks, and | 668 | // if the account may not do it. Other failures (a bad token, a network fault) still fail. |
| 667 | // leaves the actual merge or write to say why if the account may not do it. Any other failure | ||
| 668 | // (a bad token, a network fault, an unreadable body) still fails as it did before. | ||
| 669 | getRepositoryPermission: (input) => | 669 | getRepositoryPermission: (input) => |
| 670 | withRepository(input.repository, () => | 670 | withRepository(input.repository, (_path, workspace, slug) => |
| 671 | readPage({ | 671 | readPage({ |
| 672 | operation: "getRepositoryPermission", | 672 | operation: "getRepositoryPermission", |
| 673 | url: `/user/permissions/repositories?q=${encodeURIComponent( | 673 | url: `/user/workspaces/${encodeURIComponent(workspace)}/permissions/repositories?q=${encodeURIComponent( |
| 674 | `repository.full_name="${filterLiteral(input.repository.trim())}"`, | 674 | `repository.full_name="${filterLiteral(`${workspace}/${slug}`)}"`, |
| 675 | )}`, | 675 | )}`, |
| 676 | decode: decodeRepositoryPermissionJson, | 676 | decode: decodeRepositoryPermissionJson, |
| 677 | }), | 677 | }), |
| 678 | ).pipe(Effect.catchIf(isRepositoryPermissionRemovedError, () => Effect.succeed(true))), | 678 | ).pipe(Effect.catchIf(isRepositoryPermissionUnavailableError, () => Effect.succeed(true))), |
| 679 | 679 | ||
| 680 | getPullRequestDiff: pullRequestDiff, | 680 | getPullRequestDiff: pullRequestDiff, |
| 681 | 681 |
packages/source-control-bitbucket/src/server/bitbucketPullRequestJson.ts+2-3| ... | @@ -163,9 +163,8 @@ const RawViewerSchema = Schema.Struct({ | ... | @@ -163,9 +163,8 @@ const RawViewerSchema = Schema.Struct({ |
| 163 | }); | 163 | }); |
| 164 | 164 | ||
| 165 | /** | 165 | /** |
| 166 | * `/user/permissions/repositories` filtered to one repository, which is the only place Bitbucket | 166 | * The caller's workspace-scoped repository permissions, filtered to one repository. |
| 167 | * states what the credentials may do with it: nothing on the repository, the pull request or the | 167 | * One row, or none where Bitbucket names no permission for this account. |
| 168 | * workspace carries it. One row, or none where Bitbucket names no permission for this account. | ||
| 169 | */ | 168 | */ |
| 170 | const RawRepositoryPermissionsSchema = Schema.Struct({ | 169 | const RawRepositoryPermissionsSchema = Schema.Struct({ |
| 171 | values: Schema.optional( | 170 | values: Schema.optional( |