fix(agent-route): reject unsafe attachment filenames to prevent path traversal
Filenames in forwardAttachedFiles arrived from the source agent's messages_out content and were used directly in path.join on both source outbox read and target inbox write. A value like `../evil.sh` could escape `inbox/<a2a-id>/` on the target session (and similarly the source outbox on read), breaking session isolation — an adversarial or hallucinating sub-agent could overwrite files in a sibling session. Adds isSafeAttachmentName(name) — exported so it's unit-testable — which rejects empty, `.`, `..`, anything containing `/`, `\`, or NUL, and anything path.basename would strip. Guard runs before any I/O. Unsafe names are dropped with a warning log, same pattern as missing-source-file handling; a bad filename in one attachment doesn't kill the whole route's text delivery. Addresses Codex Review P1 on qwibitai/nanoclaw#1967. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -36,6 +36,26 @@ export interface ForwardedAttachment {
|
||||
localPath: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Is `name` safe to use as the last segment of a path inside the target
|
||||
* agent's inbox directory? Filenames arrive in messages_out content from
|
||||
* the source agent — under a multi-agent setup with heterogenous providers
|
||||
* (or a compromised / hallucinating sub-agent) they can't be trusted.
|
||||
*
|
||||
* Rejects:
|
||||
* - empty string
|
||||
* - `.` / `..` (traversal sentinels that path.basename returns as-is)
|
||||
* - anything containing a path separator (`/` or `\`) or NUL
|
||||
* - any value where `path.basename(name) !== name`, catching OS-specific
|
||||
* separators and covering drives/prefixes on Windows runtimes
|
||||
*/
|
||||
export function isSafeAttachmentName(name: string): boolean {
|
||||
if (typeof name !== 'string' || name.length === 0) return false;
|
||||
if (name === '.' || name === '..') return false;
|
||||
if (/[\\/\0]/.test(name)) return false;
|
||||
return path.basename(name) === name;
|
||||
}
|
||||
|
||||
/**
|
||||
* Copy file attachments from the source agent's outbox into the target
|
||||
* agent's inbox. Returns attachments using the formatter's existing
|
||||
@@ -43,9 +63,9 @@ export interface ForwardedAttachment {
|
||||
* as relative to `/workspace/`, matching how channel-inbound attachments
|
||||
* are surfaced today.
|
||||
*
|
||||
* Missing source files are skipped with a warning rather than failing
|
||||
* the whole route — a bad filename reference shouldn't kill the
|
||||
* accompanying text.
|
||||
* Missing source files and unsafe (path-traversal) filenames are skipped
|
||||
* with a warning rather than failing the whole route — a bad filename
|
||||
* reference shouldn't kill the accompanying text.
|
||||
*/
|
||||
export function forwardAttachedFiles(
|
||||
source: { agentGroupId: string; sessionId: string; messageId: string; filenames: string[] },
|
||||
@@ -67,6 +87,13 @@ export function forwardAttachedFiles(
|
||||
|
||||
const attachments: ForwardedAttachment[] = [];
|
||||
for (const filename of source.filenames) {
|
||||
if (!isSafeAttachmentName(filename)) {
|
||||
log.warn('agent-route: rejecting unsafe attachment filename (path traversal attempt?)', {
|
||||
sourceMsgId: source.messageId,
|
||||
filename,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
const src = path.join(sourceDir, filename);
|
||||
if (!fs.existsSync(src)) {
|
||||
log.warn('agent-route: referenced file missing in source outbox, skipped', {
|
||||
|
||||
Reference in New Issue
Block a user