mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-05 06:59:42 +02:00
feat(files): preview text files from outside the workspace, and stop routing them at a viewer that cannot read them
A .json/.log/.yaml/code path outside the session workspace was refused as an unsupported type, and clicking one in the terminal made it worse: text goes to the log viewer, which spawns `tail -f` and allows only the workspace, /var/log and ~/logs, so it answered "Path must be within working directory or allowed log directories" while the same path clicked in the response viewer previewed fine. Two surfaces, two answers, for a file the session can already cat. - TEXT_ATTACHMENT_EXTENSIONS IS EDITABLE_EXTENSIONS (config/file-editing.ts), not a second curated list that would drift from it. The rule reads: if the viewer would open a file for editing inside the workspace, the same file outside it can be read. The suffix was never the confidentiality gate here, the path guard is (sensitive-file blocklist, /root and /etc trees, realpath before the check), and it still runs on every registration. - Widening what can be READ must not widen what can RUN. html/htm join svg in serveRawFile's download-only branch, so markup is never served with a renderable type on our own origin; other text goes out as inert text/plain; charset=utf-8 with nosniff, matching what the path picker does. The preview reads through fetch(), which ignores the disposition, so a clicked .html still shows its source. - ~/.codeman*/state.json joins isSensitivePath. It persists SessionState.envOverrides and the env allowlist admits key-shaped names (GEMINI_API_KEY, CLAUDE_CODE_*), so it can hold a live credential. Same treatment as hook-secret and users.json, and the rest of the tree stays attachable. - The terminal sends an out-of-workspace path to the preview instead of the log viewer. In-workspace text keeps the tail viewer, which is the point of it, and file-stream-manager's allowlist is untouched: no `tail -f` on arbitrary host paths. - The by-id text preview is bounded like the workspace one: a Range request for the first 512KB (a real partial read, not a discarded 50MB download) plus a 500-line cap, with the footer saying so. Verified on an isolated instance: a 1.1MB external log opens in ~1.8s showing 500 lines with "showing first 500 lines" in the footer; json, yaml and code preview; an .html carrying a script tag renders as source and does not execute; .svg is still refused; a terminal click on an external .yaml opens the preview with no log viewer and no attachment card; an in-workspace .log still opens the streaming tail viewer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -438,6 +438,124 @@ describe('file-routes attachment path guard (COD-53)', () => {
|
||||
});
|
||||
});
|
||||
|
||||
// ===== Text family (code, config and logs outside the workspace) =====
|
||||
// The agent in the session can already `cat` these, so refusing the click
|
||||
// bought no confidentiality. The gate that matters is the path guard, which
|
||||
// still runs, and markup must not become executable just because it is now
|
||||
// readable.
|
||||
describe('text attachments', () => {
|
||||
it.each([
|
||||
['/tmp/run.log', 'log'],
|
||||
['/tmp/data.json', 'json'],
|
||||
['/tmp/conf/app.yaml', 'yaml'],
|
||||
['/tmp/src/index.ts', 'ts'],
|
||||
['/tmp/export.csv', 'csv'],
|
||||
])('registers %s as a text attachment', async (path, extension) => {
|
||||
mockedStat.mockResolvedValue({ size: 40, isFile: () => true, mtimeMs: 5 } as never);
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
|
||||
payload: { path, notify: false },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(200);
|
||||
const body = JSON.parse(res.body);
|
||||
expect(body.data.extension).toBe(extension);
|
||||
expect(body.data.attachmentType).toBe('text');
|
||||
});
|
||||
|
||||
it('serves a text file with no dedicated MIME as inert text/plain', async () => {
|
||||
const content = Buffer.from('boot ok\nstarted\n');
|
||||
mockedStat.mockResolvedValue({ size: content.length, isFile: () => true, mtimeMs: 5 } as never);
|
||||
mockedCreateReadStream.mockReturnValue(Readable.from([content]) as never);
|
||||
|
||||
const reg = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
|
||||
payload: { path: '/tmp/run.log', notify: false },
|
||||
});
|
||||
const rawRes = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments/${JSON.parse(reg.body).data.attachmentId}/raw`,
|
||||
});
|
||||
|
||||
expect(rawRes.statusCode).toBe(200);
|
||||
expect(rawRes.headers['content-type']).toBe('text/plain; charset=utf-8');
|
||||
expect(rawRes.headers['x-content-type-options']).toBe('nosniff');
|
||||
});
|
||||
|
||||
it('keeps HTML download-only so readable never means executable', async () => {
|
||||
// Serving markup with a renderable type on our own origin is stored XSS.
|
||||
// The preview reads it through fetch(), which ignores the disposition, so
|
||||
// a clicked .html still shows its source.
|
||||
const content = Buffer.from('<script>alert(1)</script>');
|
||||
mockedStat.mockResolvedValue({ size: content.length, isFile: () => true, mtimeMs: 5 } as never);
|
||||
mockedCreateReadStream.mockReturnValue(Readable.from([content]) as never);
|
||||
|
||||
const reg = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
|
||||
payload: { path: '/tmp/report.html', notify: false },
|
||||
});
|
||||
const rawRes = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments/${JSON.parse(reg.body).data.attachmentId}/raw`,
|
||||
});
|
||||
|
||||
expect(rawRes.headers['content-type']).toBe('application/octet-stream');
|
||||
expect(String(rawRes.headers['content-disposition'])).toContain('attachment');
|
||||
});
|
||||
|
||||
it('answers a byte range for text so a huge log is a partial read', async () => {
|
||||
const content = Buffer.from('0123456789abcdef');
|
||||
mockedStat.mockResolvedValue({ size: content.length, isFile: () => true, mtimeMs: 5 } as never);
|
||||
mockedCreateReadStream.mockReturnValue(Readable.from([content.subarray(0, 8)]) as never);
|
||||
|
||||
const reg = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
|
||||
payload: { path: '/tmp/big.log', notify: false },
|
||||
});
|
||||
const rawRes = await harness.app.inject({
|
||||
method: 'GET',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments/${JSON.parse(reg.body).data.attachmentId}/raw`,
|
||||
headers: { range: 'bytes=0-7' },
|
||||
});
|
||||
|
||||
expect(rawRes.statusCode).toBe(206);
|
||||
expect(rawRes.headers['content-range']).toBe(`bytes 0-7/${content.length}`);
|
||||
});
|
||||
|
||||
it.each([
|
||||
['/home/someone/.config/gh/hosts.yml', 'forge token'],
|
||||
['/home/someone/project/.env.json', 'dotenv'],
|
||||
['/home/someone/.codeman/state.json', 'codeman state (can hold envOverrides secrets)'],
|
||||
['/home/someone/deploy/credentials.yaml', 'generic credentials'],
|
||||
['/etc/codeman/dump.log', 'blocked tree'],
|
||||
])('still refuses %s (%s) now that text is servable', async (path) => {
|
||||
mockedStat.mockResolvedValue({ size: 40, isFile: () => true, mtimeMs: 5 } as never);
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
|
||||
payload: { path, notify: false },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(403);
|
||||
});
|
||||
|
||||
it('still refuses a type outside the family', async () => {
|
||||
mockedStat.mockResolvedValue({ size: 40, isFile: () => true, mtimeMs: 5 } as never);
|
||||
const res = await harness.app.inject({
|
||||
method: 'POST',
|
||||
url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
|
||||
payload: { path: '/tmp/drawing.svg', notify: false },
|
||||
});
|
||||
|
||||
expect(res.statusCode).toBe(400);
|
||||
expect(JSON.parse(res.body).error).toMatch(/unsupported/i);
|
||||
});
|
||||
});
|
||||
|
||||
// ===== Quiet registration (click-to-preview) =====
|
||||
// The file-preview overlay registers a clicked out-of-workspace path to mint
|
||||
// an id it can render by. It is already putting the file on screen, so the
|
||||
|
||||
@@ -73,6 +73,12 @@ describe('isSensitivePath', () => {
|
||||
['codeman hook secret', `${HOME}/.codeman/hook-secret`],
|
||||
['codeman user table', `${HOME}/.codeman/users.json`],
|
||||
['codeman hook secret on a named instance', `${HOME}/.codeman-beta/hook-secret`],
|
||||
// state.json persists SessionState.envOverrides, and the env allowlist
|
||||
// admits key-shaped names (GEMINI_API_KEY, CLAUDE_CODE_*), so it can hold
|
||||
// a live credential. Named once .json became previewable from outside the
|
||||
// workspace.
|
||||
['codeman state file', `${HOME}/.codeman/state.json`],
|
||||
['codeman state file on a named instance', `${HOME}/.codeman-beta/state.json`],
|
||||
];
|
||||
|
||||
it.each(blocked)('blocks the %s', (_label, path) => {
|
||||
@@ -88,6 +94,7 @@ describe('isSensitivePath', () => {
|
||||
// The publish skill and the review-card loop attach from these trees, so
|
||||
// only their named secret members are blocked, never the whole tree.
|
||||
['a codeman screenshot', `${HOME}/.codeman/screenshots/shot.png`],
|
||||
['a codeman lifecycle log', `${HOME}/.codeman/session-lifecycle.jsonl`],
|
||||
['a claude transcript', `${HOME}/.claude/projects/proj/session.jsonl`],
|
||||
['a claude team inbox', `${HOME}/.claude/teams/alpha/inboxes/bob.json`],
|
||||
// isUnderTree-style separator awareness: a sibling name that merely starts
|
||||
|
||||
Reference in New Issue
Block a user