From f8aa93969b1d39df1ae04c1567a7093291b89792 Mon Sep 17 00:00:00 2001 From: Saqeb Akhter Date: Wed, 1 Jul 2026 09:57:47 +0000 Subject: [PATCH] fix: COD-152 surface Codex generated artifacts --- src/attachment-magic.ts | 47 ++++++++ src/attachment-registry.ts | 15 ++- src/generated-artifact-attachments.ts | 112 ++++++++++++++++++++ src/session.ts | 24 +++-- src/web/server.ts | 23 +++- src/web/session-listener-wiring.ts | 8 +- test/attachment-magic.test.ts | 41 ++++++- test/generated-artifact-attachments.test.ts | 55 ++++++++++ 8 files changed, 304 insertions(+), 21 deletions(-) create mode 100644 src/generated-artifact-attachments.ts create mode 100644 test/generated-artifact-attachments.test.ts diff --git a/src/attachment-magic.ts b/src/attachment-magic.ts index f4d6d763..e0e1c142 100644 --- a/src/attachment-magic.ts +++ b/src/attachment-magic.ts @@ -3,11 +3,36 @@ */ import { isAbsolute } from 'node:path'; +import { fileURLToPath } from 'node:url'; import { isSupportedAttachmentExtension } from './attachment-registry.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 function parseAttachmentMagicLinks(data: string): string[] { + return parseMagicAttachmentRequests(data).map((request) => request.path); +} + +export function parseTerminalAttachmentRequests(data: string): TerminalAttachmentRequest[] { + const results: TerminalAttachmentRequest[] = []; + const seen = new Set(); + + for (const request of [...parseMagicAttachmentRequests(data), ...parseCodexGeneratedArtifactRequests(data)]) { + 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 +52,28 @@ 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(); + + for (const match of 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/generated-artifact-attachments.ts b/src/generated-artifact-attachments.ts new file mode 100644 index 00000000..9e050902 --- /dev/null +++ b/src/generated-artifact-attachments.ts @@ -0,0 +1,112 @@ +/** + * @fileoverview Codex generated-artifact attachment registration. + * + * Codex image generation prints paths such as `Saved to: file://...`. For local + * sessions these paths can be registered directly when they are safe. For remote + * SSH sessions the path exists on the remote host, so Codeman first copies the + * bytes into its instance data directory and then serves that cached copy through + * the existing attachment registry. + */ + +import { createHash } from 'node:crypto'; +import { execFile } from 'node:child_process'; +import { mkdir } from 'node:fs/promises'; +import { basename, join, posix as posixPath } from 'node:path'; +import { promisify } from 'node:util'; +import fs from 'node:fs/promises'; +import { dataPath } from './config/instance.js'; +import { registerExternalAttachment, type AttachmentRegistrationResult } from './attachment-registry.js'; +import { buildSshConnectionArgv, remoteSshTarget, shellescape } from './remote-hosts.js'; +import type { SessionRemote } from './types/session.js'; + +const execFileAsync = promisify(execFile); + +const MAX_GENERATED_ARTIFACT_BYTES = 50 * 1024 * 1024; +const REMOTE_FETCH_TIMEOUT_MS = 30_000; + +const CODEX_GENERATED_DIR_MARKERS = [ + '/.codex-personal/generated_images/', + '/.codex/generated_images/', + '/.codex-personal/generated_artifacts/', + '/.codex/generated_artifacts/', +]; + +export interface GeneratedArtifactRegistrationOptions { + sessionId: string; + filePath: string; + sessionWorkingDir: string; + remote?: SessionRemote; +} + +export async function registerGeneratedArtifactAttachment( + options: GeneratedArtifactRegistrationOptions +): Promise { + if (options.remote) { + if (!isAllowedGeneratedArtifactPath(options.filePath, options.remote.remotePath)) { + throw new Error('Generated artifact path is outside allowed remote locations'); + } + const localPath = await materializeRemoteGeneratedArtifact(options.sessionId, options.remote, options.filePath); + return registerExternalAttachment(options.sessionId, localPath, { sessionWorkingDir: options.sessionWorkingDir }); + } + + const forceWorkspaceConfinement = !isAllowedGeneratedArtifactPath(options.filePath, options.sessionWorkingDir); + return registerExternalAttachment(options.sessionId, options.filePath, { + sessionWorkingDir: options.sessionWorkingDir, + forceWorkspaceConfinement, + }); +} + +export function isAllowedGeneratedArtifactPath(filePath: string, workingDir: string): boolean { + const normalizedPath = posixPath.normalize(filePath); + if (isPathInside(normalizedPath, workingDir)) return true; + return CODEX_GENERATED_DIR_MARKERS.some((marker) => normalizedPath.includes(marker)); +} + +function isPathInside(filePath: string, rootPath: string): boolean { + const normalizedRoot = ensureTrailingSlash(posixPath.normalize(rootPath)); + const normalizedPath = posixPath.normalize(filePath); + return normalizedPath === normalizedRoot.slice(0, -1) || normalizedPath.startsWith(normalizedRoot); +} + +function ensureTrailingSlash(value: string): string { + return value.endsWith('/') ? value : `${value}/`; +} + +async function materializeRemoteGeneratedArtifact( + sessionId: string, + remote: SessionRemote, + remotePath: string +): Promise { + const fileName = sanitizeCacheFileName(basename(remotePath)); + const digest = createHash('sha256') + .update(`${remote.username}@${remote.host}:${remote.port ?? 22}:${remotePath}`) + .digest('hex') + .slice(0, 16); + const cacheDir = dataPath('generated-artifacts', sessionId); + await mkdir(cacheDir, { recursive: true }); + const localPath = join(cacheDir, `${digest}-${fileName}`); + + const args = buildRemoteGeneratedArtifactFetchArgs(remote, remotePath); + const { stdout } = (await execFileAsync('ssh', args, { + encoding: 'buffer', + timeout: REMOTE_FETCH_TIMEOUT_MS, + maxBuffer: MAX_GENERATED_ARTIFACT_BYTES, + })) as { stdout: Buffer }; + + await fs.writeFile(localPath, stdout); + return localPath; +} + +export function buildRemoteGeneratedArtifactFetchArgs(remote: SessionRemote, remotePath: string): string[] { + return [ + ...buildSshConnectionArgv(remote), + '-o', + 'ConnectTimeout=10', + remoteSshTarget(remote), + `cat -- ${shellescape(remotePath)}`, + ]; +} + +function sanitizeCacheFileName(fileName: string): string { + return fileName.replace(/[^A-Za-z0-9._-]/g, '_') || 'artifact'; +} diff --git a/src/session.ts b/src/session.ts index daeb1667..7ed267c7 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,24 @@ 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; Codex generated images report `Saved to: file://...`. + // The web server applies the trust boundary for each request source. + const attachmentRequests = parseTerminalAttachmentRequests(data); + 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..47d55d62 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, @@ -1345,13 +1346,25 @@ export class WebServer extends EventEmitter { * directly inside a managed session). 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' = 'external' + ): 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, + remote: session.remote, + }) + : 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..0226cafd 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..70713b02 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,42 @@ 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' + ); + + expect(requests).toEqual([ + { + path: '/Users/aamer/.codex-personal/generated_images/mockup one.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('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/generated-artifact-attachments.test.ts b/test/generated-artifact-attachments.test.ts new file mode 100644 index 00000000..353ee538 --- /dev/null +++ b/test/generated-artifact-attachments.test.ts @@ -0,0 +1,55 @@ +import { describe, expect, it } from 'vitest'; +import { + buildRemoteGeneratedArtifactFetchArgs, + isAllowedGeneratedArtifactPath, +} from '../src/generated-artifact-attachments.js'; +import type { SessionRemote } from '../src/types/session.js'; + +describe('generated artifact attachments', () => { + it('allows workspace artifacts and known Codex generated image directories', () => { + expect(isAllowedGeneratedArtifactPath('/repo/out/mockup.png', '/repo')).toBe(true); + expect(isAllowedGeneratedArtifactPath('/Users/aamer/.codex-personal/generated_images/mockup.png', '/repo')).toBe( + true + ); + expect(isAllowedGeneratedArtifactPath('/etc/secret.png', '/repo')).toBe(false); + expect( + isAllowedGeneratedArtifactPath('/Users/aamer/.codex-personal/generated_images/../../.ssh/id_rsa.png', '/repo') + ).toBe(false); + }); + + it('builds remote fetch argv from the session SSH configuration', () => { + const remote: SessionRemote = { + hostId: 'mac-mini', + label: 'mac-mini', + host: '192.168.1.20', + username: 'aamer', + port: 2222, + remotePath: '/Users/aamer/projects/app', + identityFile: '~/.ssh/remote_ed25519', + socksProxy: '127.0.0.1:1080', + jumpHost: 'jump.example.com', + extraSshOptions: ['StrictHostKeyChecking=no'], + }; + + expect( + buildRemoteGeneratedArtifactFetchArgs(remote, "/Users/aamer/.codex-personal/generated_images/a'b.png") + ).toEqual([ + '-o', + 'BatchMode=yes', + '-p', + '2222', + '-i', + expect.stringMatching(/remote_ed25519$/), + '-J', + 'jump.example.com', + '-o', + 'ProxyCommand=nc -X 5 -x 127.0.0.1:1080 %h %p', + '-o', + 'StrictHostKeyChecking=no', + '-o', + 'ConnectTimeout=10', + 'aamer@192.168.1.20', + "cat -- '/Users/aamer/.codex-personal/generated_images/a'\\''b.png'", + ]); + }); +});