Skip to content

Commit c55940e

Browse files
brendan-kellambrendan-kellam
committed
fix(worker): keep Git credentials out of process arguments
1 parent e7bf8f0 commit c55940e

9 files changed

Lines changed: 961 additions & 142 deletions

File tree

‎packages/backend/src/git.test.ts‎

Lines changed: 270 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,154 @@ 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+
request.pipe(backend.stdin);
142+
});
143+
144+
await new Promise<void>((resolve, reject) => {
145+
server.once('error', reject);
146+
server.listen(0, '127.0.0.1', () => resolve());
147+
});
148+
const address = server.address();
149+
if (!address || typeof address === 'string') {
150+
throw new Error('Git test server did not bind to a TCP port');
151+
}
152+
153+
return {
154+
cloneUrl: `http://127.0.0.1:${address.port}/repo.git`,
155+
getAuthenticatedRequestCount: () => authenticatedRequestCount,
156+
getUnauthenticatedRequestCount: () => unauthenticatedRequestCount,
157+
server,
158+
};
159+
};
160+
161+
const closeServer = async (server: Server) => {
162+
await new Promise<void>((resolve, reject) => {
163+
server.close((error) => error ? reject(error) : resolve());
164+
});
165+
};
166+
167+
const directoryContains = async (directory: string, value: string): Promise<boolean> => {
168+
const entries = await readdir(directory, { withFileTypes: true });
169+
for (const entry of entries) {
170+
const path = join(directory, entry.name);
171+
if (entry.isDirectory()) {
172+
if (await directoryContains(path, value)) {
173+
return true;
174+
}
175+
} else if (entry.isFile()) {
176+
const contents = await readFile(path);
177+
if (contents.includes(Buffer.from(value))) {
178+
return true;
179+
}
180+
}
181+
}
182+
return false;
183+
};
184+
35185
const commitFile = async ({
36186
repoPath,
37187
fileName,
@@ -134,3 +284,120 @@ describe("git ref ordering", () => {
134284
);
135285
});
136286
});
287+
288+
describe('authenticated Git operations', () => {
289+
const repoPaths: string[] = [];
290+
291+
afterEach(async () => {
292+
await Promise.all(
293+
repoPaths
294+
.splice(0)
295+
.map((repoPath) => rm(repoPath, { recursive: true, force: true })),
296+
);
297+
});
298+
299+
test('clone, fetch, and ls-remote authenticate without exposing the credential', async () => {
300+
const sourcePath = await createTempRepo();
301+
repoPaths.push(sourcePath);
302+
await commitFile({
303+
repoPath: sourcePath,
304+
fileName: 'README.md',
305+
content: 'initial\n',
306+
message: 'initial commit',
307+
timestamp: '2024-01-01T00:00:00Z',
308+
});
309+
310+
const projectRoot = await mkdtemp(join(tmpdir(), 'sourcebot-git-http-root-'));
311+
repoPaths.push(projectRoot);
312+
const bareRepoPath = join(projectRoot, 'repo.git');
313+
runGit(projectRoot, ['clone', '--bare', sourcePath, bareRepoPath]);
314+
315+
const username = 'sourcebot-test-user';
316+
const token = `sourcebot-test-token-${randomUUID()}`;
317+
const gitServer = await createAuthenticatedGitServer({
318+
projectRoot,
319+
username,
320+
password: token,
321+
});
322+
const clonePath = await mkdtemp(join(tmpdir(), 'sourcebot-git-auth-clone-'));
323+
repoPaths.push(clonePath);
324+
const tracePath = join(projectRoot, 'git-trace.json');
325+
const previousTrace = process.env.GIT_TRACE2_EVENT;
326+
process.env.GIT_TRACE2_EVENT = tracePath;
327+
let unauthenticatedRequestsBeforeProactiveAuth: number | undefined;
328+
let proactiveAuthDefaultBranch: string | undefined;
329+
330+
try {
331+
await cloneRepository({
332+
cloneUrl: gitServer.cloneUrl,
333+
credentials: {
334+
username,
335+
password: token,
336+
},
337+
path: clonePath,
338+
});
339+
340+
await commitFile({
341+
repoPath: sourcePath,
342+
fileName: 'new.txt',
343+
content: 'new commit\n',
344+
message: 'new commit',
345+
timestamp: '2024-01-02T00:00:00Z',
346+
});
347+
runGit(sourcePath, ['push', bareRepoPath, 'main']);
348+
349+
await fetchRepository({
350+
cloneUrl: gitServer.cloneUrl,
351+
credentials: {
352+
username,
353+
password: token,
354+
},
355+
path: clonePath,
356+
});
357+
358+
unauthenticatedRequestsBeforeProactiveAuth = gitServer.getUnauthenticatedRequestCount();
359+
proactiveAuthDefaultBranch = await getRemoteDefaultBranch({
360+
path: clonePath,
361+
cloneUrl: gitServer.cloneUrl,
362+
credentials: {
363+
username,
364+
password: token,
365+
proactiveAuth: 'basic',
366+
},
367+
});
368+
} finally {
369+
if (previousTrace === undefined) {
370+
delete process.env.GIT_TRACE2_EVENT;
371+
} else {
372+
process.env.GIT_TRACE2_EVENT = previousTrace;
373+
}
374+
await closeServer(gitServer.server);
375+
}
376+
377+
const expectedHead = execFileSync('git', ['rev-parse', 'HEAD'], {
378+
cwd: sourcePath,
379+
encoding: 'utf8',
380+
}).trim();
381+
const fetchedHead = execFileSync('git', ['rev-parse', 'refs/heads/main'], {
382+
cwd: clonePath,
383+
encoding: 'utf8',
384+
}).trim();
385+
const repositoryConfig = execFileSync('git', ['config', '--local', '--list', '--show-origin'], {
386+
cwd: clonePath,
387+
encoding: 'utf8',
388+
});
389+
const trace = await readFile(tracePath, 'utf8');
390+
391+
expect(fetchedHead).toBe(expectedHead);
392+
expect(gitServer.getAuthenticatedRequestCount()).toBeGreaterThan(0);
393+
expect(gitServer.getUnauthenticatedRequestCount()).toBeGreaterThan(0);
394+
expect(proactiveAuthDefaultBranch).toBe('main');
395+
expect(gitServer.getUnauthenticatedRequestCount()).toBe(unauthenticatedRequestsBeforeProactiveAuth);
396+
expect(repositoryConfig).not.toContain('remote.origin.url');
397+
expect(repositoryConfig).not.toContain('http.extraHeader');
398+
expect(repositoryConfig).not.toContain(token);
399+
expect(trace).not.toContain(token);
400+
expect(trace).not.toContain(Buffer.from(`${username}:${token}`).toString('base64'));
401+
expect(await directoryContains(clonePath, token)).toBe(false);
402+
}, 20_000);
403+
});

0 commit comments

Comments
 (0)