1. src/agents/pi-embedded-runner/run/attempt.ts, line 3198-3204 (link)

   Gate cleanup uses wrong key — memory leak remains

   The run-end cleanup calls clearAfterLlmCallGate(params.sessionId), but hook-stream-wrapper.ts stores the gate under a composite key (sessionId + ":" + runId) whenever runId is set — which it always is for normal embedded runs. Passing only params.sessionId here deletes a key that was never inserted, so the actual gate entry in the module-level gates Map is never removed. The same key mismatch exists in the turn_start subscription at line 2625.

   Both cleanup sites need to build the same composite key the wrapper does (see gateKey construction in hook-stream-wrapper.ts) and pass that to clearAfterLlmCallGate.

Description

The new LLM hook enforcement path fails open when hook execution errors occur, which can bypass security/policy plugins if they are intended as mandatory controls.

after_llm_call gate (tool execution)

• setAfterLlmCallGatePromise() converts a rejected after_llm_call hook promise into { blocked: false }.
• This means any tool calls in that turn will be allowed if the hook fails (e.g., network/policy service outage, thrown exception).

before_llm_call (prompt/tool filtering)

• wrapStreamFnWithHooks() catches errors from runBeforeLlmCall() and continues with the original context/tools.
• If a security plugin is responsible for filtering tools or blocking unsafe prompts, an attacker may be able to trigger an error condition (e.g., by causing the plugin to throw on malformed input or by inducing transient dependency failures) to bypass the protections.

Vulnerable code (after_llm_call gate):

.catch((err) => {
  log.warn(`after_llm_call hook error (failing open): ${String(err)}`);
  return { blocked: false } as GateDecision;
});

Vulnerable code (before_llm_call):

} catch (err) {
  ​// Fail-open: log and continue with the original context.
  log.warn(`before_llm_call hook failed: ${String(err)}`);
}

Recommendation

Treat policy hooks as fail-closed by default, or make the behavior explicitly configurable per hook (mandatory vs best-effort).

Minimum fix (fail-closed for after_llm_call):

.catch((err) => {
  log.error(`after_llm_call hook error (failing closed): ${String(err)}`);
  return { blocked: true, blockReason: "after_llm_call hook failed" } as GateDecision;
});

For before_llm_call, either:

• fail closed by throwing a block error when the hook fails, or
• continue but restrict capabilities (e.g., remove all tools) until policy can be evaluated.

Also consider:

• adding timeouts and circuit-breaking;
• emitting structured audit events when enforcement fails;
• documenting that hooks used for security controls must not fail open in production.

Description

The allowPromptInjection=false policy is intended to constrain prompt-mutating hooks, but the new before_llm_call hook is not treated as a prompt-injection hook by isPromptInjectionHookName(). As a result, the constraint wrapper for before_llm_call is never applied, and a plugin configured with allowPromptInjection=false can still freely mutate event.messages/event.systemPrompt.

Additionally, even in the (intended) wrapper, only messages is deep-cloned; other fields (notably tools) are passed by reference via ...event, so a handler can mutate event.tools in-place to influence subsequent handlers and downstream tool filtering decisions.

Key points:

• Policy gate never triggers for before_llm_call because PROMPT_INJECTION_HOOK_NAMES excludes it.
• Hook runner executes handlers sequentially with the same event object, so in-place mutations persist across handlers.

Vulnerable code (policy bypass and partial cloning):

if (policy?.allowPromptInjection === false && isPromptInjectionHookName(hookName)) {
  ​// ...
  if (hookName === "before_llm_call") {
    const sandboxedEvent: PluginHookBeforeLlmCallEvent = {
      ...event,
      messages: structuredClone(event.messages),
      systemPrompt: event.systemPrompt,
    };
    const result = await original(sandboxedEvent, ctx);
    const { messages: _m, systemPrompt: _s, ...safe } = result;
    return safe;
  }
}

And the prompt-injection hook allowlist omits before_llm_call:

export const PROMPT_INJECTION_HOOK_NAMES = [
  "before_prompt_build",
  "before_agent_start",
] as const;

Impact:

• A plugin believed to be “non prompt-injecting” (allowPromptInjection=false) can still modify the LLM input (messages/systemPrompt) via before_llm_call.
• In-place mutation of tools can affect later handlers’ view of the available tools and potentially weaken tool allowlisting/auditing logic that assumes events are immutable per handler.

Recommendation

1. Ensure before_llm_call is classified as a prompt-injection hook when allowPromptInjection=false, or remove the isPromptInjectionHookName() guard and explicitly check each constrained hook.

2. Prevent cross-handler interference by deep-cloning (or freezing) all mutable fields exposed to handlers (at least tools, and ideally the whole event), e.g.:

