Complete phase16-session03-input-sanitization-and-auth-hardening: input sanitization, auth hardening, exec approval tightening

- 16 security items closed (8 Apply + 8 Adapt) across injection prevention, input validation, and auth hardening
- 4 new infrastructure modules: secret-equal, base64 validator, bounded HTTP body reader, auth rate-limiter
- Exec approval chain: device binding, param allowlisting, node.invoke bypass prevention
- 113 new security-focused tests, 5935 total passing
- Version: 0.1.156 -> 0.1.157

Author: moshehbenavraham

.spec_system/PRD/phase_16/PRD_phase_16.md

@@ -4,7 +4,7 @@
 **Sessions**: 5 (initial estimate)
 **Estimated Duration**: 10-20 days
 
-**Progress**: 2/5 sessions (40%)
+**Progress**: 3/5 sessions (60%)
 
 ---
 
@@ -32,7 +32,7 @@ Audit and port ~65 upstream security patches covering SSRF bypasses, path traver
 |---------|------|--------|-------|------------|------------|-----------|
 | 01 | Security Triage and Applicability Audit | Complete | 60 triaged | 20 | ~1 | 2026-02-23 |
 | 02 | Network, SSRF, and Filesystem Hardening | Complete | 14 (5A+9Ad) | 20 | ~1 | 2026-02-23 |
-| 03 | Input Sanitization and Auth Hardening | Not Started | 16 (8A+8Ad) | 18-24 | 4-5 | - |
+| 03 | Input Sanitization and Auth Hardening | Complete | 16 (8A+8Ad) | 20 | ~1 | 2026-02-23 |
 | 04 | Execution Hardening and Data Leak Prevention | Not Started | 17 (5A+12Ad) | 20-26 | 5-6 | - |
 | 05 | ACP Fixes and Security Validation | Not Started | 5 (0A+5Ad) | 15-20 | 3-4 | - |
 
@@ -56,9 +56,18 @@ Audit and port ~65 upstream security patches covering SSRF bypasses, path traver
 
 ---
 
+### Session 03: Input Sanitization and Auth Hardening
+- **Completed**: 2026-02-23
+- **Duration**: ~53 minutes
+- **Tasks**: 20/20
+- **Result**: Closed 16 security items (8 Apply + 8 Adapt). Input sanitization: chat.send injection prevention, transcript sanitization, Unicode homoglyph detection, base64 size validation, bounded HTTP body reading, rawCommand/argv consistency. Auth hardening: OAuth CSRF protection, timing-safe token comparison, auth rate-limiting with brute-force lockout, exec approval device binding, system.run param allowlisting, node.invoke bypass prevention. 4 new infrastructure modules created. 113 security-focused tests added.
+
+---
+
 ## Upcoming Sessions
 
-- Session 03: Input Sanitization and Auth Hardening
+- Session 04: Execution Hardening and Data Leak Prevention
+- Session 05: ACP Fixes and Security Validation
 
 ---
 

.spec_system/specs/phase16-session03-input-sanitization-and-auth-hardening/IMPLEMENTATION_SUMMARY.md

