fix: block group folder path escapes

This commit is contained in:
Lawyered
2026-02-22 11:36:06 -05:00
committed by gavrielc
parent de64dab3e9
commit c6391cceb1
9 changed files with 186 additions and 25 deletions

View File

@@ -4,7 +4,6 @@
*/
import { ChildProcess, exec, spawn } from 'child_process';
import fs from 'fs';
import os from 'os';
import path from 'path';
import {
@@ -16,6 +15,7 @@ import {
IDLE_TIMEOUT,
} from './config.js';
import { readEnvFile } from './env.js';
import { resolveGroupFolderPath, resolveGroupIpcPath } from './group-folder.js';
import { logger } from './logger.js';
import { CONTAINER_RUNTIME_BIN, readonlyMountArgs, stopContainer } from './container-runtime.js';
import { validateAdditionalMounts } from './mount-security.js';
@@ -25,16 +25,6 @@ import { RegisteredGroup } from './types.js';
const OUTPUT_START_MARKER = '---NANOCLAW_OUTPUT_START---';
const OUTPUT_END_MARKER = '---NANOCLAW_OUTPUT_END---';
function getHomeDir(): string {
const home = process.env.HOME || os.homedir();
if (!home) {
throw new Error(
'Unable to determine home directory: HOME environment variable is not set and os.homedir() returned empty',
);
}
return home;
}
export interface ContainerInput {
prompt: string;
sessionId?: string;
@@ -63,8 +53,8 @@ function buildVolumeMounts(
isMain: boolean,
): VolumeMount[] {
const mounts: VolumeMount[] = [];
const homeDir = getHomeDir();
const projectRoot = process.cwd();
const groupDir = resolveGroupFolderPath(group.folder);
if (isMain) {
// Main gets the project root read-only. Writable paths the agent needs
@@ -80,14 +70,14 @@ function buildVolumeMounts(
// Main also gets its group folder as the working directory
mounts.push({
hostPath: path.join(GROUPS_DIR, group.folder),
hostPath: groupDir,
containerPath: '/workspace/group',
readonly: false,
});
} else {
// Other groups only get their own folder
mounts.push({
hostPath: path.join(GROUPS_DIR, group.folder),
hostPath: groupDir,
containerPath: '/workspace/group',
readonly: false,
});
@@ -149,7 +139,7 @@ function buildVolumeMounts(
// Per-group IPC namespace: each group gets its own IPC directory
// This prevents cross-group privilege escalation via IPC
const groupIpcDir = path.join(DATA_DIR, 'ipc', group.folder);
const groupIpcDir = resolveGroupIpcPath(group.folder);
fs.mkdirSync(path.join(groupIpcDir, 'messages'), { recursive: true });
fs.mkdirSync(path.join(groupIpcDir, 'tasks'), { recursive: true });
fs.mkdirSync(path.join(groupIpcDir, 'input'), { recursive: true });
@@ -228,7 +218,7 @@ export async function runContainerAgent(
): Promise<ContainerOutput> {
const startTime = Date.now();
const groupDir = path.join(GROUPS_DIR, group.folder);
const groupDir = resolveGroupFolderPath(group.folder);
fs.mkdirSync(groupDir, { recursive: true });
const mounts = buildVolumeMounts(group, input.isMain);
@@ -259,7 +249,7 @@ export async function runContainerAgent(
'Spawning container agent',
);
const logsDir = path.join(GROUPS_DIR, group.folder, 'logs');
const logsDir = path.join(groupDir, 'logs');
fs.mkdirSync(logsDir, { recursive: true });
return new Promise((resolve) => {
@@ -603,7 +593,7 @@ export function writeTasksSnapshot(
}>,
): void {
// Write filtered tasks to the group's IPC directory
const groupIpcDir = path.join(DATA_DIR, 'ipc', groupFolder);
const groupIpcDir = resolveGroupIpcPath(groupFolder);
fs.mkdirSync(groupIpcDir, { recursive: true });
// Main sees all tasks, others only see their own
@@ -633,7 +623,7 @@ export function writeGroupsSnapshot(
groups: AvailableGroup[],
registeredJids: Set<string>,
): void {
const groupIpcDir = path.join(DATA_DIR, 'ipc', groupFolder);
const groupIpcDir = resolveGroupIpcPath(groupFolder);
fs.mkdirSync(groupIpcDir, { recursive: true });
// Main sees all groups; others see nothing (they can't activate groups)

View File

@@ -3,6 +3,8 @@ import fs from 'fs';
import path from 'path';
import { ASSISTANT_NAME, DATA_DIR, STORE_DIR } from './config.js';
import { isValidGroupFolder } from './group-folder.js';
import { logger } from './logger.js';
import { NewMessage, RegisteredGroup, ScheduledTask, TaskRunLog } from './types.js';
let db: Database.Database;
@@ -520,6 +522,13 @@ export function getRegisteredGroup(
}
| undefined;
if (!row) return undefined;
if (!isValidGroupFolder(row.folder)) {
logger.warn(
{ jid: row.jid, folder: row.folder },
'Skipping registered group with invalid folder',
);
return undefined;
}
return {
jid: row.jid,
name: row.name,
@@ -537,6 +546,9 @@ export function setRegisteredGroup(
jid: string,
group: RegisteredGroup,
): void {
if (!isValidGroupFolder(group.folder)) {
throw new Error(`Invalid group folder "${group.folder}" for JID ${jid}`);
}
db.prepare(
`INSERT OR REPLACE INTO registered_groups (jid, name, folder, trigger_pattern, added_at, container_config, requires_trigger)
VALUES (?, ?, ?, ?, ?, ?, ?)`,
@@ -565,6 +577,13 @@ export function getAllRegisteredGroups(): Record<string, RegisteredGroup> {
}>;
const result: Record<string, RegisteredGroup> = {};
for (const row of rows) {
if (!isValidGroupFolder(row.folder)) {
logger.warn(
{ jid: row.jid, folder: row.folder },
'Skipping registered group with invalid folder',
);
continue;
}
result[row.jid] = {
name: row.name,
folder: row.folder,
@@ -629,7 +648,14 @@ function migrateJsonState(): void {
> | null;
if (groups) {
for (const [jid, group] of Object.entries(groups)) {
setRegisteredGroup(jid, group);
try {
setRegisteredGroup(jid, group);
} catch (err) {
logger.warn(
{ jid, folder: group.folder, err },
'Skipping migrated registered group with invalid folder',
);
}
}
}
}

39
src/group-folder.test.ts Normal file
View File

@@ -0,0 +1,39 @@
import path from 'path';
import { describe, expect, it } from 'vitest';
import { isValidGroupFolder, resolveGroupFolderPath, resolveGroupIpcPath } from './group-folder.js';
describe('group folder validation', () => {
it('accepts normal group folder names', () => {
expect(isValidGroupFolder('main')).toBe(true);
expect(isValidGroupFolder('family-chat')).toBe(true);
expect(isValidGroupFolder('Team_42')).toBe(true);
});
it('rejects traversal and reserved names', () => {
expect(isValidGroupFolder('../../etc')).toBe(false);
expect(isValidGroupFolder('/tmp')).toBe(false);
expect(isValidGroupFolder('global')).toBe(false);
expect(isValidGroupFolder('')).toBe(false);
});
it('resolves safe paths under groups directory', () => {
const resolved = resolveGroupFolderPath('family-chat');
expect(
resolved.endsWith(`${path.sep}groups${path.sep}family-chat`),
).toBe(true);
});
it('resolves safe paths under data ipc directory', () => {
const resolved = resolveGroupIpcPath('family-chat');
expect(
resolved.endsWith(`${path.sep}data${path.sep}ipc${path.sep}family-chat`),
).toBe(true);
});
it('throws for unsafe folder names', () => {
expect(() => resolveGroupFolderPath('../../etc')).toThrow();
expect(() => resolveGroupIpcPath('/tmp')).toThrow();
});
});

44
src/group-folder.ts Normal file
View File

@@ -0,0 +1,44 @@
import path from 'path';
import { DATA_DIR, GROUPS_DIR } from './config.js';
const GROUP_FOLDER_PATTERN = /^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/;
const RESERVED_FOLDERS = new Set(['global']);
export function isValidGroupFolder(folder: string): boolean {
if (!folder) return false;
if (folder !== folder.trim()) return false;
if (!GROUP_FOLDER_PATTERN.test(folder)) return false;
if (folder.includes('/') || folder.includes('\\')) return false;
if (folder.includes('..')) return false;
if (RESERVED_FOLDERS.has(folder.toLowerCase())) return false;
return true;
}
export function assertValidGroupFolder(folder: string): void {
if (!isValidGroupFolder(folder)) {
throw new Error(`Invalid group folder "${folder}"`);
}
}
function ensureWithinBase(baseDir: string, resolvedPath: string): void {
const rel = path.relative(baseDir, resolvedPath);
if (rel.startsWith('..') || path.isAbsolute(rel)) {
throw new Error(`Path escapes base directory: ${resolvedPath}`);
}
}
export function resolveGroupFolderPath(folder: string): string {
assertValidGroupFolder(folder);
const groupPath = path.resolve(GROUPS_DIR, folder);
ensureWithinBase(GROUPS_DIR, groupPath);
return groupPath;
}
export function resolveGroupIpcPath(folder: string): string {
assertValidGroupFolder(folder);
const ipcBaseDir = path.resolve(DATA_DIR, 'ipc');
const ipcPath = path.resolve(ipcBaseDir, folder);
ensureWithinBase(ipcBaseDir, ipcPath);
return ipcPath;
}

View File

@@ -3,7 +3,6 @@ import path from 'path';
import {
ASSISTANT_NAME,
DATA_DIR,
IDLE_TIMEOUT,
MAIN_GROUP_FOLDER,
POLL_INTERVAL,
@@ -33,6 +32,7 @@ import {
storeMessage,
} from './db.js';
import { GroupQueue } from './group-queue.js';
import { resolveGroupFolderPath } from './group-folder.js';
import { startIpcWatcher } from './ipc.js';
import { findChannel, formatMessages, formatOutbound } from './router.js';
import { startSchedulerLoop } from './task-scheduler.js';
@@ -78,11 +78,21 @@ function saveState(): void {
}
function registerGroup(jid: string, group: RegisteredGroup): void {
let groupDir: string;
try {
groupDir = resolveGroupFolderPath(group.folder);
} catch (err) {
logger.warn(
{ jid, folder: group.folder, err },
'Rejecting group registration with invalid folder',
);
return;
}
registeredGroups[jid] = group;
setRegisteredGroup(jid, group);
// Create group folder
const groupDir = path.join(DATA_DIR, '..', 'groups', group.folder);
fs.mkdirSync(path.join(groupDir, 'logs'), { recursive: true });
logger.info(

View File

@@ -301,6 +301,23 @@ describe('register_group authorization', () => {
// registeredGroups should not have changed
expect(groups['new@g.us']).toBeUndefined();
});
it('main group cannot register with unsafe folder path', async () => {
await processTaskIpc(
{
type: 'register_group',
jid: 'new@g.us',
name: 'New Group',
folder: '../../outside',
trigger: '@Andy',
},
'main',
true,
deps,
);
expect(groups['new@g.us']).toBeUndefined();
});
});
// --- refresh_groups authorization ---

View File

@@ -11,6 +11,7 @@ import {
} from './config.js';
import { AvailableGroup } from './container-runner.js';
import { createTask, deleteTask, getTaskById, updateTask } from './db.js';
import { isValidGroupFolder } from './group-folder.js';
import { logger } from './logger.js';
import { RegisteredGroup } from './types.js';
@@ -357,6 +358,13 @@ export async function processTaskIpc(
break;
}
if (data.jid && data.name && data.folder && data.trigger) {
if (!isValidGroupFolder(data.folder)) {
logger.warn(
{ sourceGroup, folder: data.folder },
'Invalid register_group request - unsafe folder name',
);
break;
}
deps.registerGroup(data.jid, {
name: data.name,
folder: data.folder,

View File

@@ -1,10 +1,8 @@
import { ChildProcess } from 'child_process';
import { CronExpressionParser } from 'cron-parser';
import fs from 'fs';
import path from 'path';
import {
GROUPS_DIR,
IDLE_TIMEOUT,
MAIN_GROUP_FOLDER,
SCHEDULER_POLL_INTERVAL,
@@ -19,6 +17,7 @@ import {
updateTaskAfterRun,
} from './db.js';
import { GroupQueue } from './group-queue.js';
import { resolveGroupFolderPath } from './group-folder.js';
import { logger } from './logger.js';
import { RegisteredGroup, ScheduledTask } from './types.js';
@@ -35,7 +34,25 @@ async function runTask(
deps: SchedulerDependencies,
): Promise<void> {
const startTime = Date.now();
const groupDir = path.join(GROUPS_DIR, task.group_folder);
let groupDir: string;
try {
groupDir = resolveGroupFolderPath(task.group_folder);
} catch (err) {
const error = err instanceof Error ? err.message : String(err);
logger.error(
{ taskId: task.id, groupFolder: task.group_folder, error },
'Task has invalid group folder',
);
logTaskRun({
task_id: task.id,
run_at: new Date().toISOString(),
duration_ms: Date.now() - startTime,
status: 'error',
result: null,
error,
});
return;
}
fs.mkdirSync(groupDir, { recursive: true });
logger.info(