Description

The before_agent_finalize hook is designed to let plugins request an additional model pass (revise) or accept the natural final answer. However, the merge logic in mergeBeforeAgentFinalize allows a later hook result with action: "finalize" to override an earlier action: "revise".

Because hooks are executed in priority order (higher priority first), this means a lower-priority plugin can negate a higher-priority plugin’s attempt to block finalization.

Implications:

• A compliance/safety plugin can return { action: "revise" } to require additional checks.
• Another plugin (including a compromised plugin) can return { action: "finalize" } and force acceptance of the natural final answer, bypassing the intended gate.
• This is especially risky because before_agent_finalize is treated as a conversation hook and is available to bundled plugins by default, and to non-bundled plugins when allowConversationAccess is enabled.

Vulnerable code:

if (next.action === "finalize") {
  return { action: "finalize", reason: next.reason };
}
...
if (acc?.action === "revise") {
  return acc;
}

This explicitly gives finalize from a later hook precedence over an earlier revise, regardless of priority.

Recommendation

Make merge semantics respect priority and prevent lower-priority handlers from overriding a higher-priority revise.

Two viable approaches:

1. “Most restrictive wins”: once any hook requests revise, keep revise (unless a trusted/core-only handler is allowed to force finalize).

const mergeBeforeAgentFinalize = (acc, next) => {
  if (acc?.action === "revise" || next.action === "revise") {
    return {
      action: "revise",
      reason: concatOptionalTextSegments({ left: acc?.reason, right: next.reason }),
    };
  }
  ​// Only accept finalize if no one requested revise.
  if (acc?.action === "finalize" || next.action === "finalize") {
    return { action: "finalize", reason: acc?.reason ?? next.reason };
  }
  return { action: "continue", reason: acc?.reason ?? next.reason };
};

2. Restrict finalize to trusted sources: enforce (at registration time) that only bundled/core plugins can return finalize, or introduce a separate hook for core-only force-finalization. This prevents third-party plugins from bypassing safety/compliance plugins.

Whichever model you choose, document it clearly so plugin authors understand whether finalize is an override and who is allowed to use it.

Description

The new typed hook before_agent_finalize is classified as a CONVERSATION_HOOK_NAME and is therefore gated only by plugins.entries.<id>.hooks.allowConversationAccess for non-bundled plugins.

This creates a capability escalation: a configuration flag that is documented/understood as allowing reading conversation content also enables run-control actions:

• { action: "revise" } can request additional model passes (cost / DoS potential if repeated each finalize attempt)
• { action: "finalize" } can override earlier revise decisions from other hooks (integrity/safety control bypass)

Because the registry does not distinguish between read-only conversation access and control over termination semantics, any non-bundled plugin that is granted allowConversationAccess=true can influence finalization behavior.

Vulnerable code (permission check treats all conversation hooks the same):

if (isConversationHookName(hookName)) {
  const explicitConversationAccess = policy?.allowConversationAccess;
  if (record.origin !== "bundled" && explicitConversationAccess !== true) {
    ​// ... block
    return;
  }
}

Recommendation

Introduce a separate explicit trust gate for finalization control (and potentially any hook that can trigger additional model work or override safety decisions).

For example:

1. Add a new policy flag, e.g. plugins.entries.<id>.hooks.allowFinalizeControl (default false for non-bundled plugins).
2. Enforce it specifically for before_agent_finalize registration.

if (hookName === "before_agent_finalize") {
  if (record.origin !== "bundled" && policy?.allowFinalizeControl !== true) {
    pushDiagnostic({
      level: "warn",
      pluginId: record.id,
      source: record.source,
      message:
        `typed hook "${hookName}" blocked because non-bundled plugins must set ` +
        `plugins.entries.${record.id}.hooks.allowFinalizeControl=true`,
    });
    return;
  }
}

Also consider adding a harness-side guard to prevent infinite revise loops (e.g., max revise count per turn/run).

Description

The new before_agent_finalize hook allows plugins to block finalization by returning { action: "revise" }. In the Codex native hook relay path this is translated into a Codex Stop-hook response { decision: "block" }, which requests another model pass.

There is no guardrail in this code to limit how many times a plugin can request revision for the same run/turn:

• input: plugin-controlled before_agent_finalize return value (via runAgentHarnessBeforeAgentFinalizeHook)
• sink: returning decision: "block" to the Codex Stop hook (renderBeforeAgentFinalizeReviseResponse), which can trigger another model pass
• missing: per-turn/per-run maximum revise attempts, backoff, or a “revise already used” latch

A buggy or malicious plugin can repeatedly return revise, potentially causing an infinite loop of model calls, runaway token spend, or a stuck session (denial of service / cost amplification). The stopHookActive flag is forwarded to plugins, but it is not enforced and does not prevent repeated blocks.

Recommendation

Introduce a strict cap on revision cycles per run/turn, and fail open (continue/finalize) once exceeded.

Example approach (conceptual):

​// Track per (runId, turnId) revision count in memory (or session state).
const key = `${registration.runId}:${invocation.turnId ?? ""}`;
const count = (reviseCounts.get(key) ?? 0) + 1;
reviseCounts.set(key, count);

const MAX_REVISES = 1; // or small number
if (outcome.action === "revise" && count > MAX_REVISES) {
  log.warn(`before_agent_finalize revise cap exceeded; allowing finalize`, { key, count });
  return adapter.renderNoopResponse(invocation.event); // or renderStopResponse
}

Additionally:

• Consider exponential backoff or cooldown before allowing another revise.
• Consider automatically treating stopHookActive === true as “revise already used” unless explicitly configured.
• Add metrics/logging for repeated revise decisions to detect plugin misbehavior early.

1. src/plugins/hooks.ts, line 933-960 (link)

   mergeBeforeAgentFinalize propagates revise with an undefined reason

   When a single plugin returns { action: "revise" } (no reason), the merge returns { action: "revise", reason: undefined }. That bubbles into normalizeBeforeAgentFinalizeResult and is silently converted to continue. This is a second place the intent is lost — if the discriminated-union type change above is adopted, the TypeScript compiler will flag this path and the merge function can be updated consistently.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/plugins/hooks.ts
   Line: 933-960
   
   Comment:
   **`mergeBeforeAgentFinalize` propagates `revise` with an `undefined` reason**
   
   When a single plugin returns `{ action: "revise" }` (no reason), the merge returns `{ action: "revise", reason: undefined }`. That bubbles into `normalizeBeforeAgentFinalizeResult` and is silently converted to `continue`. This is a second place the intent is lost — if the discriminated-union type change above is adopted, the TypeScript compiler will flag this path and the merge function can be updated consistently.
   
   How can I resolve this? If you propose a fix, please make it concise.