@@ -0,0 +1,107 @@
+# Implementation Summary
+
+**Session ID**: `phase16-session03-input-sanitization-and-auth-hardening`
+**Completed**: 2026-02-23
+**Duration**: ~53 minutes
+
+---
+
+## Overview
+
+Ported 16 upstream security fixes (8 Apply + 8 Adapt) covering injection prevention, input sanitization, authentication hardening, and exec approval tightening. Created 4 new infrastructure modules and modified 25 existing files. Closed all Section 1.3 (Injection & Input Sanitization) and Section 1.4 (Auth & Access Control) items from the phase 16 triage.
+
+---
+
+## Deliverables
+
+### Files Created
+| File | Purpose | Lines |
+|------|---------|-------|
+| `src/security/secret-equal.ts` | Timing-safe string comparison via crypto.timingSafeEqual | ~25 |
+| `src/security/secret-equal.test.ts` | Unit tests for timing-safe comparison | ~40 |
+| `src/media/base64.ts` | Oversized base64 rejection before decode | ~45 |
+| `src/media/base64.test.ts` | Unit tests for base64 validation | ~70 |
+| `src/infra/http-body.ts` | Bounded HTTP body reading with configurable size limits | ~280 |
+| `src/infra/http-body.test.ts` | Unit tests for bounded body reader | ~170 |
+| `src/gateway/auth-rate-limit.ts` | Sliding-window per-IP auth rate limiting with lockout | ~190 |
+| `src/gateway/auth-rate-limit.test.ts` | Unit + integration tests for auth rate-limiting | ~130 |
+
+### Files Modified
+| File | Changes |
+|------|---------|
+| `src/gateway/server-methods/chat.ts` | Null byte stripping and message length limit on chat.send |
+| `src/agents/session-transcript-repair.ts` | Hardened tool-call block sanitization against injection |
+| `src/agents/session-transcript-repair.test.ts` | Added sanitization test cases |
+| `src/security/external-content.ts` | Added Unicode angle bracket homoglyph detection (U+FF1C, U+FF1E, U+FE64, U+FE65, etc.) |
+| `src/security/external-content.test.ts` | Tests for new homoglyph patterns |
+| `src/gateway/server-methods/agents.ts` | Replaced unsafe .toString() with safe stringification |
+| `src/gateway/chat-attachments.ts` | Integrated base64 size validation before decode |
+| `src/media/input-files.ts` | Integrated base64 size validation |
+| `src/gateway/server-http.ts` | Integrated bounded body reading; wired auth rate-limiter |
+| `src/gateway/http-common.ts` | Used bounded body reader for JSON/text parsing |
+| `src/node-host/runner.ts` | Enforced rawCommand/argv consistency in system.run |
+| `src/commands/status.summary.ts` | Redacted sensitive details for non-admin scopes |
+| `src/agents/chutes-oauth.ts` | Validated OAuth state parameter on callback |
+| `src/commands/chutes-oauth.test.ts` | Tests for OAuth CSRF validation |
+| `src/commands/onboard-helpers.ts` | Rejected literal "undefined"/"null" tokens |
+| `src/wizard/onboarding.gateway-config.ts` | Rejected literal "undefined"/"null" tokens |
+| `src/gateway/auth.ts` | Integrated secretEqual for timing-safe token comparison; wired rate-limiter |
+| `src/gateway/auth.test.ts` | Tests for secretEqual integration and rate-limit behavior |
+| `src/gateway/node-command-policy.ts` | Added EXEC_APPROVAL_REQUIRED_COMMANDS and requiresExecApproval() |
+| `src/gateway/server-methods/nodes.ts` | Integrated sanitizer into node.invoke; added approval-required check |
+| `src/gateway/server-methods/exec-approval.ts` | Added device binding, self-approval guard, param sanitizer |
+| `src/gateway/server-methods/exec-approval.test.ts` | Integration tests for device binding, self-approval, param sanitization |
+| `src/gateway/exec-approval-manager.ts` | Added validateDeviceBinding() method |
+| `src/gateway/server-methods/types.ts` | Added execApprovalManager to GatewayRequestContext |
+| `src/gateway/server.impl.ts` | Wired auth rate-limiter and exec approval manager into server init |
+| `src/commands/auth-choice.test.ts` | Fixed pre-existing lint issue (no-base-to-string) |
+
+---
+
+## Technical Decisions
+
+1. **Sanitizer placement in exec-approval.ts**: Placed system.run param sanitizer inline in exec-approval.ts rather than a separate file (upstream pattern). Rationale: crocbot's simpler architecture means the sanitizer is tightly coupled to ExecApprovalManager and called from one place only.
+2. **Self-approval check uses clientId**: Chose clientId over deviceId or connId for self-approval prevention. Rationale: deviceId would be too restrictive for desktop users where CLI and UI run on same device but with different client IDs.
+3. **Sliding-window rate limiter**: Implemented separate auth-specific sliding-window rate limiter rather than extending existing fixed-window rate-limit.ts. Rationale: auth rate-limiting needs per-IP sliding window with lockout semantics; different algorithm and scope from general HTTP rate limiting.
+4. **Base64 size threshold**: Used upstream-aligned threshold value to balance legitimate image attachments against abuse prevention.
+
+---
+
+## Test Results
+
+| Metric | Value |
+|--------|-------|
+| Test Files | 778 |
+| Total Tests | 5935 |
+| Passed | 5935 |
+| Failed | 0 |
+| Skipped | 1 |
+| New Security Tests | 113 |
+
+---
+
+## Lessons Learned
+
+1. Exec approval chain (T015-T017) required strict sequential implementation due to tight coupling between device binding, param sanitization, and bypass prevention.
+2. Upstream file mapping is not 1:1 -- crocbot's runner.ts absorbs logic that upstream splits across invoke.ts and node-invoke-system-run-approval.ts.
+3. Pre-existing non-ASCII characters in modified files need to be documented but not "fixed" to avoid unnecessary churn.
+
+---
+
+## Future Considerations
+
+Items for future sessions:
+1. Session 04 (Execution Hardening and Data Leak Prevention) depends on the exec approval chain completed here
+2. Session 05 (ACP Fixes) depends on all prior sessions including the input sanitization and auth guards
+3. Auth rate-limiter could be extended to cover non-auth endpoints if abuse patterns emerge
+4. Consider adding rate-limit telemetry/logging for security monitoring
+
+---
+
+## Session Statistics
+
+- **Tasks**: 20 completed
+- **Files Created**: 8
+- **Files Modified**: 26
+- **Tests Added**: 113
+- **Blockers**: 0 resolved

