From f72dc00ff0e3917f9083be7044f3a35459178a45 Mon Sep 17 00:00:00 2001 From: olleepalmer Date: Wed, 25 Feb 2026 19:02:31 +1100 Subject: [PATCH 1/2] fix: patch command injection, OAuth CSRF, and path deny-list bypass - Replace execSync shell interpolation with execFileSync in macNotify() and checkBinaryExists() to prevent OS command injection (CWE-78) - Reject OAuth state mismatch instead of ignoring it to prevent CSRF - Merge ALWAYS_DENIED with user deniedPaths instead of replacing, so ~/.ssh, ~/.aws, ~/.gnupg are always protected Co-Authored-By: Claude Opus 4.6 --- src/config.ts | 2 +- src/gateway/server.ts | 6 ++---- src/providers/claude.ts | 4 ++-- src/skills/loader.ts | 4 ++-- 4 files changed, 7 insertions(+), 9 deletions(-) diff --git a/src/config.ts b/src/config.ts index f0b168c..c21aa4b 100644 --- a/src/config.ts +++ b/src/config.ts @@ -282,7 +282,7 @@ export function isPathAllowed( } // denied: always_denied + global + channel-specific (all merged) - const globalDenied = config.gateway?.deniedPaths || ALWAYS_DENIED; + const globalDenied = [...ALWAYS_DENIED, ...(config.gateway?.deniedPaths || [])]; const channelDenied = channelOverride?.deniedPaths || []; const denied = [...globalDenied, ...channelDenied].map(p => resolve(p.replace(/^~/, home))); if (denied.some(d => resolved.startsWith(d))) return false; diff --git a/src/gateway/server.ts b/src/gateway/server.ts index 0a4a8f6..e2b635e 100644 --- a/src/gateway/server.ts +++ b/src/gateway/server.ts @@ -2,7 +2,7 @@ import { WebSocketServer, WebSocket } from 'ws'; import { createServer } from 'node:http'; import { readdirSync, statSync, readFileSync, writeFileSync, existsSync, mkdirSync, rmSync, renameSync, chmodSync, unlinkSync, watch, type FSWatcher } from 'node:fs'; import { writeFile } from 'node:fs/promises'; -import { execSync } from 'node:child_process'; +import { execSync, execFileSync } from 'node:child_process'; import { resolve as pathResolve, join, dirname } from 'node:path'; import { homedir } from 'node:os'; import { createConnection } from 'node:net'; @@ -63,9 +63,7 @@ import { function macNotify(title: string, body: string) { try { - const t = title.replace(/"/g, '\\"'); - const b = body.replace(/"/g, '\\"'); - execSync(`osascript -e 'display notification "${b}" with title "${t}"'`, { stdio: 'ignore' }); + execFileSync('osascript', ['-e', `display notification "${body}" with title "${title}"`], { stdio: 'ignore' }); } catch { /* ignore */ } } diff --git a/src/providers/claude.ts b/src/providers/claude.ts index 03fa6a3..742dc57 100644 --- a/src/providers/claude.ts +++ b/src/providers/claude.ts @@ -552,8 +552,8 @@ export class ClaudeProvider implements Provider { return { authenticated: false, error: 'Invalid auth code.' }; } - if (returnedState && returnedState !== this._pkceState) { - console.warn('[claude] OAuth state mismatch, proceeding anyway'); + if (returnedState !== this._pkceState) { + return { authenticated: false, error: 'OAuth state mismatch — possible CSRF. Please retry login.' }; } try { diff --git a/src/skills/loader.ts b/src/skills/loader.ts index 92d327a..42bca56 100644 --- a/src/skills/loader.ts +++ b/src/skills/loader.ts @@ -1,6 +1,6 @@ import { readFileSync, existsSync, readdirSync, statSync } from 'node:fs'; import { join, basename, dirname } from 'node:path'; -import { execSync } from 'node:child_process'; +import { execFileSync } from 'node:child_process'; import matter from 'gray-matter'; import type { Config } from '../config.js'; @@ -28,7 +28,7 @@ export type SkillEligibility = { function checkBinaryExists(bin: string): boolean { try { - execSync(`which ${bin}`, { stdio: 'ignore' }); + execFileSync('which', [bin], { stdio: 'ignore' }); return true; } catch { return false; From 1dfd9246dcc45df082c86511772f435cc5895240 Mon Sep 17 00:00:00 2001 From: olleepalmer Date: Wed, 25 Feb 2026 19:13:13 +1100 Subject: [PATCH 2/2] fix: path boundary bypass, symlink fallback, and Telegram callback auth - Replace naive startsWith() path matching with segment-aware boundary checks so /safe no longer matches /safe_evil (CWE-22) - Resolve parent directory via realpathSync for non-existent targets to prevent symlink-parent bypasses on file creation (CWE-367) - Enforce sender allowlist on Telegram callback queries (approve/deny buttons, question responses) to prevent unauthorized users from approving tool executions (CWE-284) Co-Authored-By: Claude Opus 4.6 --- src/channels/telegram/monitor.ts | 6 ++++++ src/config.ts | 16 +++++++++++++--- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/channels/telegram/monitor.ts b/src/channels/telegram/monitor.ts index c41f3e6..b172e8a 100644 --- a/src/channels/telegram/monitor.ts +++ b/src/channels/telegram/monitor.ts @@ -87,6 +87,12 @@ export async function startTelegramMonitor(opts: TelegramMonitorOptions): Promis // handle callback queries (approvals + questions) bot.on('callback_query:data', async (ctx) => { + const callbackSenderId = String(ctx.from?.id || ''); + if (opts.allowFrom && opts.allowFrom.length > 0 && !opts.allowFrom.includes(callbackSenderId)) { + await ctx.answerCallbackQuery('Unauthorized'); + return; + } + const data = ctx.callbackQuery.data; const sep = data.indexOf(':'); if (sep < 0) return; diff --git a/src/config.ts b/src/config.ts index c21aa4b..57ff6c0 100644 --- a/src/config.ts +++ b/src/config.ts @@ -278,19 +278,29 @@ export function isPathAllowed( try { resolved = realpathSync(targetPath); } catch { - resolved = resolve(targetPath); + // For non-existent targets, resolve the parent via realpath to prevent + // symlink-parent bypasses, then append the final segment. + const parent = dirname(resolve(targetPath)); + try { + resolved = join(realpathSync(parent), resolve(targetPath).slice(parent.length)); + } catch { + resolved = resolve(targetPath); + } } + const pathMatch = (dir: string, target: string) => + target === dir || target.startsWith(dir + '/'); + // denied: always_denied + global + channel-specific (all merged) const globalDenied = [...ALWAYS_DENIED, ...(config.gateway?.deniedPaths || [])]; const channelDenied = channelOverride?.deniedPaths || []; const denied = [...globalDenied, ...channelDenied].map(p => resolve(p.replace(/^~/, home))); - if (denied.some(d => resolved.startsWith(d))) return false; + if (denied.some(d => pathMatch(d, resolved))) return false; // allowed: channel-specific overrides global if set const allowedRaw = channelOverride?.allowedPaths?.length ? channelOverride.allowedPaths : (config.gateway?.allowedPaths || [home, '/tmp']); const allowed = allowedRaw.map(p => resolve(p.replace(/^~/, home))); - return allowed.some(a => resolved.startsWith(a)); + return allowed.some(a => pathMatch(a, resolved)); }