mirror of
https://github.com/affaan-m/everything-claude-code.git
synced 2026-04-14 22:13:41 +08:00
fix: address P1 review feedback from greptile bot
1. Use run-with-flags.js wrapper (supports ECC_HOOK_PROFILE, ECC_DISABLED_HOOKS) 2. Add session timeout (30min inactivity = state reset, fixes "once ever" bug) 3. Add 9 integration tests (deny/allow/timeout/sanitize/disable) Refactored hook to module.exports.run() pattern for direct require() by run-with-flags.js (~50-100ms faster per invocation). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
seto
@@ -132,7 +132,7 @@
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/gateguard-fact-force.js\"",
|
||||
"command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/run-with-flags.js\" \"pre:edit-write:gateguard-fact-force\" \"scripts/hooks/gateguard-fact-force.js\" \"standard,strict\"",
|
||||
"timeout": 5
|
||||
}
|
||||
],
|
||||
@@ -144,7 +144,7 @@
|
||||
"hooks": [
|
||||
{
|
||||
"type": "command",
|
||||
"command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/gateguard-fact-force.js\"",
|
||||
"command": "node \"${CLAUDE_PLUGIN_ROOT}/scripts/hooks/run-with-flags.js\" \"pre:bash:gateguard-fact-force\" \"scripts/hooks/gateguard-fact-force.js\" \"standard,strict\"",
|
||||
"timeout": 5
|
||||
}
|
||||
],
|
||||
|
||||
@@ -13,10 +13,7 @@
|
||||
* - Bash (destructive): list targets, rollback plan, quote instruction
|
||||
* - Bash (routine): quote current instruction (once per session)
|
||||
*
|
||||
* Exit codes:
|
||||
* 0 - Allow (gate already passed for this target)
|
||||
* 2 - Block (force investigation first)
|
||||
*
|
||||
* Compatible with run-with-flags.js via module.exports.run().
|
||||
* Cross-platform (Windows, macOS, Linux).
|
||||
*
|
||||
* Full package with config support: pip install gateguard-ai
|
||||
@@ -28,27 +25,35 @@
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
|
||||
const MAX_STDIN = 1024 * 1024;
|
||||
|
||||
// Session state file for tracking which files have been gated
|
||||
const STATE_DIR = path.join(process.env.HOME || process.env.USERPROFILE || '/tmp', '.gateguard');
|
||||
const STATE_FILE = path.join(STATE_DIR, '.session_state.json');
|
||||
|
||||
// State expires after 30 minutes of inactivity (= new session)
|
||||
const SESSION_TIMEOUT_MS = 30 * 60 * 1000;
|
||||
|
||||
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 ---
|
||||
// --- State management (with session timeout) ---
|
||||
|
||||
function loadState() {
|
||||
try {
|
||||
if (fs.existsSync(STATE_FILE)) {
|
||||
return JSON.parse(fs.readFileSync(STATE_FILE, 'utf8'));
|
||||
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
|
||||
return { checked: [], last_active: Date.now() };
|
||||
}
|
||||
return state;
|
||||
}
|
||||
} catch (_) { /* ignore */ }
|
||||
return { checked: [], read_files: [] };
|
||||
return { checked: [], last_active: Date.now() };
|
||||
}
|
||||
|
||||
function saveState(state) {
|
||||
try {
|
||||
state.last_active = Date.now();
|
||||
fs.mkdirSync(STATE_DIR, { recursive: true });
|
||||
fs.writeFileSync(STATE_FILE, JSON.stringify(state, null, 2), 'utf8');
|
||||
} catch (_) { /* ignore */ }
|
||||
@@ -64,6 +69,8 @@ function markChecked(key) {
|
||||
|
||||
function isChecked(key) {
|
||||
const state = loadState();
|
||||
// Touch last_active on every check
|
||||
saveState(state);
|
||||
return state.checked.includes(key);
|
||||
}
|
||||
|
||||
@@ -130,42 +137,29 @@ function routineBashMsg() {
|
||||
].join('\n');
|
||||
}
|
||||
|
||||
// --- Output helpers ---
|
||||
// --- Deny helper ---
|
||||
|
||||
function deny(reason) {
|
||||
const output = {
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PreToolUse',
|
||||
permissionDecision: 'deny',
|
||||
permissionDecisionReason: reason
|
||||
}
|
||||
function denyResult(reason) {
|
||||
return {
|
||||
stdout: JSON.stringify({
|
||||
hookSpecificOutput: {
|
||||
hookEventName: 'PreToolUse',
|
||||
permissionDecision: 'deny',
|
||||
permissionDecisionReason: reason
|
||||
}
|
||||
}),
|
||||
exitCode: 0
|
||||
};
|
||||
process.stdout.write(JSON.stringify(output));
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
function allow() {
|
||||
// Output nothing = allow
|
||||
process.exit(0);
|
||||
}
|
||||
|
||||
// --- Main ---
|
||||
|
||||
function main() {
|
||||
let raw = '';
|
||||
try {
|
||||
raw = fs.readFileSync(0, 'utf8').slice(0, MAX_STDIN);
|
||||
} catch (_) {
|
||||
allow();
|
||||
return;
|
||||
}
|
||||
// --- Core logic (exported for run-with-flags.js) ---
|
||||
|
||||
function run(rawInput) {
|
||||
let data;
|
||||
try {
|
||||
data = JSON.parse(raw);
|
||||
data = typeof rawInput === 'string' ? JSON.parse(rawInput) : rawInput;
|
||||
} catch (_) {
|
||||
allow();
|
||||
return;
|
||||
return rawInput; // allow on parse error
|
||||
}
|
||||
|
||||
const toolName = data.tool_name || '';
|
||||
@@ -174,43 +168,34 @@ function main() {
|
||||
if (toolName === 'Edit' || toolName === 'Write') {
|
||||
const filePath = toolInput.file_path || '';
|
||||
if (!filePath) {
|
||||
allow();
|
||||
return;
|
||||
return rawInput; // allow
|
||||
}
|
||||
|
||||
// Gate: first action per file
|
||||
if (!isChecked(filePath)) {
|
||||
markChecked(filePath);
|
||||
const msg = toolName === 'Edit' ? editGateMsg(filePath) : writeGateMsg(filePath);
|
||||
deny(msg);
|
||||
return;
|
||||
return denyResult(msg);
|
||||
}
|
||||
|
||||
allow();
|
||||
return;
|
||||
return rawInput; // allow
|
||||
}
|
||||
|
||||
if (toolName === 'Bash') {
|
||||
const command = toolInput.command || '';
|
||||
|
||||
// Destructive commands: always gate
|
||||
if (DESTRUCTIVE_BASH.test(command)) {
|
||||
deny(destructiveBashMsg());
|
||||
return;
|
||||
return denyResult(destructiveBashMsg());
|
||||
}
|
||||
|
||||
// Routine bash: once per session
|
||||
if (!isChecked('__bash_session__')) {
|
||||
markChecked('__bash_session__');
|
||||
deny(routineBashMsg());
|
||||
return;
|
||||
return denyResult(routineBashMsg());
|
||||
}
|
||||
|
||||
allow();
|
||||
return;
|
||||
return rawInput; // allow
|
||||
}
|
||||
|
||||
allow();
|
||||
return rawInput; // allow
|
||||
}
|
||||
|
||||
main();
|
||||
module.exports = { run };
|
||||
|
||||
261
tests/hooks/gateguard-fact-force.test.js
Normal file
261
tests/hooks/gateguard-fact-force.test.js
Normal file
@@ -0,0 +1,261 @@
|
||||
/**
|
||||
* Tests for scripts/hooks/gateguard-fact-force.js via run-with-flags.js
|
||||
*/
|
||||
|
||||
const assert = require('assert');
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const { spawnSync } = require('child_process');
|
||||
|
||||
const runner = path.join(__dirname, '..', '..', 'scripts', 'hooks', 'run-with-flags.js');
|
||||
const stateDir = path.join(process.env.HOME || process.env.USERPROFILE || '/tmp', '.gateguard');
|
||||
const stateFile = path.join(stateDir, '.session_state.json');
|
||||
|
||||
function test(name, fn) {
|
||||
try {
|
||||
fn();
|
||||
console.log(` ✓ ${name}`);
|
||||
return true;
|
||||
} catch (error) {
|
||||
console.log(` ✗ ${name}`);
|
||||
console.log(` Error: ${error.message}`);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function clearState() {
|
||||
try {
|
||||
if (fs.existsSync(stateFile)) {
|
||||
fs.unlinkSync(stateFile);
|
||||
}
|
||||
} catch (_) { /* ignore */ }
|
||||
}
|
||||
|
||||
function writeExpiredState() {
|
||||
try {
|
||||
fs.mkdirSync(stateDir, { recursive: true });
|
||||
const expired = {
|
||||
checked: ['some_file.js', '__bash_session__'],
|
||||
last_active: Date.now() - (31 * 60 * 1000) // 31 minutes ago
|
||||
};
|
||||
fs.writeFileSync(stateFile, JSON.stringify(expired), 'utf8');
|
||||
} catch (_) { /* ignore */ }
|
||||
}
|
||||
|
||||
function runHook(input, env = {}) {
|
||||
const rawInput = typeof input === 'string' ? input : JSON.stringify(input);
|
||||
const result = spawnSync('node', [
|
||||
runner,
|
||||
'pre:edit-write:gateguard-fact-force',
|
||||
'scripts/hooks/gateguard-fact-force.js',
|
||||
'standard,strict'
|
||||
], {
|
||||
input: rawInput,
|
||||
encoding: 'utf8',
|
||||
env: {
|
||||
...process.env,
|
||||
ECC_HOOK_PROFILE: 'standard',
|
||||
...env
|
||||
},
|
||||
timeout: 15000,
|
||||
stdio: ['pipe', 'pipe', 'pipe']
|
||||
});
|
||||
|
||||
return {
|
||||
code: Number.isInteger(result.status) ? result.status : 1,
|
||||
stdout: result.stdout || '',
|
||||
stderr: result.stderr || ''
|
||||
};
|
||||
}
|
||||
|
||||
function runBashHook(input, env = {}) {
|
||||
const rawInput = typeof input === 'string' ? input : JSON.stringify(input);
|
||||
const result = spawnSync('node', [
|
||||
runner,
|
||||
'pre:bash:gateguard-fact-force',
|
||||
'scripts/hooks/gateguard-fact-force.js',
|
||||
'standard,strict'
|
||||
], {
|
||||
input: rawInput,
|
||||
encoding: 'utf8',
|
||||
env: {
|
||||
...process.env,
|
||||
ECC_HOOK_PROFILE: 'standard',
|
||||
...env
|
||||
},
|
||||
timeout: 15000,
|
||||
stdio: ['pipe', 'pipe', 'pipe']
|
||||
});
|
||||
|
||||
return {
|
||||
code: Number.isInteger(result.status) ? result.status : 1,
|
||||
stdout: result.stdout || '',
|
||||
stderr: result.stderr || ''
|
||||
};
|
||||
}
|
||||
|
||||
function parseOutput(stdout) {
|
||||
try {
|
||||
return JSON.parse(stdout);
|
||||
} catch (_) {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function runTests() {
|
||||
console.log('\n=== Testing gateguard-fact-force ===\n');
|
||||
|
||||
let passed = 0;
|
||||
let failed = 0;
|
||||
|
||||
// --- Test 1: denies first Edit per file ---
|
||||
clearState();
|
||||
if (test('denies first Edit per file with fact-forcing message', () => {
|
||||
const input = {
|
||||
tool_name: 'Edit',
|
||||
tool_input: { file_path: '/src/app.js', old_string: 'foo', new_string: 'bar' }
|
||||
};
|
||||
const result = runHook(input);
|
||||
const output = parseOutput(result.stdout);
|
||||
assert.ok(output, 'should produce JSON output');
|
||||
assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny');
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Fact-Forcing Gate'));
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('import/require'));
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('/src/app.js'));
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 2: allows second Edit on same file ---
|
||||
if (test('allows second Edit on same file (gate already passed)', () => {
|
||||
const input = {
|
||||
tool_name: 'Edit',
|
||||
tool_input: { file_path: '/src/app.js', old_string: 'foo', new_string: 'bar' }
|
||||
};
|
||||
const result = runHook(input);
|
||||
const output = parseOutput(result.stdout);
|
||||
if (output && output.hookSpecificOutput) {
|
||||
assert.notStrictEqual(output.hookSpecificOutput.permissionDecision, 'deny',
|
||||
'should not deny second edit on same file');
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 3: denies first Write per file ---
|
||||
clearState();
|
||||
if (test('denies first Write per file with fact-forcing message', () => {
|
||||
const input = {
|
||||
tool_name: 'Write',
|
||||
tool_input: { file_path: '/src/new-file.js', content: 'console.log("hello")' }
|
||||
};
|
||||
const result = runHook(input);
|
||||
const output = parseOutput(result.stdout);
|
||||
assert.ok(output, 'should produce JSON output');
|
||||
assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny');
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('creating'));
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('call this new file'));
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 4: denies destructive Bash ---
|
||||
clearState();
|
||||
if (test('denies destructive Bash commands', () => {
|
||||
const input = {
|
||||
tool_name: 'Bash',
|
||||
tool_input: { command: 'rm -rf /important/data' }
|
||||
};
|
||||
const result = runBashHook(input);
|
||||
const output = parseOutput(result.stdout);
|
||||
assert.ok(output, 'should produce JSON output');
|
||||
assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny');
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('Destructive'));
|
||||
assert.ok(output.hookSpecificOutput.permissionDecisionReason.includes('rollback'));
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 5: denies first routine Bash, allows second ---
|
||||
clearState();
|
||||
if (test('denies first routine Bash, allows second', () => {
|
||||
const input = {
|
||||
tool_name: 'Bash',
|
||||
tool_input: { command: 'ls -la' }
|
||||
};
|
||||
|
||||
// First call: should deny
|
||||
const result1 = runBashHook(input);
|
||||
const output1 = parseOutput(result1.stdout);
|
||||
assert.ok(output1, 'first call should produce JSON output');
|
||||
assert.strictEqual(output1.hookSpecificOutput.permissionDecision, 'deny');
|
||||
|
||||
// Second call: should allow
|
||||
const result2 = runBashHook(input);
|
||||
const output2 = parseOutput(result2.stdout);
|
||||
if (output2 && output2.hookSpecificOutput) {
|
||||
assert.notStrictEqual(output2.hookSpecificOutput.permissionDecision, 'deny');
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 6: session state resets after timeout ---
|
||||
if (test('session state resets after 30-minute timeout', () => {
|
||||
writeExpiredState();
|
||||
const input = {
|
||||
tool_name: 'Edit',
|
||||
tool_input: { file_path: 'some_file.js', old_string: 'a', new_string: 'b' }
|
||||
};
|
||||
const result = runHook(input);
|
||||
const output = parseOutput(result.stdout);
|
||||
assert.ok(output, 'should produce JSON output after expired state');
|
||||
assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny',
|
||||
'should deny again after session timeout (state was reset)');
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 7: allows unknown tool names ---
|
||||
clearState();
|
||||
if (test('allows unknown tool names through', () => {
|
||||
const input = {
|
||||
tool_name: 'Read',
|
||||
tool_input: { file_path: '/src/app.js' }
|
||||
};
|
||||
const result = runHook(input);
|
||||
const output = parseOutput(result.stdout);
|
||||
if (output && output.hookSpecificOutput) {
|
||||
assert.notStrictEqual(output.hookSpecificOutput.permissionDecision, 'deny');
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 8: sanitizes file paths with newlines ---
|
||||
clearState();
|
||||
if (test('sanitizes file paths containing newlines', () => {
|
||||
const input = {
|
||||
tool_name: 'Edit',
|
||||
tool_input: { file_path: '/src/app.js\ninjected content', old_string: 'a', new_string: 'b' }
|
||||
};
|
||||
const result = runHook(input);
|
||||
const output = parseOutput(result.stdout);
|
||||
assert.ok(output, 'should produce JSON output');
|
||||
assert.strictEqual(output.hookSpecificOutput.permissionDecision, 'deny');
|
||||
const reason = output.hookSpecificOutput.permissionDecisionReason;
|
||||
assert.ok(!reason.includes('injected content\n'), 'newline injection should be sanitized');
|
||||
})) passed++; else failed++;
|
||||
|
||||
// --- Test 9: respects ECC_DISABLED_HOOKS ---
|
||||
clearState();
|
||||
if (test('respects ECC_DISABLED_HOOKS (skips when disabled)', () => {
|
||||
const input = {
|
||||
tool_name: 'Edit',
|
||||
tool_input: { file_path: '/src/disabled.js', old_string: 'a', new_string: 'b' }
|
||||
};
|
||||
const result = runHook(input, {
|
||||
ECC_DISABLED_HOOKS: 'pre:edit-write:gateguard-fact-force'
|
||||
});
|
||||
|
||||
const output = parseOutput(result.stdout);
|
||||
if (output && output.hookSpecificOutput) {
|
||||
assert.notStrictEqual(output.hookSpecificOutput.permissionDecision, 'deny',
|
||||
'should not deny when hook is disabled');
|
||||
}
|
||||
})) passed++; else failed++;
|
||||
|
||||
// Cleanup
|
||||
clearState();
|
||||
|
||||
console.log(`\n ${passed} passed, ${failed} failed\n`);
|
||||
process.exit(failed > 0 ? 1 : 0);
|
||||
}
|
||||
|
||||
runTests();
|
||||
Reference in New Issue
Block a user