mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-11-15 10:59:32 +01:00
fix(files): harden the outside-worktree root after review
Keep the 1.49.0 API meaning: liveWorktree and root=live stay inside the session folder, and an outside worktree is published as outsideWorktree and walked only with root=live-any. Refuse a symlinked .git folder as the main worktree, sensitive files inside a followed worktree, and any outside worktree while attachment confinement is on or around a blocked tree. The file routes re-check the guard settings on every request, and a throwing rule check refuses. The panel keeps its Back to worktree button while pinned to the session folder. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
c9b79097b0
commit
8d33f53fd0
28 files changed
+856
-198
No files matched your search
@@ -81,6 +81,21 @@ describe('Files panel listing of a live worktree', () => {
|
||||
expect(data.tree[0].children[0].path).toBe(`${PREFIX}/src/worktree-only.ts`);
|
||||
});
|
||||
|
||||
it('lists an inside worktree for root=live-any, the root the Files panel sends', async () => {
|
||||
const data = await list('&root=live-any');
|
||||
expect(data.root).toBe(worktree);
|
||||
expect(data.worktree).toEqual({ name: 'probe', path: worktree });
|
||||
expect(names(data.tree)).toEqual(['src']);
|
||||
expect(data.tree[0].path).toBe(`${PREFIX}/src`);
|
||||
expect(data.tree[0].children[0].path).toBe(`${PREFIX}/src/worktree-only.ts`);
|
||||
});
|
||||
|
||||
it('walks the session folder for root=live-any when a symlink replaced the worktree', async () => {
|
||||
rmSync(worktree, { recursive: true });
|
||||
symlinkSync(makeOutsideFolder(), worktree);
|
||||
expectSessionFolderListing(await list('&root=live-any&showHidden=true'));
|
||||
});
|
||||
|
||||
it('does not need hidden files on, though the worktree lives under a dot folder', async () => {
|
||||
const data = await list('&root=live&showHidden=false');
|
||||
expect(data.totalFiles).toBe(1);
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
/**
|
||||
* @fileoverview The file routes for a worktree OUTSIDE the session's folder (#765 follow-up).
|
||||
*
|
||||
* `liveWorktree.outside` makes that worktree a second root: `GET .../files?root=live` lists it with
|
||||
* ABSOLUTE paths, and the read, raw, edit and download routes confine an absolute path inside it to
|
||||
* it. What must not change: a relative path still resolves against `workingDir` only, a path that
|
||||
* A session's outside worktree is a second root: `GET .../files?root=live-any` lists it with
|
||||
* ABSOLUTE paths (`root=live` keeps walking `workingDir`), and the read, raw, edit and download
|
||||
* routes confine an absolute path inside it to it. What must not change: a relative path still resolves against `workingDir` only, a path that
|
||||
* escapes the worktree through a symlink is refused, and a worktree folder replaced by a symlink is
|
||||
* neither listed nor served. Real filesystem on purpose, since realpath is the thing under test.
|
||||
*/
|
||||
@@ -11,10 +11,18 @@ import { createHash } from 'node:crypto';
|
||||
import { mkdirSync, mkdtempSync, realpathSync, renameSync, rmSync, symlinkSync, writeFileSync } from 'node:fs';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { tmpdir } from 'node:os';
|
||||
import { dirname } from 'node:path';
|
||||
import { join } from 'node:path';
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
|
||||
import { registerFileRoutes } from '../../src/web/routes/file-routes.js';
|
||||
import { SETTINGS_PATH } from '../../src/web/route-helpers.js';
|
||||
|
||||
// The converter would shell out to pdftoppm/LibreOffice; the route's own path check is under test.
|
||||
vi.mock('../../src/document-thumbnailer.js', () => ({
|
||||
generateFirstPageThumbnail: vi.fn(async () => ({ content: Buffer.from('\x89PNG fake'), contentType: 'image/png' })),
|
||||
}));
|
||||
import { generateFirstPageThumbnail } from '../../src/document-thumbnailer.js';
|
||||
|
||||
const sha256 = (text: string) => createHash('sha256').update(text).digest('hex');
|
||||
|
||||
@@ -55,11 +63,12 @@ describe('file routes for a worktree outside the session folder', () => {
|
||||
afterEach(async () => {
|
||||
await harness.app.close();
|
||||
rmSync(base, { recursive: true, force: true });
|
||||
rmSync(SETTINGS_PATH, { force: true });
|
||||
});
|
||||
|
||||
describe('GET .../files', () => {
|
||||
it('lists the worktree with absolute paths', async () => {
|
||||
const data = await list('&root=live');
|
||||
const data = await list('&root=live-any');
|
||||
expect(data.root).toBe(sibling);
|
||||
expect(data.worktree).toEqual({ name: 'myapp-feature', path: sibling, outside: true });
|
||||
expect(data.tree[0].path).toBe(join(sibling, 'src'));
|
||||
@@ -67,20 +76,29 @@ describe('file routes for a worktree outside the session folder', () => {
|
||||
});
|
||||
|
||||
it('searches the worktree, matching on paths relative to it and answering absolute ones', async () => {
|
||||
const data = await list(`&root=live&q=${encodeURIComponent('src/feature*')}`);
|
||||
const data = await list(`&root=live-any&q=${encodeURIComponent('src/feature*')}`);
|
||||
expect(data.matches.map((m: { path: string }) => m.path)).toEqual([join(sibling, 'src', 'feature-only.ts')]);
|
||||
});
|
||||
|
||||
it('keeps walking the session folder, with relative paths, for root=live', async () => {
|
||||
const data = await list('&root=live');
|
||||
expect(data.root).toBe(repo);
|
||||
expect(data.worktree).toBeUndefined();
|
||||
expect(data.tree[0].children[0].path).toBe('src/main-only.ts');
|
||||
expect(JSON.stringify(data.tree)).not.toContain('feature-only.ts');
|
||||
});
|
||||
|
||||
it('keeps the session folder, with relative paths, without the parameter', async () => {
|
||||
const data = await list();
|
||||
expect(data.root).toBe(repo);
|
||||
expect(data.worktree).toBeUndefined();
|
||||
expect(data.tree[0].children[0].path).toBe('src/main-only.ts');
|
||||
});
|
||||
|
||||
it('falls back to the session folder once a symlink has taken the worktree folder’s place', async () => {
|
||||
renameSync(sibling, `${sibling}-moved`);
|
||||
symlinkSync(secret, sibling);
|
||||
const data = await list('&root=live');
|
||||
const data = await list('&root=live-any');
|
||||
expect(data.root).toBe(repo);
|
||||
expect(JSON.stringify(data.tree)).not.toContain('key.txt');
|
||||
});
|
||||
@@ -93,6 +111,32 @@ describe('file routes for a worktree outside the session folder', () => {
|
||||
expect(JSON.stringify(res.json())).toContain('feature');
|
||||
});
|
||||
|
||||
it('refuses a sensitive file in the worktree on file-content and file-raw, and serves its neighbours', async () => {
|
||||
writeFileSync(join(sibling, '.env'), 'TOKEN=abc');
|
||||
mkdirSync(join(sibling, '.aws'));
|
||||
writeFileSync(join(sibling, '.aws', 'credentials'), 'aws_secret=abc');
|
||||
writeFileSync(join(sibling, 'plain.ts'), 'plain');
|
||||
for (const blocked of [join(sibling, '.env'), join(sibling, '.aws', 'credentials')]) {
|
||||
const content = await readContent(blocked);
|
||||
expect(content.json().success).toBe(false);
|
||||
expect(content.body).not.toContain('abc');
|
||||
const raw = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: sessionUrl(`file-raw?path=${encodeURIComponent(blocked)}`),
|
||||
});
|
||||
expect(raw.statusCode).not.toBe(200);
|
||||
expect(raw.body).not.toContain('abc');
|
||||
}
|
||||
const plain = await readContent(join(sibling, 'plain.ts'));
|
||||
expect(plain.json().success).toBe(true);
|
||||
const plainRaw = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: sessionUrl(`file-raw?path=${encodeURIComponent(join(sibling, 'plain.ts'))}`),
|
||||
});
|
||||
expect(plainRaw.statusCode).toBe(200);
|
||||
expect(plainRaw.body).toBe('plain');
|
||||
});
|
||||
|
||||
it('still resolves a relative path against the session folder only', async () => {
|
||||
expect((await readContent('src/main-only.ts')).statusCode).toBe(200);
|
||||
const escaped = await readContent('../myapp-feature/src/feature-only.ts');
|
||||
@@ -168,4 +212,88 @@ describe('file routes for a worktree outside the session folder', () => {
|
||||
expect(readFileSync(join(secret, 'key.txt'), 'utf8')).toBe('secret');
|
||||
});
|
||||
});
|
||||
|
||||
describe('file-preview and file-thumbnail', () => {
|
||||
const preview = (path: string) =>
|
||||
harness.app.inject({ method: 'GET', url: sessionUrl(`file-preview?path=${encodeURIComponent(path)}`) });
|
||||
const thumbnail = (path: string) =>
|
||||
harness.app.inject({ method: 'GET', url: sessionUrl(`file-thumbnail?path=${encodeURIComponent(path)}`) });
|
||||
|
||||
beforeEach(() => {
|
||||
vi.mocked(generateFirstPageThumbnail).mockClear();
|
||||
writeFileSync(join(sibling, 'doc.pdf'), '%PDF-1.4 fake');
|
||||
writeFileSync(join(sibling, 'pic.png'), 'png');
|
||||
});
|
||||
|
||||
it('answers file-preview for an absolute path inside the worktree', async () => {
|
||||
const res = await preview(join(sibling, 'doc.pdf'));
|
||||
expect(res.statusCode).toBeGreaterThanOrEqual(300);
|
||||
expect(res.statusCode).toBeLessThan(400);
|
||||
expect(res.headers.location).toContain('/file-raw?path=');
|
||||
});
|
||||
|
||||
it('serves file-thumbnail for an absolute path inside the worktree', async () => {
|
||||
const res = await thumbnail(join(sibling, 'pic.png'));
|
||||
expect(res.statusCode).toBe(200);
|
||||
expect(res.headers['content-type']).toContain('image/png');
|
||||
expect(generateFirstPageThumbnail).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('refuses both once a symlink has taken the worktree folder’s place', async () => {
|
||||
writeFileSync(join(secret, 'doc.pdf'), '%PDF-1.4 fake');
|
||||
writeFileSync(join(secret, 'pic.png'), 'png');
|
||||
renameSync(sibling, `${sibling}-moved`);
|
||||
symlinkSync(secret, sibling);
|
||||
expect((await preview(join(sibling, 'doc.pdf'))).statusCode).toBe(404);
|
||||
expect((await thumbnail(join(sibling, 'pic.png'))).statusCode).toBe(404);
|
||||
expect(generateFirstPageThumbnail).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
describe('the attachment guard, judged again on every request', () => {
|
||||
const target = () => join(sibling, 'src', 'feature-only.ts');
|
||||
const writeSettings = (settings: Record<string, unknown>) => {
|
||||
mkdirSync(dirname(SETTINGS_PATH), { recursive: true });
|
||||
writeFileSync(SETTINGS_PATH, JSON.stringify(settings));
|
||||
};
|
||||
const get = (route: string) =>
|
||||
harness.app.inject({ method: 'GET', url: sessionUrl(`${route}?path=${encodeURIComponent(target())}`) });
|
||||
|
||||
it('serves the worktree while no guard rule applies', async () => {
|
||||
writeSettings({});
|
||||
expect((await get('file-raw')).statusCode).toBe(200);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['workspace confinement is turned on', () => ({ attachmentConfineToWorkspace: true })],
|
||||
['the worktree’s parent folder is blocked', () => ({ attachmentBlockedPaths: [base] })],
|
||||
])('stops serving it once %s', async (_label, settings) => {
|
||||
writeSettings(settings());
|
||||
|
||||
const content = await get('file-content');
|
||||
expect(content.json().success).toBe(false);
|
||||
expect(content.body).not.toContain('feature');
|
||||
expect((await get('file-raw')).statusCode).toBe(404);
|
||||
|
||||
const before = readFileSync(target(), 'utf8');
|
||||
const put = await harness.app.inject({
|
||||
method: 'PUT',
|
||||
url: sessionUrl('file-content'),
|
||||
payload: { path: target(), content: 'changed', baseHash: sha256(before) },
|
||||
});
|
||||
expect(put.statusCode).toBe(404);
|
||||
expect(readFileSync(target(), 'utf8')).toBe(before);
|
||||
|
||||
const download = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/download?sessionId=${harness.ctx._sessionId}&path=${encodeURIComponent(target())}`,
|
||||
});
|
||||
expect(download.statusCode).toBe(404);
|
||||
|
||||
// The listing walks the session folder, and keeps reporting the worktree so the panel settles.
|
||||
const data = await list('&root=live-any');
|
||||
expect(data.root).toBe(repo);
|
||||
expect(data.tree[0].children[0].path).toBe('src/main-only.ts');
|
||||
expect(data.worktree).toEqual({ name: 'myapp-feature', path: sibling, outside: true });
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -34,6 +34,9 @@ describe('html-view routes', () => {
|
||||
writeFileSync(join(root, 'outside.txt'), 'outside');
|
||||
symlinkSync(join(root, 'outside.txt'), join(workspace, 'escape.txt'));
|
||||
|
||||
mkdirSync(join(root, 'sibling-feature'));
|
||||
writeFileSync(join(root, 'sibling-feature', 'report.html'), '<p>feature</p>');
|
||||
|
||||
harness = await createRouteTestHarness(registerHtmlViewRoutes);
|
||||
sid = harness.ctx._sessionId as string;
|
||||
(harness.ctx.sessions.get(sid) as { workingDir: string }).workingDir = workspace;
|
||||
@@ -270,6 +273,64 @@ describe('html-view routes', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('in a worktree outside the workspace (#765 follow-up)', () => {
|
||||
let sibling: string;
|
||||
const saved = {
|
||||
confine: process.env.CODEMAN_ATTACHMENT_CONFINE,
|
||||
blocked: process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS,
|
||||
};
|
||||
|
||||
beforeEach(() => {
|
||||
sibling = realpathSync(join(root, 'sibling-feature'));
|
||||
Object.assign(harness.ctx.sessions.get(sid) as object, {
|
||||
liveWorktree: { name: 'sibling-feature', path: sibling, outside: true },
|
||||
});
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
for (const [key, value] of [
|
||||
['CODEMAN_ATTACHMENT_CONFINE', saved.confine],
|
||||
['CODEMAN_ATTACHMENT_BLOCKED_PATHS', saved.blocked],
|
||||
] as const) {
|
||||
if (value === undefined) delete process.env[key];
|
||||
else process.env[key] = value;
|
||||
}
|
||||
});
|
||||
|
||||
it('renders a page from the worktree, minted as outside the workspace', async () => {
|
||||
const res = await open(join(sibling, 'report.html'));
|
||||
expect(res.statusCode).toBe(200);
|
||||
const url = JSON.parse(res.body).data.url as string;
|
||||
const page = await harness.app.inject({ method: 'GET', url });
|
||||
expect(page.statusCode).toBe(200);
|
||||
expect(page.body).toBe('<p>feature</p>');
|
||||
// Minted as outside the workspace, so the serve route's per-request confinement check covers it.
|
||||
expect(htmlViewCapabilities.resolve(capabilityFromHtmlViewPath(url) as string)?.outsideWorkspace).toBe(true);
|
||||
});
|
||||
|
||||
it('answers to the attachment guard: confinement on, or a blocked tree', async () => {
|
||||
const res = await open(join(sibling, 'report.html'));
|
||||
const url = JSON.parse(res.body).data.url as string;
|
||||
|
||||
process.env.CODEMAN_ATTACHMENT_CONFINE = '1';
|
||||
expect((await open(join(sibling, 'report.html'))).statusCode).toBe(403);
|
||||
// A page minted before confinement was switched on stops serving too.
|
||||
expect((await harness.app.inject({ method: 'GET', url })).statusCode).toBe(404);
|
||||
delete process.env.CODEMAN_ATTACHMENT_CONFINE;
|
||||
|
||||
process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS = sibling;
|
||||
expect((await open(join(sibling, 'report.html'))).statusCode).toBe(403);
|
||||
});
|
||||
|
||||
it('leaves a path outside both roots as it was', async () => {
|
||||
mkdirSync(join(root, 'reports'));
|
||||
writeFileSync(join(root, 'reports', 'report.html'), '<p>report</p>');
|
||||
expect((await open(join(root, 'reports', 'report.html'))).statusCode).toBe(200);
|
||||
process.env.CODEMAN_ATTACHMENT_CONFINE = '1';
|
||||
expect((await open(join(root, 'reports', 'report.html'))).statusCode).toBe(403);
|
||||
});
|
||||
});
|
||||
|
||||
it('refuses an HTML file inside a hidden directory, in or out of the workspace', async () => {
|
||||
mkdirSync(join(workspace, '.cache'));
|
||||
writeFileSync(join(workspace, '.cache', 'r.html'), '<p>r</p>');
|
||||
|
||||
Reference in new issue
Block a user