From 4ce02ee569aa96752045783d74abbecc5c000eea Mon Sep 17 00:00:00 2001 From: bot-hermes Date: Tue, 18 Aug 2026 20:35:46 +0000 Subject: [PATCH] fix: address bot review findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add codeberg.org to deriveLinkedIssueProvider built-in Gitea check - Fix Gitea PR review prompt: !N → #N (GitLab vs Gitea syntax) - Update gitlab.mdx: remove 'read-only' claim (write support ships) - Gate walkthrough Gitea PR source (server can't fulfill yet) - Thread per-project API base URL override into walkthrough GitLab diff - Stop following cross-origin redirects in GitLab/Gitea clients (auth leak prevention) --- packages/docs/content/docs/gitlab.mdx | 2 +- .../views/walkthrough/WalkthroughView.tsx | 6 ++---- packages/ui/src/lib/linkedIssues.ts | 2 +- packages/ui/src/lib/magicPrompts.ts | 2 +- packages/web/server/lib/gitea/client.js | 3 +++ packages/web/server/lib/gitlab/client.js | 5 ++++- .../web/server/lib/walkthrough/pull-request.js | 16 +++++++++++++++- 7 files changed, 27 insertions(+), 9 deletions(-) diff --git a/packages/docs/content/docs/gitlab.mdx b/packages/docs/content/docs/gitlab.mdx index 7fbab1b5..c519c576 100644 --- a/packages/docs/content/docs/gitlab.mdx +++ b/packages/docs/content/docs/gitlab.mdx @@ -5,7 +5,7 @@ description: Connect GitLab and start sessions from issues and merge requests. # GitLab Issues & MRs -Connect your GitLab account and OpenChamber can pull in issues and merge requests, and start a session straight from one. GitLab support is new and currently read-only — OpenChamber can't create or merge MRs for you yet. +Connect your GitLab account and OpenChamber can pull in issues and merge requests, and start a session straight from one. You can also create, update, and merge MRs directly from OpenChamber. ## Connect GitLab diff --git a/packages/ui/src/components/views/walkthrough/WalkthroughView.tsx b/packages/ui/src/components/views/walkthrough/WalkthroughView.tsx index ae078733..287a1b43 100644 --- a/packages/ui/src/components/views/walkthrough/WalkthroughView.tsx +++ b/packages/ui/src/components/views/walkthrough/WalkthroughView.tsx @@ -265,10 +265,8 @@ export const WalkthroughView = ({ directory }: WalkthroughViewProps) => { const number = gitLabMr.mr?.number; return number ? { kind: 'pr', number } : null; } - if (gitProvider === 'gitea') { - const number = giteaPr.pr?.number; - return number ? { kind: 'pr', number } : null; - } + // Gitea PR diff is not yet supported server-side; omit the source to + // avoid offering a review that would fail with "no GitHub remote". return branchPrNumber ? { kind: 'pr', number: branchPrNumber } : null; }, [branchPrNumber, giteaPr.pr, gitLabMr.mr, gitProvider, source]); diff --git a/packages/ui/src/lib/linkedIssues.ts b/packages/ui/src/lib/linkedIssues.ts index 12c0a7e8..e35ec879 100644 --- a/packages/ui/src/lib/linkedIssues.ts +++ b/packages/ui/src/lib/linkedIssues.ts @@ -132,7 +132,7 @@ export const deriveLinkedIssueProvider = (url: string): ForgeProviderKind | null if (host === 'github.com') return 'github'; if (host === 'gitlab.com') return 'gitlab'; - if (host === 'gitea.com') return 'gitea'; + if (host === 'gitea.com' || host === 'codeberg.org') return 'gitea'; const { domains, apiBaseUrls } = useGitProviderDomainsStore.getState(); const gitlabAccounts = useGitLabAuthStore.getState().status?.accounts; diff --git a/packages/ui/src/lib/magicPrompts.ts b/packages/ui/src/lib/magicPrompts.ts index bc6d40e8..a30820ff 100644 --- a/packages/ui/src/lib/magicPrompts.ts +++ b/packages/ui/src/lib/magicPrompts.ts @@ -443,7 +443,7 @@ Do not implement changes until I confirm; end with: "Next actions: <1 sentence>" placeholders: [ { key: 'pr_number', description: 'Pull request number.' }, ], - template: 'Review this pull request !{{pr_number}} using the provided PR context', + template: 'Review this pull request #{{pr_number}} using the provided PR context', }, { id: 'gitea.pr.review.instructions', diff --git a/packages/web/server/lib/gitea/client.js b/packages/web/server/lib/gitea/client.js index 3ac96569..147a4abc 100644 --- a/packages/web/server/lib/gitea/client.js +++ b/packages/web/server/lib/gitea/client.js @@ -227,7 +227,9 @@ export function createGiteaClient({ token, baseUrl }) { // Follow redirects (301/302/308) exactly once. Gitea serves them for moved // repos/users; a manual redirect keeps our Authorization header across the hop. + // Only follow same-origin redirects to avoid leaking the token to a different host. let redirects = 0; + const baseHost = new URL(url).host; while ( (response.status === 301 || response.status === 302 || response.status === 308) && headerValue(response.headers, 'location') @@ -235,6 +237,7 @@ export function createGiteaClient({ token, baseUrl }) { ) { const location = headerValue(response.headers, 'location'); const nextUrl = new URL(location, url).toString(); + if (new URL(nextUrl).host !== baseHost) break; response = await conditionalFetch(nextUrl, fetchOptions); redirects += 1; } diff --git a/packages/web/server/lib/gitlab/client.js b/packages/web/server/lib/gitlab/client.js index 42c487f0..fddf2ff4 100644 --- a/packages/web/server/lib/gitlab/client.js +++ b/packages/web/server/lib/gitlab/client.js @@ -222,8 +222,10 @@ export function createGitLabClient({ token, baseUrl }) { // Follow a project-move redirect exactly once. GitLab redirects // (301/302/308) come with a `Location` for the new project URL; a manual - // redirect keeps our PRIVATE-TOKEN header across the hop. + // redirect keeps our PRIVATE-TOKEN header across the hop. Only follow + // same-origin redirects to avoid leaking the token to a different host. let redirects = 0; + const baseHost = new URL(url).host; while ( (response.status === 301 || response.status === 302 || response.status === 308) && headerValue(response.headers, 'location') @@ -231,6 +233,7 @@ export function createGitLabClient({ token, baseUrl }) { ) { const location = headerValue(response.headers, 'location'); const nextUrl = new URL(location, url).toString(); + if (new URL(nextUrl).host !== baseHost) break; response = await conditionalFetch(nextUrl, fetchOptions); redirects += 1; } diff --git a/packages/web/server/lib/walkthrough/pull-request.js b/packages/web/server/lib/walkthrough/pull-request.js index 22afef4d..d64b990b 100644 --- a/packages/web/server/lib/walkthrough/pull-request.js +++ b/packages/web/server/lib/walkthrough/pull-request.js @@ -1,6 +1,9 @@ import { getOctokitOrNull } from '../github/octokit.js'; import { resolveGitHubRepoFromDirectory } from '../github/repo/index.js'; import { getGitLabClientOrNull } from '../gitlab/client.js'; +import { createGitLabClient } from '../gitlab/client.js'; +import { getGitLabAuth, getGitLabDefaultBaseUrl } from '../gitlab/auth.js'; +import { getEffectiveProviderApiBaseUrl } from '../git-providers/project-config.js'; import { resolveGitLabRepoFromDirectory } from '../gitlab/repo.js'; // GitLab diff pagination cap: never loop more than 10 pages of 100 files, @@ -60,7 +63,18 @@ async function getGitHubPullRequestDiff(directory, number) { * here. */ async function getGitLabMergeRequestDiff(repo, number) { - const client = getGitLabClientOrNull(); + // Resolve per-project API base override, mirroring getClient() in routes.js. + const auth = getGitLabAuth(); + if (!auth?.accessToken) { + throw Object.assign(new Error('Connect a GitLab account to review merge requests'), { + statusCode: 401, + code: 'gitlab-not-connected', + }); + } + const effectiveBaseUrl = getEffectiveProviderApiBaseUrl('gitlab', directory) || getGitLabDefaultBaseUrl(); + const client = effectiveBaseUrl !== getGitLabDefaultBaseUrl() + ? createGitLabClient({ token: auth.accessToken, baseUrl: effectiveBaseUrl }) + : getGitLabClientOrNull(); if (!client) { throw Object.assign(new Error('Connect a GitLab account to review merge requests'), { statusCode: 401,