Merge remote-tracking branch 'origin/main' into feat/nested-git-repos
# Conflicts: # packages/web/server/lib/fs/routes.test.js
This commit is contained in:
@@ -3,6 +3,7 @@ import path from 'path';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
import { mintOutsideFileGrant, registerFsRoutes } from './routes.js';
|
||||
import { createProjectDirectoryRuntime } from '../opencode/project-directory-runtime.js';
|
||||
|
||||
const createRouteRegistry = () => {
|
||||
const routes = new Map();
|
||||
@@ -140,6 +141,30 @@ const registerWrite = (fsPromises) => {
|
||||
return getRoute('POST', '/api/fs/write');
|
||||
};
|
||||
|
||||
const registerUpload = (fsPromises) => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
registerFsRoutes(app, {
|
||||
os: { homedir: () => '/home/user' },
|
||||
path: path.posix,
|
||||
fsPromises: {
|
||||
realpath: async (targetPath) => {
|
||||
if (targetPath === '/repo') return targetPath;
|
||||
throw Object.assign(new Error('not found'), { code: 'ENOENT' });
|
||||
},
|
||||
stat: async () => ({ isDirectory: () => false }),
|
||||
...fsPromises,
|
||||
},
|
||||
spawn: vi.fn(),
|
||||
crypto: { randomUUID: () => 'job-0' },
|
||||
normalizeDirectoryPath: (p) => p,
|
||||
resolveProjectDirectory: async () => ({ directory: '/repo' }),
|
||||
buildAugmentedPath: () => '/usr/bin',
|
||||
resolveGitBinaryForSpawn: () => 'git',
|
||||
openchamberUserConfigRoot: '/home/user/.config',
|
||||
});
|
||||
return getRoute('POST', '/api/fs/upload');
|
||||
};
|
||||
|
||||
const registerRead = (fsPromises) => {
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
registerFsRoutes(app, {
|
||||
@@ -233,6 +258,28 @@ const callWrite = async (handler, body) => {
|
||||
return res;
|
||||
};
|
||||
|
||||
const callUpload = async (handler, {
|
||||
body = Buffer.from('upload'),
|
||||
chunks,
|
||||
includeContentLength = true,
|
||||
path: filePath = '/repo/file.bin',
|
||||
overwrite = false,
|
||||
} = {}) => {
|
||||
const res = createMockResponse();
|
||||
const uploadChunks = chunks ?? [body];
|
||||
const headers = { 'content-type': 'application/octet-stream' };
|
||||
if (includeContentLength) headers['content-length'] = String(body.length);
|
||||
const req = {
|
||||
headers,
|
||||
query: { path: filePath, overwrite: overwrite ? 'true' : undefined },
|
||||
async *[Symbol.asyncIterator]() {
|
||||
yield* uploadChunks;
|
||||
},
|
||||
};
|
||||
await handler(req, res);
|
||||
return res;
|
||||
};
|
||||
|
||||
const callRead = async (handler, query) => {
|
||||
const res = createMockResponse();
|
||||
await handler({ query }, res);
|
||||
@@ -340,7 +387,186 @@ describe('fs write', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('fs upload', () => {
|
||||
it('streams a binary file to temp storage before committing it without overwrite', async () => {
|
||||
const write = vi.fn(async (_buffer, _offset, length) => ({ bytesWritten: length }));
|
||||
const close = vi.fn(async () => undefined);
|
||||
const fsPromises = {
|
||||
open: vi.fn(async () => ({ write, close })),
|
||||
link: vi.fn(async () => undefined),
|
||||
rename: vi.fn(async () => undefined),
|
||||
unlink: vi.fn(async () => undefined),
|
||||
};
|
||||
const handler = registerUpload(fsPromises);
|
||||
|
||||
const body = Buffer.from([0, 1, 2, 255]);
|
||||
const res = await callUpload(handler, {
|
||||
body,
|
||||
chunks: [body.subarray(0, 2), body.subarray(2)],
|
||||
});
|
||||
|
||||
expect(res.body).toEqual({ success: true, path: '/repo/file.bin' });
|
||||
const tmp = fsPromises.open.mock.calls[0][0];
|
||||
expect(tmp).toMatch(/^\/repo\/file\.bin\.upload-/);
|
||||
expect(fsPromises.open).toHaveBeenCalledWith(tmp, 'wx');
|
||||
expect(write).toHaveBeenNthCalledWith(1, Buffer.from([0, 1]), 0, 2, null);
|
||||
expect(write).toHaveBeenNthCalledWith(2, Buffer.from([2, 255]), 0, 2, null);
|
||||
expect(close).toHaveBeenCalledTimes(1);
|
||||
expect(fsPromises.link).toHaveBeenCalledWith(tmp, '/repo/file.bin');
|
||||
expect(fsPromises.unlink).toHaveBeenCalledWith(tmp);
|
||||
expect(fsPromises.rename).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns a conflict instead of silently replacing an existing file', async () => {
|
||||
const fsPromises = {
|
||||
realpath: vi.fn(async (targetPath) => targetPath),
|
||||
stat: vi.fn(async () => ({ isDirectory: () => false })),
|
||||
open: vi.fn(async () => ({ write: vi.fn(), close: vi.fn() })),
|
||||
};
|
||||
const handler = registerUpload(fsPromises);
|
||||
|
||||
const res = await callUpload(handler);
|
||||
|
||||
expect(res.statusCode).toBe(409);
|
||||
expect(res.body).toEqual({ error: 'File already exists', reason: 'already-exists' });
|
||||
expect(fsPromises.open).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('atomically replaces a file only when overwrite is explicit', async () => {
|
||||
const write = vi.fn(async (_buffer, _offset, length) => ({ bytesWritten: length }));
|
||||
const fsPromises = {
|
||||
realpath: vi.fn(async (targetPath) => targetPath),
|
||||
stat: vi.fn(async () => ({ isDirectory: () => false })),
|
||||
open: vi.fn(async () => ({ write, close: vi.fn(async () => undefined) })),
|
||||
rename: vi.fn(async () => undefined),
|
||||
unlink: vi.fn(async () => undefined),
|
||||
};
|
||||
const handler = registerUpload(fsPromises);
|
||||
|
||||
const res = await callUpload(handler, { overwrite: true });
|
||||
|
||||
expect(res.body).toEqual({ success: true, path: '/repo/file.bin' });
|
||||
const tmp = fsPromises.open.mock.calls[0][0];
|
||||
expect(tmp).toMatch(/^\/repo\/file\.bin\.upload-/);
|
||||
expect(write).toHaveBeenCalledWith(Buffer.from('upload'), 0, 6, null);
|
||||
expect(fsPromises.rename).toHaveBeenCalledWith(tmp, '/repo/file.bin');
|
||||
});
|
||||
|
||||
it('rejects an existing directory before reading the upload body', async () => {
|
||||
const fsPromises = {
|
||||
realpath: vi.fn(async (targetPath) => targetPath),
|
||||
stat: vi.fn(async () => ({ isDirectory: () => true })),
|
||||
open: vi.fn(async () => ({ write: vi.fn(), close: vi.fn() })),
|
||||
};
|
||||
const handler = registerUpload(fsPromises);
|
||||
|
||||
const res = await callUpload(handler);
|
||||
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(res.body).toEqual({ error: 'Specified path is a directory' });
|
||||
expect(fsPromises.open).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('rejects a destination parent that resolves outside the workspace', async () => {
|
||||
const fsPromises = {
|
||||
realpath: vi.fn(async (targetPath) => targetPath === '/repo/link' ? '/outside' : targetPath),
|
||||
open: vi.fn(async () => ({ write: vi.fn(), close: vi.fn() })),
|
||||
};
|
||||
const handler = registerUpload(fsPromises);
|
||||
|
||||
const res = await callUpload(handler, { path: '/repo/link/file.bin' });
|
||||
|
||||
expect(res.statusCode).toBe(403);
|
||||
expect(res.body).toEqual({ error: 'Access denied' });
|
||||
expect(fsPromises.open).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('cleans up a partial temp file when the configured streaming limit is exceeded', async () => {
|
||||
const previous = process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES;
|
||||
process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES = '5';
|
||||
const write = vi.fn(async (_buffer, _offset, length) => ({ bytesWritten: length }));
|
||||
const fsPromises = {
|
||||
open: vi.fn(async () => ({ write, close: vi.fn(async () => undefined) })),
|
||||
link: vi.fn(async () => undefined),
|
||||
unlink: vi.fn(async () => undefined),
|
||||
};
|
||||
try {
|
||||
const handler = registerUpload(fsPromises);
|
||||
const res = await callUpload(handler, {
|
||||
body: Buffer.from('123456'),
|
||||
chunks: [Buffer.from('123'), Buffer.from('456')],
|
||||
includeContentLength: false,
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(413);
|
||||
expect(res.body).toEqual({ error: 'File exceeds maximum size of 5 bytes' });
|
||||
expect(write).toHaveBeenCalledWith(Buffer.from('123'), 0, 3, null);
|
||||
expect(fsPromises.link).not.toHaveBeenCalled();
|
||||
expect(fsPromises.unlink).toHaveBeenCalledWith(expect.stringMatching(/^\/repo\/file\.bin\.upload-/));
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES;
|
||||
else process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES = previous;
|
||||
}
|
||||
});
|
||||
|
||||
it('rejects a declared oversized upload before opening a temp file', async () => {
|
||||
const previous = process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES;
|
||||
process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES = '5';
|
||||
const fsPromises = {
|
||||
open: vi.fn(async () => ({ write: vi.fn(), close: vi.fn() })),
|
||||
};
|
||||
try {
|
||||
const handler = registerUpload(fsPromises);
|
||||
const res = await callUpload(handler, { body: Buffer.from('123456') });
|
||||
|
||||
expect(res.statusCode).toBe(413);
|
||||
expect(res.body).toEqual({ error: 'File exceeds maximum size of 5 bytes' });
|
||||
expect(fsPromises.open).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES;
|
||||
else process.env.OPENCHAMBER_FS_UPLOAD_MAX_BYTES = previous;
|
||||
}
|
||||
});
|
||||
|
||||
it('keeps the existing file when a target appears before the atomic commit', async () => {
|
||||
const error = Object.assign(new Error('exists'), { code: 'EEXIST' });
|
||||
const fsPromises = {
|
||||
open: vi.fn(async () => ({
|
||||
write: vi.fn(async (_buffer, _offset, length) => ({ bytesWritten: length })),
|
||||
close: vi.fn(async () => undefined),
|
||||
})),
|
||||
link: vi.fn(async () => { throw error; }),
|
||||
unlink: vi.fn(async () => undefined),
|
||||
};
|
||||
const handler = registerUpload(fsPromises);
|
||||
|
||||
const res = await callUpload(handler);
|
||||
|
||||
expect(res.statusCode).toBe(409);
|
||||
expect(res.body).toEqual({ error: 'File already exists', reason: 'already-exists' });
|
||||
expect(fsPromises.unlink).toHaveBeenCalledWith(expect.stringMatching(/^\/repo\/file\.bin\.upload-/));
|
||||
});
|
||||
});
|
||||
|
||||
describe('fs read', () => {
|
||||
it('reads workspace files through symlinks that resolve outside the workspace', async () => {
|
||||
const fsPromises = {
|
||||
realpath: vi.fn(async (targetPath) => {
|
||||
if (targetPath === '/repo/link.txt') return '/shared/target.txt';
|
||||
return targetPath;
|
||||
}),
|
||||
stat: vi.fn(async () => ({ isFile: () => true, size: 6 })),
|
||||
readFile: vi.fn(async () => 'shared'),
|
||||
};
|
||||
const handler = registerRead(fsPromises);
|
||||
|
||||
const res = await callRead(handler, { path: '/repo/link.txt' });
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.body).toBe('shared');
|
||||
expect(fsPromises.readFile).toHaveBeenCalledWith('/shared/target.txt', 'utf8');
|
||||
});
|
||||
|
||||
it('rejects outside workspace reads without a grant', async () => {
|
||||
const warn = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
||||
const fsPromises = {
|
||||
@@ -1042,3 +1268,78 @@ describe('fs git-dirs', () => {
|
||||
expect(res.body.repositories).toEqual([{ path: '/workspace/open', name: 'open' }]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('fs stat directory scope (issue 3019)', () => {
|
||||
// Wires the real project-directory runtime so the stat route resolves the
|
||||
// workspace exactly as the server does: explicit x-opencode-directory header
|
||||
// first, then the settings.lastDirectory fallback. The renderer's file
|
||||
// reference probes must send the header because lastDirectory reflects the
|
||||
// directory the UI last browsed, not the session's directory.
|
||||
const registerStatWithProjectDirectoryRuntime = () => {
|
||||
const projectDirectoryRuntime = createProjectDirectoryRuntime({
|
||||
fsPromises: {
|
||||
stat: async (targetPath) => {
|
||||
if (targetPath === '/repo-a' || targetPath === '/repo-b') {
|
||||
return { isDirectory: () => true };
|
||||
}
|
||||
return { isDirectory: () => false, isFile: () => true, size: 12 };
|
||||
},
|
||||
realpath: async (targetPath) => targetPath,
|
||||
},
|
||||
path: { resolve: (p) => path.posix.resolve(p) },
|
||||
normalizeDirectoryPath: (p) => p,
|
||||
readSettingsFromDiskMigrated: async () => ({ lastDirectory: '/repo-a', projects: [] }),
|
||||
getReadSettingsFromDiskMigrated: undefined,
|
||||
sanitizeProjects: (input) => input,
|
||||
});
|
||||
|
||||
const { app, getRoute } = createRouteRegistry();
|
||||
registerFsRoutes(app, {
|
||||
os: { homedir: () => '/home/user' },
|
||||
path: path.posix,
|
||||
fsPromises: {
|
||||
realpath: async (targetPath) => targetPath,
|
||||
stat: async () => ({ isFile: () => true, size: 12 }),
|
||||
},
|
||||
spawn: vi.fn(),
|
||||
crypto: { randomUUID: () => 'job-0' },
|
||||
normalizeDirectoryPath: (p) => p,
|
||||
resolveProjectDirectory: projectDirectoryRuntime.resolveProjectDirectory,
|
||||
buildAugmentedPath: () => '/usr/bin',
|
||||
resolveGitBinaryForSpawn: () => 'git',
|
||||
openchamberUserConfigRoot: '/home/user/.config',
|
||||
});
|
||||
return getRoute('GET', '/api/fs/stat');
|
||||
};
|
||||
|
||||
const callStat = async (handler, { headers = {}, query }) => {
|
||||
const res = createMockResponse();
|
||||
const req = {
|
||||
query,
|
||||
get: (name) => headers[name.toLowerCase()] ?? undefined,
|
||||
};
|
||||
await handler(req, res);
|
||||
return res;
|
||||
};
|
||||
|
||||
it('rejects a stat for a file under the session directory when only lastDirectory resolves the workspace', async () => {
|
||||
const handler = registerStatWithProjectDirectoryRuntime();
|
||||
|
||||
const res = await callStat(handler, { query: { path: '/repo-b/src/index.ts', optional: 'true' } });
|
||||
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(res.body).toEqual({ error: 'Path is outside of active workspace' });
|
||||
});
|
||||
|
||||
it('accepts the same stat when the session directory rides the x-opencode-directory header', async () => {
|
||||
const handler = registerStatWithProjectDirectoryRuntime();
|
||||
|
||||
const res = await callStat(handler, {
|
||||
headers: { 'x-opencode-directory': '/repo-b' },
|
||||
query: { path: '/repo-b/src/index.ts', optional: 'true' },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.body.isFile).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user