Skip to content

fix(gitlab): stop leaking the PAT (SSRF-scoped MR fetch, header-based clone auth) - #314

Merged
jeff-r2026 merged 1 commit into
Tencent:mainfrom
jeff-r2026:fix-gitlab-token-leak
Aug 24, 2026
Merged

fix(gitlab): stop leaking the PAT (SSRF-scoped MR fetch, header-based clone auth)#314
jeff-r2026 merged 1 commit into
Tencent:mainfrom
jeff-r2026:fix-gitlab-token-leak

Conversation

@jeff-r2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes three security issues in the GitLab provider added in #307. All three were flagged by automated security review; none are in scope of any other open PR.

1. [HIGH] SSRF / credential exfiltration — src/providers/gitlab/mr-fetch.ts

fetchGitLabMR derived the API base and sent the PAT to whatever host appeared in the user-supplied MR URL, with no check against the configured instance. A hand-crafted MR URL pointing at an attacker-controlled host would receive the token.

Fix: the URL host must match the configured instance (GITLAB_URL / TEAMAI_GITLAB_HOST, default gitlab.com), or the fetch is rejected before any network call. Scheme/port are still derived from the URL so self-hosted instances keep working — but only for the trusted host.

2. [MEDIUM] Credential persistence in .git/configsrc/providers/gitlab/gitlab-api.ts

gitlabRepoClone embedded the token in the clone URL (oauth2:<token>@…), which git writes into the cloned repo's .git/config and reuses for every later fetch/push.

Fix: inject the token via -c http.extraHeader=Authorization: Basic <base64(oauth2:token)> instead, so it never enters the URL or .git/config. Mirrors the existing http.extraHeader pattern in clone.ts.

3. [MEDIUM] Inconsistent redaction — src/providers/gitlab/gitlab-api.ts

The clone-error path used an inline oauth2:[^@]+@ regex. Now uses the shared sanitizeGitUrl from utils/redact.ts for consistent coverage.

Test plan

  • npx tsc --noEmit — no errors in src/ (test-file vitest-drift errors are pre-existing, unrelated)
  • npx vitest run — 2138 passed; the only failures are the pre-existing iwiki/import vitest-4.x drift files, unaffected by this change
  • Updated gitlab-provider.test.ts — asserts header-based auth, clean clone URL, and the SSRF rejection (token never sent to a non-configured host)
  • Added gitlab-clone-realspawn.test.ts — a real git spawn against a fake git on PATH, confirming the token never appears in the argv/URL (only base64 inside http.extraHeader) and the surfaced error is sanitized
  • docs/providers.md updated to match the new clone auth and MR-host behavior

🤖 Generated with Claude Code

…host, keep clone token out of the URL

Three security issues in the GitLab provider (Tencent#307):

1. [HIGH] SSRF / credential exfiltration in mr-fetch.ts — fetchGitLabMR derived
   the API base (and sent the PAT) from the host in the user-supplied MR URL,
   with no check against the configured instance. A hand-crafted MR URL on an
   attacker-controlled host would receive the token. Now the URL host must match
   GITLAB_HOST (GITLAB_URL / TEAMAI_GITLAB_HOST, default gitlab.com) or the fetch
   is rejected before any network call. We still derive scheme/port from the URL
   so self-hosted instances keep working — but only for the trusted host.

2. [MEDIUM] Credential persistence in gitlab-api.ts — gitlabRepoClone embedded
   the token in the clone URL (oauth2:<token>@...), which git writes into the
   cloned repo's .git/config for every later fetch/push. Inject it via
   `-c http.extraHeader=Authorization: Basic <base64(oauth2:token)>` instead, so
   the token never enters the URL or .git/config. Mirrors clone.ts.

3. [MEDIUM] Inconsistent redaction in gitlab-api.ts — the clone-error path used
   an inline `oauth2:[^@]+@` regex. Use the shared sanitizeGitUrl from
   utils/redact.ts so redaction coverage stays consistent with clone.ts.

Tests: updated gitlab-provider.test.ts to assert the header-based auth, the
clean clone URL, and the SSRF rejection; added gitlab-clone-realspawn.test.ts
which runs a REAL git spawn against a fake `git` on PATH and confirms the token
never appears in the argv/URL (only base64 in the extraHeader) and that the
surfaced error is sanitized. docs/providers.md updated to match.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@jeff-r2026
jeff-r2026 merged commit 828a951 into Tencent:main Aug 24, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant