fix(gateway): harden build workspace boundaries
This commit is contained in:
@@ -17,6 +17,11 @@ export interface BuildRunner {
|
|||||||
run(commands: BuildCommand[]): Promise<BuildResult>;
|
run(commands: BuildCommand[]): Promise<BuildResult>;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
export const MAX_BUILD_OUTPUT_CHARS = 64 * 1024;
|
||||||
|
|
||||||
|
const appendOutputTail = (current: string, chunk: unknown): string =>
|
||||||
|
`${current}${String(chunk)}`.slice(-MAX_BUILD_OUTPUT_CHARS);
|
||||||
|
|
||||||
const runCommand = (command: BuildCommand): Promise<BuildResult> =>
|
const runCommand = (command: BuildCommand): Promise<BuildResult> =>
|
||||||
new Promise((resolve) => {
|
new Promise((resolve) => {
|
||||||
const child = spawn(command.command, command.args, {
|
const child = spawn(command.command, command.args, {
|
||||||
@@ -26,10 +31,17 @@ const runCommand = (command: BuildCommand): Promise<BuildResult> =>
|
|||||||
});
|
});
|
||||||
let output = '';
|
let output = '';
|
||||||
child.stdout.on('data', (chunk) => {
|
child.stdout.on('data', (chunk) => {
|
||||||
output += chunk.toString();
|
output = appendOutputTail(output, chunk);
|
||||||
});
|
});
|
||||||
child.stderr.on('data', (chunk) => {
|
child.stderr.on('data', (chunk) => {
|
||||||
output += chunk.toString();
|
output = appendOutputTail(output, chunk);
|
||||||
|
});
|
||||||
|
child.on('error', (error) => {
|
||||||
|
resolve({
|
||||||
|
ok: false,
|
||||||
|
exitCode: null,
|
||||||
|
output: appendOutputTail(output, error.message),
|
||||||
|
});
|
||||||
});
|
});
|
||||||
child.on('close', (code) => {
|
child.on('close', (code) => {
|
||||||
resolve({
|
resolve({
|
||||||
@@ -45,7 +57,7 @@ export class PnpmBuildRunner implements BuildRunner {
|
|||||||
let mergedOutput = '';
|
let mergedOutput = '';
|
||||||
for (const command of commands) {
|
for (const command of commands) {
|
||||||
const result = await runCommand(command);
|
const result = await runCommand(command);
|
||||||
mergedOutput += result.output;
|
mergedOutput = appendOutputTail(mergedOutput, result.output);
|
||||||
if (!result.ok) {
|
if (!result.ok) {
|
||||||
return {
|
return {
|
||||||
ok: false,
|
ok: false,
|
||||||
|
|||||||
@@ -28,6 +28,9 @@ const runGit = (args: string[], cwd: string, env?: Record<string, string>): Prom
|
|||||||
child.stderr.on('data', (chunk) => {
|
child.stderr.on('data', (chunk) => {
|
||||||
output += chunk.toString();
|
output += chunk.toString();
|
||||||
});
|
});
|
||||||
|
child.on('error', (error) => {
|
||||||
|
resolve({ ok: false, output: `${output}${error.message}` });
|
||||||
|
});
|
||||||
child.on('close', (code) => {
|
child.on('close', (code) => {
|
||||||
resolve({ ok: code === 0, output });
|
resolve({ ok: code === 0, output });
|
||||||
});
|
});
|
||||||
@@ -41,6 +44,7 @@ const ensureDir = (dir: string): void => {
|
|||||||
|
|
||||||
const hasInstallMarker = (dir: string): boolean => fs.existsSync(path.join(dir, 'node_modules', '.pnpm'));
|
const hasInstallMarker = (dir: string): boolean => fs.existsSync(path.join(dir, 'node_modules', '.pnpm'));
|
||||||
const GIT_REF_PATTERN = /^[0-9A-Za-z._/-]+$/;
|
const GIT_REF_PATTERN = /^[0-9A-Za-z._/-]+$/;
|
||||||
|
const COMMIT_SHA_PATTERN = /^[0-9a-f]{40}$/i;
|
||||||
|
|
||||||
const assertGitRef = (value: string): string => {
|
const assertGitRef = (value: string): string => {
|
||||||
const ref = value.trim();
|
const ref = value.trim();
|
||||||
@@ -84,6 +88,9 @@ export class GitWorkspaceManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async prepare(commitSha: string): Promise<WorkspaceInfo> {
|
async prepare(commitSha: string): Promise<WorkspaceInfo> {
|
||||||
|
if (!COMMIT_SHA_PATTERN.test(commitSha)) {
|
||||||
|
throw new Error('Invalid commit SHA.');
|
||||||
|
}
|
||||||
const workspacePath = path.join(this.worktreeRoot, commitSha);
|
const workspacePath = path.join(this.worktreeRoot, commitSha);
|
||||||
ensureDir(this.worktreeRoot);
|
ensureDir(this.worktreeRoot);
|
||||||
|
|
||||||
@@ -101,6 +108,8 @@ export class GitWorkspaceManager {
|
|||||||
if (!result.ok) {
|
if (!result.ok) {
|
||||||
throw new Error(result.output || 'Failed to create git worktree.');
|
throw new Error(result.output || 'Failed to create git worktree.');
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
await this.assertReusableWorkspace(workspacePath, commitSha);
|
||||||
}
|
}
|
||||||
|
|
||||||
return {
|
return {
|
||||||
@@ -111,18 +120,61 @@ export class GitWorkspaceManager {
|
|||||||
}
|
}
|
||||||
|
|
||||||
async remove(workspacePath: string): Promise<boolean> {
|
async remove(workspacePath: string): Promise<boolean> {
|
||||||
const resolved = path.resolve(workspacePath);
|
const resolved = this.assertManagedWorkspacePath(workspacePath);
|
||||||
const root = path.resolve(this.worktreeRoot);
|
|
||||||
if (!resolved.startsWith(root)) {
|
|
||||||
throw new Error('Workspace path is outside the configured worktree root.');
|
|
||||||
}
|
|
||||||
if (!fs.existsSync(resolved)) {
|
if (!fs.existsSync(resolved)) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
|
await this.assertRegisteredWorkspace(resolved);
|
||||||
const result = await runGit(['worktree', 'remove', '--force', resolved], this.repoRoot, this.baseEnv);
|
const result = await runGit(['worktree', 'remove', '--force', resolved], this.repoRoot, this.baseEnv);
|
||||||
if (!result.ok) {
|
if (!result.ok) {
|
||||||
fs.rmSync(resolved, { recursive: true, force: true });
|
fs.rmSync(resolved, { recursive: true, force: true });
|
||||||
}
|
}
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private assertManagedWorkspacePath(workspacePath: string): string {
|
||||||
|
const resolved = path.resolve(workspacePath);
|
||||||
|
const root = path.resolve(this.worktreeRoot);
|
||||||
|
const relative = path.relative(root, resolved);
|
||||||
|
if (!relative || relative.startsWith(`..${path.sep}`) || path.isAbsolute(relative)) {
|
||||||
|
throw new Error('Workspace path must be a child of the configured worktree root.');
|
||||||
|
}
|
||||||
|
if (relative.includes(path.sep) || !COMMIT_SHA_PATTERN.test(relative)) {
|
||||||
|
throw new Error('Workspace path is not a managed commit workspace.');
|
||||||
|
}
|
||||||
|
return resolved;
|
||||||
|
}
|
||||||
|
|
||||||
|
private async assertRegisteredWorkspace(workspacePath: string, expectedCommitSha?: string): Promise<void> {
|
||||||
|
const listed = await runGit(['worktree', 'list', '--porcelain'], this.repoRoot, this.baseEnv);
|
||||||
|
if (!listed.ok) {
|
||||||
|
throw new Error(listed.output || 'Failed to inspect git worktrees.');
|
||||||
|
}
|
||||||
|
const blocks = listed.output.split(/\n\n+/);
|
||||||
|
const matchingBlock = blocks.find((block) => {
|
||||||
|
const worktreeLine = block.split('\n').find((line) => line.startsWith('worktree '));
|
||||||
|
return worktreeLine && path.resolve(worktreeLine.slice('worktree '.length)) === workspacePath;
|
||||||
|
});
|
||||||
|
if (!matchingBlock) {
|
||||||
|
throw new Error('Workspace is not registered as a git worktree.');
|
||||||
|
}
|
||||||
|
if (expectedCommitSha) {
|
||||||
|
const headLine = matchingBlock.split('\n').find((line) => line.startsWith('HEAD '));
|
||||||
|
if (headLine?.slice('HEAD '.length).toLowerCase() !== expectedCommitSha.toLowerCase()) {
|
||||||
|
throw new Error('Existing workspace HEAD does not match the requested commit.');
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private async assertReusableWorkspace(workspacePath: string, expectedCommitSha: string): Promise<void> {
|
||||||
|
const resolved = this.assertManagedWorkspacePath(workspacePath);
|
||||||
|
await this.assertRegisteredWorkspace(resolved, expectedCommitSha);
|
||||||
|
const status = await runGit(['status', '--porcelain'], resolved, this.baseEnv);
|
||||||
|
if (!status.ok) {
|
||||||
|
throw new Error(status.output || 'Failed to inspect existing workspace.');
|
||||||
|
}
|
||||||
|
if (status.output.trim()) {
|
||||||
|
throw new Error('Existing workspace has uncommitted changes.');
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,43 @@
|
|||||||
|
import path from 'node:path';
|
||||||
|
|
||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import { MAX_BUILD_OUTPUT_CHARS, PnpmBuildRunner } from '../src/orchestrator/buildRunner.js';
|
||||||
|
|
||||||
|
describe('PnpmBuildRunner', () => {
|
||||||
|
it('returns a failed result when a command cannot be spawned', async () => {
|
||||||
|
const runner = new PnpmBuildRunner();
|
||||||
|
|
||||||
|
const result = await runner.run([
|
||||||
|
{
|
||||||
|
command: path.join(process.cwd(), 'missing-build-command'),
|
||||||
|
args: [],
|
||||||
|
cwd: process.cwd(),
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(result.ok).toBe(false);
|
||||||
|
expect(result.exitCode).toBeNull();
|
||||||
|
expect(result.output).toContain('ENOENT');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('retains only a bounded tail across command output', async () => {
|
||||||
|
const runner = new PnpmBuildRunner();
|
||||||
|
const result = await runner.run([
|
||||||
|
{
|
||||||
|
command: process.execPath,
|
||||||
|
args: ['-e', `process.stdout.write('a'.repeat(${MAX_BUILD_OUTPUT_CHARS}));`],
|
||||||
|
cwd: process.cwd(),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
command: process.execPath,
|
||||||
|
args: ['-e', "process.stdout.write('tail-marker');"],
|
||||||
|
cwd: process.cwd(),
|
||||||
|
},
|
||||||
|
]);
|
||||||
|
|
||||||
|
expect(result.ok).toBe(true);
|
||||||
|
expect(result.output.length).toBe(MAX_BUILD_OUTPUT_CHARS);
|
||||||
|
expect(result.output.endsWith('tail-marker')).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -85,4 +85,65 @@ describe('GitWorkspaceManager source resolution', () => {
|
|||||||
await expect(manager.resolveCommit('BRANCH', '--upload-pack=bad')).rejects.toThrow('Invalid git ref');
|
await expect(manager.resolveCommit('BRANCH', '--upload-pack=bad')).rejects.toThrow('Invalid git ref');
|
||||||
await expect(manager.resolveCommit('COMMIT', 'HEAD..main')).rejects.toThrow('Invalid git ref');
|
await expect(manager.resolveCommit('COMMIT', 'HEAD..main')).rejects.toThrow('Invalid git ref');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('reuses only a clean registered worktree at the requested commit', async () => {
|
||||||
|
const fixture = createRepositoryFixture();
|
||||||
|
const manager = new GitWorkspaceManager({
|
||||||
|
repoRoot: fixture.checkout,
|
||||||
|
worktreeRoot: fixture.worktrees,
|
||||||
|
});
|
||||||
|
|
||||||
|
const created = await manager.prepare(fixture.firstCommit);
|
||||||
|
expect(created.created).toBe(true);
|
||||||
|
const reused = await manager.prepare(fixture.firstCommit);
|
||||||
|
expect(reused).toMatchObject({ root: created.root, created: false });
|
||||||
|
|
||||||
|
fs.writeFileSync(path.join(created.root, 'untracked.txt'), 'dirty\n');
|
||||||
|
await expect(manager.prepare(fixture.firstCommit)).rejects.toThrow('uncommitted changes');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects an unregistered directory that occupies a commit workspace path', async () => {
|
||||||
|
const fixture = createRepositoryFixture();
|
||||||
|
const manager = new GitWorkspaceManager({
|
||||||
|
repoRoot: fixture.checkout,
|
||||||
|
worktreeRoot: fixture.worktrees,
|
||||||
|
});
|
||||||
|
const occupied = path.join(fixture.worktrees, fixture.firstCommit);
|
||||||
|
fs.mkdirSync(occupied, { recursive: true });
|
||||||
|
|
||||||
|
await expect(manager.prepare(fixture.firstCommit)).rejects.toThrow('not registered as a git worktree');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('removes only registered direct commit workspaces and never the root or sibling prefixes', async () => {
|
||||||
|
const fixture = createRepositoryFixture();
|
||||||
|
const manager = new GitWorkspaceManager({
|
||||||
|
repoRoot: fixture.checkout,
|
||||||
|
worktreeRoot: fixture.worktrees,
|
||||||
|
});
|
||||||
|
const workspace = await manager.prepare(fixture.firstCommit);
|
||||||
|
const siblingPrefix = `${fixture.worktrees}-outside`;
|
||||||
|
fs.mkdirSync(siblingPrefix, { recursive: true });
|
||||||
|
|
||||||
|
await expect(manager.remove(fixture.worktrees)).rejects.toThrow('must be a child');
|
||||||
|
await expect(manager.remove(path.join(siblingPrefix, fixture.firstCommit))).rejects.toThrow('must be a child');
|
||||||
|
expect(fs.existsSync(siblingPrefix)).toBe(true);
|
||||||
|
await expect(manager.remove(workspace.root)).resolves.toBe(true);
|
||||||
|
expect(fs.existsSync(workspace.root)).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects deletion of unregistered and nested paths under the worktree root', async () => {
|
||||||
|
const fixture = createRepositoryFixture();
|
||||||
|
const manager = new GitWorkspaceManager({
|
||||||
|
repoRoot: fixture.checkout,
|
||||||
|
worktreeRoot: fixture.worktrees,
|
||||||
|
});
|
||||||
|
const unregistered = path.join(fixture.worktrees, fixture.firstCommit);
|
||||||
|
fs.mkdirSync(unregistered, { recursive: true });
|
||||||
|
|
||||||
|
await expect(manager.remove(unregistered)).rejects.toThrow('not registered as a git worktree');
|
||||||
|
await expect(manager.remove(path.join(unregistered, 'nested'))).rejects.toThrow(
|
||||||
|
'not a managed commit workspace'
|
||||||
|
);
|
||||||
|
expect(fs.existsSync(unregistered)).toBe(true);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user