diff --git a/src/attachment-magic.ts b/src/attachment-magic.ts index f4d6d763..086700c7 100644 --- a/src/attachment-magic.ts +++ b/src/attachment-magic.ts @@ -3,11 +3,52 @@ */ import { isAbsolute } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { isSupportedAttachmentExtension } from './attachment-registry.js'; +import { stripAnsi } from './utils/index.js'; const MAGIC_LINK_RE = /codeman:\/\/attach\?([^\s<>"']+)/g; +const CODEX_SAVED_FILE_RE = /\bSaved to:\s*(file:\/\/[^\s<>"']+)/gi; + +export interface TerminalAttachmentRequest { + path: string; + source: 'external' | 'codex-generated'; +} + +export interface ParseTerminalAttachmentOptions { + /** + * Enable the Codex `Saved to: file://...` scanner. Only codex-mode sessions + * may set this — the relaxed codex-generated trust policy must never be + * reachable from other modes' (prompt-injectable) terminal output. + */ + codexArtifacts?: boolean; +} export function parseAttachmentMagicLinks(data: string): string[] { + return parseMagicAttachmentRequests(data).map((request) => request.path); +} + +export function parseTerminalAttachmentRequests( + data: string, + options: ParseTerminalAttachmentOptions = {} +): TerminalAttachmentRequest[] { + const results: TerminalAttachmentRequest[] = []; + const seen = new Set(); + const requests = options.codexArtifacts + ? [...parseMagicAttachmentRequests(data), ...parseCodexGeneratedArtifactRequests(data)] + : parseMagicAttachmentRequests(data); + + for (const request of requests) { + const key = `${request.source}:${request.path}`; + if (seen.has(key)) continue; + seen.add(key); + results.push(request); + } + + return results; +} + +function parseMagicAttachmentRequests(data: string): TerminalAttachmentRequest[] { const results: string[] = []; const seen = new Set(); @@ -27,6 +68,31 @@ export function parseAttachmentMagicLinks(data: string): string[] { } } + return results.map((path) => ({ path, source: 'external' })); +} + +function parseCodexGeneratedArtifactRequests(data: string): TerminalAttachmentRequest[] { + const results: TerminalAttachmentRequest[] = []; + const seen = new Set(); + + // Codex styles its TUI output — strip ANSI first so a trailing SGR reset + // (e.g. `...mockup.png\x1b[0m`) doesn't ride into the captured URL and break + // the extension allowlist check. + for (const match of stripAnsi(data).matchAll(CODEX_SAVED_FILE_RE)) { + const rawUrl = trimTrailingPunctuation(match[1] || ''); + try { + const filePath = fileURLToPath(rawUrl); + if (!isAbsolute(filePath)) continue; + const extension = filePath.split('.').pop()?.toLowerCase() || ''; + if (!isSupportedAttachmentExtension(extension)) continue; + if (seen.has(filePath)) continue; + seen.add(filePath); + results.push({ path: filePath, source: 'codex-generated' }); + } catch { + // Ignore malformed terminal text. Generated-artifact links are advisory. + } + } + return results; } diff --git a/src/attachment-registry.ts b/src/attachment-registry.ts index 3bfd4b40..cfd6fb85 100644 --- a/src/attachment-registry.ts +++ b/src/attachment-registry.ts @@ -14,7 +14,18 @@ import { isBlockedAttachmentPath, loadAttachmentGuardConfig } from './config/att import { validateSessionFilePath } from './web/route-helpers.js'; import type { AttachmentDetectedEvent, AttachmentDetectedType } from './types.js'; -const SUPPORTED_ATTACHMENT_EXTENSIONS = new Set(['png', 'pdf', 'docx', 'pptx', 'md', 'txt']); +const SUPPORTED_ATTACHMENT_EXTENSIONS = new Set([ + 'png', + 'jpg', + 'jpeg', + 'gif', + 'webp', + 'pdf', + 'docx', + 'pptx', + 'md', + 'txt', +]); export type AttachmentSource = 'detected' | 'external'; @@ -96,7 +107,7 @@ export function isSupportedAttachmentExtension(extension: string): boolean { export function getAttachmentType(extension: string): AttachmentDetectedType { const normalized = extension.toLowerCase().replace(/^\./, ''); - if (normalized === 'png') return 'image'; + if (['png', 'jpg', 'jpeg', 'gif', 'webp'].includes(normalized)) return 'image'; if (normalized === 'pdf') return 'pdf'; if (normalized === 'pptx') return 'presentation'; if (normalized === 'md') return 'markdown'; diff --git a/src/document-thumbnailer.ts b/src/document-thumbnailer.ts index 8972bebb..9da8d027 100644 --- a/src/document-thumbnailer.ts +++ b/src/document-thumbnailer.ts @@ -13,9 +13,18 @@ import { runWithConversionLimit } from './document-conversion-limiter.js'; const execFileAsync = promisify(execFile); const THUMBNAIL_CONVERSION_TIMEOUT_MS = 5 * 60_000; +/** Browser-renderable image formats served as-is (no conversion). */ +const IMAGE_PASSTHROUGH_CONTENT_TYPES: Record = { + png: 'image/png', + jpg: 'image/jpeg', + jpeg: 'image/jpeg', + gif: 'image/gif', + webp: 'image/webp', +}; + export interface ThumbnailResult { content: Buffer; - contentType: 'image/png'; + contentType: string; } export async function generateFirstPageThumbnail(filePath: string, extension: string): Promise { @@ -24,8 +33,9 @@ export async function generateFirstPageThumbnail(filePath: string, extension: st try { await fs.stat(filePath); - if (ext === 'png') { - return { content: await fs.readFile(filePath), contentType: 'image/png' }; + const passthroughContentType = IMAGE_PASSTHROUGH_CONTENT_TYPES[ext]; + if (passthroughContentType) { + return { content: await fs.readFile(filePath), contentType: passthroughContentType }; } if (ext === 'pdf') { diff --git a/src/generated-artifact-attachments.ts b/src/generated-artifact-attachments.ts new file mode 100644 index 00000000..afe0c52e --- /dev/null +++ b/src/generated-artifact-attachments.ts @@ -0,0 +1,70 @@ +/** + * @fileoverview Codex generated-artifact attachment registration. + * + * Codex image generation prints paths such as `Saved to: file://...`. These + * paths are registered directly when they fall within allowed locations (the + * session workspace or the well-known Codex generated-artifact directories + * anchored at the user's home). The trust decision is made on the + * realpath-RESOLVED path so a symlink staged at an allowed location cannot + * smuggle an arbitrary host file past workspace confinement. + */ + +import { realpathSync } from 'node:fs'; +import { homedir } from 'node:os'; +import { join, normalize, sep } from 'node:path'; +import { registerExternalAttachment, type AttachmentRegistrationResult } from './attachment-registry.js'; + +export interface GeneratedArtifactRegistrationOptions { + sessionId: string; + filePath: string; + sessionWorkingDir: string; +} + +export async function registerGeneratedArtifactAttachment( + options: GeneratedArtifactRegistrationOptions +): Promise { + // Decide trust on the symlink-resolved path. If it can't be resolved, fall + // back to the strict force-confined policy (registration will 404 a missing + // file anyway). + let forceWorkspaceConfinement = true; + try { + const resolvedPath = realpathSync(options.filePath); + forceWorkspaceConfinement = !isAllowedGeneratedArtifactPath(resolvedPath, options.sessionWorkingDir); + } catch { + // Keep force confinement. + } + return registerExternalAttachment(options.sessionId, options.filePath, { + sessionWorkingDir: options.sessionWorkingDir, + forceWorkspaceConfinement, + }); +} + +/** Well-known Codex generated-artifact directories, anchored at the user's home. */ +function codexGeneratedDirs(): string[] { + const home = homedir(); + return [ + join(home, '.codex-personal', 'generated_images'), + join(home, '.codex', 'generated_images'), + join(home, '.codex-personal', 'generated_artifacts'), + join(home, '.codex', 'generated_artifacts'), + ]; +} + +/** + * True when `filePath` (absolute; callers should pass the realpath-resolved + * path) is inside the session workspace or one of the well-known Codex + * generated-artifact directories under the current user's home. The marker + * directories are prefix-anchored to `os.homedir()` — a `.codex/...` subtree + * elsewhere on the filesystem does NOT qualify. + */ +export function isAllowedGeneratedArtifactPath(filePath: string, workingDir: string): boolean { + const normalizedPath = normalize(filePath); + if (isPathInside(normalizedPath, workingDir)) return true; + return codexGeneratedDirs().some((dir) => isPathInside(normalizedPath, dir)); +} + +function isPathInside(filePath: string, rootPath: string): boolean { + const normalizedRoot = normalize(rootPath); + if (filePath === normalizedRoot) return true; + return filePath.startsWith(normalizedRoot.endsWith(sep) ? normalizedRoot : normalizedRoot + sep); +} diff --git a/src/session.ts b/src/session.ts index daeb1667..5532f955 100644 --- a/src/session.ts +++ b/src/session.ts @@ -82,7 +82,7 @@ import { import { SessionAutoOps } from './session-auto-ops.js'; import { detectUsageLimitPause } from './usage-limit-patterns.js'; import { SessionTaskCache } from './session-task-cache.js'; -import { parseAttachmentMagicLinks } from './attachment-magic.js'; +import { parseTerminalAttachmentRequests } from './attachment-magic.js'; import { sanitizeAttachmentHistory, upsertAttachmentHistory as upsertAttachmentHistoryList, @@ -1230,18 +1230,26 @@ export class Session extends EventEmitter { .replace(/\x1b\[\?(?:1000|1001|1002|1003|1005|1006|1007)[hl]/g, ''); } - // Scan terminal output for `codeman://attach?path=...` magic links and emit - // an attachmentRequested event for each newly-seen absolute path. The web - // server turns these into registered attachment cards. - const attachmentPaths = parseAttachmentMagicLinks(data); - for (const attachmentPath of attachmentPaths) { - if (this._attachmentMagicSeen.has(attachmentPath)) continue; - this._attachmentMagicSeen.add(attachmentPath); + // Scan terminal output for attachment requests. `codeman://attach?...` is an + // explicit magic link (all modes); Codex generated images report + // `Saved to: file://...` — that scanner (and its relaxed trust policy) is + // only enabled for codex-mode sessions. The web server applies the trust + // boundary for each request source. + const attachmentRequests = parseTerminalAttachmentRequests(data, { codexArtifacts: this.mode === 'codex' }); + for (const request of attachmentRequests) { + const seenKey = `${request.source}:${request.path}`; + if (this._attachmentMagicSeen.has(seenKey)) continue; + this._attachmentMagicSeen.add(seenKey); if (this._attachmentMagicSeen.size > 200) { const oldest = this._attachmentMagicSeen.values().next().value; if (oldest) this._attachmentMagicSeen.delete(oldest); } - this.emit('attachmentRequested', { sessionId: this.id, path: attachmentPath, timestamp: Date.now() }); + this.emit('attachmentRequested', { + sessionId: this.id, + path: request.path, + source: request.source, + timestamp: Date.now(), + }); } // BufferAccumulator handles auto-trimming when max size exceeded diff --git a/src/web/server.ts b/src/web/server.ts index 428630cc..f3884b57 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -62,6 +62,7 @@ import { import { imageWatcher } from '../image-watcher.js'; import { workflowRunWatcher, summarizeRun } from '../workflow-run-watcher.js'; import { attachmentRegistry, buildFileThumbnailRoute, registerExternalAttachment } from '../attachment-registry.js'; +import { registerGeneratedArtifactAttachment } from '../generated-artifact-attachments.js'; import { buildDetectedAttachmentHistoryItem, buildExternalAttachmentHistoryItem, @@ -1329,7 +1330,8 @@ export class WebServer extends EventEmitter { } }, getStore: () => this.store, - registerAttachment: (id: string, filePath: string) => this.registerAttachment(id, filePath), + registerAttachment: (id: string, filePath: string, source: 'external' | 'codex-generated') => + this.registerAttachment(id, filePath, source), }; } @@ -1342,16 +1344,31 @@ export class WebServer extends EventEmitter { * session workspace — passive magic links can't expose arbitrary host files. * Deliberate cross-workspace attachment goes through the explicit, * Origin-guarded `POST /attachments` route (and `codeman attach`, which POSTs - * directly inside a managed session). Registration also enforces the COD-53 - * blocklist as defense-in-depth. + * directly inside a managed session). Codex-mode `Saved to:` requests + * (`source: 'codex-generated'`) instead go through + * registerGeneratedArtifactAttachment, which stays force-confined unless the + * realpath-resolved target is inside the workspace or a home-anchored + * `~/.codex*` generated-artifact directory. Registration also enforces the + * COD-53 blocklist as defense-in-depth. */ - private async registerAttachment(sessionId: string, filePath: string): Promise { + private async registerAttachment( + sessionId: string, + filePath: string, + source: 'external' | 'codex-generated' + ): Promise { const session = this.sessions.get(sessionId); if (!session) return; - const event = await registerExternalAttachment(sessionId, filePath, { - sessionWorkingDir: session.workingDir, - forceWorkspaceConfinement: true, - }); + const event = + source === 'codex-generated' + ? await registerGeneratedArtifactAttachment({ + sessionId, + filePath, + sessionWorkingDir: session.workingDir, + }) + : await registerExternalAttachment(sessionId, filePath, { + sessionWorkingDir: session.workingDir, + forceWorkspaceConfinement: true, + }); const record = attachmentRegistry.get(sessionId, event.attachmentId); if (record) { session.upsertAttachmentHistory( diff --git a/src/web/session-listener-wiring.ts b/src/web/session-listener-wiring.ts index 5a0f9a4f..ba53626e 100644 --- a/src/web/session-listener-wiring.ts +++ b/src/web/session-listener-wiring.ts @@ -58,7 +58,7 @@ export interface SessionListenerRefs { bashToolStart: (tool: ActiveBashTool) => void; bashToolEnd: (tool: ActiveBashTool) => void; bashToolsUpdate: (tools: ActiveBashTool[]) => void; - attachmentRequested: (event: { path: string }) => void; + attachmentRequested: (event: { path: string; source: 'external' | 'codex-generated' }) => void; } /** Dependencies injected by WebServer — keeps listener creation decoupled from server internals. */ @@ -78,7 +78,7 @@ interface SessionListenerDeps { removeSessionListenerRefs(sessionId: string): void; cleanupRespawnOnExit(sessionId: string): void; getStore(): import('../state-store.js').StateStore; - registerAttachment(sessionId: string, filePath: string): Promise; + registerAttachment(sessionId: string, filePath: string, source: 'external' | 'codex-generated'): Promise; } /** @@ -359,8 +359,8 @@ export function createSessionListeners(session: Session, deps: SessionListenerDe }, /** Registers an explicit attachment card requested by terminal magic text. */ - attachmentRequested: (event: { path: string }) => { - deps.registerAttachment(session.id, event.path).catch((err) => { + attachmentRequested: (event: { path: string; source: 'external' | 'codex-generated' }) => { + deps.registerAttachment(session.id, event.path, event.source).catch((err) => { console.error(`[Attachment] Failed to register ${event.path} for ${session.id}:`, err); }); }, diff --git a/test/attachment-magic.test.ts b/test/attachment-magic.test.ts index 0acbae2d..2f734de6 100644 --- a/test/attachment-magic.test.ts +++ b/test/attachment-magic.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it } from 'vitest'; import { Session } from '../src/session.js'; -import { parseAttachmentMagicLinks } from '../src/attachment-magic.js'; +import { parseAttachmentMagicLinks, parseTerminalAttachmentRequests } from '../src/attachment-magic.js'; +import { isSupportedAttachmentExtension } from '../src/attachment-registry.js'; describe('attachment magic links', () => { it('extracts absolute paths from codeman attach magic URLs', () => { @@ -54,4 +55,79 @@ describe('attachment magic links', () => { expect(requested).toEqual(['/tmp/deck.pptx']); }); + + it('extracts Codex generated image file URLs from saved-to terminal output', () => { + const requests = parseTerminalAttachmentRequests( + 'Saved to: file:///Users/aamer/.codex-personal/generated_images/mockup%20one.png', + { codexArtifacts: true } + ); + + expect(requests).toEqual([ + { + path: '/Users/aamer/.codex-personal/generated_images/mockup one.png', + source: 'codex-generated', + }, + ]); + }); + + it('ignores Codex saved-to output unless the codex scanner is enabled', () => { + const requests = parseTerminalAttachmentRequests( + 'Saved to: file:///Users/aamer/.codex-personal/generated_images/mockup.png' + ); + + expect(requests).toEqual([]); + }); + + it('strips ANSI styling around Codex saved-to lines before capturing the URL', () => { + const requests = parseTerminalAttachmentRequests( + '\x1b[1mSaved to:\x1b[0m file:///Users/aamer/.codex/generated_images/mockup.png\x1b[0m\r\n', + { codexArtifacts: true } + ); + + expect(requests).toEqual([ + { + path: '/Users/aamer/.codex/generated_images/mockup.png', + source: 'codex-generated', + }, + ]); + }); + + it('emits generated artifact requests from Codex saved-to output', () => { + const session = new Session({ id: 'session-generated-artifact-test', workingDir: '/tmp', mode: 'codex' }); + const requested: Array<{ path: string; source?: string }> = []; + session.on('attachmentRequested', (event: { path: string; source?: string }) => requested.push(event)); + + (session as unknown as { _handleTerminalOutput(data: string): void })._handleTerminalOutput( + 'Saved to: file:///Users/aamer/.codex-personal/generated_images/output.png' + ); + + expect(requested).toEqual([ + { + sessionId: 'session-generated-artifact-test', + path: '/Users/aamer/.codex-personal/generated_images/output.png', + source: 'codex-generated', + timestamp: expect.any(Number), + }, + ]); + }); + + it('does not emit codex-generated requests from non-codex session modes', () => { + for (const mode of ['claude', 'shell'] as const) { + const session = new Session({ id: `session-generated-artifact-${mode}`, workingDir: '/tmp', mode }); + const requested: Array<{ path: string }> = []; + session.on('attachmentRequested', (event: { path: string }) => requested.push(event)); + + (session as unknown as { _handleTerminalOutput(data: string): void })._handleTerminalOutput( + 'Saved to: file:///Users/aamer/.codex-personal/generated_images/output.png' + ); + + expect(requested).toEqual([]); + } + }); + + it('supports generated image attachment extensions beyond png', () => { + expect(isSupportedAttachmentExtension('jpg')).toBe(true); + expect(isSupportedAttachmentExtension('jpeg')).toBe(true); + expect(isSupportedAttachmentExtension('webp')).toBe(true); + }); }); diff --git a/test/document-thumbnailer.test.ts b/test/document-thumbnailer.test.ts index 1652afaf..785951e6 100644 --- a/test/document-thumbnailer.test.ts +++ b/test/document-thumbnailer.test.ts @@ -71,6 +71,22 @@ describe('document-thumbnailer', () => { ); }); + it('passes through generated image formats with per-extension content types', async () => { + const expectations: Array<[string, string]> = [ + ['jpg', 'image/jpeg'], + ['jpeg', 'image/jpeg'], + ['gif', 'image/gif'], + ['webp', 'image/webp'], + ['png', 'image/png'], + ]; + + for (const [ext, contentType] of expectations) { + const result = await generateFirstPageThumbnail(`/tmp/mockup.${ext}`, ext); + expect(result).toEqual({ content: Buffer.from('large thumbnail'), contentType }); + } + expect(mockedExecFile).not.toHaveBeenCalled(); + }); + it('renders Office thumbnails from the cached converted PDF after conversion cleanup', async () => { mockedMkdtemp.mockImplementation(async (prefix) => String(prefix).includes('codeman-document-preview-cache') diff --git a/test/generated-artifact-attachments.test.ts b/test/generated-artifact-attachments.test.ts new file mode 100644 index 00000000..938cb806 --- /dev/null +++ b/test/generated-artifact-attachments.test.ts @@ -0,0 +1,78 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import fs from 'node:fs/promises'; +import { homedir, tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { + isAllowedGeneratedArtifactPath, + registerGeneratedArtifactAttachment, +} from '../src/generated-artifact-attachments.js'; +import { attachmentRegistry } from '../src/attachment-registry.js'; + +describe('generated artifact attachments', () => { + it('allows workspace artifacts and home-anchored Codex generated image directories', () => { + const home = homedir(); + expect(isAllowedGeneratedArtifactPath('/repo/out/mockup.png', '/repo')).toBe(true); + expect( + isAllowedGeneratedArtifactPath(join(home, '.codex-personal', 'generated_images', 'mockup.png'), '/repo') + ).toBe(true); + expect(isAllowedGeneratedArtifactPath(join(home, '.codex', 'generated_artifacts', 'report.pdf'), '/repo')).toBe( + true + ); + expect(isAllowedGeneratedArtifactPath('/etc/secret.png', '/repo')).toBe(false); + expect( + isAllowedGeneratedArtifactPath( + join(home, '.codex-personal', 'generated_images', '..', '..', '.ssh', 'id_rsa.png'), + '/repo' + ) + ).toBe(false); + }); + + it('rejects .codex marker directories that are not anchored at the user home', () => { + expect(isAllowedGeneratedArtifactPath('/var/tmp/staging/.codex/generated_images/leak.png', '/repo')).toBe(false); + expect(isAllowedGeneratedArtifactPath('/var/tmp/.codex-personal/generated_artifacts/leak.md', '/repo')).toBe(false); + }); + + describe('symlink resolution', () => { + let workspaceDir: string | undefined; + let outsideDir: string | undefined; + const sessionId = 'generated-artifact-symlink-test'; + + afterEach(async () => { + attachmentRegistry.clearSession(sessionId); + for (const dir of [workspaceDir, outsideDir]) { + if (dir) await fs.rm(dir, { recursive: true, force: true }); + } + workspaceDir = undefined; + outsideDir = undefined; + }); + + it('confines on the resolved path: a workspace symlink to an outside file is rejected', async () => { + // realpath so a symlinked tmpdir (e.g. macOS /var -> /private/var) can't skew containment checks + workspaceDir = await fs.realpath(await fs.mkdtemp(join(tmpdir(), 'codeman-genart-ws-'))); + outsideDir = await fs.realpath(await fs.mkdtemp(join(tmpdir(), 'codeman-genart-out-'))); + const outsideFile = join(outsideDir, 'private-notes.md'); + await fs.writeFile(outsideFile, 'secret'); + const linkPath = join(workspaceDir, 'x.md'); + await fs.symlink(outsideFile, linkPath); + + await expect( + registerGeneratedArtifactAttachment({ sessionId, filePath: linkPath, sessionWorkingDir: workspaceDir }) + ).rejects.toMatchObject({ statusCode: 403 }); + }); + + it('registers a real workspace file', async () => { + workspaceDir = await fs.realpath(await fs.mkdtemp(join(tmpdir(), 'codeman-genart-ws-'))); + const filePath = join(workspaceDir, 'mockup.png'); + await fs.writeFile(filePath, 'png-bytes'); + + const event = await registerGeneratedArtifactAttachment({ + sessionId, + filePath, + sessionWorkingDir: workspaceDir, + }); + + expect(event.fileName).toBe('mockup.png'); + expect(event.attachmentType).toBe('image'); + }); + }); +}); diff --git a/test/session-listener-wiring.test.ts b/test/session-listener-wiring.test.ts new file mode 100644 index 00000000..74ea312c --- /dev/null +++ b/test/session-listener-wiring.test.ts @@ -0,0 +1,23 @@ +import { describe, expect, it, vi } from 'vitest'; +import { Session } from '../src/session.js'; +import { createSessionListeners } from '../src/web/session-listener-wiring.js'; + +describe('session listener wiring', () => { + it('forwards the attachment request source through registerAttachment', async () => { + const session = new Session({ id: 'wiring-attach-source-test', workingDir: '/tmp', mode: 'codex' }); + const registerAttachment = vi.fn(async () => undefined); + const deps = { registerAttachment } as unknown as Parameters[1]; + + const refs = createSessionListeners(session, deps); + refs.attachmentRequested({ path: '/tmp/mockup.png', source: 'codex-generated' }); + refs.attachmentRequested({ path: '/tmp/report.pdf', source: 'external' }); + + expect(registerAttachment).toHaveBeenNthCalledWith( + 1, + 'wiring-attach-source-test', + '/tmp/mockup.png', + 'codex-generated' + ); + expect(registerAttachment).toHaveBeenNthCalledWith(2, 'wiring-attach-source-test', '/tmp/report.pdf', 'external'); + }); +});