fix(security): push-endpoint SSRF guard + tmux name validation; document tail-file roots

- M7 (SSRF): add isSafePushEndpoint (https-only; reject internal/loopback/link-local/metadata IPs incl. IPv4-mapped); enforce in PushSubscribeSchema and re-check before webpush.sendNotification. + unit test.
- M1 (command injection): validate tmux session names with isValidMuxName in sessionExists, killSession, and reconcileSessions before they reach a shell call site.
- M5: keep the intentional /var/log + ~/logs log-tail roots (a tested feature) and document the wider read scope in docs/security-architecture.md section 5 instead of dropping it.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

docs/security-architecture.md

@@ -306,6 +306,20 @@ injected from API JSON (`innerHTML`), not via `file-raw`, so they are unaffected
 is **defense‑in‑depth, not the primary boundary** — the realpath containment is
 the control.
 
+### SSE log‑tail route — intentional extra read roots
+
+The live file‑tail SSE route (`FileStreamManager`, used to stream a growing log
+into the UI) does **not** use `validateSessionFilePath`; it has its own validator
+with a deliberately **wider** allowlist: the session `workingDir` **plus two
+read‑only log roots — `/var/log` and `~/logs`** — so operators can tail
+system/app logs. `/tmp` is intentionally excluded (world‑writable). Like the
+other routes it `realpath`s the target and re‑checks right before spawning `tail`
+(TOCTOU guard), and it is read‑only. This is the one place the per‑session
+boundary is intentionally relaxed; on a password‑protected remote deployment an
+authenticated user can therefore read `/var/log` and `~/logs` outside their
+session dir. (Security review M5: this divergence is by design and is now
+documented here rather than silently diverging from the per‑session claim above.)
+
 ### Known limitation — `workingDir` scope
 
 The file‑route boundary is the session's `workingDir`, and `POST /api/sessions`

src/file-stream-manager.ts

