third/nanoclaw
1
0
Fork 0
mirror of https://github.com/qwibitai/nanoclaw.git synced 2026-06-04 10:14:47 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(session-manager): apply outbox path-confinement to inbound attachments

Browse Source 
Mirrors the four defenses on the outbound side onto extractAttachmentFiles:

  1. Reject unsafe messageId via isSafeAttachmentName before any inbox path
     is built. WhatsApp passes msg.key.id through raw and that field is
     client generated, so a peer can craft it; future end to end encrypted
     adapters will have the same property.
  2. lstatSync on the inbox dir refuses a pre placed symlink before
     mkdirSync would silently follow it.
  3. realpathSync + isPathInside contains the resolved dir under the
     session inbox root.
  4. writeFileSync uses the wx flag so a pre placed symlink at the file
     path is refused atomically by the kernel; EEXIST surfaces as a
     logged skip.

Threat: the session dir is mounted writable into the container at
/workspace, so a compromised agent can pre place inbox/<future msgId>/
as a symlink and wait for a chat message with a matching id to redirect
the host write. The four guards together close that window.

Consolidates with the existing isSafeAttachmentName helper from
attachment-safety.ts rather than introducing a duplicate basename
validator inside session-manager.

Co-Authored-By: Daisuke Tsuji <dim0627@gmail.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
gavrielc
2026-05-01 01:26:18 +03:00
parent 852009dcb1
commit fc3c11b6b9
2 changed files with 169 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/host-core.test.ts
+89
View File
@@ -162,6 +162,95 @@ describe('session manager', () => {
expect(fs.existsSync(msgOutbox)).toBe(false);
});
it('should reject inbound attachment writes through a pre-placed symlinked inbox dir', () => {
initSessionFolder('ag-1', 'sess-test');
const { session } = resolveSession('ag-1', 'mg-1', null, 'shared');
// The container has /workspace write access, so it can pre create
// inbox/<msgId> as a symlink to escape.
const inboxRoot = path.join(sessionDir('ag-1', session.id), 'inbox');
fs.mkdirSync(inboxRoot, { recursive: true });
const evilTarget = path.join(TEST_DIR, 'evil-target');
fs.mkdirSync(evilTarget, { recursive: true });
fs.symlinkSync(evilTarget, path.join(inboxRoot, 'msg-evil'));
writeSessionMessage('ag-1', session.id, {
id: 'msg-evil',
kind: 'chat',
timestamp: now(),
content: JSON.stringify({
text: 'evil',
attachments: [{ name: 'photo.png', data: Buffer.from('PNGBYTES').toString('base64'), size: 8 }],
}),
});
expect(fs.existsSync(path.join(evilTarget, 'photo.png'))).toBe(false);
});
it('should refuse to follow a pre-existing symlink at the inbound attachment path', () => {
initSessionFolder('ag-1', 'sess-test');
const { session } = resolveSession('ag-1', 'mg-1', null, 'shared');
// The container pre creates inbox/<msgId>/photo.png as a symlink to a
// host file. Without the wx flag, writeFileSync would follow it.
const inboxDir = path.join(sessionDir('ag-1', session.id), 'inbox', 'msg-sym');
fs.mkdirSync(inboxDir, { recursive: true });
const outside = path.join(TEST_DIR, 'outside.txt');
fs.writeFileSync(outside, 'ORIGINAL');
fs.symlinkSync(outside, path.join(inboxDir, 'photo.png'));
writeSessionMessage('ag-1', session.id, {
id: 'msg-sym',
kind: 'chat',
timestamp: now(),
content: JSON.stringify({
text: 'sym',
attachments: [{ name: 'photo.png', data: Buffer.from('PNGBYTES').toString('base64'), size: 8 }],
}),
});
expect(fs.readFileSync(outside, 'utf-8')).toBe('ORIGINAL');
});
it('should reject inbound attachments when messageId is unsafe', () => {
initSessionFolder('ag-1', 'sess-test');
const { session } = resolveSession('ag-1', 'mg-1', null, 'shared');
writeSessionMessage('ag-1', session.id, {
id: '../../escape',
kind: 'chat',
timestamp: now(),
content: JSON.stringify({
text: 'msgid',
attachments: [{ name: 'photo.png', data: Buffer.from('PNGBYTES').toString('base64'), size: 8 }],
}),
});
const inboxRoot = path.join(sessionDir('ag-1', session.id), 'inbox');
if (fs.existsSync(inboxRoot)) {
expect(fs.readdirSync(inboxRoot)).toEqual([]);
}
});
it('should still save inbound attachments with safe basenames', () => {
initSessionFolder('ag-1', 'sess-test');
const { session } = resolveSession('ag-1', 'mg-1', null, 'shared');
writeSessionMessage('ag-1', session.id, {
id: 'msg-ok',
kind: 'chat',
timestamp: now(),
content: JSON.stringify({
text: 'ok',
attachments: [{ name: 'photo.png', data: Buffer.from('PNGBYTES').toString('base64'), size: 8 }],
}),
});
const expected = path.join(sessionDir('ag-1', session.id), 'inbox', 'msg-ok', 'photo.png');
expect(fs.existsSync(expected)).toBe(true);
expect(fs.readFileSync(expected, 'utf-8')).toBe('PNGBYTES');
});
it('should resolve to existing session (shared mode)', () => {
const { session: s1, created: c1 } = resolveSession('ag-1', 'mg-1', null, 'shared');
expect(c1).toBe(true);
src/session-manager.ts
+80 -25
View File
@@ -238,6 +238,20 @@ export function writeSessionMessage(
/**
* If message content has attachments with base64 `data`, save them to
* the session's inbox directory and replace with `localPath`.
*
* Both `messageId` and `att.name` originate in untrusted input. WhatsApp
* passes `msg.key.id` through raw (and that field is client generated, so a
* peer can craft it), and other adapters may follow. The session dir is
* mounted writable into the container, so a compromised agent can also
* pre-place a symlink at `inbox/<future msgId>/` and wait for a chat message
* with a matching id to redirect the host's write.
*
* Defenses, mirrored from the outbound side:
* 1. basename check on `messageId` and `filename`.
* 2. lstat of the inbox dir to refuse pre-placed symlinks.
* 3. realpath-based containment under the session inbox root.
* 4. `wx` flag on writeFileSync to refuse following a pre-existing symlink
* at the target file path or overwriting any existing file.
*/
function extractAttachmentFiles(
agentGroupId: string,
@@ -255,34 +269,75 @@ function extractAttachmentFiles(
const attachments = parsed.attachments as Array<Record<string, unknown>> | undefined;
if (!Array.isArray(attachments)) return contentStr;
if (!isSafeAttachmentName(messageId)) {
log.warn('Rejecting unsafe inbound message id', { messageId });
return contentStr;
}
let changed = false;
for (const att of attachments) {
if (typeof att.data === 'string') {
// The name field is attacker-controlled: chat platforms with E2E
// attachment encryption (WhatsApp, Matrix) cannot sanitize filename
// server-side, and other adapters pass att.name through raw. Without
// this guard, `path.join(inboxDir, '../../...')` writes anywhere the
// host process has fs permission — see Signal Desktop's Nov 2025
// attachment-fileName advisory for the same archetype.
const rawName = deriveAttachmentName(att);
const filename = isSafeAttachmentName(rawName) ? rawName : `attachment-${Date.now()}`;
if (filename !== rawName) {
log.warn('Refused unsafe attachment filename — would escape inbox', {
messageId,
rawName,
replacement: filename,
});
}
const inboxDir = path.join(sessionDir(agentGroupId, sessionId), 'inbox', messageId);
fs.mkdirSync(inboxDir, { recursive: true });
const filePath = path.join(inboxDir, filename);
fs.writeFileSync(filePath, Buffer.from(att.data as string, 'base64'));
att.name = filename;
att.localPath = `inbox/${messageId}/${filename}`;
delete att.data;
changed = true;
log.debug('Saved attachment to inbox', { messageId, filename, size: att.size });
if (typeof att.data !== 'string') continue;
const rawName = deriveAttachmentName(att);
const filename = isSafeAttachmentName(rawName) ? rawName : `attachment-${Date.now()}`;
if (filename !== rawName) {
log.warn('Refused unsafe attachment filename, would escape inbox', {
messageId,
rawName,
replacement: filename,
});
}
const inboxDir = path.join(sessionDir(agentGroupId, sessionId), 'inbox', messageId);
// Refuse to mkdir through a symlink that the container may have pre placed
// at inboxDir. With recursive:true, mkdirSync would silently no op on a
// pre existing symlink and the subsequent writeFileSync would follow it.
if (fs.existsSync(inboxDir)) {
const stat = fs.lstatSync(inboxDir);
if (stat.isSymbolicLink() || !stat.isDirectory()) {
log.warn('Rejecting unsafe inbox directory', { messageId, inboxDir });
continue;
}
}
fs.mkdirSync(inboxDir, { recursive: true });
let realInboxDir: string;
try {
realInboxDir = fs.realpathSync(inboxDir);
} catch (err) {
log.warn('Failed to resolve inbox directory', { messageId, err });
continue;
}
const inboxRoot = path.join(sessionDir(agentGroupId, sessionId), 'inbox');
if (!isPathInside(fs.realpathSync(inboxRoot), realInboxDir)) {
log.warn('Inbox directory escaped session inbox root', { messageId, inboxDir });
continue;
}
const filePath = path.join(inboxDir, filename);
try {
// wx = exclusive create. Refuses to follow a pre existing symlink or
// overwrite any existing file. The host expects to be the sole writer
// of these attachments.
fs.writeFileSync(filePath, Buffer.from(att.data as string, 'base64'), { flag: 'wx' });
} catch (err: unknown) {
const e = err as NodeJS.ErrnoException;
if (e.code === 'EEXIST') {
log.warn('Inbox attachment target already exists, refusing to overwrite', {
messageId,
filename,
});
continue;
}
throw err;
}
att.name = filename;
att.localPath = `inbox/${messageId}/${filename}`;
delete att.data;
changed = true;
log.debug('Saved attachment to inbox', { messageId, filename, size: att.size });
}
return changed ? JSON.stringify(parsed) : contentStr;