COD-37 add server-side attachment pipeline (registry, magic-link, path guard)

Adds the foundation for serving local files to the browser as live external
attachments with a stable id, so requests never carry arbitrary absolute paths.

- attachment-registry: in-memory, session-scoped registry. registerExternalAttachment
  validates an absolute path, resolves symlinks, enforces the path guard, and mints
  an `att_<uuid>` id; records are cleared when the session is removed.
- attachment path guard: a configurable blocklist (secret locations + /root,/etc
  trees, extendable via attachmentBlockedPaths / CODEMAN_ATTACHMENT_BLOCKED_PATHS)
  plus an optional, default-off workspace-confinement mode. Shares one
  sensitive-path blocklist (web/sensitive-path.ts) with /api/download, which is
  refactored to use the extracted module instead of an inline copy.
- terminal magic links: the session scans output for codeman://attach?path=... and
  emits `attachmentRequested`; the web server registers the file and broadcasts an
  `attachment:detected` SSE event. `codeman attach <path>` (CLI) prints the magic
  link or POSTs directly when a session id is known.
- image watcher: detects png/pdf/docx/pptx dropped into a session's working dir and
  emits `attachment:detected`.
- routes: POST /api/sessions/:id/attachments (register) and
  GET /api/sessions/:id/attachments/:attachmentId/raw (serve), both re-checking the
  guard before streaming.

Document previews/thumbnails and the attachment-history drawer build on this
foundation and land separately.

Verified: tsc --noEmit, lint, format, frontend-syntax, full test:ci (2846 passed),
and a server boot smoke (/api/status 200).

test/attachment-magic.test.ts

@@ -0,0 +1,57 @@
+import { describe, expect, it } from 'vitest';
+import { Session } from '../src/session.js';
+import { parseAttachmentMagicLinks } from '../src/attachment-magic.js';
+
+describe('attachment magic links', () => {
+  it('extracts absolute paths from codeman attach magic URLs', () => {
+    const links = parseAttachmentMagicLinks(
+      'Preview this: codeman://attach?path=%2Fmnt%2Fc%2FDecks%2FBoard%20Update.pptx'
+    );
+
+    expect(links).toEqual(['/mnt/c/Decks/Board Update.pptx']);
+  });
+
+  it('ignores duplicate links in one terminal chunk', () => {
+    const links = parseAttachmentMagicLinks(
+      [
+        'codeman://attach?path=/tmp/report.pdf',
+        'codeman://attach?path=/tmp/report.pdf',
+        'codeman://attach?path=/tmp/brief.docx',
+      ].join('\n')
+    );
+
+    expect(links).toEqual(['/tmp/report.pdf', '/tmp/brief.docx']);
+  });
+
+  it('accepts markdown and plain-text magic paths', () => {
+    const links = parseAttachmentMagicLinks(
+      ['codeman://attach?path=/tmp/notes.md', 'codeman://attach?path=/tmp/run.txt'].join('\n')
+    );
+
+    expect(links).toEqual(['/tmp/notes.md', '/tmp/run.txt']);
+  });
+
+  it('rejects relative or unsupported magic paths', () => {
+    const links = parseAttachmentMagicLinks(
+      [
+        'codeman://attach?path=relative.pdf',
+        'codeman://attach?path=/tmp/archive.zip',
+        'codeman://attach?path=/tmp/deck.pptx',
+      ].join('\n')
+    );
+
+    expect(links).toEqual(['/tmp/deck.pptx']);
+  });
+
+  it('emits attachmentRequested from raw terminal output', () => {
+    const session = new Session({ id: 'session-attach-test', workingDir: '/tmp', mode: 'codex' });
+    const requested: string[] = [];
+    session.on('attachmentRequested', (event: { path: string }) => requested.push(event.path));
+
+    (session as unknown as { _handleTerminalOutput(data: string): void })._handleTerminalOutput(
+      'codeman://attach?path=%2Ftmp%2Fdeck.pptx'
+    );
+
+    expect(requested).toEqual(['/tmp/deck.pptx']);
+  });
+});

test/image-watcher.test.ts

