Merge branch 'main' into fix/message-history-overflow

src/container-runtime.test.ts

@@ -39,11 +39,20 @@ describe('readonlyMountArgs', () => {
 });
 
 describe('stopContainer', () => {
-  it('returns stop command using CONTAINER_RUNTIME_BIN', () => {
-    expect(stopContainer('nanoclaw-test-123')).toBe(
+  it('calls docker stop for valid container names', () => {
+    stopContainer('nanoclaw-test-123');
+    expect(mockExecSync).toHaveBeenCalledWith(
       `${CONTAINER_RUNTIME_BIN} stop -t 1 nanoclaw-test-123`,
+      { stdio: 'pipe' },
     );
   });
+
+  it('rejects names with shell metacharacters', () => {
+    expect(() => stopContainer('foo; rm -rf /')).toThrow('Invalid container name');
+    expect(() => stopContainer('foo$(whoami)')).toThrow('Invalid container name');
+    expect(() => stopContainer('foo`id`')).toThrow('Invalid container name');
+    expect(mockExecSync).not.toHaveBeenCalled();
+  });
 });
 
 // --- ensureContainerRuntimeRunning ---

src/container-runtime.ts

@@ -27,9 +27,12 @@ export function readonlyMountArgs(
   return ['-v', `${hostPath}:${containerPath}:ro`];
 }
 
-/** Returns the shell command to stop a container by name. */
-export function stopContainer(name: string): string {
-  return `${CONTAINER_RUNTIME_BIN} stop -t 1 ${name}`;
+/** Stop a container by name. Uses execFileSync to avoid shell injection. */
+export function stopContainer(name: string): void {
+  if (!/^[a-zA-Z0-9][a-zA-Z0-9_.-]*$/.test(name)) {
+    throw new Error(`Invalid container name: ${name}`);
+  }
+  execSync(`${CONTAINER_RUNTIME_BIN} stop -t 1 ${name}`, { stdio: 'pipe' });
 }
 
 /** Ensure the container runtime is running, starting it if needed. */
@@ -82,7 +85,7 @@ export function cleanupOrphans(): void {
     const orphans = output.trim().split('\n').filter(Boolean);
     for (const name of orphans) {
       try {
-        execSync(stopContainer(name), { stdio: 'pipe' });
+        stopContainer(name);
       } catch {
         /* already stopped */
       }

src/env.ts

@@ -30,8 +30,9 @@ export function readEnvFile(keys: string[]): Record<string, string> {
     if (!wanted.has(key)) continue;
     let value = trimmed.slice(eqIdx + 1).trim();
     if (
-      (value.startsWith('"') && value.endsWith('"')) ||
-      (value.startsWith("'") && value.endsWith("'"))
+      value.length >= 2 &&
+      ((value.startsWith('"') && value.endsWith('"')) ||
+        (value.startsWith("'") && value.endsWith("'")))
     ) {
       value = value.slice(1, -1);
     }

src/index.ts

@@ -352,6 +352,7 @@ async function runAgent(
       id: t.id,
       groupFolder: t.group_folder,
       prompt: t.prompt,
+      script: t.script || undefined,
       schedule_type: t.schedule_type,
       schedule_value: t.schedule_value,
       status: t.status,
@@ -713,6 +714,7 @@ async function main(): Promise<void> {
         id: t.id,
         groupFolder: t.group_folder,
         prompt: t.prompt,
+        script: t.script || undefined,
         schedule_type: t.schedule_type,
         schedule_value: t.schedule_value,
         status: t.status,

src/ipc.ts

@@ -441,7 +441,10 @@ export async function processTaskIpc(
           );
           break;
         }
-        // Defense in depth: agent cannot set isMain via IPC
+        // Defense in depth: agent cannot set isMain via IPC.                                                                                                                                    
+        // Preserve isMain from the existing registration so IPC config                                                                                                                          
+        // updates (e.g. adding additionalMounts) don't strip the flag.                                                                                                                          
+        const existingGroup = registeredGroups[data.jid];
         deps.registerGroup(data.jid, {
           name: data.name,
           folder: data.folder,
@@ -449,6 +452,7 @@ export async function processTaskIpc(
           added_at: new Date().toISOString(),
           containerConfig: data.containerConfig,
           requiresTrigger: data.requiresTrigger,
+          isMain: existingGroup?.isMain,
         });
       } else {
         logger.warn(

src/logger.ts

@@ -1,11 +1,78 @@
-import pino from 'pino';
+const LEVELS = { debug: 20, info: 30, warn: 40, error: 50, fatal: 60 } as const;
+type Level = keyof typeof LEVELS;
 
-export const logger = pino({
-  level: process.env.LOG_LEVEL || 'info',
-  transport: { target: 'pino-pretty', options: { colorize: true } },
-});
+const COLORS: Record<Level, string> = {
+  debug: '\x1b[34m',
+  info: '\x1b[32m',
+  warn: '\x1b[33m',
+  error: '\x1b[31m',
+  fatal: '\x1b[41m\x1b[37m',
+};
+const KEY_COLOR = '\x1b[35m';
+const MSG_COLOR = '\x1b[36m';
+const RESET = '\x1b[39m';
+const FULL_RESET = '\x1b[0m';
 
-// Route uncaught errors through pino so they get timestamps in stderr
+const threshold =
+  LEVELS[(process.env.LOG_LEVEL as Level) || 'info'] ?? LEVELS.info;
+
+function formatErr(err: unknown): string {
+  if (err instanceof Error) {
+    return `{\n      "type": "${err.constructor.name}",\n      "message": "${err.message}",\n      "stack":\n          ${err.stack}\n    }`;
+  }
+  return JSON.stringify(err);
+}
+
+function formatData(data: Record<string, unknown>): string {
+  let out = '';
+  for (const [k, v] of Object.entries(data)) {
+    if (k === 'err') {
+      out += `\n    ${KEY_COLOR}err${RESET}: ${formatErr(v)}`;
+    } else {
+      out += `\n    ${KEY_COLOR}${k}${RESET}: ${JSON.stringify(v)}`;
+    }
+  }
+  return out;
+}
+
+function ts(): string {
+  const d = new Date();
+  return `${String(d.getHours()).padStart(2, '0')}:${String(d.getMinutes()).padStart(2, '0')}:${String(d.getSeconds()).padStart(2, '0')}.${String(d.getMilliseconds()).padStart(3, '0')}`;
+}
+
+function log(
+  level: Level,
+  dataOrMsg: Record<string, unknown> | string,
+  msg?: string,
+): void {
+  if (LEVELS[level] < threshold) return;
+  const tag = `${COLORS[level]}${level.toUpperCase()}${level === 'fatal' ? FULL_RESET : RESET}`;
+  const stream = LEVELS[level] >= LEVELS.warn ? process.stderr : process.stdout;
+  if (typeof dataOrMsg === 'string') {
+    stream.write(
+      `[${ts()}] ${tag} (${process.pid}): ${MSG_COLOR}${dataOrMsg}${RESET}\n`,
+    );
+  } else {
+    stream.write(
+      `[${ts()}] ${tag} (${process.pid}): ${MSG_COLOR}${msg}${RESET}${formatData(dataOrMsg)}\n`,
+    );
+  }
+}
+
+export const logger = {
+  debug: (dataOrMsg: Record<string, unknown> | string, msg?: string) =>
+    log('debug', dataOrMsg, msg),
+  info: (dataOrMsg: Record<string, unknown> | string, msg?: string) =>
+    log('info', dataOrMsg, msg),
+  warn: (dataOrMsg: Record<string, unknown> | string, msg?: string) =>
+    log('warn', dataOrMsg, msg),
+  error: (dataOrMsg: Record<string, unknown> | string, msg?: string) =>
+    log('error', dataOrMsg, msg),
+  fatal: (dataOrMsg: Record<string, unknown> | string, msg?: string) =>
+    log('fatal', dataOrMsg, msg),
+};
+
+// Route uncaught errors through logger so they get timestamps in stderr
 process.on('uncaughtException', (err) => {
   logger.fatal({ err }, 'Uncaught exception');
   process.exit(1);

src/mount-security.ts

@@ -9,16 +9,10 @@
 import fs from 'fs';
 import os from 'os';
 import path from 'path';
-import pino from 'pino';
-
 import { MOUNT_ALLOWLIST_PATH } from './config.js';
+import { logger } from './logger.js';
 import { AdditionalMount, AllowedRoot, MountAllowlist } from './types.js';
 
-const logger = pino({
-  level: process.env.LOG_LEVEL || 'info',
-  transport: { target: 'pino-pretty', options: { colorize: true } },
-});
-
 // Cache the allowlist in memory - only reloads on process restart
 let cachedAllowlist: MountAllowlist | null = null;
 let allowlistLoadError: string | null = null;
@@ -63,7 +57,8 @@ export function loadMountAllowlist(): MountAllowlist | null {
 
   try {
     if (!fs.existsSync(MOUNT_ALLOWLIST_PATH)) {
-      allowlistLoadError = `Mount allowlist not found at ${MOUNT_ALLOWLIST_PATH}`;
+      // Do NOT cache this as an error — file may be created later without restart.
+      // Only parse/structural errors are permanently cached.
       logger.warn(
         { path: MOUNT_ALLOWLIST_PATH },
         'Mount allowlist not found - additional mounts will be BLOCKED. ' +
@@ -215,6 +210,11 @@ function isValidContainerPath(containerPath: string): boolean {
     return false;
   }
 
+  // Must not contain colons — prevents Docker -v option injection (e.g., "repo:rw")
+  if (containerPath.includes(':')) {
+    return false;
+  }
+
   return true;
 }