From 6356b7683662c5c93debea59d5bd2a280bcfa1ae Mon Sep 17 00:00:00 2001 From: ejclaw Date: Sat, 2 May 2026 22:00:07 +0900 Subject: [PATCH] fix: allow runtime image attachment paths --- src/attachment-base-dirs.ts | 21 +++++++++++++ src/ipc-outbound-delivery.test.ts | 10 ++++-- src/ipc-outbound-delivery.ts | 3 +- src/message-runtime-final-delivery.test.ts | 9 ++++-- src/message-runtime-final-delivery.ts | 3 +- src/message-runtime.ts | 3 +- src/outbound-attachments.test.ts | 21 +++++++++++++ src/outbound-attachments.ts | 36 ++-------------------- 8 files changed, 66 insertions(+), 40 deletions(-) create mode 100644 src/attachment-base-dirs.ts diff --git a/src/attachment-base-dirs.ts b/src/attachment-base-dirs.ts new file mode 100644 index 0000000..8f49b98 --- /dev/null +++ b/src/attachment-base-dirs.ts @@ -0,0 +1,21 @@ +import path from 'path'; + +import { DATA_DIR } from './config.js'; +import { isValidGroupFolder } from './group-folder.js'; +import type { RegisteredGroup } from './types.js'; + +function unique(values: Array): string[] { + return [ + ...new Set(values.filter((value): value is string => Boolean(value))), + ]; +} + +export function resolveRuntimeAttachmentBaseDirs( + group: RegisteredGroup, +): string[] | undefined { + const workspaceRoot = isValidGroupFolder(group.folder) + ? path.resolve(DATA_DIR, 'workspaces', group.folder) + : null; + const dirs = unique([group.workDir, workspaceRoot]); + return dirs.length > 0 ? dirs : undefined; +} diff --git a/src/ipc-outbound-delivery.test.ts b/src/ipc-outbound-delivery.test.ts index baad6bd..aa20f3d 100644 --- a/src/ipc-outbound-delivery.test.ts +++ b/src/ipc-outbound-delivery.test.ts @@ -94,7 +94,10 @@ describe('deliverIpcOutboundMessage', () => { expect.objectContaining({ channel, item: createdItem, - attachmentBaseDirs: ['/repo'], + attachmentBaseDirs: expect.arrayContaining([ + '/repo', + expect.stringMatching(/data\/workspaces\/room-folder$/), + ]), }), ); expect(channel.sendMessage).not.toHaveBeenCalled(); @@ -144,7 +147,10 @@ describe('deliverIpcOutboundMessage', () => { expect.objectContaining({ channel: reviewerChannel, item: createdItem, - attachmentBaseDirs: ['/repo'], + attachmentBaseDirs: expect.arrayContaining([ + '/repo', + expect.stringMatching(/data\/workspaces\/room-folder$/), + ]), }), ); expect(noteDirectTerminalDelivery).toHaveBeenCalledWith( diff --git a/src/ipc-outbound-delivery.ts b/src/ipc-outbound-delivery.ts index 328d8bc..abb75fe 100644 --- a/src/ipc-outbound-delivery.ts +++ b/src/ipc-outbound-delivery.ts @@ -1,5 +1,6 @@ import { createProducedWorkItem } from './db.js'; import type { WorkItem } from './db/work-items.js'; +import { resolveRuntimeAttachmentBaseDirs } from './attachment-base-dirs.js'; import { deliverOpenWorkItem } from './message-runtime-delivery.js'; import { parseVisibleVerdict } from './paired-verdict.js'; import { resolveChannelForDeliveryRole } from './router.js'; @@ -129,7 +130,7 @@ export async function deliverCanonicalOutboundMessage( channel: route.channel, item: workItem as WorkItem, log, - attachmentBaseDirs: group.workDir ? [group.workDir] : undefined, + attachmentBaseDirs: resolveRuntimeAttachmentBaseDirs(group), isDuplicateOfLastBotFinal: () => false, openContinuation: () => {}, }); diff --git a/src/message-runtime-final-delivery.test.ts b/src/message-runtime-final-delivery.test.ts index e12718a..0934f16 100644 --- a/src/message-runtime-final-delivery.test.ts +++ b/src/message-runtime-final-delivery.test.ts @@ -149,7 +149,10 @@ describe('message-runtime-final-delivery', () => { expect.objectContaining({ channel, log: logger, - attachmentBaseDirs: ['/work/room'], + attachmentBaseDirs: expect.arrayContaining([ + '/work/room', + expect.stringMatching(/data\/workspaces\/room-folder$/), + ]), replaceMessageId: undefined, isDuplicateOfLastBotFinal, openContinuation, @@ -189,7 +192,9 @@ describe('message-runtime-final-delivery', () => { expect(deliverOpenWorkItem).toHaveBeenCalledWith( expect.objectContaining({ replaceMessageId: 'discord-message-1', - attachmentBaseDirs: undefined, + attachmentBaseDirs: expect.arrayContaining([ + expect.stringMatching(/data\/workspaces\/room-folder$/), + ]), }), ); }); diff --git a/src/message-runtime-final-delivery.ts b/src/message-runtime-final-delivery.ts index 977044d..f5c15bb 100644 --- a/src/message-runtime-final-delivery.ts +++ b/src/message-runtime-final-delivery.ts @@ -1,4 +1,5 @@ import { createProducedWorkItem } from './db.js'; +import { resolveRuntimeAttachmentBaseDirs } from './attachment-base-dirs.js'; import { logger } from './logger.js'; import { deliverOpenWorkItem } from './message-runtime-delivery.js'; import type { @@ -65,7 +66,7 @@ export async function deliverMessageRuntimeFinalText(args: { channel: args.channel, item: workItem, log: logger, - attachmentBaseDirs: args.group.workDir ? [args.group.workDir] : undefined, + attachmentBaseDirs: resolveRuntimeAttachmentBaseDirs(args.group), replaceMessageId: args.replaceMessageId, isDuplicateOfLastBotFinal: args.isDuplicateOfLastBotFinal, openContinuation: args.openContinuation, diff --git a/src/message-runtime.ts b/src/message-runtime.ts index edcb830..9245310 100644 --- a/src/message-runtime.ts +++ b/src/message-runtime.ts @@ -9,6 +9,7 @@ import { isSessionCommandSenderAllowed, SERVICE_SESSION_SCOPE, } from './config.js'; +import { resolveRuntimeAttachmentBaseDirs } from './attachment-base-dirs.js'; import { GroupQueue, GroupRunContext } from './group-queue.js'; import { findChannel, formatMessages } from './router.js'; import { enqueueGenericFollowUpAfterDeliveryRetry as enqueueDeliveryRetryFollowUp } from './message-runtime-dispatch.js'; @@ -311,7 +312,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): { channel, roleToChannel, log, - attachmentBaseDirs: group.workDir ? [group.workDir] : undefined, + attachmentBaseDirs: resolveRuntimeAttachmentBaseDirs(group), isPairedRoom: hasReviewerLease(chatJid), getMissingRoleChannelMessage, isDuplicateOfLastBotFinal: checkDuplicateOfLastBotFinal, diff --git a/src/outbound-attachments.test.ts b/src/outbound-attachments.test.ts index a7f97fe..6911bb9 100644 --- a/src/outbound-attachments.test.ts +++ b/src/outbound-attachments.test.ts @@ -111,6 +111,27 @@ describe('validateOutboundAttachments', () => { ]); }); + it('accepts generated image files in nested temp directories', () => { + const dir = makeTempDir(os.tmpdir(), 'paladin-character-'); + const imagePath = writeFile(dir, 'sheet_12x.png', ONE_PIXEL_PNG); + + const result = validateOutboundAttachments([ + { + path: imagePath, + name: 'sheet_12x.png', + mime: 'image/png', + }, + ]); + + expect(result.rejected).toEqual([]); + expect(result.files).toEqual([ + { + attachment: fs.realpathSync(imagePath), + name: 'sheet_12x.png', + }, + ]); + }); + it('requires workspace paths to be explicitly allowlisted', () => { const dir = makeTempDir(process.cwd(), '.ejclaw-attachment-'); const imagePath = writeFile(dir, 'workspace-shot.png', ONE_PIXEL_PNG); diff --git a/src/outbound-attachments.ts b/src/outbound-attachments.ts index 96a05ab..2787f67 100644 --- a/src/outbound-attachments.ts +++ b/src/outbound-attachments.ts @@ -17,11 +17,6 @@ export interface ValidateOutboundAttachmentsResult { const MAX_ATTACHMENT_BYTES = 8 * 1024 * 1024; const IMAGE_EXTS = /\.(png|jpe?g|gif|webp|bmp)$/i; -const DEFAULT_TEMP_ATTACHMENT_DIR_PREFIXES = [ - 'ejclaw-attachment-', - 'ejclaw-discord-image-', -] as const; - function unique(values: Array): string[] { return [ ...new Set(values.filter((value): value is string => Boolean(value))), @@ -65,37 +60,13 @@ function matchesAllowedBaseDir(realPath: string, baseDirs: string[]): boolean { return baseDirs.some((baseDir) => isWithinBaseDir(realPath, baseDir)); } -function isWithinDefaultTempAttachmentDir( - realPath: string, - tempDir: string | null, -): boolean { +function isWithinTempDir(realPath: string, tempDir: string | null): boolean { if (!tempDir) return false; const relative = path.relative(tempDir, realPath); if (!relative || relative.startsWith('..') || path.isAbsolute(relative)) { return false; } - const [firstSegment, ...rest] = relative.split(path.sep); - return ( - rest.length > 0 && - DEFAULT_TEMP_ATTACHMENT_DIR_PREFIXES.some((prefix) => - firstSegment.startsWith(prefix), - ) - ); -} - -function isDirectTempAttachmentFile( - realPath: string, - tempDir: string | null, -): boolean { - if (!tempDir) return false; - const relative = path.relative(tempDir, realPath); - if (!relative || relative.startsWith('..') || path.isAbsolute(relative)) { - return false; - } - // Agents commonly write Playwright screenshots and generated images directly - // under /tmp with task-specific names. Keep this limited to direct children; - // nested temp directories still need an EJClaw prefix or explicit baseDir. - return !relative.includes(path.sep); + return true; } function detectImageMime(filePath: string): string | null { @@ -186,8 +157,7 @@ export function validateOutboundAttachments( } if ( !matchesAllowedBaseDir(realPath, baseDirs) && - !isWithinDefaultTempAttachmentDir(realPath, defaultTempDir) && - !isDirectTempAttachmentFile(realPath, defaultTempDir) + !isWithinTempDir(realPath, defaultTempDir) ) { rejected.push({ path: requestedPath, reason: 'outside-allowed-dirs' }); continue;