@@ -145,9 +145,9 @@ describe('ImageWatcher', () => {
   // ========== Image Detection ==========
 
   describe('image detection', () => {
-    it('should emit image:detected for .png files', () => {
+    it('should emit attachment:detected for .png files', () => {
       const handler = vi.fn();
-      watcher.on('image:detected', handler);
+      watcher.on('attachment:detected', handler);
 
       watcher.watchSession('session-1', '/home/user/project');
       const chokidarWatcher = mockWatchers.get('/home/user/project')!;
@@ -161,9 +161,42 @@ describe('ImageWatcher', () => {
       expect(event.fileName).toBe('screenshot.png');
       expect(event.filePath).toBe('/home/user/project/screenshot.png');
       expect(event.relativePath).toBe('screenshot.png');
+      expect(event.extension).toBe('png');
+      expect(event.attachmentType).toBe('image');
       expect(event.size).toBe(2048);
     });
 
+    it('should not emit legacy image:detected for .png attachment cards', () => {
+      const handler = vi.fn();
+      watcher.on('image:detected', handler);
+
+      watcher.watchSession('session-1', '/home/user/project');
+      mockWatchers.get('/home/user/project')!.emit('add', '/home/user/project/screenshot.png');
+      vi.advanceTimersByTime(300);
+
+      expect(handler).not.toHaveBeenCalled();
+    });
+
+    it.each([
+      ['report.pdf', 'pdf'],
+      ['brief.docx', 'document'],
+      ['deck.pptx', 'presentation'],
+    ])('should emit attachment:detected for %s files', (fileName, attachmentType) => {
+      const handler = vi.fn();
+      watcher.on('attachment:detected', handler);
+
+      watcher.watchSession('session-1', '/home/user/project');
+      mockWatchers.get('/home/user/project')!.emit('add', `/home/user/project/${fileName}`);
+      vi.advanceTimersByTime(300);
+
+      expect(handler).toHaveBeenCalledTimes(1);
+      expect(handler.mock.calls[0][0]).toMatchObject({
+        sessionId: 'session-1',
+        fileName,
+        attachmentType,
+      });
+    });
+
     it('should emit for .jpg files', () => {
       const handler = vi.fn();
       watcher.on('image:detected', handler);
@@ -250,10 +283,10 @@ describe('ImageWatcher', () => {
       watcher.on('image:detected', handler);
 
       watcher.watchSession('session-1', '/home/user/project');
-      mockWatchers.get('/home/user/project')!.emit('add', '/home/user/project/assets/img.png');
+      mockWatchers.get('/home/user/project')!.emit('add', '/home/user/project/assets/img.jpg');
       vi.advanceTimersByTime(300);
 
-      expect(handler.mock.calls[0][0].relativePath).toBe('assets/img.png');
+      expect(handler.mock.calls[0][0].relativePath).toBe('assets/img.jpg');
     });
   });
 
@@ -268,11 +301,11 @@ describe('ImageWatcher', () => {
       const chokidarWatcher = mockWatchers.get('/home/user/project')!;
 
       // Rapid adds of the same file
-      chokidarWatcher.emit('add', '/home/user/project/screenshot.png');
+      chokidarWatcher.emit('add', '/home/user/project/screenshot.jpg');
       vi.advanceTimersByTime(100); // not yet past debounce
-      chokidarWatcher.emit('add', '/home/user/project/screenshot.png');
+      chokidarWatcher.emit('add', '/home/user/project/screenshot.jpg');
       vi.advanceTimersByTime(100);
-      chokidarWatcher.emit('add', '/home/user/project/screenshot.png');
+      chokidarWatcher.emit('add', '/home/user/project/screenshot.jpg');
       vi.advanceTimersByTime(300); // now past debounce from last emit
 
       // Should only emit once (the last debounced one)
@@ -286,8 +319,8 @@ describe('ImageWatcher', () => {
       watcher.watchSession('session-1', '/home/user/project');
       const chokidarWatcher = mockWatchers.get('/home/user/project')!;
 
-      chokidarWatcher.emit('add', '/home/user/project/a.png');
-      chokidarWatcher.emit('add', '/home/user/project/b.png');
+      chokidarWatcher.emit('add', '/home/user/project/a.jpg');
+      chokidarWatcher.emit('add', '/home/user/project/b.jpg');
       vi.advanceTimersByTime(300);
 
       expect(handler).toHaveBeenCalledTimes(2);
@@ -306,7 +339,7 @@ describe('ImageWatcher', () => {
 
       // Emit 25 unique images in quick succession
       for (let i = 0; i < 25; i++) {
-        chokidarWatcher.emit('add', `/home/user/project/img${i}.png`);
+        chokidarWatcher.emit('add', `/home/user/project/img${i}.jpg`);
         vi.advanceTimersByTime(250); // past debounce, within burst window
       }
 
@@ -323,7 +356,7 @@ describe('ImageWatcher', () => {
 
       // Fill up burst limit
       for (let i = 0; i < 20; i++) {
-        chokidarWatcher.emit('add', `/home/user/project/img${i}.png`);
+        chokidarWatcher.emit('add', `/home/user/project/img${i}.jpg`);
         vi.advanceTimersByTime(250);
       }
       expect(handler).toHaveBeenCalledTimes(20);
@@ -332,7 +365,7 @@ describe('ImageWatcher', () => {
       vi.advanceTimersByTime(11_000);
 
       // Should accept new images
-      chokidarWatcher.emit('add', '/home/user/project/new.png');
+      chokidarWatcher.emit('add', '/home/user/project/new.jpg');
       vi.advanceTimersByTime(300);
 
       expect(handler).toHaveBeenCalledTimes(21);

test/routes/file-routes-attachment-path-guard.test.ts

@@ -0,0 +1,324 @@
+/**
+ * @fileoverview COD-53 — attachment path-traversal / sensitive-file guard.
+ *
+ * Verifies the sensitive-path blocklist is enforced at:
+ *  - attachment registration (POST /api/sessions/:id/attachments)
+ *  - raw / preview / thumbnail serving (defense-in-depth against a record that
+ *    was crafted or registered before the guard existed)
+ * while still allowing legitimate cross-workspace attachment (codeman-publish
+ * skill + the ~/.codeman review-card loop) to succeed.
+ *
+ * Uses app.inject() — no real HTTP ports needed.
+ * Port: N/A (app.inject doesn't open ports)
+ */
+
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { Readable } from 'node:stream';
+import { homedir } from 'node:os';
+import { createRouteTestHarness, type RouteTestHarness } from './_route-test-utils.js';
+import { registerFileRoutes } from '../../src/web/routes/file-routes.js';
+
+// Mock fs/promises for file operations
+vi.mock('node:fs/promises', () => ({
+  default: {
+    readdir: vi.fn(async () => []),
+    readFile: vi.fn(async () => 'file content'),
+    writeFile: vi.fn(async () => undefined),
+    stat: vi.fn(async () => ({ size: 100, isFile: () => true, mtimeMs: 1 })),
+    mkdir: vi.fn(async () => undefined),
+    mkdtemp: vi.fn(async () => '/tmp/codeman-preview-test'),
+    rename: vi.fn(async () => undefined),
+    rm: vi.fn(async () => undefined),
+  },
+}));
+
+// Mock realpathSync for symlink resolution (identity by default)
+vi.mock('node:fs', async (importOriginal) => {
+  const actual = await importOriginal<typeof import('node:fs')>();
+  return {
+    ...actual,
+    realpathSync: vi.fn((p: string) => p),
+    createReadStream: vi.fn(() => Readable.from([Buffer.from('file content')])),
+  };
+});
+
+vi.mock('../../src/file-stream-manager.js', () => ({
+  fileStreamManager: {
+    createStream: vi.fn(async () => ({ success: true, streamId: 'stream-1' })),
+    closeStream: vi.fn(() => true),
+  },
+}));
+
+import fs from 'node:fs/promises';
+import { createReadStream, realpathSync } from 'node:fs';
+import { attachmentRegistry, type AttachmentRecord } from '../../src/attachment-registry.js';
+
+const mockedStat = vi.mocked(fs.stat);
+const mockedRealpathSync = vi.mocked(realpathSync);
+const mockedCreateReadStream = vi.mocked(createReadStream);
+
+describe('file-routes attachment path guard (COD-53)', () => {
+  let harness: RouteTestHarness;
+
+  beforeEach(async () => {
+    harness = await createRouteTestHarness(registerFileRoutes);
+    vi.clearAllMocks();
+    attachmentRegistry.clearSession('test-session-1');
+    mockedRealpathSync.mockImplementation((p: string) => p as never);
+    mockedStat.mockResolvedValue({ size: 100, isFile: () => true, mtimeMs: 1 } as never);
+    mockedCreateReadStream.mockReturnValue(Readable.from([Buffer.from('file content')]) as never);
+  });
+
+  afterEach(async () => {
+    await harness.app.close();
+    attachmentRegistry.clearSession(harness.ctx._sessionId);
+    // Reset attachment-guard env knobs so one test can't leak into the next.
+    delete process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS;
+    delete process.env.CODEMAN_ATTACHMENT_CONFINE;
+  });
+
+  // ===== BLOCK: registration rejects a sensitive path =====
+
+  it('rejects registering a .env file that carries a supported extension', async () => {
+    // A dotenv-style secret file named with a supported extension still leaks
+    // secrets; the blocklist's /\.env\./ pattern catches `.env.<ext>`.
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/home/someone/project/.env.txt' },
+    });
+
+    expect(res.statusCode).toBe(403);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(false);
+  });
+
+  it('rejects registering an SSH key path even with a supported extension', async () => {
+    const sshTxt = `${homedir()}/.ssh/id_rsa.txt`;
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: sshTxt },
+    });
+
+    expect(res.statusCode).toBe(403);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(false);
+  });
+
+  it('rejects registering a sensitive path that a symlink resolves to', async () => {
+    // The requested path looks innocent (.md) but realpath resolves it to an SSH key dir.
+    mockedRealpathSync.mockReturnValue(`${homedir()}/.ssh/known_hosts.md` as never);
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/home/someone/project/innocent.md' },
+    });
+
+    expect(res.statusCode).toBe(403);
+  });
+
+  // ===== BLOCK (defense-in-depth): raw serving rejects a sensitive record =====
+
+  it('refuses to serve raw bytes for a record whose path is sensitive', async () => {
+    // Simulate a record that was registered before the guard existed (or crafted).
+    const record: AttachmentRecord = {
+      attachmentId: 'att_sensitive',
+      sessionId: harness.ctx._sessionId,
+      filePath: `${homedir()}/.ssh/id_rsa.txt`,
+      fileName: 'id_rsa.txt',
+      extension: 'txt',
+      attachmentType: 'text',
+      size: 100,
+      mtimeMs: 1,
+      timestamp: Date.now(),
+      source: 'external',
+    };
+    attachmentRegistry.register(record);
+
+    const res = await harness.app.inject({
+      method: 'GET',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments/att_sensitive/raw`,
+    });
+
+    expect(res.statusCode).toBe(403);
+    expect(mockedCreateReadStream).not.toHaveBeenCalled();
+  });
+
+  // ===== PRESERVE: legitimate cross-workspace attachment still works =====
+
+  it('still registers a normal cross-workspace file (codeman-publish / loop review card)', async () => {
+    mockedStat.mockResolvedValue({ size: 512, isFile: () => true, mtimeMs: 5 } as never);
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: `${homedir()}/.codeman/jira-autoloop-questions.md` },
+    });
+
+    expect(res.statusCode).toBe(200);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(true);
+    expect(body.data.fileName).toBe('jira-autoloop-questions.md');
+    expect(body.data.extension).toBe('md');
+  });
+
+  it('still registers an arbitrary project-dir file (WSL path)', async () => {
+    mockedStat.mockResolvedValue({ size: 4096, isFile: () => true, mtimeMs: 5 } as never);
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/mnt/c/decks/board-update.pdf' },
+    });
+
+    expect(res.statusCode).toBe(200);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(true);
+    expect(body.data.fileName).toBe('board-update.pdf');
+  });
+
+  it('still serves raw bytes for a legitimately registered cross-workspace file', async () => {
+    const content = Buffer.from('# notes');
+    mockedCreateReadStream.mockReturnValue(Readable.from([content]) as never);
+    mockedStat.mockResolvedValue({ size: content.length, isFile: () => true, mtimeMs: 5 } as never);
+
+    const registerRes = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: `${homedir()}/.codeman/review-card.md` },
+    });
+    const attachmentId = JSON.parse(registerRes.body).data.attachmentId;
+
+    const rawRes = await harness.app.inject({
+      method: 'GET',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments/${attachmentId}/raw`,
+    });
+
+    expect(rawRes.statusCode).toBe(200);
+    expect(rawRes.headers['content-type']).toBe('text/markdown');
+  });
+
+  // ===== BLOCK (broadened defaults): /root and /etc trees =====
+
+  it('rejects registering a file anywhere under /root by default', async () => {
+    // /root is the root account home — blocked as a whole tree by default,
+    // even for an ordinary-looking note with a supported extension.
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/root/secret-notes.md' },
+    });
+
+    expect(res.statusCode).toBe(403);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(false);
+  });
+
+  it('rejects registering a file anywhere under /etc by default', async () => {
+    // The whole /etc tree is blocked by default (not just /etc/shadow).
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/etc/codeman/config-dump.txt' },
+    });
+
+    expect(res.statusCode).toBe(403);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(false);
+  });
+
+  it('does not block a lookalike sibling dir like /etcetera (separator-aware)', async () => {
+    // The /etc tree block must be path-separator-aware so an unrelated
+    // /etcetera/... path is NOT caught by accident.
+    mockedStat.mockResolvedValue({ size: 10, isFile: () => true, mtimeMs: 5 } as never);
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/etcetera/notes.md' },
+    });
+
+    expect(res.statusCode).toBe(200);
+  });
+
+  // ===== CONFIG: extend the blocked set via env =====
+
+  it('rejects a path added via the extra-blocked-paths config', async () => {
+    process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS = '/srv/secrets,/data/private';
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/srv/secrets/keys.pdf' },
+    });
+
+    expect(res.statusCode).toBe(403);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(false);
+  });
+
+  it('still allows a normal path NOT in the configured blocked set', async () => {
+    process.env.CODEMAN_ATTACHMENT_BLOCKED_PATHS = '/srv/secrets,/data/private';
+    mockedStat.mockResolvedValue({ size: 20, isFile: () => true, mtimeMs: 5 } as never);
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/srv/public/report.pdf' },
+    });
+
+    expect(res.statusCode).toBe(200);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(true);
+    expect(body.data.fileName).toBe('report.pdf');
+  });
+
+  // ===== CONFINEMENT MODE ON (opt-in) =====
+
+  it('confinement ON: rejects a file OUTSIDE the session workspace', async () => {
+    process.env.CODEMAN_ATTACHMENT_CONFINE = '1';
+    // Mock session workspace is /tmp/test-workdir; this file resolves elsewhere.
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: '/home/someone/elsewhere/report.pdf' },
+    });
+
+    expect(res.statusCode).toBe(403);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(false);
+  });
+
+  it('confinement ON: allows a file INSIDE the session workspace', async () => {
+    process.env.CODEMAN_ATTACHMENT_CONFINE = '1';
+    // Mock session workspace is /tmp/test-workdir (see MockSession).
+    const insidePath = '/tmp/test-workdir/docs/report.pdf';
+    mockedRealpathSync.mockReturnValue(insidePath as never);
+    mockedStat.mockResolvedValue({ size: 30, isFile: () => true, mtimeMs: 5 } as never);
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: insidePath },
+    });
+
+    expect(res.statusCode).toBe(200);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(true);
+    expect(body.data.fileName).toBe('report.pdf');
+  });
+
+  // ===== CONFINEMENT OFF (default) regression: legit cross-workspace attach =====
+
+  it('confinement OFF (default): legit cross-workspace attach still succeeds', async () => {
+    // No CODEMAN_ATTACHMENT_CONFINE set → default OFF. A ~/.codeman review-card
+    // file lives OUTSIDE the /tmp/test-workdir session workspace and must still
+    // attach (protects codeman-publish + the loop's review-card channel).
+    mockedStat.mockResolvedValue({ size: 64, isFile: () => true, mtimeMs: 5 } as never);
+    const res = await harness.app.inject({
+      method: 'POST',
+      url: `/api/sessions/${harness.ctx._sessionId}/attachments`,
+      payload: { path: `${homedir()}/.codeman/jira-autoloop-questions.md` },
+    });
+
+    expect(res.statusCode).toBe(200);
+    const body = JSON.parse(res.body);
+    expect(body.success).toBe(true);
+    expect(body.data.fileName).toBe('jira-autoloop-questions.md');
+  });
+});