.spec_system/state.json

@@ -9,7 +9,7 @@
       "name": "Critical Security Hardening II",
       "description": "Audit and port ~65 upstream security patches covering SSRF bypasses, path traversal, input sanitization, auth hardening, execution safety, data leak prevention, and ACP policy fixes.",
       "status": "in_progress",
-      "sessions_completed": 2,
+      "sessions_completed": 3,
       "session_count": 5
     },
     "17": {
@@ -50,7 +50,8 @@
   },
   "completed_sessions": [
     "phase16-session01-security-triage-and-applicability-audit",
-    "phase16-session02-network-ssrf-and-filesystem-hardening"
+    "phase16-session02-network-ssrf-and-filesystem-hardening",
+    "phase16-session03-input-sanitization-and-auth-hardening"
   ],
   "next_session_history": [
     {
@@ -62,6 +63,11 @@
       "date": "2026-02-23",
       "session": "phase16-session02-network-ssrf-and-filesystem-hardening",
       "status": "completed"
+    },
+    {
+      "date": "2026-02-23",
+      "session": "phase16-session03-input-sanitization-and-auth-hardening",
+      "status": "completed"
     }
   ]
 }

package.json

@@ -1,6 +1,6 @@
 {
   "name": "crocbot",
-  "version": "0.1.156",
+  "version": "0.1.157",
   "description": "Telegram gateway CLI with Pi RPC agent",
   "type": "module",
   "main": "dist/index.js",

src/agents/chutes-oauth.ts

@@ -53,12 +53,23 @@ export function parseOAuthCallbackInput(
     if (!state) {
       return { error: "Missing 'state' parameter. Paste the full URL." };
     }
+    if (state !== expectedState) {
+      return { error: "OAuth state mismatch -- possible CSRF. Restart the flow." };
+    }
     return { code, state };
   } catch {
-    if (!expectedState) {
-      return { error: "Paste the full redirect URL, not just the code." };
+    // Try parsing as a query string (e.g. "code=abc&state=xyz")
+    const qs = new URLSearchParams(trimmed);
+    const qsCode = qs.get("code");
+    const qsState = qs.get("state");
+    if (qsCode && qsState) {
+      if (qsState !== expectedState) {
+        return { error: "OAuth state mismatch -- possible CSRF. Restart the flow." };
+      }
+      return { code: qsCode, state: qsState };
     }
-    return { code: trimmed, state: expectedState };
+    // Reject bare code pastes: require the full URL for CSRF safety
+    return { error: "Paste the full redirect URL, not just the code." };
   }
 }
 

src/agents/session-transcript-repair.test.ts

@@ -148,3 +148,51 @@ describe("sanitizeToolCallInputs", () => {
     expect(types).toEqual(["text", "toolUse"]);
   });
 });
