From 978ca57343ebd8f18882b5a5e2d15fcb5c576bd9 Mon Sep 17 00:00:00 2001 From: Saqeb Akhter Date: Wed, 1 Jul 2026 10:40:09 +0000 Subject: [PATCH] fix: COD-152 preserve generated artifact filenames --- src/generated-artifact-attachments.ts | 71 ++------------------- src/web/server.ts | 1 - test/generated-artifact-attachments.test.ts | 42 +----------- 3 files changed, 5 insertions(+), 109 deletions(-) diff --git a/src/generated-artifact-attachments.ts b/src/generated-artifact-attachments.ts index 9e050902..3c8d2dde 100644 --- a/src/generated-artifact-attachments.ts +++ b/src/generated-artifact-attachments.ts @@ -1,28 +1,13 @@ /** * @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. + * Codex image generation prints paths such as `Saved to: file://...`. These + * paths are registered directly when they fall within allowed locations (workspace + * or well-known Codex generated-image directories). */ -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 { posix as posixPath } from 'node:path'; 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/', @@ -35,20 +20,11 @@ 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, @@ -71,42 +47,3 @@ function isPathInside(filePath: string, rootPath: string): boolean { 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/web/server.ts b/src/web/server.ts index 47d55d62..feec0d05 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -1359,7 +1359,6 @@ export class WebServer extends EventEmitter { sessionId, filePath, sessionWorkingDir: session.workingDir, - remote: session.remote, }) : await registerExternalAttachment(sessionId, filePath, { sessionWorkingDir: session.workingDir, diff --git a/test/generated-artifact-attachments.test.ts b/test/generated-artifact-attachments.test.ts index 353ee538..6b7b7070 100644 --- a/test/generated-artifact-attachments.test.ts +++ b/test/generated-artifact-attachments.test.ts @@ -1,9 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { - buildRemoteGeneratedArtifactFetchArgs, - isAllowedGeneratedArtifactPath, -} from '../src/generated-artifact-attachments.js'; -import type { SessionRemote } from '../src/types/session.js'; +import { isAllowedGeneratedArtifactPath } from '../src/generated-artifact-attachments.js'; describe('generated artifact attachments', () => { it('allows workspace artifacts and known Codex generated image directories', () => { @@ -16,40 +12,4 @@ describe('generated artifact attachments', () => { 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'", - ]); - }); });