diff --git a/src/message-runtime-final-delivery.ts b/src/message-runtime-final-delivery.ts index aec805e..f5c15bb 100644 --- a/src/message-runtime-final-delivery.ts +++ b/src/message-runtime-final-delivery.ts @@ -2,10 +2,6 @@ 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 { - getOutboundStageDir, - stageOutboundAttachments, -} from './outbound-attachments.js'; import type { AgentType, Channel, @@ -54,19 +50,6 @@ export async function deliverMessageRuntimeFinalText(args: { return true; } - const attachmentBaseDirs = resolveRuntimeAttachmentBaseDirs(args.group); - // Relocate attachments that live outside the room's allowed directories into - // a safe staging dir before persisting the work item, so the path delivery - // actually uses (and revalidates) is one the validator accepts. Without this, - // agent-generated files written to an arbitrary working path are rejected as - // "outside-allowed-dirs" and silently dropped. - const attachments = args.attachments?.length - ? stageOutboundAttachments(args.attachments, { - baseDirs: attachmentBaseDirs, - stageDir: getOutboundStageDir(args.group.folder), - }) - : args.attachments; - const workItem = createProducedWorkItem({ group_folder: args.group.folder, chat_jid: args.chatJid, @@ -76,14 +59,14 @@ export async function deliverMessageRuntimeFinalText(args: { start_seq: args.startSeq, end_seq: args.endSeq, result_payload: args.text, - attachments, + attachments: args.attachments, }); return deliverOpenWorkItem({ channel: args.channel, item: workItem, log: logger, - attachmentBaseDirs, + attachmentBaseDirs: resolveRuntimeAttachmentBaseDirs(args.group), replaceMessageId: args.replaceMessageId, isDuplicateOfLastBotFinal: args.isDuplicateOfLastBotFinal, openContinuation: args.openContinuation, diff --git a/src/outbound-attachments.test.ts b/src/outbound-attachments.test.ts index bd0c206..4fd6429 100644 --- a/src/outbound-attachments.test.ts +++ b/src/outbound-attachments.test.ts @@ -7,7 +7,6 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { appendRejectionNotice, describeRejectedAttachments, - stageOutboundAttachments, validateOutboundAttachments, } from './outbound-attachments.js'; @@ -19,11 +18,6 @@ const MINIMAL_MP4 = Buffer.from([ 0x00, 0x00, 0x00, 0x18, 0x66, 0x74, 0x79, 0x70, 0x69, 0x73, 0x6f, 0x6d, 0x00, 0x00, 0x02, 0x00, 0x69, 0x73, 0x6f, 0x6d, 0x69, 0x73, 0x6f, 0x32, ]); -const WAV_BYTES = Buffer.concat([ - Buffer.from('RIFF', 'ascii'), - Buffer.from([0x24, 0x00, 0x00, 0x00]), - Buffer.from('WAVE', 'ascii'), -]); const MINIMAL_PDF = Buffer.from('%PDF-1.4\n', 'ascii'); const MINIMAL_ZIP = Buffer.from([0x50, 0x4b, 0x03, 0x04, 0x14, 0x00]); @@ -343,7 +337,10 @@ describe('describeRejectedAttachments', () => { it('summarizes rejected attachments by basename and reason', () => { const note = describeRejectedAttachments([ - { path: '/home/claude/jarvis-tts/sample.wav', reason: 'outside-allowed-dirs' }, + { + path: '/home/claude/jarvis-tts/sample.wav', + reason: 'outside-allowed-dirs', + }, { path: '/tmp/missing.png', reason: 'not-found' }, ]); @@ -381,56 +378,10 @@ describe('appendRejectionNotice', () => { { path: '/tmp/missing.png', reason: 'not-found' }, ]); - expect(body).toBe(describeRejectedAttachments([ - { path: '/tmp/missing.png', reason: 'not-found' }, - ])); - }); -}); - -describe('stageOutboundAttachments', () => { - it('copies attachments outside the allowed dirs into the stage dir and keeps them deliverable', () => { - const sourceDir = makeTempDir(process.cwd(), '.ejclaw-external-src-'); - const stageDir = makeTempDir(os.tmpdir(), 'ejclaw-outbound-stage-'); - const sourcePath = writeFile(sourceDir, 'sample.wav', WAV_BYTES); - - const [staged] = stageOutboundAttachments([{ path: sourcePath }], { - stageDir, - }); - - expect(staged.path.startsWith(stageDir)).toBe(true); - expect(staged.path).not.toBe(sourcePath); - expect(fs.readFileSync(staged.path)).toEqual(WAV_BYTES); - - // The relocated copy is now accepted by the validator (delivery succeeds), - // which is the regression the silent drop used to break. - const validation = validateOutboundAttachments([{ path: staged.path }], { - baseDirs: [stageDir], - }); - expect(validation.rejected).toEqual([]); - expect(validation.files).toHaveLength(1); - }); - - it('leaves attachments already inside an allowed dir untouched', () => { - const allowedDir = makeTempDir(os.tmpdir(), 'ejclaw-attachment-'); - const stageDir = makeTempDir(os.tmpdir(), 'ejclaw-outbound-stage-'); - const sourcePath = writeFile(allowedDir, 'inside.png', ONE_PIXEL_PNG); - - const [staged] = stageOutboundAttachments([{ path: sourcePath }], { - baseDirs: [allowedDir], - stageDir, - }); - - expect(staged.path).toBe(sourcePath); - }); - - it('returns missing or relative paths unchanged so validation can reject them', () => { - const stageDir = makeTempDir(os.tmpdir(), 'ejclaw-outbound-stage-'); - - expect( - stageOutboundAttachments( - [{ path: '/does/not/exist.png' }, { path: 'relative.png' }], - { stageDir }, - ), - ).toEqual([{ path: '/does/not/exist.png' }, { path: 'relative.png' }]); + expect(body).toBe( + describeRejectedAttachments([ + { path: '/tmp/missing.png', reason: 'not-found' }, + ]), + ); }); }); diff --git a/src/outbound-attachments.ts b/src/outbound-attachments.ts index 2de9e9c..e2ec5aa 100644 --- a/src/outbound-attachments.ts +++ b/src/outbound-attachments.ts @@ -1,4 +1,3 @@ -import crypto from 'crypto'; import fs from 'fs'; import os from 'os'; import path from 'path'; @@ -288,76 +287,6 @@ export function appendRejectionNotice( return cleanText ? `${cleanText}\n\n${note}` : note; } -export function isAttachmentWithinAllowedDirs( - realPath: string, - baseDirs?: string[], -): boolean { - const allowed = resolveAllowedBaseDirs(baseDirs); - const tempDir = resolveExistingDir(os.tmpdir()); - return ( - matchesAllowedBaseDir(realPath, allowed) || - isWithinTempDir(realPath, tempDir) - ); -} - -export function getOutboundStageDir(groupFolder: string): string { - const safe = groupFolder.replace(/[^a-zA-Z0-9._-]+/g, '-') || 'group'; - return path.join(DATA_DIR, 'attachments', 'outbound', safe); -} - -/** - * Relocates outbound attachments that live outside the room's allowed - * directories into a safe staging directory inside DATA_DIR, returning the - * rewritten attachment list. This is what makes agent-generated files (e.g. TTS - * audio written to an arbitrary working path) actually deliverable: the staged - * copy lands inside an allowlisted directory, so validateOutboundAttachments - * accepts it while still enforcing every signature/size/type check on the copy. - * - * Attachments already inside an allowed directory are returned untouched so the - * existing isolation checks (including symlink-escape rejection) keep applying. - * Anything that cannot be staged is returned as-is; validation then rejects it - * and appendRejectionNotice surfaces the failure visibly instead of silently. - */ -export function stageOutboundAttachments( - attachments: OutboundAttachment[] | undefined, - options: { baseDirs?: string[]; stageDir: string }, -): OutboundAttachment[] { - if (!attachments || attachments.length === 0) return []; - return attachments.map((attachment) => { - try { - const requested = attachment.path; - if (!path.isAbsolute(requested) || !fs.existsSync(requested)) { - return attachment; - } - const realPath = fs.realpathSync(requested); - if (!fs.statSync(realPath).isFile()) return attachment; - if (isAttachmentWithinAllowedDirs(realPath, options.baseDirs)) { - return attachment; - } - fs.mkdirSync(options.stageDir, { recursive: true }); - const hash = crypto - .createHash('sha1') - .update(realPath) - .digest('hex') - .slice(0, 10); - const targetPath = path.join( - options.stageDir, - `${hash}-${path.basename(realPath)}`, - ); - fs.copyFileSync(realPath, targetPath); - return { - ...attachment, - path: targetPath, - name: attachment.name - ? path.basename(attachment.name) - : path.basename(realPath), - }; - } catch { - return attachment; - } - }); -} - export function validateOutboundAttachments( attachments: OutboundAttachment[] | undefined, options: { baseDirs?: string[] } = {},