diff --git a/docs/providers.md b/docs/providers.md index 0c28bc70..51d6efab 100644 --- a/docs/providers.md +++ b/docs/providers.md @@ -208,11 +208,11 @@ git@git.example.com:Group/Subgroup/repo.git | 操作 | 实现 | |------------------------|-------------------------------------------------------------| -| clone | `git clone /...`，token 以 `oauth2:` 基本认证注入 | +| clone | `git clone /...`，token 以 `oauth2:` 基本认证经 `-c http.extraHeader` 注入（不写进 URL，因此不会残留在克隆仓库的 `.git/config`） | | 创建仓库 | `POST /api/v4/projects`（用户 namespace，或按路径精确解析 group；解析不到直接报错，不会退回个人 namespace） | | 创建 MR | `POST /api/v4/projects/:id/merge_requests` | | 指定 reviewer | 解析 username → user id，提交 `reviewer_ids` | -| 拉取 MR 数据 | `GET /api/v4/projects/:id/merge_requests/:iid` + commits + changes | +| 拉取 MR 数据 | `GET /api/v4/projects/:id/merge_requests/:iid` + commits + changes；MR URL 的 host 必须与已配置实例（`GITLAB_URL` / `TEAMAI_GITLAB_HOST`，默认 gitlab.com）一致，否则拒绝请求，避免把 token 发往未配置的 host | | 列出 group 仓库 | `GET /api/v4/groups/:path/projects`（分页，`include_subgroups=true` 含子组） | ### 默认 email 域 diff --git a/src/__tests__/gitlab-clone-realspawn.test.ts b/src/__tests__/gitlab-clone-realspawn.test.ts new file mode 100644 index 00000000..100f41aa --- /dev/null +++ b/src/__tests__/gitlab-clone-realspawn.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +// NOTE: no vi.mock('node:child_process') here — this exercises a REAL git spawn +// against a fake `git` on PATH, to verify the token never reaches the URL/argv +// and that error output is sanitized end-to-end. +import { gitlabRepoClone } from '../providers/gitlab/gitlab-api.js'; + +describe('gitlabRepoClone — real spawn (e2e)', () => { + let tmp: string; + const origPath = process.env.PATH; + const origEnv = { ...process.env }; + + beforeEach(() => { + tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'gl-clone-e2e-')); + const bin = path.join(tmp, 'bin'); + fs.mkdirSync(bin); + // Fake git: records its argv, then fails like a 403 with creds in the URL. + const fakeGit = path.join(bin, 'git'); + fs.writeFileSync( + fakeGit, + [ + '#!/bin/bash', + `printf '%s\\n' "$@" > "${path.join(tmp, 'argv.txt')}"`, + `echo "fatal: unable to access 'https://oauth2:glpat_e2e_secret@gitlab.example.com/org/repo.git/': The requested URL returned error: 403" >&2`, + 'exit 128', + ].join('\n'), + { mode: 0o755 }, + ); + process.env.PATH = `${bin}:${origPath}`; + process.env.GITLAB_TOKEN = 'glpat_e2e_secret'; + }); + + afterEach(() => { + process.env.PATH = origPath; + process.env = { ...origEnv }; + fs.rmSync(tmp, { recursive: true, force: true }); + }); + + it('never passes the token in the clone URL/argv and sanitizes the error', () => { + let err: Error | null = null; + try { + gitlabRepoClone('org/repo', path.join(tmp, 'dest')); + } catch (e) { + err = e as Error; + } + + // The real git argv, as the child process saw it. + const argv = fs.readFileSync(path.join(tmp, 'argv.txt'), 'utf-8').split('\n'); + const urlArg = argv.find((a) => a.endsWith('.git')); + expect(urlArg).toBeDefined(); + // Token must NOT be embedded in the clone URL. + expect(urlArg).not.toContain('glpat_e2e_secret'); + expect(urlArg).not.toContain('oauth2:'); + // Token travels only inside the http.extraHeader arg (base64), never plaintext. + expect(argv.some((a) => a.includes('glpat_e2e_secret'))).toBe(false); + expect(argv.some((a) => a.startsWith('http.extraHeader=Authorization: Basic '))).toBe(true); + + // Error surfaced to the caller is sanitized. + expect(err).not.toBeNull(); + expect(err!.message).not.toContain('glpat_e2e_secret'); + expect(err!.message).toContain('***@'); + }); +}); diff --git a/src/__tests__/gitlab-provider.test.ts b/src/__tests__/gitlab-provider.test.ts index bed1cab3..014dd7da 100644 --- a/src/__tests__/gitlab-provider.test.ts +++ b/src/__tests__/gitlab-provider.test.ts @@ -212,7 +212,7 @@ describe('gitlabRepoClone', () => { expect(() => gitlabRepoClone('org/missing', '/tmp/clone')).toThrow(GitLabRepoNotFoundError); }); - it('succeeds when git clone exits 0', () => { + it('injects the token via http.extraHeader, never into the clone URL', () => { process.env.GITLAB_TOKEN = 'glpat_secret'; mockedSpawnSync.mockReturnValue({ status: 0, @@ -220,19 +220,44 @@ describe('gitlabRepoClone', () => { stderr: '', }); expect(() => gitlabRepoClone('org/repo', '/tmp/clone')).not.toThrow(); - const args = mockedSpawnSync.mock.calls[0][1]; + const args = mockedSpawnSync.mock.calls[0][1] as string[]; + // -c http.extraHeader=Authorization: Basic + expect(args[0]).toBe('-c'); + const expectedHeader = `http.extraHeader=Authorization: Basic ${Buffer.from('oauth2:glpat_secret').toString('base64')}`; + expect(args[1]).toBe(expectedHeader); + expect(args[2]).toBe('clone'); + // The token must NOT appear anywhere in the clone URL. + const cloneUrlArg = args.find((a) => a.endsWith('.git')); + expect(cloneUrlArg).toBeDefined(); + expect(cloneUrlArg).not.toContain('glpat_secret'); + expect(cloneUrlArg).not.toContain('oauth2:'); + // And the raw token must not appear in any argument. + expect(args.some((a) => a.includes('glpat_secret'))).toBe(false); + }); + + it('clones anonymously (no auth header) when no token is set', () => { + delete process.env.GITLAB_TOKEN; + delete process.env.GITLAB_PRIVATE_TOKEN; + delete process.env.GITLAB_PAT; + mockedSpawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); + gitlabRepoClone('org/repo', '/tmp/clone'); + const args = mockedSpawnSync.mock.calls[0][1] as string[]; expect(args[0]).toBe('clone'); - expect(args[1]).toContain('oauth2:glpat_secret@'); + expect(args.some((a) => a.includes('http.extraHeader'))).toBe(false); }); - it('sanitizes token from error output', () => { + it('sanitizes credentials from error output via the shared helper', () => { process.env.GITLAB_TOKEN = 'glpat_secret'; mockedSpawnSync.mockReturnValue({ status: 128, stdout: '', - stderr: 'fatal: Authentication failed for host oauth2:glpat_secret@gitlab.example.com', + // git echoes the full URL (with any embedded userinfo) on access errors. + stderr: "fatal: unable to access 'https://oauth2:glpat_secret@gitlab.example.com/org/repo.git/': The requested URL returned error: 403", }); - expect(() => gitlabRepoClone('org/repo', '/tmp/clone')).toThrow(/oauth2:\*\*\*@/); + const err = (() => { try { gitlabRepoClone('org/repo', '/tmp/clone'); return null; } catch (e) { return e as Error; } })(); + expect(err).not.toBeNull(); + expect(err!.message).not.toContain('glpat_secret'); + expect(err!.message).toContain('***@'); }); }); @@ -531,17 +556,21 @@ describe('gitlabRepoClone — self-hosted clone URL', () => { process.env = { ...originalEnv }; }); - it('preserves scheme, port and path prefix when embedding the token', () => { + it('preserves scheme, port and path prefix in the clone URL — without the token', () => { process.env.GITLAB_TOKEN = 'glpat_secret'; process.env.GITLAB_URL = 'http://gitlab.internal:8929/gitlab'; mockedSpawnSync.mockReturnValue({ status: 0, stdout: '', stderr: '' }); gitlabRepoClone('group/repo', '/tmp/clone'); - const cloneTarget = mockedSpawnSync.mock.calls[0][1][1]; - expect(cloneTarget).toBe( - 'http://oauth2:glpat_secret@gitlab.internal:8929/gitlab/group/repo.git', - ); + const args = mockedSpawnSync.mock.calls[0][1] as string[]; + const cloneTarget = args.find((a) => a.endsWith('.git')); + // URL keeps the self-hosted scheme/port/prefix but carries NO credentials. + expect(cloneTarget).toBe('http://gitlab.internal:8929/gitlab/group/repo.git'); + // Token travels in the extraHeader instead. + const expectedHeader = `http.extraHeader=Authorization: Basic ${Buffer.from('oauth2:glpat_secret').toString('base64')}`; + expect(args).toContain(expectedHeader); + expect(args.some((a) => a.includes('glpat_secret') && a.endsWith('.git'))).toBe(false); }); }); @@ -560,8 +589,9 @@ describe('fetchGitLabMR', () => { process.env = { ...originalEnv }; }); - it('queries the host named by the MR URL, not the configured instance', async () => { - process.env.GITLAB_URL = 'https://gitlab.com'; + it('queries the MR URL host when it matches the configured instance', async () => { + // Default GITLAB_HOST in the test process is gitlab.com; an MR URL on that + // host is trusted and the token is sent to it. const seen: string[] = []; global.fetch = vi.fn(async (url: string) => { seen.push(String(url)); @@ -577,16 +607,27 @@ describe('fetchGitLabMR', () => { ); }) as never; - const mr = await fetchGitLabMR('https://git.corp.example.com/team/repo/-/merge_requests/42'); + const mr = await fetchGitLabMR('https://gitlab.com/team/repo/-/merge_requests/42'); - expect(seen.every((u) => u.startsWith('https://git.corp.example.com/api/v4/'))).toBe(true); - expect(seen.some((u) => u.includes('gitlab.com'))).toBe(false); + expect(seen.every((u) => u.startsWith('https://gitlab.com/api/v4/'))).toBe(true); expect(mr.title).toBe('T'); expect(mr.author).toBe('bob'); expect(mr.commits).toEqual([{ hash: 'abc123', message: 'first' }]); expect(mr.diff).toContain('@@'); }); + 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. + const fetchSpy = vi.fn(async () => new Response('{}', { status: 200 })); + global.fetch = fetchSpy as never; + + await expect( + fetchGitLabMR('https://git.corp.example.com/team/repo/-/merge_requests/42'), + ).rejects.toThrow(/does not match the configured GitLab instance/); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + it('encodes a nested group path into the project id', async () => { const seen: string[] = []; global.fetch = vi.fn(async (url: string) => { diff --git a/src/providers/gitlab/gitlab-api.ts b/src/providers/gitlab/gitlab-api.ts index 1bf7c3cc..814e015b 100644 --- a/src/providers/gitlab/gitlab-api.ts +++ b/src/providers/gitlab/gitlab-api.ts @@ -1,5 +1,6 @@ import { spawnSync } from 'node:child_process'; import { GITLAB_HOST } from './repo-url.js'; +import { sanitizeGitUrl } from '../../utils/redact.js'; /** * GitLab REST API client (API v4). @@ -139,29 +140,47 @@ export class GitLabRepoNotFoundError extends Error { } /** - * Build a git clone URL embedding the token (oauth2 scheme, per GitLab docs). + * Build the git clone URL WITHOUT credentials. * * Derived from the instance base URL rather than re-assembled from GITLAB_HOST, * so a self-hosted instance keeps its scheme (`http://` internal deployments), * its port, and any relative-URL-root prefix (`https://example.com/gitlab`). + * The token is injected out-of-band via `http.extraHeader` (see gitlabRepoClone) + * so it never lands in the remote URL, and therefore is not persisted to the + * cloned repo's `.git/config` (where a URL-embedded credential would remain for + * every later fetch/push). This mirrors the http.extraHeader approach already + * used in clone.ts. */ function cloneUrl(repo: string): string { - const token = getGitLabToken(); - if (!token) return `${gitlabBaseUrl()}/${repo}.git`; - - const url = new URL(gitlabBaseUrl()); - url.username = 'oauth2'; - url.password = token; - const base = url.toString().replace(/\/+$/, ''); + const base = gitlabBaseUrl().replace(/\/+$/, ''); return `${base}/${repo}.git`; } /** - * Clone a GitLab repo to localPath. Embeds the token in the remote URL so - * subsequent git ops work without extra auth. + * HTTP Basic auth header for a GitLab PAT (username fixed to `oauth2`), passed + * to git via `-c http.extraHeader=...` so the token stays out of the URL. + */ +function gitlabAuthHeaderArg(token: string): string { + const encoded = Buffer.from(`oauth2:${token}`).toString('base64'); + return `http.extraHeader=Authorization: Basic ${encoded}`; +} + +/** + * Clone a GitLab repo to localPath. The token is injected via http.extraHeader + * rather than embedded in the remote URL, so it is not persisted to + * `.git/config` for subsequent git operations. */ export function gitlabRepoClone(repo: string, localPath: string): void { - const result = spawnSync('git', ['clone', cloneUrl(repo), localPath], { + const token = getGitLabToken(); + // `-c =` is a git-level option and must precede the `clone` + // subcommand, matching the http.extraHeader pattern in clone.ts. + const args: string[] = []; + if (token) { + args.push('-c', gitlabAuthHeaderArg(token)); + } + args.push('clone', cloneUrl(repo), localPath); + + const result = spawnSync('git', args, { encoding: 'utf-8', stdio: ['pipe', 'pipe', 'pipe'], timeout: 120_000, @@ -180,7 +199,9 @@ export function gitlabRepoClone(repo: string, localPath: string): void { throw new GitLabRepoNotFoundError(repo); } - const sanitized = allOutput.replace(/oauth2:[^@]+@/g, 'oauth2:***@'); + // Redact any credentials git may have echoed back, using the shared helper so + // coverage stays consistent with the rest of the codebase. + const sanitized = sanitizeGitUrl(allOutput); throw new Error(`git clone failed: ${sanitized.trim()}`); } diff --git a/src/providers/gitlab/mr-fetch.ts b/src/providers/gitlab/mr-fetch.ts index 0315674c..0caf3e35 100644 --- a/src/providers/gitlab/mr-fetch.ts +++ b/src/providers/gitlab/mr-fetch.ts @@ -1,6 +1,7 @@ import { type MRData } from '../../types.js'; import { log } from '../../utils/logger.js'; import { getGitLabToken } from './gitlab-api.js'; +import { GITLAB_HOST } from './repo-url.js'; /** GitLab MR URL 解析结果 */ interface ParsedGitLabMR { @@ -22,20 +23,45 @@ function parseGitLabMRUrl(url: string): ParsedGitLabMR { if (!match) { throw new Error(`Invalid GitLab MR URL: ${url}`); } + const scheme = match[1].toLowerCase(); + const host = match[2]; const projectPath = match[3]; if (!projectPath.includes('/')) { throw new Error(`Invalid GitLab MR URL: ${url}`); } - // Query the instance the URL actually names. Defaulting to the configured - // instance would send a self-hosted PAT to gitlab.com and read a different - // project that merely shares the same path. + // Only ever send the token to the *configured* instance. We still derive the + // API base from the URL's own origin (so a self-hosted instance keeps its + // scheme/port), but the host component must match GITLAB_HOST — otherwise a + // hand-crafted MR URL on an attacker-controlled host would receive the PAT + // (SSRF / credential exfiltration). The host compare is case-insensitive and + // strips a leading `www.`; ports must match exactly. + if (!hostMatchesConfigured(host)) { + throw new Error( + `Refusing to fetch GitLab MR from "${host}": it does not match the configured ` + + `GitLab instance "${GITLAB_HOST}". Set GITLAB_URL / TEAMAI_GITLAB_HOST to this ` + + `instance if it is trusted.`, + ); + } return { - apiBase: `${match[1].toLowerCase()}://${match[2]}/api/v4`, + apiBase: `${scheme}://${host}/api/v4`, projectPath, mrIid: match[4], }; } +/** Normalize a host for comparison: lowercase, drop a leading `www.`. */ +function normalizeHost(host: string): string { + return host.trim().toLowerCase().replace(/^www\./, ''); +} + +/** + * True when `urlHost` (host[:port] from an MR URL) refers to the same instance + * as the configured GITLAB_HOST. GITLAB_HOST may itself carry a port. + */ +function hostMatchesConfigured(urlHost: string): boolean { + return normalizeHost(urlHost) === normalizeHost(GITLAB_HOST); +} + /** GitLab REST API 返回的 MR 元信息（仅使用的字段） */ interface GitLabMR { title: string;