Skip to content

Commit f223602

Browse files
authored
fix(yara): repeat-block escalation + scan only edit replacement text (stacked on #804)
The YARA guard split out of #816, rebuilt on top of #804's warlock-based pi fence, plus the one surviving piece of the closed #818: 1. Repeat-block escalation: agents retry identically-blocked payloads (~4.2 blocks per affected run) despite the prompt saying not to. Both fences (pi pre-execution scan, anthropic PreToolUse Bash hook) now hash the payloads they block; an identical retry gets an escalated reason ("this exact content was ALREADY blocked — change the code, not the retry"), and from the third attempt the agent is told to note it in the setup report and move on. The escalation decorates #804's remediation-bearing block message, never replaces it. Tracker lives in yara-hooks, shared by both fences; policy denies are unaffected. 2. Edit scanning scoped to replacement text: #804 still scans JSON.stringify(input.edits), which includes each edit's oldText — pre-existing file content. That blocked edits whose surroundings contained a violation and even edits REMOVING one (field FPs: capture edits adjacent to existing identify() PII; pre-existing violations blocking their own replacement). Now scans only the joined newText, matching the anthropic path which scans new_string only. Generated-By: PostHog Code Task-Id: bcb96bcb-e37f-421d-841d-738647c73b8e
1 parent 40cc77b commit f223602

4 files changed

Lines changed: 301 additions & 6 deletions

File tree

src/lib/__tests__/yara-hooks.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import {
22
createPreToolUseYaraHooks,
33
createPostToolUseYaraHooks,
4+
createRepeatBlockTracker,
45
formatScanReport,
6+
repeatBlockReason,
57
writeScanReport,
68
captureScanReport,
79
resetScanReport,
@@ -1048,3 +1050,73 @@ describe('yara-hooks', () => {
10481050
});
10491051
});
10501052
});
1053+
1054+
describe('repeat-block tracker (identical retries after a block)', () => {
1055+
test('counts attempts per exact (tool, content) payload', () => {
1056+
const tracker = createRepeatBlockTracker();
1057+
expect(tracker.attempt('Edit', 'const a = 1;')).toBe(1);
1058+
expect(tracker.attempt('Edit', 'const a = 1;')).toBe(2);
1059+
expect(tracker.attempt('Edit', 'const a = 1;')).toBe(3);
1060+
// Different content or a different tool is a fresh first attempt.
1061+
expect(tracker.attempt('Edit', 'const a = 2;')).toBe(1);
1062+
expect(tracker.attempt('Write', 'const a = 1;')).toBe(1);
1063+
});
1064+
1065+
test('reason is unchanged on the first block', () => {
1066+
expect(repeatBlockReason(1, 'Edit', '[YARA] rule: bad.')).toBe(
1067+
'[YARA] rule: bad.',
1068+
);
1069+
});
1070+
1071+
test('second attempt says the retry can never work', () => {
1072+
const reason = repeatBlockReason(2, 'Edit', '[YARA] rule: bad.');
1073+
expect(reason).toContain('[YARA] rule: bad.');
1074+
expect(reason).toContain('ALREADY blocked');
1075+
expect(reason).toContain('Change the code');
1076+
});
1077+
1078+
test('third and later attempts tell the agent to report and move on', () => {
1079+
for (const attempt of [3, 4, 7]) {
1080+
const reason = repeatBlockReason(attempt, 'Write', '[YARA] rule: bad.');
1081+
expect(reason).toContain(`blocked ${attempt} times`);
1082+
expect(reason).toContain('setup report');
1083+
expect(reason).toContain('move on');
1084+
}
1085+
});
1086+
1087+
test('PreToolUse Bash escalates when the same blocked command is retried', async () => {
1088+
mockScan.mockResolvedValue({
1089+
matched: true,
1090+
matches: [
1091+
{
1092+
rule: 'destructive_rm',
1093+
metadata: {
1094+
severity: 'critical',
1095+
category: 'destructive_operations',
1096+
description: 'Detects destructive rm',
1097+
scan_context: 'command',
1098+
},
1099+
matchedStrings: [],
1100+
},
1101+
],
1102+
});
1103+
const [matcher] = createPreToolUseYaraHooks();
1104+
const run = () =>
1105+
matcher.hooks[0](
1106+
{ tool_name: 'Bash', tool_input: { command: 'rm -rf build' } },
1107+
undefined,
1108+
{ signal: dummySignal },
1109+
);
1110+
1111+
const first = await run();
1112+
expect(first.decision).toBe('block');
1113+
expect(first.reason).not.toContain('ALREADY blocked');
1114+
1115+
const second = await run();
1116+
expect(second.reason).toContain('ALREADY blocked');
1117+
1118+
const third = await run();
1119+
expect(third.reason).toContain('blocked 3 times');
1120+
expect(third.reason).toContain('setup report');
1121+
});
1122+
});

