claude-bot/EJClaw
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

revert: drop unsafe outbound attachment relocation, keep only the visible failure notice

Browse Source 
The staging logic in 9c46cf6 copied any agent-declared file from outside the
room's allowed directories into a safe folder and attached it, which bypassed
the attachment directory allowlist (cross-room isolation / sensitive-file
protection). Removing it.

Kept: appendRejectionNotice / describeRejectedAttachments so rejected
attachments are surfaced in the visible message instead of being silently
dropped. This changes no security behavior — it only adds text when an
attachment was already going to be rejected.

Verified: outbound-attachments + final-delivery + discord tests 70/70, tsc clean.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
This commit is contained in:
Codex
2026-06-12 00:52:42 +09:00
parent 9c46cf6761
commit 73fd71b39a
3 changed files with 11 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
src/message-runtime-final-delivery.ts
View File

@@ -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,

65
src/outbound-attachments.test.ts
View File

@@ -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([
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' }]);
]),
);
});
});

71
src/outbound-attachments.ts
View File

@@ -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[] } = {},