From 4e5be8da36ee4685dfd1cbf273fc5946c7c0b9c6 Mon Sep 17 00:00:00 2001 From: ydflow <314143294+ydflow@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:49:08 +0800 Subject: [PATCH] fix(gitlab): preserve instance subpath when importing MRs Signed-off-by: ydflow <314143294+ydflow@users.noreply.github.com> --- docs/guide/providers.md | 2 +- docs/guide/zh-CN/providers.md | 2 +- skill-data/core/references/troubleshooting.md | 4 +++ src/__tests__/gitlab-provider.test.ts | 32 +++++++++++++++++++ src/providers/gitlab/mr-fetch.ts | 15 +++++++-- 5 files changed, 50 insertions(+), 5 deletions(-) diff --git a/docs/guide/providers.md b/docs/guide/providers.md index db351fc71..ca810504a 100644 --- a/docs/guide/providers.md +++ b/docs/guide/providers.md @@ -176,7 +176,7 @@ export GITLAB_API_PREFIX=api/v4 # API path prefix; default api/ Three names are accepted for the token, in priority order: `GITLAB_TOKEN` > `GITLAB_PRIVATE_TOKEN` > `GITLAB_PAT`. An empty or whitespace-only value counts as unset and the next alias is tried. -`GITLAB_URL` **must carry a scheme** (`https://` or `http://`). `gitlab.example.com` on its own makes GitLab operations exit with an error instead of silently falling back to gitlab.com. Intranet http instances, non-standard ports and sub-path deployments (`https://example.com/gitlab`) are preserved in full, clone URLs included. +`GITLAB_URL` **must carry a scheme** (`https://` or `http://`). `gitlab.example.com` on its own makes GitLab operations exit with an error instead of silently falling back to gitlab.com. Intranet http instances, non-standard ports and sub-path deployments (`https://example.com/gitlab`) are preserved in full, clone URLs included. MR import also preserves this deployment prefix in API requests and leaves it out of the project id; the MR URL must belong to that instance path. `GITLAB_API_PREFIX` is for gateway setups where the GitLab API is mounted on a non-standard path (`/api/gitlab` instead of `/api/v4`). It also applies to the group repository listing used by `teamai import --from-org`, every page included; unset or empty, `api/v4` is used. Setting it avoids 405 errors in those setups. Examples: diff --git a/docs/guide/zh-CN/providers.md b/docs/guide/zh-CN/providers.md index abf5abde2..bf78935a0 100644 --- a/docs/guide/zh-CN/providers.md +++ b/docs/guide/zh-CN/providers.md @@ -179,7 +179,7 @@ export GITLAB_API_PREFIX=api/v4 # API 路径前缀;默认 api token 变量支持三个名字(按优先级):`GITLAB_TOKEN` > `GITLAB_PRIVATE_TOKEN` > `GITLAB_PAT`。空值/纯空白视为未设置,会继续尝试下一个别名。 -`GITLAB_URL` **必须带 scheme**(`https://` 或 `http://`)。写成 `gitlab.example.com` 会在执行 GitLab 操作时报错退出,而不是静默回落到 gitlab.com。内网 http 实例、非标准端口、以及挂在子路径下的部署(`https://example.com/gitlab`)都会被完整保留,包括 clone URL。 +`GITLAB_URL` **必须带 scheme**(`https://` 或 `http://`)。写成 `gitlab.example.com` 会在执行 GitLab 操作时报错退出,而不是静默回落到 gitlab.com。内网 http 实例、非标准端口、以及挂在子路径下的部署(`https://example.com/gitlab`)都会被完整保留,包括 clone URL。MR 导入也会在 API 请求中保留这个部署前缀,并从项目 ID 中排除它;MR URL 必须位于该实例路径下。 `GITLAB_API_PREFIX` 用于网关代理场景:某些自托管实例通过网关统一路由,GitLab API 被挂载到非标准路径(如 `/api/gitlab` 而不是标准的 `/api/v4`)。该配置也适用于 `teamai import --from-org` 的 group 仓库列表请求(包括所有分页);未设置或为空时仍使用 `api/v4`。此时设置该变量可避免 405 错误。示例: diff --git a/skill-data/core/references/troubleshooting.md b/skill-data/core/references/troubleshooting.md index 486fbe8a8..5a98c66dc 100644 --- a/skill-data/core/references/troubleshooting.md +++ b/skill-data/core/references/troubleshooting.md @@ -229,6 +229,10 @@ teamai init https://git.example.com/yourgroup/yourrepo A member who only syncs and never needs the CLI to open merge requests can skip both: `teamai init --provider git` uses their existing Git authentication. +For a GitLab deployed at a sub-path, include that path in `GITLAB_URL`, for +example `https://git.example.com/gitlab`. MR import keeps it in the API base +and removes it from the project id. An MR outside that instance path is refused. + ## Which tools actually get hooks `teamai hooks inject` prints **"Hooks injected into all AI tool settings"** even diff --git a/src/__tests__/gitlab-provider.test.ts b/src/__tests__/gitlab-provider.test.ts index 4327aa387..6a4d06b6c 100644 --- a/src/__tests__/gitlab-provider.test.ts +++ b/src/__tests__/gitlab-provider.test.ts @@ -639,6 +639,38 @@ describe('fetchGitLabMR', () => { expect(mr.diff).toContain('@@'); }); + it.each(['api/v4', 'api/gateway'])('keeps the instance root with API prefix %s when fetching an MR', async (prefix) => { + process.env.GITLAB_URL = 'https://gitlab.com/gitlab/'; + process.env.GITLAB_API_PREFIX = prefix; + const base = `https://gitlab.com/gitlab/${prefix}/projects/g%2Fsub%2Frepo/merge_requests/7`; + const seen: string[] = []; + global.fetch = vi.fn(async (url: string) => { + seen.push(String(url)); + if (String(url) === `${base}/commits?per_page=50`) { + return new Response(JSON.stringify([{ id: 'abc123', title: 'Fix' }]), { status: 200 }); + } + if (String(url) === `${base}/changes`) { + return new Response(JSON.stringify({ changes: [{ diff: '+fixed' }] }), { status: 200 }); + } + return new Response(JSON.stringify({ title: 'T', description: null, author: { username: 'a' } }), { status: 200 }); + }) as never; + + const mr = await fetchGitLabMR('https://gitlab.com/gitlab/g/sub/repo/-/merge_requests/7'); + + expect(seen).toEqual([base, `${base}/commits?per_page=50`, `${base}/changes`]); + expect(mr.commits).toEqual([{ hash: 'abc123', message: 'Fix' }]); + expect(mr.diff).toBe('+fixed'); + }); + + it('rejects an MR outside the configured instance root before fetching', async () => { + process.env.GITLAB_URL = 'https://gitlab.com/gitlab'; + const fetchSpy = vi.fn(); + global.fetch = fetchSpy; + await expect(fetchGitLabMR('https://gitlab.com/gitlab-other/g/repo/-/merge_requests/7')) + .rejects.toThrow(/does not match the configured GitLab instance path/); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + it('refuses to send the token to a host that is not the configured instance (SSRF guard)', async () => { // GITLAB_HOST defaults to gitlab.com; an MR URL on a different host must be // rejected BEFORE any network call, so the PAT is never exfiltrated. diff --git a/src/providers/gitlab/mr-fetch.ts b/src/providers/gitlab/mr-fetch.ts index 8c89e1eac..37ff19af9 100644 --- a/src/providers/gitlab/mr-fetch.ts +++ b/src/providers/gitlab/mr-fetch.ts @@ -1,6 +1,6 @@ import { type MRData } from '../../types.js'; import { log } from '../../utils/logger.js'; -import { getGitLabToken } from './gitlab-api.js'; +import { getGitLabToken, gitlabBaseUrl } from './gitlab-api.js'; import { GITLAB_HOST } from './repo-url.js'; /** GitLab MR URL 解析结果 */ @@ -25,7 +25,7 @@ function parseGitLabMRUrl(url: string): ParsedGitLabMR { } const scheme = match[1].toLowerCase(); const host = match[2]; - const projectPath = match[3]; + let projectPath = match[3]; if (!projectPath.includes('/')) { throw new Error(`Invalid GitLab MR URL: ${url}`); } @@ -42,10 +42,19 @@ function parseGitLabMRUrl(url: string): ParsedGitLabMR { `instance if it is trusted.`, ); } + const instancePath = new URL(gitlabBaseUrl()).pathname.replace(/\/+$/, ''); + if (instancePath) { + const prefix = `${instancePath.slice(1)}/`; + if (!projectPath.startsWith(prefix)) { + throw new Error(`GitLab MR URL does not match the configured GitLab instance path "${instancePath}": ${url}`); + } + projectPath = projectPath.slice(prefix.length); + if (!projectPath.includes('/')) throw new Error(`Invalid GitLab MR URL: ${url}`); + } // Use the same API prefix as gitlabApiBase() for consistency with MR creation. const apiPrefix = process.env.GITLAB_API_PREFIX?.trim().replace(/^\/+|\/+$/g, '') || 'api/v4'; return { - apiBase: `${scheme}://${host}/${apiPrefix}`, + apiBase: `${scheme}://${host}${instancePath}/${apiPrefix}`, projectPath, mrIid: match[4], };