​// hook-types.ts
export const PROMPT_INJECTION_HOOK_NAMES = [
  "before_prompt_build",
  "before_agent_start",
  "before_llm_call",
] as const;

​// registry.ts wrapper
const sandboxedEvent: PluginHookBeforeLlmCallEvent = structuredClone(event);
​// or at minimum:
const sandboxedEvent: PluginHookBeforeLlmCallEvent = {
  ...event,
  messages: structuredClone(event.messages),
  tools: structuredClone(event.tools),
};
Object.freeze(sandboxedEvent.tools);

This ensures plugins cannot bypass the configured policy and cannot mutate shared objects to influence other handlers.

Description

In fireAfterLlmCallGate() the toolCalls array is built by directly reusing the arguments/input object from finalMessage.content without cloning.

• finalMessage is the same assistant message object that will later be used by the agent runtime when executing tool calls.
• after_llm_call handlers receive event.toolCalls[i].arguments and can mutate it (including nested fields).
• Because these arguments objects are shared references to finalMessage.content[*].arguments/input, such mutations can alter the tool-call payload that will subsequently be executed.

This enables plugin-driven tool argument tampering (confused-deputy) even though the after-LLM hook/gate is intended to only filter/block tool calls.

Vulnerable code:

arguments:
  (part.arguments as Record<string, unknown>) ??
  (part.input as Record<string, unknown>) ??
  {},

Recommendation

Deep-clone tool-call argument objects before exposing them to plugins (and ideally freeze/sandbox the event) so plugin handlers cannot mutate the assistant message that will be executed.

Example fix:

const rawArgs =
  (part.arguments as Record<string, unknown>) ??
  (part.input as Record<string, unknown>) ??
  {};

toolCalls.push({
  id: String(part.id ?? ""),
  name: String(part.name ?? ""),
  arguments: structuredClone(rawArgs),
});

Additionally consider cloning the entire toolCalls array passed to runAfterLlmCall (or freezing it) and/or ensure tool execution uses a separately-parsed immutable copy of tool calls rather than the mutable finalMessage object.

Description

In runEmbeddedAttempt, the finally cleanup block performs an awaited dynamic import and gate cleanup without a protective try/catch.

• If await import("./after-llm-call-gate.js") fails (packaging/bundling issues, missing file, transient loader errors) or clearAfterLlmCallGate(...) throws, the exception will be thrown from the finally block.
• Exceptions thrown from finally can mask the original error from the main try block.
• More importantly, throwing here can prevent subsequent teardown steps from running (e.g., detachBackend, clearActiveEmbeddedRun, abort listener removal), potentially causing resource leaks and unbounded growth / stuck sessions.

Vulnerable code:

{
  const { clearAfterLlmCallGate } = await import("./after-llm-call-gate.js");
  clearAfterLlmCallGate(params.sessionId);
}

Recommendation

Wrap the dynamic import and gate cleanup in a try/catch inside finally, and ensure later teardown steps always run.

try {
  const mod = await import("./after-llm-call-gate.js");
  mod.clearAfterLlmCallGate(params.sessionId);
} catch (err) {
  log.error(
    `CRITICAL: failed to clear after_llm_call gate: runId=${params.runId} sessionId=${params.sessionId} ${String(err)}`,
  );
}

Optionally, avoid dynamic import in teardown by importing the module statically at top-level if feasible in your build, or guard the call when params.sessionId is falsy.

Description

after_llm_call gate entries are stored under a per-run key (sessionId:runId) in the stream wrapper and are read under the same key in the tool wrapper, but runEmbeddedAttempt() clears the gate using only sessionId.

• Write key (hook stream wrapper): gateKey = ${sessionId}:${runId}`` when runId is present
• Read key (before-tool-call): same sessionId:runId key
• Clear key (attempt.ts turn_start + finally): clearAfterLlmCallGate(params.sessionId)

Impact:

• When params.runId is set (normal for runs), the module-level gates Map in after-llm-call-gate.ts never deletes the sessionId:runId entry.
• Over time, repeated runs/sessions accumulate in the Map, causing unbounded memory growth and potential process OOM / DoS.

Vulnerable code (cleanup uses wrong key):

​// attempt.ts
clearAfterLlmCallGate(params.sessionId);

Recommendation

Ensure clearAfterLlmCallGate() is called with the same key used by writers/readers (i.e., include runId when present). Ideally centralize key construction in a helper to avoid future drift.

Example fix:

​// attempt.ts
const gateKey = params.runId ? `${params.sessionId}:${params.runId}` : params.sessionId;
clearAfterLlmCallGate(gateKey);

Also apply the same gateKey derivation in both the turn_start subscription and the finally cleanup block.