src/lib/agent/runner/harness/pi/__tests__/security.test.ts

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -259,3 +259,128 @@ describe('pi-security: extension state machine (fail-closed + runaway + latch)',
259259
expect(state.toolCalls).toBeGreaterThan(MAX_TOOL_CALLS);
260260
});
261261
});
262+
263+
describe('pi-security: edit scanning is scoped to the replacement text', () => {
264+
test('scans only newText — oldText (pre-existing content) never reaches the scanner', async () => {
265+
// Field FPs: a violation in the surrounding/replaced code blocked edits
266+
// that were clean — including edits REMOVING the violation.
267+
await evaluateToolCall('edit', {
268+
path: 'src/analytics.ts',
269+
edits: [
270+
{
271+
oldText: "posthog.capture('signup', { email: user.email })",
272+
newText: "posthog.capture('signup', { plan: user.plan })",
273+
},
274+
{ oldText: 'const a = 1;', newText: 'const a = 2;' },
275+
],
276+
});
277+
expect(mockedScan).toHaveBeenCalledTimes(1);
278+
const scanned = mockedScan.mock.calls[0][0];
279+
expect(scanned).toContain("posthog.capture('signup', { plan: user.plan })");
280+
expect(scanned).toContain('const a = 2;');
281+
expect(scanned).not.toContain('email');
282+
expect(scanned).not.toContain('const a = 1;');
283+
});
284+
285+
test('an edit whose newText introduces a violation still blocks', async () => {
286+
mockedScan.mockResolvedValueOnce({ matched: true, matches: [piiMatch] });
287+
const decision = await evaluateToolCall('edit', {
288+
path: 'src/analytics.ts',
289+
edits: [
290+
{
291+
oldText: "posthog.capture('signup')",
292+
newText: "posthog.capture('signup', { email: user.email })",
293+
},
294+
],
295+
});
296+
expect(decision.block).toBe(true);
297+
expect(decision.reason).toContain('posthog_pii_in_capture_call');
298+
});
299+
300+
test('an edits array with only empty newText skips the scan entirely', async () => {
301+
const decision = await evaluateToolCall('edit', {
302+
path: 'src/analytics.ts',
303+
edits: [{ oldText: 'delete me', newText: '' }],
304+
});
305+
expect(decision.block).toBe(false);
306+
expect(mockedScan).not.toHaveBeenCalled();
307+
});
308+
});
309+
310+
describe('pi-security: repeat-block escalation (identical retries after a YARA block)', () => {
311+
function fakePi() {
312+
const handlers: Record<string, (e: any) => any> = {};
313+
const pi: PiExtensionApiLike = {
314+
on: (event: string, handler: (e: any) => any) => {
315+
handlers[event] = handler;
316+
},
317+
} as PiExtensionApiLike;
318+
return { pi, handlers };
319+
}
320+
321+
const piiWrite = {
322+
toolName: 'write',
323+
input: {
324+
path: 'src/analytics.ts',
325+
content: "posthog.capture('login', { email: user.email })",
326+
},
327+
};
328+
329+
test('an identical YARA-blocked write escalates, then says report-and-move-on', async () => {
330+
const { factory } = createSecurityExtension();
331+
const { pi, handlers } = fakePi();
332+
factory(pi);
333+
334+
mockedScan.mockResolvedValueOnce({ matched: true, matches: [piiMatch] });
335+
const first = await handlers.tool_call(piiWrite);
336+
expect(first.block).toBe(true);
337+
// The remediation still leads — escalation decorates, never replaces it.
338+
expect(first.reason).toContain('identify()');
339+
expect(first.reason).not.toContain('ALREADY blocked');
340+
341+
mockedScan.mockResolvedValueOnce({ matched: true, matches: [piiMatch] });
342+
const second = await handlers.tool_call(piiWrite);
343+
expect(second.block).toBe(true);
344+
expect(second.reason).toContain('ALREADY blocked');
345+
expect(second.reason).toContain('Change the code');
346+
347+
mockedScan.mockResolvedValueOnce({ matched: true, matches: [piiMatch] });
348+
const third = await handlers.tool_call(piiWrite);
349+
expect(third.block).toBe(true);
350+
expect(third.reason).toContain('blocked 3 times');
351+
expect(third.reason).toContain('setup report');
352+
});
353+
354+
test('different blocked content is a fresh first attempt, not a repeat', async () => {
355+
const { factory } = createSecurityExtension();
356+
const { pi, handlers } = fakePi();
357+
factory(pi);
358+
359+
mockedScan.mockResolvedValueOnce({ matched: true, matches: [piiMatch] });
360+
await handlers.tool_call(piiWrite);
361+
362+
mockedScan.mockResolvedValueOnce({ matched: true, matches: [piiMatch] });
363+
const other = await handlers.tool_call({
364+
toolName: 'write',
365+
input: {
366+
path: 'src/checkout.ts',
367+
content: "posthog.capture('purchase', { phone: user.phone })",
368+
},
369+
});
370+
expect(other.block).toBe(true);
371+
expect(other.reason).not.toContain('ALREADY blocked');
372+
});
373+
374+
test('policy denies (non-YARA) never gain repeat-escalation text', async () => {
375+
const { factory } = createSecurityExtension();
376+
const { pi, handlers } = fakePi();
377+
factory(pi);
378+
379+
const deny = () =>
380+
handlers.tool_call({ toolName: 'bash', input: { command: 'cat .env' } });
381+
await deny();
382+
const second = await deny();
383+
expect(second.block).toBe(true);
384+
expect(second.reason).not.toContain('ALREADY blocked');
385+
});
386+
});