+
+describe("tool-call block field validation", () => {
+  it("skips tool calls with empty id", () => {
+    const input: AgentMessage[] = [
+      {
+        role: "assistant",
+        content: [{ type: "toolCall", id: "", name: "read", arguments: {} }],
+      },
+      { role: "user", content: "hello" },
+    ];
+
+    const out = sanitizeToolUseResultPairing(input);
+    const assistant = out[0] as Extract<AgentMessage, { role: "assistant" }>;
+    expect(Array.isArray(assistant.content) ? assistant.content.length : 0).toBe(1);
+  });
+
+  it("skips tool calls with oversized id", () => {
+    const longId = "x".repeat(200);
+    const input: AgentMessage[] = [
+      {
+        role: "assistant",
+        content: [{ type: "toolCall", id: longId, name: "read", arguments: {} }],
+      },
+      { role: "user", content: "hello" },
+    ];
+
+    const out = sanitizeToolUseResultPairing(input);
+    const results = out.filter((m) => m.role === "toolResult");
+    // No synthetic result for oversized ID - it gets truncated or skipped
+    expect(results.length).toBe(0);
+  });
+
+  it("sanitizes tool names with invalid characters", () => {
+    const input: AgentMessage[] = [
+      {
+        role: "assistant",
+        content: [{ type: "toolCall", id: "call_1", name: "read<script>", arguments: {} }],
+      },
+    ];
+
+    const out = sanitizeToolUseResultPairing(input);
+    const results = out.filter((m) => m.role === "toolResult") as Array<{
+      toolName?: string;
+    }>;
+    expect(results.length).toBe(1);
+    expect(results[0]?.toolName).not.toContain("<");
+  });
+});

src/agents/session-transcript-repair.ts

