Fix runtime image attachment paths

Allow nested /tmp generated images and per-room runtime workspace attachments after realpath/image validation.
This commit is contained in:
Eyejoker
2026-05-02 22:02:13 +09:00
committed by GitHub
parent 6ea4ec5b68
commit 4859c58f48
8 changed files with 66 additions and 40 deletions

View File

@@ -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 | null | undefined>): 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;
}

View File

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

View File

@@ -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: () => {},
});

View File

@@ -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$/),
]),
}),
);
});

View File

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

View File

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

View File

@@ -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);

View File

@@ -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 | null | undefined>): 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;