fix(storage): strip credentials from remote URLs before they are persisted (#2914) (#2928)

`git config --get remote.origin.url` returns whatever the checkout was
configured with, and the HTTPS token form
`https://x-access-token:<token>@host/owner/repo` is how CI checkouts and
credential helpers routinely authenticate. `getRemoteUrl` kept that value
verbatim, so it reached `~/.gitnexus/registry.json` and the per-repo meta,
and MCP `list_repos` echoed it back — repository discovery doubled as
credential disclosure.

Three edges, one helper:

- `stripUrlCredentials` drops `user[:password]@` userinfo from http(s) URLs.
  `ssh://git@host/…` and SCP-like `git@host:owner/repo` are left alone: that
  is an SSH user name, not a secret, and rewriting it would repoint the
  sibling-clone fingerprint (#2054) for every registered repo.
- `getRemoteUrl` strips at capture, before the existing host lower-casing —
  that regex treats the whole `user:pass@host` span as the host, so it was
  also mangling the credential's case on the way to disk.
- The registry sanitizes on read AND write, so a `registry.json` (or a
  per-repo meta copied forward by a re-register) written by an older version
  is neither emitted nor rewritten with the credential still in it.

Also strips both URLs from the clone/remote mismatch error in
`assertRemoteMatchesRequestedUrl`, which is echoed to API callers and the
server log.

Sanitized values compare equal to a freshly captured remote on both sides,
so sibling matching, drift checks and `--name` inference are unchanged.


Claude-Session: https://claude.ai/code/session_01W8QYxYvd5ntikRUvpLZD5e

Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Gergő Magyar 2026-08-11 14:19:24 +01:00 committed by GitHub
parent 22d3c2ad74
commit 5f9648744c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
5 changed files with 252 additions and 16 deletions

View file

@ -10,7 +10,7 @@ import path from 'path';
import fs from 'fs/promises';
import { isIP } from 'net';
import { logger } from '../core/logger.js';
import { parseRepoNameFromUrl } from '../storage/git.js';
import { parseRepoNameFromUrl, stripUrlCredentials } from '../storage/git.js';
import { getGlobalDir } from '../storage/repo-manager.js';
/**
@ -410,7 +410,10 @@ export async function assertRemoteMatchesRequestedUrl(
}
if (normalizeGitUrlForCompare(remoteUrl) !== normalizeGitUrlForCompare(requestedUrl)) {
throw new Error(
`Existing clone at ${targetDir} has remote ${remoteUrl}, not the requested URL ${requestedUrl}`,
// Both URLs are echoed to the API caller and the server log, and either
// can carry `https://user:token@` userinfo — strip it here too (#2914).
`Existing clone at ${targetDir} has remote ${stripUrlCredentials(remoteUrl)}, ` +
`not the requested URL ${stripUrlCredentials(requestedUrl)}`,
);
}
}

View file

@ -201,6 +201,28 @@ export const getCurrentCommit = (repoPath: string): string => {
}
};
/**
* Remove `user[:password]@` userinfo from an http(s) URL.
*
* `git config remote.origin.url` returns whatever was configured, and the
* HTTPS token form `https://x-access-token:<token>@host/owner/repo` is how
* CI checkouts and credential helpers routinely authenticate. That string
* reached `registry.json`, the per-repo meta and the MCP `list_repos`
* payload verbatim, which turned repository discovery into credential
* disclosure (#2914).
*
* Only `http`/`https` are rewritten. `ssh://git@host/…` and the SCP-like
* `git@host:owner/repo` carry an SSH *user name*, not a secret, and are part
* of the remote's identity dropping it would repoint the sibling-clone
* fingerprint (#2054) for every already-registered repo.
*
* The match is bounded by the authority (`[^/]*` cannot cross the first `/`
* after the scheme) and greedy to the last `@` in it, so a password
* containing `@` is removed whole rather than leaving its tail behind.
*/
export const stripUrlCredentials = (url: string): string =>
url.replace(/^(https?:\/\/)[^/]*@/i, '$1');
/**
* Get a stable canonical identifier for the repo's `origin` remote, if any.
*
@ -212,6 +234,10 @@ export const getCurrentCommit = (repoPath: string): string => {
* survives those conventions.
*
* Normalisation strategy:
* - Strip http(s) userinfo credentials (see {@link stripUrlCredentials}).
* Done FIRST, before the host lower-casing below that regex treats the
* whole `user:pass@host` span as the host and would mangle the secret's
* case on its way into the registry (#2914).
* - Strip a trailing `.git` so `https://x/y` and `https://x/y.git` collapse.
* - Strip a trailing `/` for the same reason.
* - `git@github.com:foo/bar` and `https://github.com/foo/bar` are
@ -239,7 +265,9 @@ export const getRemoteUrl = (repoPath: string): string | undefined => {
}
if (!raw) return undefined;
let normalised = raw.replace(/\/$/, '').replace(/\.git$/, '');
let normalised = stripUrlCredentials(raw)
.replace(/\/$/, '')
.replace(/\.git$/, '');
// Lower-case the host segment of `scheme://[user@]host[:port]/...`
// and the host segment of `git@host:owner/repo` SCP form.

View file

@ -18,7 +18,7 @@ import fs from 'fs/promises';
import { realpathSync } from 'fs';
import path from 'path';
import os from 'os';
import { getInferredRepoName, resolveRepoIdentityRoot } from './git.js';
import { getInferredRepoName, resolveRepoIdentityRoot, stripUrlCredentials } from './git.js';
import { stripWindowsLongPathPrefix } from '../lib/utils.js';
import { writeFileAtomic } from './fs-atomic.js';
import { logger } from '../core/logger.js';
@ -1115,6 +1115,27 @@ const withRegistryLock = async <T>(operation: () => Promise<T>): Promise<T> => {
}
};
/**
* Drop credentials from every entry's `remoteUrl` (#2914).
*
* Applied on BOTH registry edges. Capture-time stripping in `getRemoteUrl`
* only covers values this version writes; a `registry.json` (or a per-repo
* meta that a re-register copies forward) written by an older version still
* holds the credential. Reading through here keeps it out of every consumer
* `listRegisteredRepos`, MCP `list_repos`, `gitnexus list`, group sync and
* writing through here means the next registry write drops it at rest instead
* of round-tripping it back to disk.
*
* Sanitised values compare equal to a freshly captured `getRemoteUrl`, so
* sibling-clone matching (#2054) is unaffected: both sides lose the same span.
*/
const sanitizeEntries = (entries: RegistryEntry[]): RegistryEntry[] =>
entries.map((e) => {
if (!e.remoteUrl) return e;
const cleaned = stripUrlCredentials(e.remoteUrl);
return cleaned === e.remoteUrl ? e : { ...e, remoteUrl: cleaned };
});
/**
* Read the global registry. Returns empty array if not found.
*/
@ -1122,7 +1143,7 @@ export const readRegistry = async (): Promise<RegistryEntry[]> => {
try {
const raw = await fs.readFile(getGlobalRegistryPath(), 'utf-8');
const data = JSON.parse(raw);
return Array.isArray(data) ? data : [];
return Array.isArray(data) ? sanitizeEntries(data) : [];
} catch {
return [];
}
@ -1142,7 +1163,11 @@ export const readRegistry = async (): Promise<RegistryEntry[]> => {
*/
const writeRegistry = async (entries: RegistryEntry[], attempts?: number): Promise<void> => {
await fs.mkdir(getGlobalDir(), { recursive: true });
await writeFileAtomic(getGlobalRegistryPath(), JSON.stringify(entries, null, 2), attempts);
await writeFileAtomic(
getGlobalRegistryPath(),
JSON.stringify(sanitizeEntries(entries), null, 2),
attempts,
);
};
/**

View file

@ -189,17 +189,17 @@ describe('getGitRoot', () => {
// ─── getRemoteUrl ─────────────────────────────────────────────────────────
describe('getRemoteUrl', () => {
const setupRepoWithRemote = (remoteUrl: string): string => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-remote-'));
// Use real fs paths and shellouts — the helper itself shells out to
// `git config`, so we need a real git repo for the assertion to be
// meaningful.
execSync('git init -q', { cwd: tmpDir });
execSync(`git remote add origin ${remoteUrl}`, { cwd: tmpDir });
return tmpDir;
};
const setupRepoWithRemote = (remoteUrl: string): string => {
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-remote-'));
// Use real fs paths and shellouts — the helper itself shells out to
// `git config`, so we need a real git repo for the assertion to be
// meaningful.
execSync('git init -q', { cwd: tmpDir });
execSync(`git remote add origin ${remoteUrl}`, { cwd: tmpDir });
return tmpDir;
};
describe('getRemoteUrl', () => {
it('returns undefined for a non-git directory', async () => {
const { getRemoteUrl } = await import('../../src/storage/git.js');
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gitnexus-test-'));
@ -255,6 +255,80 @@ describe('getRemoteUrl', () => {
});
});
// ─── credentials in remote URLs (#2914) ──────────────────────────────────
//
// `git config remote.origin.url` hands back whatever CI or a credential
// helper configured, including `https://x-access-token:<token>@host/…`.
// That value is persisted (registry.json, the per-repo meta) and echoed by
// MCP `list_repos`, so it must lose its userinfo at capture time. Every
// credential below is an obviously fake constant.
describe('remote URL credentials (#2914)', () => {
const FAKE_TOKEN = 'ExAmPle-FAKE-SECRET';
it('strips userinfo from an HTTPS remote before it can be persisted', async () => {
const { getRemoteUrl } = await import('../../src/storage/git.js');
const tmpDir = setupRepoWithRemote(
`https://x-access-token:${FAKE_TOKEN}@github.com/example/project.git`,
);
try {
expect(getRemoteUrl(tmpDir)).toBe('https://github.com/example/project');
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it('strips a username-only HTTPS remote (the PAT-as-username form)', async () => {
const { getRemoteUrl } = await import('../../src/storage/git.js');
const tmpDir = setupRepoWithRemote(`https://${FAKE_TOKEN}@github.com/example/project.git`);
try {
expect(getRemoteUrl(tmpDir)).toBe('https://github.com/example/project');
} finally {
fs.rmSync(tmpDir, { recursive: true, force: true });
}
});
it('leaves plain HTTPS, SSH-URL and SCP-like remotes untouched', async () => {
const { stripUrlCredentials } = await import('../../src/storage/git.js');
// The SSH forms carry a user NAME, not a secret, and are part of the
// remote's identity — rewriting them would repoint the #2054 fingerprint.
expect(stripUrlCredentials('https://github.com/example/project.git')).toBe(
'https://github.com/example/project.git',
);
expect(stripUrlCredentials('ssh://git@github.com/example/project.git')).toBe(
'ssh://git@github.com/example/project.git',
);
expect(stripUrlCredentials('git@github.com:example/project.git')).toBe(
'git@github.com:example/project.git',
);
// An `@` in the PATH is not userinfo — the authority ends at the first `/`.
expect(stripUrlCredentials('https://github.com/example/pro@ject')).toBe(
'https://github.com/example/pro@ject',
);
});
it('removes a password containing @ whole, leaving no tail behind', async () => {
const { stripUrlCredentials } = await import('../../src/storage/git.js');
expect(stripUrlCredentials(`https://user:pa@ss-${FAKE_TOKEN}@host.example/o/r`)).toBe(
'https://host.example/o/r',
);
});
it('keeps the same fingerprint for credentialed and clean clones of one repo', async () => {
const { getRemoteUrl } = await import('../../src/storage/git.js');
const withCreds = setupRepoWithRemote(
`https://x-access-token:${FAKE_TOKEN}@example.com/foo/bar.git`,
);
const clean = setupRepoWithRemote('https://example.com/foo/bar');
try {
expect(getRemoteUrl(withCreds)).toBe(getRemoteUrl(clean));
} finally {
fs.rmSync(withCreds, { recursive: true, force: true });
fs.rmSync(clean, { recursive: true, force: true });
}
});
});
// ─── getCanonicalRepoRoot (#1259) ────────────────────────────────────────
//
// Critical for the worktree-naming bug: when `gitnexus analyze` runs from a

View file

@ -975,6 +975,112 @@ describe('registerRepo name override + collision guard (#829)', () => {
// ─── registerRepo branch nesting (#2106) ─────────────────────────────
// ─── remoteUrl credentials (#2914) ───────────────────────────────────
//
// The registry is the surface `list_repos` (MCP), `gitnexus list` and group
// sync read from, so a `remoteUrl` carrying `https://user:token@` turns repo
// discovery into credential disclosure. Capture-time stripping in
// `getRemoteUrl` only covers what THIS version writes — a registry.json (or a
// per-repo meta a re-register copies forward) written by an older version
// still holds one, so both registry edges sanitize. Fake credential only.
describe('registry never emits or persists remoteUrl credentials (#2914)', () => {
const FAKE_TOKEN = 'ExAmPle-FAKE-SECRET';
const CREDENTIALED = `https://x-access-token:${FAKE_TOKEN}@github.com/example/project`;
const CLEAN = 'https://github.com/example/project';
let tmpHome: Awaited<ReturnType<typeof createTempDir>>;
let tmpRepo: Awaited<ReturnType<typeof createTempDir>>;
let savedGitnexusHome: string | undefined;
let registryPath: string;
const meta: RepoMeta = {
repoPath: '',
lastCommit: 'abc1234',
indexedAt: '2026-08-11T12:00:00.000Z',
stats: { files: 1, nodes: 1 },
};
beforeEach(async () => {
tmpHome = await createTempDir('gitnexus-2914-home-');
tmpRepo = await createTempDir('gitnexus-2914-repo-');
savedGitnexusHome = process.env.GITNEXUS_HOME;
process.env.GITNEXUS_HOME = tmpHome.dbPath;
registryPath = path.join(tmpHome.dbPath, 'registry.json');
});
afterEach(async () => {
if (savedGitnexusHome === undefined) delete process.env.GITNEXUS_HOME;
else process.env.GITNEXUS_HOME = savedGitnexusHome;
await tmpHome.cleanup();
await tmpRepo.cleanup();
});
/** A registry.json as an older version would have left it. */
const seedLegacyRegistry = async (entryPath: string): Promise<void> => {
const legacy: RegistryEntry[] = [
{
name: 'legacy',
path: entryPath,
storagePath: path.join(entryPath, '.gitnexus'),
indexedAt: meta.indexedAt,
lastCommit: meta.lastCommit,
remoteUrl: CREDENTIALED,
},
];
await fs.writeFile(registryPath, JSON.stringify(legacy, null, 2), 'utf-8');
};
it('sanitizes a legacy on-disk entry before listRegisteredRepos returns it', async () => {
await seedLegacyRegistry(tmpRepo.dbPath);
const entries = await listRegisteredRepos();
expect(entries).toHaveLength(1);
expect(entries[0].remoteUrl).toBe(CLEAN);
expect(JSON.stringify(entries)).not.toContain(FAKE_TOKEN);
});
it('never writes a credentialed remoteUrl to registry.json', async () => {
// meta.remoteUrl bypasses getRemoteUrl entirely — this is the legacy
// per-repo gitnexus.json being copied forward into a fresh registry.
await registerRepo(tmpRepo.dbPath, { ...meta, remoteUrl: CREDENTIALED }, { name: 'repro' });
const raw = await fs.readFile(registryPath, 'utf-8');
expect(raw).not.toContain(FAKE_TOKEN);
expect((JSON.parse(raw) as RegistryEntry[])[0].remoteUrl).toBe(CLEAN);
});
it('scrubs an untouched legacy entry when some other repo is registered', async () => {
const other = await createTempDir('gitnexus-2914-other-');
try {
await seedLegacyRegistry(other.dbPath);
await registerRepo(tmpRepo.dbPath, meta, { name: 'fresh' });
const raw = await fs.readFile(registryPath, 'utf-8');
expect(raw).not.toContain(FAKE_TOKEN);
// The legacy entry survives — it is scrubbed, not dropped.
expect(JSON.parse(raw)).toHaveLength(2);
} finally {
await other.cleanup();
}
});
it('still matches sibling clones after sanitization (#2054 fingerprint)', async () => {
await registerRepo(tmpRepo.dbPath, { ...meta, remoteUrl: CREDENTIALED }, { name: 'with-cred' });
const other = await createTempDir('gitnexus-2914-sibling-');
try {
await registerRepo(other.dbPath, { ...meta, remoteUrl: CLEAN }, { name: 'clean' });
const entries = await listRegisteredRepos();
const remotes = entries.map((e) => e.remoteUrl);
expect(remotes).toEqual([CLEAN, CLEAN]);
} finally {
await other.cleanup();
}
});
});
describe('registerRepo branch nesting (#2106)', () => {
let tmpHome: Awaited<ReturnType<typeof createTempDir>>;
let tmpRepo: Awaited<ReturnType<typeof createTempDir>>;