@@ -7,6 +7,10 @@ type ToolCallLike = {
 
 const TOOL_CALL_TYPES = new Set(["toolCall", "toolUse", "functionCall"]);
 
+const MAX_TOOL_CALL_ID_LENGTH = 128;
+const MAX_TOOL_NAME_LENGTH = 64;
+const TOOL_NAME_PATTERN = /^[A-Za-z0-9_\-.]+$/;
+
 type ToolCallBlock = {
   type?: unknown;
   id?: unknown;
@@ -23,6 +27,17 @@ function isToolCallBlock(block: unknown): block is ToolCallBlock {
   return typeof type === "string" && TOOL_CALL_TYPES.has(type);
 }
 
+function isValidToolCallId(id: unknown): boolean {
+  return typeof id === "string" && id.length > 0 && id.length <= MAX_TOOL_CALL_ID_LENGTH;
+}
+
+function isValidToolName(name: unknown): boolean {
+  if (typeof name !== "string") {
+    return true;
+  }
+  return name.length <= MAX_TOOL_NAME_LENGTH && TOOL_NAME_PATTERN.test(name);
+}
+
 function hasToolCallInput(block: ToolCallBlock): boolean {
   const hasInput = "input" in block ? block.input !== undefined && block.input !== null : false;
   const hasArguments =
@@ -44,15 +59,20 @@ function extractToolCallsFromAssistant(
       continue;
     }
     const rec = block as { type?: unknown; id?: unknown; name?: unknown };
-    if (typeof rec.id !== "string" || !rec.id) {
+    if (!isValidToolCallId(rec.id)) {
       continue;
     }
 
     if (rec.type === "toolCall" || rec.type === "toolUse" || rec.type === "functionCall") {
-      toolCalls.push({
-        id: rec.id,
-        name: typeof rec.name === "string" ? rec.name : undefined,
-      });
+      const id = (rec.id as string).slice(0, MAX_TOOL_CALL_ID_LENGTH);
+      const rawName = typeof rec.name === "string" ? rec.name : undefined;
+      const name =
+        rawName && isValidToolName(rawName)
+          ? rawName
+          : rawName
+            ? rawName.slice(0, MAX_TOOL_NAME_LENGTH).replace(/[^A-Za-z0-9_\-.]/g, "_")
+            : undefined;
+      toolCalls.push({ id, name });
     }
   }
   return toolCalls;

src/commands/auth-choice.test.ts

@@ -437,7 +437,19 @@ describe("applyAuthChoice", () => {
     });
     vi.stubGlobal("fetch", fetchSpy);
 
-    const text = vi.fn().mockResolvedValue("code_manual");
+    let capturedState = "";
+    const logFn = vi.fn((...args: unknown[]) => {
+      const msg = typeof args[0] === "string" ? args[0] : "";
+      const match = /[?&]state=([^&\s]+)/.exec(msg);
+      if (match) {
+        capturedState = match[1];
+      }
+    });
+    const text = vi
+      .fn()
+      .mockImplementation(
+        async () => `http://127.0.0.1:1456/oauth-callback?code=code_manual&state=${capturedState}`,
+      );
     const select: WizardPrompter["select"] = vi.fn(
       async (params) => params.options[0]?.value as never,
     );
@@ -453,7 +465,7 @@ describe("applyAuthChoice", () => {
       progress: vi.fn(() => ({ update: noop, stop: noop })),
     };
     const runtime: RuntimeEnv = {
-      log: vi.fn(),
+      log: logFn,
       error: vi.fn(),
       exit: vi.fn((code: number) => {
         throw new Error(`exit:${code}`);

src/commands/chutes-oauth.test.ts

@@ -68,7 +68,7 @@ describe("loginChutes", () => {
     expect(creds.email).toBe("local-user");
   });
 
-  it("supports manual flow with pasted code", async () => {
+  it("supports manual flow with pasted redirect URL", async () => {
     const fetchFn: typeof fetch = async (input) => {
       const url = String(input);
       if (url === CHUTES_TOKEN_ENDPOINT) {
@@ -90,15 +90,19 @@ describe("loginChutes", () => {
       return new Response("not found", { status: 404 });
     };
 
+    let capturedState = "";
     const creds = await loginChutes({
       app: {
         clientId: "cid_test",
         redirectUri: "http://127.0.0.1:1456/oauth-callback",
         scopes: ["openid"],
       },
       manual: true,
-      onAuth: async () => {},
-      onPrompt: async () => "code_manual",
+      onAuth: async ({ url }) => {
+        capturedState = new URL(url).searchParams.get("state") ?? "";
+      },
+      onPrompt: async () =>
+        `http://127.0.0.1:1456/oauth-callback?code=code_manual&state=${capturedState}`,
       fetchFn,
     });
 
@@ -149,7 +153,7 @@ describe("loginChutes", () => {
         expect(parsed.searchParams.get("state")).toBe("state_456");
         expect(parsed.searchParams.get("state")).not.toBe("verifier_123");
       },
-      onPrompt: async () => "code_manual",
+      onPrompt: async () => "http://127.0.0.1:1456/oauth-callback?code=code_manual&state=state_456",
       fetchFn,
     });