fix: allow runtime image attachment paths
This commit is contained in:
21
src/attachment-base-dirs.ts
Normal file
21
src/attachment-base-dirs.ts
Normal 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;
|
||||||
|
}
|
||||||
@@ -94,7 +94,10 @@ describe('deliverIpcOutboundMessage', () => {
|
|||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
channel,
|
channel,
|
||||||
item: createdItem,
|
item: createdItem,
|
||||||
attachmentBaseDirs: ['/repo'],
|
attachmentBaseDirs: expect.arrayContaining([
|
||||||
|
'/repo',
|
||||||
|
expect.stringMatching(/data\/workspaces\/room-folder$/),
|
||||||
|
]),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
expect(channel.sendMessage).not.toHaveBeenCalled();
|
expect(channel.sendMessage).not.toHaveBeenCalled();
|
||||||
@@ -144,7 +147,10 @@ describe('deliverIpcOutboundMessage', () => {
|
|||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
channel: reviewerChannel,
|
channel: reviewerChannel,
|
||||||
item: createdItem,
|
item: createdItem,
|
||||||
attachmentBaseDirs: ['/repo'],
|
attachmentBaseDirs: expect.arrayContaining([
|
||||||
|
'/repo',
|
||||||
|
expect.stringMatching(/data\/workspaces\/room-folder$/),
|
||||||
|
]),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
expect(noteDirectTerminalDelivery).toHaveBeenCalledWith(
|
expect(noteDirectTerminalDelivery).toHaveBeenCalledWith(
|
||||||
|
|||||||
@@ -1,5 +1,6 @@
|
|||||||
import { createProducedWorkItem } from './db.js';
|
import { createProducedWorkItem } from './db.js';
|
||||||
import type { WorkItem } from './db/work-items.js';
|
import type { WorkItem } from './db/work-items.js';
|
||||||
|
import { resolveRuntimeAttachmentBaseDirs } from './attachment-base-dirs.js';
|
||||||
import { deliverOpenWorkItem } from './message-runtime-delivery.js';
|
import { deliverOpenWorkItem } from './message-runtime-delivery.js';
|
||||||
import { parseVisibleVerdict } from './paired-verdict.js';
|
import { parseVisibleVerdict } from './paired-verdict.js';
|
||||||
import { resolveChannelForDeliveryRole } from './router.js';
|
import { resolveChannelForDeliveryRole } from './router.js';
|
||||||
@@ -129,7 +130,7 @@ export async function deliverCanonicalOutboundMessage(
|
|||||||
channel: route.channel,
|
channel: route.channel,
|
||||||
item: workItem as WorkItem,
|
item: workItem as WorkItem,
|
||||||
log,
|
log,
|
||||||
attachmentBaseDirs: group.workDir ? [group.workDir] : undefined,
|
attachmentBaseDirs: resolveRuntimeAttachmentBaseDirs(group),
|
||||||
isDuplicateOfLastBotFinal: () => false,
|
isDuplicateOfLastBotFinal: () => false,
|
||||||
openContinuation: () => {},
|
openContinuation: () => {},
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -149,7 +149,10 @@ describe('message-runtime-final-delivery', () => {
|
|||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
channel,
|
channel,
|
||||||
log: logger,
|
log: logger,
|
||||||
attachmentBaseDirs: ['/work/room'],
|
attachmentBaseDirs: expect.arrayContaining([
|
||||||
|
'/work/room',
|
||||||
|
expect.stringMatching(/data\/workspaces\/room-folder$/),
|
||||||
|
]),
|
||||||
replaceMessageId: undefined,
|
replaceMessageId: undefined,
|
||||||
isDuplicateOfLastBotFinal,
|
isDuplicateOfLastBotFinal,
|
||||||
openContinuation,
|
openContinuation,
|
||||||
@@ -189,7 +192,9 @@ describe('message-runtime-final-delivery', () => {
|
|||||||
expect(deliverOpenWorkItem).toHaveBeenCalledWith(
|
expect(deliverOpenWorkItem).toHaveBeenCalledWith(
|
||||||
expect.objectContaining({
|
expect.objectContaining({
|
||||||
replaceMessageId: 'discord-message-1',
|
replaceMessageId: 'discord-message-1',
|
||||||
attachmentBaseDirs: undefined,
|
attachmentBaseDirs: expect.arrayContaining([
|
||||||
|
expect.stringMatching(/data\/workspaces\/room-folder$/),
|
||||||
|
]),
|
||||||
}),
|
}),
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,4 +1,5 @@
|
|||||||
import { createProducedWorkItem } from './db.js';
|
import { createProducedWorkItem } from './db.js';
|
||||||
|
import { resolveRuntimeAttachmentBaseDirs } from './attachment-base-dirs.js';
|
||||||
import { logger } from './logger.js';
|
import { logger } from './logger.js';
|
||||||
import { deliverOpenWorkItem } from './message-runtime-delivery.js';
|
import { deliverOpenWorkItem } from './message-runtime-delivery.js';
|
||||||
import type {
|
import type {
|
||||||
@@ -65,7 +66,7 @@ export async function deliverMessageRuntimeFinalText(args: {
|
|||||||
channel: args.channel,
|
channel: args.channel,
|
||||||
item: workItem,
|
item: workItem,
|
||||||
log: logger,
|
log: logger,
|
||||||
attachmentBaseDirs: args.group.workDir ? [args.group.workDir] : undefined,
|
attachmentBaseDirs: resolveRuntimeAttachmentBaseDirs(args.group),
|
||||||
replaceMessageId: args.replaceMessageId,
|
replaceMessageId: args.replaceMessageId,
|
||||||
isDuplicateOfLastBotFinal: args.isDuplicateOfLastBotFinal,
|
isDuplicateOfLastBotFinal: args.isDuplicateOfLastBotFinal,
|
||||||
openContinuation: args.openContinuation,
|
openContinuation: args.openContinuation,
|
||||||
|
|||||||
@@ -9,6 +9,7 @@ import {
|
|||||||
isSessionCommandSenderAllowed,
|
isSessionCommandSenderAllowed,
|
||||||
SERVICE_SESSION_SCOPE,
|
SERVICE_SESSION_SCOPE,
|
||||||
} from './config.js';
|
} from './config.js';
|
||||||
|
import { resolveRuntimeAttachmentBaseDirs } from './attachment-base-dirs.js';
|
||||||
import { GroupQueue, GroupRunContext } from './group-queue.js';
|
import { GroupQueue, GroupRunContext } from './group-queue.js';
|
||||||
import { findChannel, formatMessages } from './router.js';
|
import { findChannel, formatMessages } from './router.js';
|
||||||
import { enqueueGenericFollowUpAfterDeliveryRetry as enqueueDeliveryRetryFollowUp } from './message-runtime-dispatch.js';
|
import { enqueueGenericFollowUpAfterDeliveryRetry as enqueueDeliveryRetryFollowUp } from './message-runtime-dispatch.js';
|
||||||
@@ -311,7 +312,7 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
|||||||
channel,
|
channel,
|
||||||
roleToChannel,
|
roleToChannel,
|
||||||
log,
|
log,
|
||||||
attachmentBaseDirs: group.workDir ? [group.workDir] : undefined,
|
attachmentBaseDirs: resolveRuntimeAttachmentBaseDirs(group),
|
||||||
isPairedRoom: hasReviewerLease(chatJid),
|
isPairedRoom: hasReviewerLease(chatJid),
|
||||||
getMissingRoleChannelMessage,
|
getMissingRoleChannelMessage,
|
||||||
isDuplicateOfLastBotFinal: checkDuplicateOfLastBotFinal,
|
isDuplicateOfLastBotFinal: checkDuplicateOfLastBotFinal,
|
||||||
|
|||||||
@@ -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', () => {
|
it('requires workspace paths to be explicitly allowlisted', () => {
|
||||||
const dir = makeTempDir(process.cwd(), '.ejclaw-attachment-');
|
const dir = makeTempDir(process.cwd(), '.ejclaw-attachment-');
|
||||||
const imagePath = writeFile(dir, 'workspace-shot.png', ONE_PIXEL_PNG);
|
const imagePath = writeFile(dir, 'workspace-shot.png', ONE_PIXEL_PNG);
|
||||||
|
|||||||
@@ -17,11 +17,6 @@ export interface ValidateOutboundAttachmentsResult {
|
|||||||
|
|
||||||
const MAX_ATTACHMENT_BYTES = 8 * 1024 * 1024;
|
const MAX_ATTACHMENT_BYTES = 8 * 1024 * 1024;
|
||||||
const IMAGE_EXTS = /\.(png|jpe?g|gif|webp|bmp)$/i;
|
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[] {
|
function unique(values: Array<string | null | undefined>): string[] {
|
||||||
return [
|
return [
|
||||||
...new Set(values.filter((value): value is string => Boolean(value))),
|
...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));
|
return baseDirs.some((baseDir) => isWithinBaseDir(realPath, baseDir));
|
||||||
}
|
}
|
||||||
|
|
||||||
function isWithinDefaultTempAttachmentDir(
|
function isWithinTempDir(realPath: string, tempDir: string | null): boolean {
|
||||||
realPath: string,
|
|
||||||
tempDir: string | null,
|
|
||||||
): boolean {
|
|
||||||
if (!tempDir) return false;
|
if (!tempDir) return false;
|
||||||
const relative = path.relative(tempDir, realPath);
|
const relative = path.relative(tempDir, realPath);
|
||||||
if (!relative || relative.startsWith('..') || path.isAbsolute(relative)) {
|
if (!relative || relative.startsWith('..') || path.isAbsolute(relative)) {
|
||||||
return false;
|
return false;
|
||||||
}
|
}
|
||||||
const [firstSegment, ...rest] = relative.split(path.sep);
|
return true;
|
||||||
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);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function detectImageMime(filePath: string): string | null {
|
function detectImageMime(filePath: string): string | null {
|
||||||
@@ -186,8 +157,7 @@ export function validateOutboundAttachments(
|
|||||||
}
|
}
|
||||||
if (
|
if (
|
||||||
!matchesAllowedBaseDir(realPath, baseDirs) &&
|
!matchesAllowedBaseDir(realPath, baseDirs) &&
|
||||||
!isWithinDefaultTempAttachmentDir(realPath, defaultTempDir) &&
|
!isWithinTempDir(realPath, defaultTempDir)
|
||||||
!isDirectTempAttachmentFile(realPath, defaultTempDir)
|
|
||||||
) {
|
) {
|
||||||
rejected.push({ path: requestedPath, reason: 'outside-allowed-dirs' });
|
rejected.push({ path: requestedPath, reason: 'outside-allowed-dirs' });
|
||||||
continue;
|
continue;
|
||||||
|
|||||||
Reference in New Issue
Block a user