Skip to content

fix: prevent false positives for deny rules and PreToolUse block hooks #9

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kai-agent-free wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: prevent false positives for deny rules and PreToolUse block hooks #9

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1e775cc
fix: prevent false positives for deny rules and PreToolUse block hooks
kai-agent-free Mar 13, 2026
07cb0c4
fix: address CodeRabbit review comments on hooks.ts
kai-agent-free Mar 13, 2026
a39d9ae
fix: string-aware bracket counting and raw-text object boundary finding
kai-agent-free Mar 13, 2026
1266bce
fix: replace ambiguous needle-based hook lookup with index-based forw…
kai-agent-free Mar 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 97 additions & 1 deletion dist/action.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,6 +1225,88 @@ function findLineNumber2(content, matchIndex) {
}

// src/rules/hooks.ts
function findStringRangesAtPath(content, path) {
const ranges = [];
try {
const config = JSON.parse(content);
let target = config;
for (const key of path) {
if (target && typeof target === "object" && !Array.isArray(target)) {
target = target[key];
} else {
return ranges;
}
}
if (!Array.isArray(target)) return ranges;
for (const entry of target) {
if (typeof entry !== "string") continue;
const needle = JSON.stringify(entry);
let idx = 0;
while ((idx = content.indexOf(needle, idx)) !== -1) {
ranges.push({ start: idx, end: idx + needle.length });
idx += needle.length;
}
}
} catch {
}
return ranges;
}
function findBlockHookRanges(content) {
const ranges = [];
try {
const config = JSON.parse(content);
const preToolUseHooks = config?.hooks?.PreToolUse ?? [];
for (const hookEntry of preToolUseHooks) {
const h = hookEntry;
const commands = [];
if (typeof h.command === "string") commands.push(h.command);
if (typeof h.hook === "string") commands.push(h.hook);
if (Array.isArray(h.hooks)) {
for (const sub of h.hooks) {
const s = sub;
if (typeof s.command === "string") commands.push(s.command);
if (typeof s.hook === "string") commands.push(s.hook);
}
}
const isBlock = commands.some((c) => /exit\s+[12]\b/.test(c));
if (!isBlock) continue;
const strings = collectStrings(hookEntry);
for (const s of strings) {
const needle = JSON.stringify(s);
let idx = 0;
while ((idx = content.indexOf(needle, idx)) !== -1) {
ranges.push({ start: idx, end: idx + needle.length });
idx += needle.length;
}
}
}
} catch {
}
return ranges;
}
function collectStrings(obj) {
const result = [];
if (typeof obj === "string") {
result.push(obj);
} else if (Array.isArray(obj)) {
for (const item of obj) result.push(...collectStrings(item));
} else if (obj && typeof obj === "object") {
for (const val of Object.values(obj)) {
result.push(...collectStrings(val));
}
}
return result;
}
function buildSafeRanges(content) {
return [
...findStringRangesAtPath(content, ["permissions", "deny"]),
...findStringRangesAtPath(content, ["permissions", "allow"]),
...findBlockHookRanges(content)
];
}
function isInSafeRange(ranges, matchIndex) {
return ranges.some((r) => matchIndex >= r.start && matchIndex < r.end);
}
var INJECTION_PATTERNS = [
{
name: "var-interpolation",
Expand Down Expand Up @@ -1276,8 +1358,22 @@ var EXFILTRATION_PATTERNS = [
function findLineNumber3(content, matchIndex) {
return content.substring(0, matchIndex).split("\n").length;
}
var safeRangeCache = /* @__PURE__ */ new WeakMap();
var contentKeyMap = /* @__PURE__ */ new Map();
function getSafeRanges(content) {
let key = contentKeyMap.get(content);
if (!key) {
key = {};
contentKeyMap.set(content, key);
safeRangeCache.set(key, buildSafeRanges(content));
}
return safeRangeCache.get(key);
}
function findAllMatches2(content, pattern) {
return [...content.matchAll(new RegExp(pattern.source, pattern.flags.includes("g") ? pattern.flags : pattern.flags + "g"))];
const matches = [...content.matchAll(new RegExp(pattern.source, pattern.flags.includes("g") ? pattern.flags : pattern.flags + "g"))];
const safeRanges = getSafeRanges(content);
if (safeRanges.length === 0) return matches;
return matches.filter((m) => !isInSafeRange(safeRanges, m.index ?? 0));
}
var hookRules = [
{
Expand Down
100 changes: 98 additions & 2 deletions dist/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1248,13 +1248,107 @@ var init_permissions = __esm({
});

// src/rules/hooks.ts
function findStringRangesAtPath(content, path) {
const ranges = [];
try {
const config = JSON.parse(content);
let target = config;
for (const key of path) {
if (target && typeof target === "object" && !Array.isArray(target)) {
target = target[key];
} else {
return ranges;
}
}
if (!Array.isArray(target)) return ranges;
for (const entry of target) {
if (typeof entry !== "string") continue;
const needle = JSON.stringify(entry);
let idx = 0;
while ((idx = content.indexOf(needle, idx)) !== -1) {
ranges.push({ start: idx, end: idx + needle.length });
idx += needle.length;
}
}
} catch {
}
return ranges;
}
function findBlockHookRanges(content) {
const ranges = [];
try {
const config = JSON.parse(content);
const preToolUseHooks = config?.hooks?.PreToolUse ?? [];
for (const hookEntry of preToolUseHooks) {
const h = hookEntry;
const commands = [];
if (typeof h.command === "string") commands.push(h.command);
if (typeof h.hook === "string") commands.push(h.hook);
if (Array.isArray(h.hooks)) {
for (const sub of h.hooks) {
const s = sub;
if (typeof s.command === "string") commands.push(s.command);
if (typeof s.hook === "string") commands.push(s.hook);
}
}
const isBlock = commands.some((c) => /exit\s+[12]\b/.test(c));
if (!isBlock) continue;
const strings = collectStrings(hookEntry);
for (const s of strings) {
const needle = JSON.stringify(s);
let idx = 0;
while ((idx = content.indexOf(needle, idx)) !== -1) {
ranges.push({ start: idx, end: idx + needle.length });
idx += needle.length;
}
}
}
} catch {
}
return ranges;
}
function collectStrings(obj) {
const result = [];
if (typeof obj === "string") {
result.push(obj);
} else if (Array.isArray(obj)) {
for (const item of obj) result.push(...collectStrings(item));
} else if (obj && typeof obj === "object") {
for (const val of Object.values(obj)) {
result.push(...collectStrings(val));
}
}
return result;
}
function buildSafeRanges(content) {
return [
...findStringRangesAtPath(content, ["permissions", "deny"]),
...findStringRangesAtPath(content, ["permissions", "allow"]),
...findBlockHookRanges(content)
];
}
function isInSafeRange(ranges, matchIndex) {
return ranges.some((r) => matchIndex >= r.start && matchIndex < r.end);
}
function findLineNumber3(content, matchIndex) {
return content.substring(0, matchIndex).split("\n").length;
}
function getSafeRanges(content) {
let key = contentKeyMap.get(content);
if (!key) {
key = {};
contentKeyMap.set(content, key);
safeRangeCache.set(key, buildSafeRanges(content));
}
return safeRangeCache.get(key);
}
function findAllMatches2(content, pattern) {
return [...content.matchAll(new RegExp(pattern.source, pattern.flags.includes("g") ? pattern.flags : pattern.flags + "g"))];
const matches = [...content.matchAll(new RegExp(pattern.source, pattern.flags.includes("g") ? pattern.flags : pattern.flags + "g"))];
const safeRanges = getSafeRanges(content);
if (safeRanges.length === 0) return matches;
return matches.filter((m) => !isInSafeRange(safeRanges, m.index ?? 0));
}
var INJECTION_PATTERNS, EXFILTRATION_PATTERNS, hookRules;
var INJECTION_PATTERNS, EXFILTRATION_PATTERNS, safeRangeCache, contentKeyMap, hookRules;
var init_hooks = __esm({
"src/rules/hooks.ts"() {
"use strict";
Expand Down Expand Up @@ -1306,6 +1400,8 @@ var init_hooks = __esm({
description: "Hook sends email \u2014 potential data exfiltration"
}
];
safeRangeCache = /* @__PURE__ */ new WeakMap();
contentKeyMap = /* @__PURE__ */ new Map();
hookRules = [
{
id: "hooks-injection",
Expand Down
151 changes: 150 additions & 1 deletion src/rules/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,137 @@
import type { ConfigFile, Finding, Rule } from "../types.js";

// ─── Safe-context detection ────────────────────────────────
// Deny-list entries and PreToolUse block hooks (exit 2) are security
// controls, not threats. We compute byte-ranges for those contexts
// so individual rules can skip matches that fall inside them.

interface SafeRange {
start: number;
end: number;
}
Comment on lines +8 to +11
Copy link

@coderabbitai coderabbitai bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Use immutable typings for interfaces and arrays in changed code

Changed segments still use mutable fields/array types (start, end, SafeRange[], string[]). This violates the repository immutability rule.

🧩 Minimal typing updates 
 interface SafeRange {
-  start: number;
-  end: number;
+  readonly start: number;
+  readonly end: number;
 }

 function findStringRangesAtPath(
   content: string,
-  path: string[],
-): SafeRange[] {
+  path: ReadonlyArray<string>,
+): ReadonlyArray<SafeRange> {

 function findBlockHookRanges(content: string): SafeRange[] {
-  const ranges: SafeRange[] = [];
+  const ranges: Array<SafeRange> = [];

 function collectStrings(obj: unknown): string[] {
-  const result: string[] = [];
+  const result: Array<string> = [];

As per coding guidelines: src/**/*.ts: "Use ReadonlyArray for all array types and readonly fields in all interfaces for immutability."

Also applies to: 21-23, 62-63, 105-107, 123-124, 211-211

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/rules/hooks.ts` around lines 8 - 11, Update the mutable types to
immutable variants: make the SafeRange interface fields readonly (e.g., readonly
start and readonly end) and replace all plain array types in this file
(instances of SafeRange[] and string[]) with ReadonlyArray (e.g.,
ReadonlyArray<SafeRange>, ReadonlyArray<string>); search for usages referenced
in this file (including the occurrences around lines shown: 21-23, 62-63,
105-107, 123-124, 211) and change any parameter, variable, or return type
annotations to use readonly fields and ReadonlyArray to comply with the
repository immutability rule.


/**
* Find the start and end byte offsets of every JSON string literal
* that is a child of a given JSON path. Works on the raw text so
* it handles any formatting/indentation.
*/
function findStringRangesAtPath(
content: string,
path: string[],
): SafeRange[] {
const ranges: SafeRange[] = [];

// Walk through JSON tokens manually to find strings under the target path.
// This is simpler and more reliable than trying to match JSON.stringify output.
try {
const config = JSON.parse(content);
// Navigate to the target path
let target: unknown = config;
for (const key of path) {
if (target && typeof target === "object" && !Array.isArray(target)) {
target = (target as Record<string, unknown>)[key];
} else {
return ranges;
}
}
if (!Array.isArray(target)) return ranges;

// For each string in the array, find its quoted occurrence in the file
// We need to find them in order to handle duplicates correctly
for (const entry of target) {
if (typeof entry !== "string") continue;
const needle = JSON.stringify(entry); // e.g. "\"Bash(sudo)\""
let idx = 0;
while ((idx = content.indexOf(needle, idx)) !== -1) {
ranges.push({ start: idx, end: idx + needle.length });
idx += needle.length;
}
}
} catch {
// Not valid JSON
}
return ranges;
}

/**
* Find ranges of PreToolUse block hooks (hooks that exit 1 or exit 2).
* These are security controls that block actions, not threats.
* We mark a broad region around each such hook entry.
*/
function findBlockHookRanges(content: string): SafeRange[] {
const ranges: SafeRange[] = [];
try {
const config = JSON.parse(content);
const preToolUseHooks: unknown[] = config?.hooks?.PreToolUse ?? [];

for (const hookEntry of preToolUseHooks) {
const h = hookEntry as Record<string, unknown>;

// Collect all command strings from the hook
const commands: string[] = [];
if (typeof h.command === "string") commands.push(h.command);
if (typeof h.hook === "string") commands.push(h.hook);
if (Array.isArray(h.hooks)) {
for (const sub of h.hooks) {
const s = sub as Record<string, unknown>;
if (typeof s.command === "string") commands.push(s.command);
if (typeof s.hook === "string") commands.push(s.hook);
}
}

// Only mark as safe if the hook blocks/rejects (exit 1 or exit 2)
const isBlock = commands.some((c) => /exit\s+[12]\b/.test(c));
if (!isBlock) continue;

// Find all string values from this hook entry in the content and
// mark them as safe. We collect all string leaves from the object.
const strings = collectStrings(hookEntry);
for (const s of strings) {
const needle = JSON.stringify(s);
let idx = 0;
while ((idx = content.indexOf(needle, idx)) !== -1) {
ranges.push({ start: idx, end: idx + needle.length });
idx += needle.length;
}
}
}
} catch {
// Not valid JSON
}
return ranges;
}

/** Recursively collect all string values from an object/array. */
function collectStrings(obj: unknown): string[] {
const result: string[] = [];
if (typeof obj === "string") {
result.push(obj);
} else if (Array.isArray(obj)) {
for (const item of obj) result.push(...collectStrings(item));
} else if (obj && typeof obj === "object") {
for (const val of Object.values(obj as Record<string, unknown>)) {
result.push(...collectStrings(val));
}
}
return result;
}

/**
* Build safe ranges: deny list entries, allow list entries (permissions,
* not hooks), and PreToolUse block hooks.
*/
function buildSafeRanges(content: string): SafeRange[] {
return [
...findStringRangesAtPath(content, ["permissions", "deny"]),
...findStringRangesAtPath(content, ["permissions", "allow"]),
...findBlockHookRanges(content),
];
}

function isInSafeRange(ranges: SafeRange[], matchIndex: number): boolean {
return ranges.some((r) => matchIndex >= r.start && matchIndex < r.end);
}

/**
* Patterns in hooks that could enable injection or information disclosure.
*/
Expand Down Expand Up @@ -72,8 +204,25 @@ function findLineNumber(content: string, matchIndex: number): number {
return content.substring(0, matchIndex).split("\n").length;
}

// Cache safe ranges per content string to avoid re-parsing JSON for every pattern
const safeRangeCache = new WeakMap<object, SafeRange[]>();
const contentKeyMap = new Map<string, object>();

function getSafeRanges(content: string): SafeRange[] {
let key = contentKeyMap.get(content);
if (!key) {
key = {};
contentKeyMap.set(content, key);
safeRangeCache.set(key, buildSafeRanges(content));
}
return safeRangeCache.get(key)!;
}

function findAllMatches(content: string, pattern: RegExp): Array<RegExpMatchArray> {
return [...content.matchAll(new RegExp(pattern.source, pattern.flags.includes("g") ? pattern.flags : pattern.flags + "g"))];
const matches = [...content.matchAll(new RegExp(pattern.source, pattern.flags.includes("g") ? pattern.flags : pattern.flags + "g"))];
const safeRanges = getSafeRanges(content);
if (safeRanges.length === 0) return matches;
return matches.filter((m) => !isInSafeRange(safeRanges, m.index ?? 0));
}

export const hookRules: ReadonlyArray<Rule> = [
Expand Down
Loading