Skip to content

Commit 6ce7a86

Browse files
fix(worker): keep Git credentials out of process arguments (#1584)
* fix(worker): keep Git credentials out of process arguments * chore: add changelog entry for PR 1584 * feedback * feedback
1 parent e7bf8f0 commit 6ce7a86

10 files changed

Lines changed: 1017 additions & 142 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99

1010
### Fixed
1111
- Fixed memory leak attributed to CodeMirror allocating objects on heap that were never freed. [#1580](https://github.com/sourcebot-dev/sourcebot/pull/1580)
12+
- Kept Git provider credentials out of subprocess arguments and on-disk configuration by using isolated in-memory credential caches. [#1584](https://github.com/sourcebot-dev/sourcebot/pull/1584)
1213

1314
## [5.1.7] - 2026-08-13
1415

packages/backend/src/git.test.ts

Lines changed: 273 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1-
import { mkdtemp, rm, writeFile } from "node:fs/promises";
1+
import { execFileSync, spawn } from "node:child_process";
2+
import { randomUUID } from "node:crypto";
3+
import { createServer, type Server } from "node:http";
4+
import { mkdtemp, readFile, readdir, rm, writeFile } from "node:fs/promises";
25
import { join } from "node:path";
36
import { tmpdir } from "node:os";
4-
import { execFileSync } from "node:child_process";
57
import { afterEach, describe, expect, test } from "vitest";
6-
import { getBranches, getTags } from "./git.js";
8+
import { cloneRepository, fetchRepository, getBranches, getRemoteDefaultBranch, getTags } from "./git.js";
79

810
const runGit = (
911
repoPath: string,
@@ -32,6 +34,157 @@ const createTempRepo = async () => {
3234
return repoPath;
3335
};
3436

37+
const createAuthenticatedGitServer = async ({
38+
projectRoot,
39+
username,
40+
password,
41+
}: {
42+
projectRoot: string;
43+
username: string;
44+
password: string;
45+
}) => {
46+
const expectedAuthorization = `Basic ${Buffer.from(`${username}:${password}`).toString('base64')}`;
47+
let authenticatedRequestCount = 0;
48+
let unauthenticatedRequestCount = 0;
49+
50+
const server = createServer((request, response) => {
51+
if (request.headers.authorization !== expectedAuthorization) {
52+
unauthenticatedRequestCount++;
53+
response.writeHead(401, {
54+
'WWW-Authenticate': 'Basic realm="Sourcebot Git Test"',
55+
});
56+
response.end();
57+
return;
58+
}
59+
60+
authenticatedRequestCount++;
61+
const requestUrl = new URL(request.url ?? '/', 'http://127.0.0.1');
62+
const backend = spawn('git', ['http-backend'], {
63+
env: {
64+
...process.env,
65+
GIT_HTTP_EXPORT_ALL: '1',
66+
GIT_PROJECT_ROOT: projectRoot,
67+
PATH_INFO: requestUrl.pathname,
68+
QUERY_STRING: requestUrl.searchParams.toString(),
69+
REQUEST_METHOD: request.method ?? 'GET',
70+
CONTENT_TYPE: request.headers['content-type'] ?? '',
71+
CONTENT_LENGTH: request.headers['content-length'] ?? '',
72+
REMOTE_USER: username,
73+
SERVER_PROTOCOL: 'HTTP/1.1',
74+
},
75+
stdio: ['pipe', 'pipe', 'pipe'],
76+
});
77+
let headerBuffer = Buffer.alloc(0);
78+
let headersSent = false;
79+
const stderr: Buffer[] = [];
80+
81+
backend.stderr.on('data', (chunk: Buffer) => stderr.push(chunk));
82+
backend.stdout.on('data', (chunk: Buffer) => {
83+
if (headersSent) {
84+
response.write(chunk);
85+
return;
86+
}
87+
88+
headerBuffer = Buffer.concat([headerBuffer, chunk]);
89+
const crlfTerminatorIndex = headerBuffer.indexOf('\r\n\r\n');
90+
const lfTerminatorIndex = headerBuffer.indexOf('\n\n');
91+
const terminatorIndex = crlfTerminatorIndex >= 0
92+
? crlfTerminatorIndex
93+
: lfTerminatorIndex;
94+
if (terminatorIndex < 0) {
95+
return;
96+
}
97+
98+
const terminatorLength = crlfTerminatorIndex >= 0 ? 4 : 2;
99+
const rawHeaders = headerBuffer.subarray(0, terminatorIndex).toString('utf8');
100+
const responseHeaders: Record<string, string> = {};
101+
let statusCode = 200;
102+
for (const line of rawHeaders.split(/\r?\n/)) {
103+
const separatorIndex = line.indexOf(':');
104+
if (separatorIndex < 0) {
105+
continue;
106+
}
107+
108+
const name = line.slice(0, separatorIndex).trim();
109+
const value = line.slice(separatorIndex + 1).trim();
110+
if (name.toLowerCase() === 'status') {
111+
statusCode = Number.parseInt(value, 10);
112+
} else {
113+
responseHeaders[name] = value;
114+
}
115+
}
116+
117+
response.writeHead(statusCode, responseHeaders);
118+
headersSent = true;
119+
response.write(headerBuffer.subarray(terminatorIndex + terminatorLength));
120+
headerBuffer = Buffer.alloc(0);
121+
});
122+
backend.once('error', (error) => {
123+
if (!response.headersSent) {
124+
response.writeHead(500);
125+
}
126+
response.end(error.message);
127+
});
128+
backend.once('close', (code) => {
129+
if (!headersSent) {
130+
response.writeHead(500);
131+
response.end(Buffer.concat(stderr));
132+
return;
133+
}
134+
if (code !== 0) {
135+
response.destroy(new Error(Buffer.concat(stderr).toString('utf8')));
136+
return;
137+
}
138+
response.end();
139+
});
140+
141+
backend.stdin.on('error', () => {
142+
// The child process error and close handlers report the actionable failure.
143+
});
144+
request.pipe(backend.stdin);
145+
});
146+
147+
await new Promise<void>((resolve, reject) => {
148+
server.once('error', reject);
149+
server.listen(0, '127.0.0.1', () => resolve());
150+
});
151+
const address = server.address();
152+
if (!address || typeof address === 'string') {
153+
throw new Error('Git test server did not bind to a TCP port');
154+
}
155+
156+
return {
157+
cloneUrl: `http://127.0.0.1:${address.port}/repo.git`,
158+
getAuthenticatedRequestCount: () => authenticatedRequestCount,
159+
getUnauthenticatedRequestCount: () => unauthenticatedRequestCount,
160+
server,
161+
};
162+
};
163+
164+
const closeServer = async (server: Server) => {
165+
await new Promise<void>((resolve, reject) => {
166+
server.close((error) => error ? reject(error) : resolve());
167+
});
168+
};
169+
170+
const directoryContains = async (directory: string, value: string): Promise<boolean> => {
171+
const entries = await readdir(directory, { withFileTypes: true });
172+
for (const entry of entries) {
173+
const path = join(directory, entry.name);
174+
if (entry.isDirectory()) {
175+
if (await directoryContains(path, value)) {
176+
return true;
177+
}
178+
} else if (entry.isFile()) {
179+
const contents = await readFile(path);
180+
if (contents.includes(Buffer.from(value))) {
181+
return true;
182+
}
183+
}
184+
}
185+
return false;
186+
};
187+
35188
const commitFile = async ({
36189
repoPath,
37190
fileName,
@@ -134,3 +287,120 @@ describe("git ref ordering", () => {
134287
);
135288
});
136289
});
290+
291+
describe('authenticated Git operations', () => {
292+
const repoPaths: string[] = [];
293+
294+
afterEach(async () => {
295+
await Promise.all(
296+
repoPaths
297+
.splice(0)
298+
.map((repoPath) => rm(repoPath, { recursive: true, force: true })),
299+
);
300+
});
301+
302+
test('clone, fetch, and ls-remote authenticate without exposing the credential', async () => {
303+
const sourcePath = await createTempRepo();
304+
repoPaths.push(sourcePath);
305+
await commitFile({
306+
repoPath: sourcePath,
307+
fileName: 'README.md',
308+
content: 'initial\n',
309+
message: 'initial commit',
310+
timestamp: '2024-01-01T00:00:00Z',
311+
});
312+
313+
const projectRoot = await mkdtemp(join(tmpdir(), 'sourcebot-git-http-root-'));
314+
repoPaths.push(projectRoot);
315+
const bareRepoPath = join(projectRoot, 'repo.git');
316+
runGit(projectRoot, ['clone', '--bare', sourcePath, bareRepoPath]);
317+
318+
const username = 'sourcebot-test-user';
319+
const token = `sourcebot-test-token-${randomUUID()}`;
320+
const gitServer = await createAuthenticatedGitServer({
321+
projectRoot,
322+
username,
323+
password: token,
324+
});
325+
const clonePath = await mkdtemp(join(tmpdir(), 'sourcebot-git-auth-clone-'));
326+
repoPaths.push(clonePath);
327+
const tracePath = join(projectRoot, 'git-trace.json');
328+
const previousTrace = process.env.GIT_TRACE2_EVENT;
329+
process.env.GIT_TRACE2_EVENT = tracePath;
330+
let unauthenticatedRequestsBeforeProactiveAuth: number | undefined;
331+
let proactiveAuthDefaultBranch: string | undefined;
332+
333+
try {
334+
await cloneRepository({
335+
cloneUrl: gitServer.cloneUrl,
336+
credentials: {
337+
username,
338+
password: token,
339+
},
340+
path: clonePath,
341+
});
342+
343+
await commitFile({
344+
repoPath: sourcePath,
345+
fileName: 'new.txt',
346+
content: 'new commit\n',
347+
message: 'new commit',
348+
timestamp: '2024-01-02T00:00:00Z',
349+
});
350+
runGit(sourcePath, ['push', bareRepoPath, 'main']);
351+
352+
await fetchRepository({
353+
cloneUrl: gitServer.cloneUrl,
354+
credentials: {
355+
username,
356+
password: token,
357+
},
358+
path: clonePath,
359+
});
360+
361+
unauthenticatedRequestsBeforeProactiveAuth = gitServer.getUnauthenticatedRequestCount();
362+
proactiveAuthDefaultBranch = await getRemoteDefaultBranch({
363+
path: clonePath,
364+
cloneUrl: gitServer.cloneUrl,
365+
credentials: {
366+
username,
367+
password: token,
368+
proactiveAuth: 'basic',
369+
},
370+
});
371+
} finally {
372+
if (previousTrace === undefined) {
373+
delete process.env.GIT_TRACE2_EVENT;
374+
} else {
375+
process.env.GIT_TRACE2_EVENT = previousTrace;
376+
}
377+
await closeServer(gitServer.server);
378+
}
379+
380+
const expectedHead = execFileSync('git', ['rev-parse', 'HEAD'], {
381+
cwd: sourcePath,
382+
encoding: 'utf8',
383+
}).trim();
384+
const fetchedHead = execFileSync('git', ['rev-parse', 'refs/heads/main'], {
385+
cwd: clonePath,
386+
encoding: 'utf8',
387+
}).trim();
388+
const repositoryConfig = execFileSync('git', ['config', '--local', '--list', '--show-origin'], {
389+
cwd: clonePath,
390+
encoding: 'utf8',
391+
});
392+
const trace = await readFile(tracePath, 'utf8');
393+
394+
expect(fetchedHead).toBe(expectedHead);
395+
expect(gitServer.getAuthenticatedRequestCount()).toBeGreaterThan(0);
396+
expect(gitServer.getUnauthenticatedRequestCount()).toBeGreaterThan(0);
397+
expect(proactiveAuthDefaultBranch).toBe('main');
398+
expect(gitServer.getUnauthenticatedRequestCount()).toBe(unauthenticatedRequestsBeforeProactiveAuth);
399+
expect(repositoryConfig).not.toContain('remote.origin.url');
400+
expect(repositoryConfig).not.toContain('http.extraHeader');
401+
expect(repositoryConfig).not.toContain(token);
402+
expect(trace).not.toContain(token);
403+
expect(trace).not.toContain(Buffer.from(`${username}:${token}`).toString('base64'));
404+
expect(await directoryContains(clonePath, token)).toBe(false);
405+
}, 20_000);
406+
});

0 commit comments

Comments
 (0)