From 45823fcedecb4ecd516a86b281b28bbb8ea8e5ff Mon Sep 17 00:00:00 2001
From: seto <hirokazu.seto@gmail.com>
Date: Mon, 13 Apr 2026 15:30:34 +0900
Subject: [PATCH] fix: session-scoped state to prevent cross-session race

Addresses reviewer feedback from @affaan-m:

1. State keyed by CLAUDE_SESSION_ID / ECC_SESSION_ID
   - Falls back to pid-based isolation when env vars absent
   - State file: state-{sessionId}.json (was .session_state.json)

2. Atomic write+rename semantics
   - Write to temp file, then fs.renameSync to final path
   - Prevents partial reads from concurrent hooks

3. Bounded checked list (MAX_CHECKED_ENTRIES = 500)
   - Prunes to last 500 entries when cap exceeded
   - Stale session files auto-deleted after 1 hour

9/9 tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 scripts/hooks/gateguard-fact-force.js    | 41 ++++++++++++++++++++----
 tests/hooks/gateguard-fact-force.test.js |  4 ++-
 2 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/scripts/hooks/gateguard-fact-force.js b/scripts/hooks/gateguard-fact-force.js
index 76e1665e..4126c545 100644
--- a/scripts/hooks/gateguard-fact-force.js
+++ b/scripts/hooks/gateguard-fact-force.js
@@ -25,16 +25,21 @@
 const fs = require('fs');
 const path = require('path');
 
-// Session state file for tracking which files have been gated
+// Session state — scoped per session to avoid cross-session races.
+// Uses CLAUDE_SESSION_ID (set by Claude Code) or falls back to PID-based isolation.
 const STATE_DIR = process.env.GATEGUARD_STATE_DIR || path.join(process.env.HOME || process.env.USERPROFILE || '/tmp', '.gateguard');
-const STATE_FILE = path.join(STATE_DIR, '.session_state.json');
+const SESSION_ID = process.env.CLAUDE_SESSION_ID || process.env.ECC_SESSION_ID || `pid-${process.ppid || process.pid}`;
+const STATE_FILE = path.join(STATE_DIR, `state-${SESSION_ID.replace(/[^a-zA-Z0-9_-]/g, '_')}.json`);
 
-// State expires after 30 minutes of inactivity (= new session)
+// State expires after 30 minutes of inactivity
 const SESSION_TIMEOUT_MS = 30 * 60 * 1000;
 
+// Maximum checked entries to prevent unbounded growth
+const MAX_CHECKED_ENTRIES = 500;
+
 const DESTRUCTIVE_BASH = /\b(rm\s+-rf|git\s+reset\s+--hard|git\s+checkout\s+--|git\s+clean\s+-f|drop\s+table|delete\s+from|truncate|git\s+push\s+--force|dd\s+if=)\b/i;
 
-// --- State management (with session timeout) ---
+// --- State management (per-session, atomic writes, bounded) ---
 
 function loadState() {
   try {
@@ -42,7 +47,7 @@ function loadState() {
       const state = JSON.parse(fs.readFileSync(STATE_FILE, 'utf8'));
       const lastActive = state.last_active || 0;
       if (Date.now() - lastActive > SESSION_TIMEOUT_MS) {
-        // Session expired — start fresh
+        try { fs.unlinkSync(STATE_FILE); } catch (_) { /* ignore */ }
         return { checked: [], last_active: Date.now() };
       }
       return state;
@@ -54,8 +59,15 @@ function loadState() {
 function saveState(state) {
   try {
     state.last_active = Date.now();
+    // Prune checked list if it exceeds the cap
+    if (state.checked.length > MAX_CHECKED_ENTRIES) {
+      state.checked = state.checked.slice(-MAX_CHECKED_ENTRIES);
+    }
     fs.mkdirSync(STATE_DIR, { recursive: true });
-    fs.writeFileSync(STATE_FILE, JSON.stringify(state, null, 2), 'utf8');
+    // Atomic write: temp file + rename prevents partial reads
+    const tmpFile = STATE_FILE + '.tmp.' + process.pid;
+    fs.writeFileSync(tmpFile, JSON.stringify(state, null, 2), 'utf8');
+    fs.renameSync(tmpFile, STATE_FILE);
   } catch (_) { /* ignore */ }
 }
 
@@ -69,11 +81,26 @@ function markChecked(key) {
 
 function isChecked(key) {
   const state = loadState();
-  // Touch last_active on every check
   saveState(state);
   return state.checked.includes(key);
 }
 
+// Prune stale session files older than 1 hour
+(function pruneStaleFiles() {
+  try {
+    const files = fs.readdirSync(STATE_DIR);
+    const now = Date.now();
+    for (const f of files) {
+      if (!f.startsWith('state-') || !f.endsWith('.json')) continue;
+      const fp = path.join(STATE_DIR, f);
+      const stat = fs.statSync(fp);
+      if (now - stat.mtimeMs > SESSION_TIMEOUT_MS * 2) {
+        fs.unlinkSync(fp);
+      }
+    }
+  } catch (_) { /* ignore */ }
+})();
+
 // --- Sanitize file path against injection ---
 
 function sanitizePath(filePath) {
diff --git a/tests/hooks/gateguard-fact-force.test.js b/tests/hooks/gateguard-fact-force.test.js
index ef1f9905..9ff3e432 100644
--- a/tests/hooks/gateguard-fact-force.test.js
+++ b/tests/hooks/gateguard-fact-force.test.js
@@ -9,7 +9,9 @@ const { spawnSync } = require('child_process');
 
 const runner = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js');
 const stateDir = process.env.GATEGUARD_STATE_DIR || path.join(process.env.HOME || process.env.USERPROFILE || '/tmp', '.gateguard-test-' + process.pid);
-const stateFile = path.join(stateDir, '.session_state.json');
+// State files are per-session. In tests, falls back to pid-based name.
+const sessionId = process.env.CLAUDE_SESSION_ID || process.env.ECC_SESSION_ID || `pid-${process.ppid || process.pid}`;
+const stateFile = path.join(stateDir, `state-${sessionId.replace(/[^a-zA-Z0-9_-]/g, '_')}.json`);
 
 function test(name, fn) {
   try {