Merge PR #150 from aakhter/cod-166-codex-generated-artifact-attachments: Codex generated artifacts as attachment cards

Includes review fixes: source arg threaded through the deps lambda (was silently dropped), codex-mode gating, realpath-first trust decisions with homedir-anchored markers, image thumbnail passthrough, ANSI-stripped scanning.

# Conflicts:
#	src/session.ts

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<string>();
+  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<string>();
 
@@ -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<string>();
+
+  // 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;
 }
 

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';

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<string, string> = {
+  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<ThumbnailResult | null> {
@@ -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') {

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<AttachmentRegistrationResult> {
+  // 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);
+}

src/session.ts

@@ -84,7 +84,7 @@ import { SessionAutoOps } from './session-auto-ops.js';
 import { detectUsageLimitPause } from './usage-limit-patterns.js';
 import { SessionTaskCache } from './session-task-cache.js';
 import { InteractivePtyExitBreaker } from './session-pty-exit-breaker.js';
-import { parseAttachmentMagicLinks } from './attachment-magic.js';
+import { parseTerminalAttachmentRequests } from './attachment-magic.js';
 import {
   sanitizeAttachmentHistory,
   upsertAttachmentHistory as upsertAttachmentHistoryList,
@@ -1257,18 +1257,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

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,
@@ -1365,7 +1366,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),
     };
   }
 
@@ -1378,16 +1380,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<void> {
+  private async registerAttachment(
+    sessionId: string,
+    filePath: string,
+    source: 'external' | 'codex-generated'
+  ): Promise<void> {
     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(

src/web/session-listener-wiring.ts

@@ -59,7 +59,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. */
@@ -79,7 +79,7 @@ interface SessionListenerDeps {
   removeSessionListenerRefs(sessionId: string): void;
   cleanupRespawnOnExit(sessionId: string): void;
   getStore(): import('../state-store.js').StateStore;
-  registerAttachment(sessionId: string, filePath: string): Promise<void>;
+  registerAttachment(sessionId: string, filePath: string, source: 'external' | 'codex-generated'): Promise<void>;
 }
 
 /**
@@ -381,8 +381,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);
       });
     },