src/lib/agent/runner/harness/pi/security.ts

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,11 @@
2020
import type { LLMProvider, ScanMatch } from '@posthog/warlock';
2121
import { wizardCanUseTool } from '@lib/agent/agent-interface';
2222
import {
23+
createRepeatBlockTracker,
2324
isWizardDocumentationPath,
25+
repeatBlockReason,
2426
scanAndTriage,
27+
type RepeatBlockTracker,
2528
type ScanContext,
2629
} from '@lib/yara-hooks';
2730
import {
@@ -44,6 +47,14 @@ export interface ToolGateContext {
4447
* on (fail-closed, but no false-positive filtering).
4548
*/
4649
triageAuth?: TriageGatewayAuth;
50+
/**
51+
* Per-run tracker of YARA-blocked payloads. When the agent retries content
52+
* that was already blocked, the block reason escalates ("change the code,
53+
* not the retry" → "report it and move on"). The extension wires one in;
54+
* absent (tests calling `evaluateToolCall` directly) every block reads as
55+
* a first attempt.
56+
*/
57+
repeatTracker?: RepeatBlockTracker;
4758
}
4859

4960
export interface GateDecision {
@@ -110,6 +121,7 @@ async function preExecutionYaraBlock(
110121
toolName: string,
111122
input: Record<string, unknown>,
112123
llmProvider: LLMProvider | undefined,
124+
repeatTracker?: RepeatBlockTracker,
113125
): Promise<string | undefined> {
114126
let content: string;
115127
let ctx: ScanContext;
@@ -126,7 +138,15 @@ async function preExecutionYaraBlock(
126138
triage = llmProvider;
127139
break;
128140
case 'edit':
129-
content = JSON.stringify(input.edits ?? '');
141+
// Scan ONLY the replacement text. `edits` is [{oldText, newText}] and
142+
// oldText is pre-existing file content the agent is changing — scanning
143+
// it blocks edits whose *surroundings* contain a violation, and even
144+
// edits that are removing one (field FPs: capture edits adjacent to
145+
// existing identify() PII; pre-existing violations blocking their own
146+
// replacement). Matches the anthropic path, which scans new_string only.
147+
content = (Array.isArray(input.edits) ? input.edits : [])
148+
.map((e) => str((e as Record<string, unknown>)?.newText))
149+
.join('\n');
130150
ctx = 'output';
131151
triage = llmProvider;
132152
break;
@@ -141,7 +161,8 @@ async function preExecutionYaraBlock(
141161
}
142162
if (matches.length === 0) return undefined;
143163

144-
return blockMessage(matches[0]);
164+
const attempt = repeatTracker?.attempt(toolName, content) ?? 1;
165+
return repeatBlockReason(attempt, toolName, blockMessage(matches[0]));
145166
}
146167

147168
/**
@@ -169,6 +190,7 @@ export async function evaluateToolCall(
169190
toolName,
170191
input,
171192
llmProvider,
193+
ctx.repeatTracker,
172194
);
173195
if (yaraReason) return { block: true, reason: yaraReason };
174196

@@ -213,6 +235,13 @@ export function createSecurityExtension(ctx: ToolGateContext = {}): {
213235
// triage is skipped and every flagged match is acted on — fail closed.
214236
const llmProvider = createTriageLLMProvider(ctx.triageAuth);
215237

238+
// One tracker per extension = per (sub)agent session, so an identical
239+
// payload retried after a block gets the escalating reason.
240+
const gateCtx: ToolGateContext = {
241+
...ctx,
242+
repeatTracker: ctx.repeatTracker ?? createRepeatBlockTracker(),
243+
};
244+
216245
const factory = (pi: PiExtensionApiLike): void => {
217246
pi.on('tool_call', async (event) => {
218247
// A latched post-scan violation blocks everything that follows.
@@ -232,7 +261,7 @@ export function createSecurityExtension(ctx: ToolGateContext = {}): {
232261
const decision = await evaluateToolCall(
233262
event.toolName,
234263
event.input ?? {},
235-
ctx,
264+
gateCtx,
236265
llmProvider,
237266
);
238267
if (decision.block) {

0 commit comments

Comments
 (0)