@@ -395,8 +395,12 @@ export class FileStreamManager extends EventEmitter {
     // Normalize the working directory
     const normalizedWorkingDir = resolve(workingDir);
 
-    // Check if the resolved path is within the working directory
+    // Allowed read roots for log tailing: the session working dir plus the
-    // or common log directories (/tmp intentionally excluded — world-writable)
+    // INTENTIONAL log directories (/var/log, ~/logs). This is wider than the
+    // per-session boundary used by validateSessionFilePath — a deliberate,
+    // tested design choice for tailing system/app logs, documented as such in
+    // docs/security-architecture.md (security review M5). /tmp is excluded
+    // (world-writable).
     const allowedPaths = [normalizedWorkingDir, '/var/log', resolve(homedir(), 'logs')];
 
     const isAllowed = allowedPaths.some((allowed) => {

src/tmux-manager.ts

@@ -920,6 +920,7 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
 
   private sessionExists(muxName: string): boolean {
     if (IS_TEST_MODE) return false;
+    if (!isValidMuxName(muxName)) return false;
 
     try {
       execSync(`${this.tmux()} has-session -t "${muxName}" 2>/dev/null`, {
@@ -1060,13 +1061,15 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
       }
     }
 
-    // Strategy 3: Kill tmux session by name
+    // Strategy 3: Kill tmux session by name (guard the name before it reaches the shell)
-    try {
+    if (isValidMuxName(session.muxName)) {
-      execSync(`${this.tmux()} kill-session -t "${session.muxName}" 2>/dev/null`, {
+      try {
-        timeout: EXEC_TIMEOUT_MS,
+        execSync(`${this.tmux()} kill-session -t "${session.muxName}" 2>/dev/null`, {
-      });
+          timeout: EXEC_TIMEOUT_MS,
-    } catch {
+        });
-      // Session may already be dead
+      } catch {
+        // Session may already be dead
+      }
     }
 
     // Strategy 4: Direct kill by PID as final fallback
@@ -1166,6 +1169,14 @@ export class TmuxManager extends EventEmitter implements TerminalMultiplexer {
 
     for (const [sessionName, pid] of active) {
       if (!sessionName.startsWith('codeman-') && !sessionName.startsWith('claudeman-')) continue;
+      // Only admit names that pass the safe-name pattern. A foreign process on the
+      // shared `tmux -L codeman` socket could create a `codeman-…` session whose name
+      // contains shell metacharacters; rejecting it here keeps it out of this.sessions
+      // and away from the name-interpolating tmux call sites (M1).
+      if (!isValidMuxName(sessionName)) {
+        console.warn(`[TmuxManager] Skipping discovered tmux session with unsafe name: ${sessionName}`);
+        continue;
+      }
       if (knownMuxNames.has(sessionName)) continue;
 
       const fragment = sessionName.replace(/^(?:codeman|claudeman)-/, '');

src/utils/index.ts

@@ -22,6 +22,7 @@ export {
   execPattern,
 } from './regex-patterns.js';
 export { MAX_SESSION_TOKENS } from './token-validation.js';
+export { isSafePushEndpoint } from './push-endpoint-validation.js';
 export { stringSimilarity, fuzzyPhraseMatch, todoContentHash } from './string-similarity.js';
 export { assertNever } from './type-safety.js';
 export { wrapWithNice } from './nice-wrapper.js';

src/utils/push-endpoint-validation.ts

@@ -0,0 +1,69 @@
+/**
+ * @fileoverview SSRF guard for web-push subscription endpoints (security review M7).
+ *
+ * A push `endpoint` is an attacker-suppliable URL that the server fetches via
+ * `webpush.sendNotification`. On the no-auth loopback default a local page (or any
+ * non-browser client) could register an endpoint pointing at the cloud metadata
+ * service (169.254.169.254) or an internal host, turning the server into an SSRF
+ * proxy. We require https and reject IP-literal hosts in private/loopback/
+ * link-local/reserved ranges. DNS-named hosts are allowed (every real push service
+ * — FCM, Mozilla, Apple, WNS — uses a public DNS name); this is checked both at
+ * subscribe time (schema) and again at send time (defense-in-depth).
+ *
+ * Note: a hostname that *resolves* to an internal IP (DNS rebinding) is not caught
+ * here without async resolution; the realistic, documented vector (a direct
+ * internal IP literal) is closed.
+ */
+import { isIP } from 'node:net';
+
+/** True if `host` is an IP literal in a private, loopback, link-local, or reserved range. */
+function isPrivateOrReservedIp(host: string): boolean {
+  const kind = isIP(host);
+  if (kind === 0) return false; // not an IP literal — a DNS name
+
+  if (kind === 4) {
+    const [a, b] = host.split('.').map(Number);
+    if (a === 0 || a === 10 || a === 127) return true; // unspecified, private, loopback
+    if (a === 169 && b === 254) return true; // link-local (incl. 169.254.169.254 metadata)
+    if (a === 172 && b >= 16 && b <= 31) return true; // private
+    if (a === 192 && b === 168) return true; // private
+    if (a === 100 && b >= 64 && b <= 127) return true; // CGNAT (RFC 6598)
+    if (a >= 224) return true; // multicast + reserved (224.0.0.0+)
+    return false;
+  }
+
+  // IPv6
+  const h = host.toLowerCase();
+  if (h === '::1' || h === '::') return true; // loopback, unspecified
+  if (h.startsWith('fe8') || h.startsWith('fe9') || h.startsWith('fea') || h.startsWith('feb')) return true; // fe80::/10 link-local
+  if (h.startsWith('fc') || h.startsWith('fd')) return true; // fc00::/7 unique-local
+  // IPv4-mapped (::ffff:a.b.c.d). URL/Node may normalize the dotted tail to hex
+  // (::ffff:7f00:1), so handle both forms and re-check the embedded IPv4.
+  const mappedDotted = h.match(/^::ffff:(\d{1,3}\.\d{1,3}\.\d{1,3}\.\d{1,3})$/);
+  if (mappedDotted) return isPrivateOrReservedIp(mappedDotted[1]);
+  const mappedHex = h.match(/^::ffff:([0-9a-f]{1,4}):([0-9a-f]{1,4})$/);
+  if (mappedHex) {
+    const hi = parseInt(mappedHex[1], 16);
+    const lo = parseInt(mappedHex[2], 16);
+    return isPrivateOrReservedIp(`${(hi >> 8) & 0xff}.${hi & 0xff}.${(lo >> 8) & 0xff}.${lo & 0xff}`);
+  }
+  return false;
+}
+
+/**
+ * Validate a web-push endpoint URL is safe to fetch server-side.
+ * Requires an https URL whose host is not an internal/reserved IP literal.
+ */
+export function isSafePushEndpoint(endpoint: string): boolean {
+  let url: URL;
+  try {
+    url = new URL(endpoint);
+  } catch {
+    return false;
+  }
+  if (url.protocol !== 'https:') return false;
+  if (!url.hostname) return false;
+  // URL.hostname wraps IPv6 literals in brackets ([::1]); strip them for isIP().
+  const host = url.hostname.replace(/^\[|\]$/g, '');
+  return !isPrivateOrReservedIp(host);
+}

src/web/schemas.ts

@@ -8,7 +8,7 @@
  */
 
 import { z } from 'zod';
-import { SAFE_PATH_PATTERN } from '../utils/index.js';
+import { SAFE_PATH_PATTERN, isSafePushEndpoint } from '../utils/index.js';
 
 // ========== Path Validation ==========
 
@@ -531,7 +531,11 @@ export const RespawnEnableSchema = z.object({
 
 /** POST /api/push/subscribe */
 export const PushSubscribeSchema = z.object({
-  endpoint: z.string().url().max(2000),
+  endpoint: z
+    .string()
+    .url()
+    .max(2000)
+    .refine(isSafePushEndpoint, { message: 'endpoint must be an https URL to a public (non-internal) host' }),
   keys: z.object({
     p256dh: z.string().min(1).max(500),
     auth: z.string().min(1).max(500),

src/web/server.ts

@@ -97,7 +97,13 @@ import {
   type ImageDetectedEvent,
   DEFAULT_NICE_CONFIG,
 } from '../types.js';
-import { CleanupManager, KeyedDebouncer, StaleExpirationMap, startEventLoopMonitor } from '../utils/index.js';
+import {
+  CleanupManager,
+  KeyedDebouncer,
+  StaleExpirationMap,
+  startEventLoopMonitor,
+  isSafePushEndpoint,
+} from '../utils/index.js';
 import type { EventLoopMonitorHandle } from '../utils/index.js';
 import { MAX_CONCURRENT_SESSIONS, MAX_SSE_CLIENTS } from '../config/map-limits.js';
 import { SseEvent } from './sse-events.js';
@@ -1639,6 +1645,14 @@ export class WebServer extends EventEmitter {
       // Check per-subscription preferences
       if (sub.pushPreferences[event] === false) continue;
 
+      // Re-validate the stored endpoint before fetching it server-side (SSRF, M7).
+      // Defense-in-depth: subscribe-time validation already rejects unsafe URLs.
+      if (!isSafePushEndpoint(sub.endpoint)) {
+        console.warn('[push] skipping notification to unsafe endpoint:', sub.endpoint);
+        this.pushStore.removeByEndpoint(sub.endpoint);
+        continue;
+      }
+
       const pushSub = {
         endpoint: sub.endpoint,
         keys: sub.keys,

test/push-endpoint-validation.test.ts

@@ -0,0 +1,45 @@
+/**
+ * SSRF guard for web-push endpoints (security review M7).
+ */
+import { describe, it, expect } from 'vitest';
+import { isSafePushEndpoint } from '../src/utils/push-endpoint-validation.js';
+
+describe('isSafePushEndpoint (SSRF guard, M7)', () => {
+  it('accepts real https push-service endpoints (public DNS hosts)', () => {
+    expect(isSafePushEndpoint('https://fcm.googleapis.com/fcm/send/abc123')).toBe(true);
+    expect(isSafePushEndpoint('https://updates.push.services.mozilla.com/wpush/v2/abc')).toBe(true);
+    expect(isSafePushEndpoint('https://web.push.apple.com/abc')).toBe(true);
+    expect(isSafePushEndpoint('https://foo.notify.windows.com/w/?token=x')).toBe(true);
+  });
+
+  it('accepts a public IP literal over https', () => {
+    expect(isSafePushEndpoint('https://93.184.216.34/x')).toBe(true);
+  });
+
+  it('rejects non-https schemes', () => {
+    expect(isSafePushEndpoint('http://fcm.googleapis.com/x')).toBe(false);
+    expect(isSafePushEndpoint('ftp://example.com/x')).toBe(false);
+  });
+
+  it('rejects the cloud-metadata IP and internal IPv4 ranges', () => {
+    expect(isSafePushEndpoint('https://169.254.169.254/latest/meta-data/')).toBe(false);
+    expect(isSafePushEndpoint('https://127.0.0.1/x')).toBe(false);
+    expect(isSafePushEndpoint('https://10.0.0.5/x')).toBe(false);
+    expect(isSafePushEndpoint('https://192.168.1.10/x')).toBe(false);
+    expect(isSafePushEndpoint('https://172.16.0.1/x')).toBe(false);
+    expect(isSafePushEndpoint('https://100.64.0.1/x')).toBe(false);
+    expect(isSafePushEndpoint('https://0.0.0.0/x')).toBe(false);
+  });
+
+  it('rejects internal IPv6 (incl. bracketed + IPv4-mapped)', () => {
+    expect(isSafePushEndpoint('https://[::1]/x')).toBe(false);
+    expect(isSafePushEndpoint('https://[fe80::1]/x')).toBe(false);
+    expect(isSafePushEndpoint('https://[fd00::1]/x')).toBe(false);
+    expect(isSafePushEndpoint('https://[::ffff:127.0.0.1]/x')).toBe(false);
+  });
+
+  it('rejects garbage / empty input', () => {
+    expect(isSafePushEndpoint('not a url')).toBe(false);
+    expect(isSafePushEndpoint('')).toBe(false);
+  });
+});