mirror of
https://github.com/qwibitai/nanoclaw.git
synced 2026-06-04 10:14:47 +08:00
fix(container): confine outbound attachment paths
This commit is contained in:
@@ -23,6 +23,8 @@ import {
|
||||
sessionDir,
|
||||
inboundDbPath,
|
||||
outboundDbPath,
|
||||
readOutboxFiles,
|
||||
clearOutbox,
|
||||
} from './session-manager.js';
|
||||
import { getSession, findSession } from './db/sessions.js';
|
||||
import type { InboundEvent } from './channels/adapter.js';
|
||||
@@ -108,6 +110,58 @@ describe('session manager', () => {
|
||||
outDb.close();
|
||||
});
|
||||
|
||||
it('should reject outbound attachment filenames that escape the message outbox', () => {
|
||||
initSessionFolder('ag-1', 'sess-test');
|
||||
const dir = sessionDir('ag-1', 'sess-test');
|
||||
const msgOutbox = path.join(dir, 'outbox', 'msg-1');
|
||||
fs.mkdirSync(msgOutbox, { recursive: true });
|
||||
|
||||
const outside = path.join(TEST_DIR, 'outside.txt');
|
||||
fs.writeFileSync(outside, 'outside secret');
|
||||
|
||||
expect(readOutboxFiles('ag-1', 'sess-test', 'msg-1', ['../../../../../outside.txt'])).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should reject outbound attachment symlinks that escape the message outbox', () => {
|
||||
initSessionFolder('ag-1', 'sess-test');
|
||||
const dir = sessionDir('ag-1', 'sess-test');
|
||||
const msgOutbox = path.join(dir, 'outbox', 'msg-1');
|
||||
fs.mkdirSync(msgOutbox, { recursive: true });
|
||||
|
||||
const outside = path.join(TEST_DIR, 'outside.txt');
|
||||
fs.writeFileSync(outside, 'outside secret');
|
||||
fs.symlinkSync('../../../../../outside.txt', path.join(msgOutbox, 'safe-name.txt'));
|
||||
|
||||
expect(readOutboxFiles('ag-1', 'sess-test', 'msg-1', ['safe-name.txt'])).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should not recursively delete outside the outbox for unsafe message ids', () => {
|
||||
initSessionFolder('ag-1', 'sess-test');
|
||||
const victimDir = path.join(TEST_DIR, 'victim-dir');
|
||||
fs.mkdirSync(victimDir, { recursive: true });
|
||||
fs.writeFileSync(path.join(victimDir, 'keep.txt'), 'do not delete');
|
||||
|
||||
clearOutbox('ag-1', 'sess-test', '../../../../victim-dir');
|
||||
|
||||
expect(fs.existsSync(path.join(victimDir, 'keep.txt'))).toBe(true);
|
||||
});
|
||||
|
||||
it('should still read and clear normal basename outbox files', () => {
|
||||
initSessionFolder('ag-1', 'sess-test');
|
||||
const dir = sessionDir('ag-1', 'sess-test');
|
||||
const msgOutbox = path.join(dir, 'outbox', 'msg-1');
|
||||
fs.mkdirSync(msgOutbox, { recursive: true });
|
||||
fs.writeFileSync(path.join(msgOutbox, 'result.txt'), 'ok');
|
||||
|
||||
const files = readOutboxFiles('ag-1', 'sess-test', 'msg-1', ['result.txt']);
|
||||
expect(files).toHaveLength(1);
|
||||
expect(files?.[0]?.filename).toBe('result.txt');
|
||||
expect(files?.[0]?.data.toString()).toBe('ok');
|
||||
|
||||
clearOutbox('ag-1', 'sess-test', 'msg-1');
|
||||
expect(fs.existsSync(msgOutbox)).toBe(false);
|
||||
});
|
||||
|
||||
it('should resolve to existing session (shared mode)', () => {
|
||||
const { session: s1, created: c1 } = resolveSession('ag-1', 'mg-1', null, 'shared');
|
||||
expect(c1).toBe(true);
|
||||
|
||||
+56
-7
@@ -21,7 +21,6 @@ import { DATA_DIR } from './config.js';
|
||||
import { getMessagingGroup } from './db/messaging-groups.js';
|
||||
import {
|
||||
createSession,
|
||||
findSession,
|
||||
findSessionByAgentGroup,
|
||||
findSessionForAgent,
|
||||
getSession,
|
||||
@@ -38,6 +37,11 @@ import {
|
||||
import { log } from './log.js';
|
||||
import type { Session } from './types.js';
|
||||
|
||||
function isPathInside(parent: string, child: string): boolean {
|
||||
const relative = path.relative(parent, child);
|
||||
return relative === '' || (!relative.startsWith('..') && !path.isAbsolute(relative));
|
||||
}
|
||||
|
||||
/** Root directory for all session data. */
|
||||
export function sessionsBaseDir(): string {
|
||||
return path.join(DATA_DIR, 'v2-sessions');
|
||||
@@ -369,19 +373,48 @@ export function readOutboxFiles(
|
||||
messageId: string,
|
||||
filenames: string[],
|
||||
): OutboundFile[] | undefined {
|
||||
if (!isSafeAttachmentName(messageId)) {
|
||||
log.warn('Rejecting unsafe outbox message id', { messageId });
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const outboxDir = path.join(sessionDir(agentGroupId, sessionId), 'outbox', messageId);
|
||||
if (!fs.existsSync(outboxDir)) return undefined;
|
||||
|
||||
let realOutboxDir: string;
|
||||
try {
|
||||
const stat = fs.lstatSync(outboxDir);
|
||||
if (!stat.isDirectory() || stat.isSymbolicLink()) {
|
||||
log.warn('Rejecting unsafe outbox directory', { messageId, outboxDir });
|
||||
return undefined;
|
||||
}
|
||||
realOutboxDir = fs.realpathSync(outboxDir);
|
||||
} catch (err) {
|
||||
log.warn('Failed to inspect outbox directory', { messageId, err });
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const files: OutboundFile[] = [];
|
||||
for (const filename of filenames) {
|
||||
// Reject any name that isn't a bare basename before touching the filesystem.
|
||||
if (!isSafeAttachmentName(filename)) {
|
||||
log.warn('Refused unsafe outbox filename — would escape outbox', { messageId, filename });
|
||||
log.warn('Refused unsafe outbox filename, would escape outbox', { messageId, filename });
|
||||
continue;
|
||||
}
|
||||
|
||||
const filePath = path.join(outboxDir, filename);
|
||||
if (fs.existsSync(filePath)) {
|
||||
files.push({ filename, data: fs.readFileSync(filePath) });
|
||||
} else {
|
||||
try {
|
||||
const stat = fs.lstatSync(filePath);
|
||||
if (!stat.isFile() || stat.isSymbolicLink()) {
|
||||
log.warn('Rejecting unsafe outbox file', { messageId, filename });
|
||||
continue;
|
||||
}
|
||||
const realFilePath = fs.realpathSync(filePath);
|
||||
if (!isPathInside(realOutboxDir, realFilePath)) {
|
||||
log.warn('Rejecting outbox file outside message directory', { messageId, filename });
|
||||
continue;
|
||||
}
|
||||
files.push({ filename, data: fs.readFileSync(realFilePath) });
|
||||
} catch {
|
||||
log.warn('Outbox file not found', { messageId, filename });
|
||||
}
|
||||
}
|
||||
@@ -395,10 +428,26 @@ export function readOutboxFiles(
|
||||
* thrown error would trigger the delivery retry path and deliver twice.
|
||||
*/
|
||||
export function clearOutbox(agentGroupId: string, sessionId: string, messageId: string): void {
|
||||
if (!isSafeAttachmentName(messageId)) {
|
||||
log.warn('Rejecting unsafe outbox cleanup message id', { messageId });
|
||||
return;
|
||||
}
|
||||
|
||||
const outboxDir = path.join(sessionDir(agentGroupId, sessionId), 'outbox', messageId);
|
||||
if (!fs.existsSync(outboxDir)) return;
|
||||
try {
|
||||
fs.rmSync(outboxDir, { recursive: true, force: true });
|
||||
const stat = fs.lstatSync(outboxDir);
|
||||
if (!stat.isDirectory() || stat.isSymbolicLink()) {
|
||||
log.warn('Rejecting unsafe outbox cleanup directory', { messageId, outboxDir });
|
||||
return;
|
||||
}
|
||||
const realOutboxBase = fs.realpathSync(path.join(sessionDir(agentGroupId, sessionId), 'outbox'));
|
||||
const realOutboxDir = fs.realpathSync(outboxDir);
|
||||
if (!isPathInside(realOutboxBase, realOutboxDir)) {
|
||||
log.warn('Rejecting outbox cleanup outside session outbox', { messageId, outboxDir });
|
||||
return;
|
||||
}
|
||||
fs.rmSync(realOutboxDir, { recursive: true, force: true });
|
||||
} catch (err) {
|
||||
log.warn('Outbox cleanup failed (message already delivered)', { messageId, err });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user