security-guidance: emit schema-valid Stop-hook output (#2159) — 2.0.2 → 2.0.3

Fixes #2159. The Stop hook emits feedback via
`hookSpecificOutput: {hookEventName: "Stop", additionalContext}`, but
`Stop`/`SubagentStop` are NOT members of CC's `hookSpecificOutput`
discriminated union (coreSchemas.ts — valid members are PreToolUse,
PostToolUse, UserPromptSubmit, SessionStart, etc.). So the emitted shape
violates CC's documented hook-output schema.

Impact is CC-version-dependent — important nuance, established empirically:

  - Reporter (array0224-cloud) on CLI 2.1.150 / 2.1.152: CC rejects the
    Stop feedback; the block/reason never reaches the model, so the
    auto-rewake/fix loop is lost. (Detection still runs + logs.)

  - On CLI 2.1.160 (current) the asyncRewake completion path is lenient:
    its gate is `isSyncHookJSONOutput` (hooks.ts) which is just
    `!(json.async === true)` — NOT a strict schema parse. So the invalid
    hookSpecificOutput is tolerated: metrics + rewakeSummary are still
    consumed and the model still receives the findings. I could NOT
    reproduce the rejection on 2.1.160, and BQ confirms Stop-path
    vulns_found metrics are recorded normally (~21k with-vuln fires / 3d),
    i.e. NOT dropped. (An earlier draft of this message claimed metrics
    were dropped — that was wrong; corrected after checking telemetry +
    repro'ing the old plugin on 2.1.160.)

So this is defensive schema-correctness: the plugin should emit output
that conforms to CC's documented union regardless of how strictly a given
CC version validates it. The reporter's environment validates strictly;
relying on the current version's leniency is fragile.

Fix (CC's documented asyncRewake "clean pattern" — hooks.ts: "error text
on stderr, JSON on stdout"):
  - For Stop/SubagentStop, emit_metrics writes guidance to stderr (the
    asyncRewake body channel CC delivers via `stderr || stdout`) and sets
    top-level `decision: "block"` + `reason` (valid SyncHookJSONOutput
    fields; also the documented sync Stop-hook contract for the `-p`
    fallback). It does NOT emit a Stop hookSpecificOutput.
  - PostToolUse (commit-review, push-sweep) is unchanged — valid union
    member, keeps the modern hookSpecificOutput protocol.

Verified locally on macOS Python 3.13:

  - py_compile clean.
  - 11 new tests (test_2159_stop_hook_schema.py) pin the contract: Stop
    output carries no hookSpecificOutput, uses top-level decision/reason,
    writes guidance to stderr; the emitted hookEventName (when present) is
    a valid union member. 2 existing tests that asserted the buggy
    Stop->hookSpecificOutput shape were corrected. Full suite 464/464
    pass + 2 skipped.

  - END-TO-END in /tmux on CLI 2.1.160:
    * FIXED plugin (2.0.3): staged pickle.loads + os.system, benign edit
      pulls the file into review_set; Stop LLM review found 2 critical
      vulns; CC delivered a clean rewake ("Background security review
      found issues" + both findings). All hooks (UPS, PostToolUse[Edit]
      pattern, PostToolUse[Bash] commit-review, Stop) fired clean; zero
      schema rejections / errors / http_err in the debug log.
    * OLD plugin (2.0.2) on the SAME 2.1.160: also delivered Stop feedback
      (confirming the no-repro-on-latest finding above) — which proves the
      fix carries NO regression risk on current CC while making the output
      robust for the stricter versions where it actually breaks.

Version bumped 2.0.2 -> 2.0.3 per the per-PR-bump policy (#2114: a bump is
the only way the fix reaches the existing fleet — relevant for users still
on the CC versions where this breaks).

Closes #2159.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

.claude-plugin/marketplace.json

@@ -2215,7 +2215,7 @@
     {
       "name": "security-guidance",
       "description": "Security review for Claude-generated code. Pattern-based warnings on edits, LLM-powered diff review on Stop, and an agentic commit reviewer that catches injection, XSS, SSRF, hardcoded secrets, and 25+ other vulnerability classes.",
-      "version": "2.0.2",
+      "version": "2.0.3",
       "author": {
         "name": "Anthropic",
         "email": "support@anthropic.com"

plugins/security-guidance/.claude-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "security-guidance",
-  "version": "2.0.2",
+  "version": "2.0.3",
   "description": "Security review for Claude-generated code. Pattern-based warnings on edits, LLM-powered diff review on Stop, and an agentic commit reviewer that catches injection, XSS, SSRF, hardcoded secrets, and 25+ other vulnerability classes.",
   "author": {
     "name": "David Dworken",

plugins/security-guidance/hooks/security_reminder_hook.py

@@ -221,15 +221,34 @@ def emit_metrics(
     task-notification one-liner. Must be in the same JSON line as the metrics
     because CC stops scanning stdout after the first {-prefixed line.
 
-    `additional_context` (asyncRewake findings): model-visible guidance text
-    that CC surfaces via the modern hook-output protocol
-    (hookSpecificOutput.additionalContext) instead of the legacy stderr +
-    exit(2) pair. The caller passes the finding-explanation text it would
-    have written to stderr; the JSON channel carries it cleanly so CC's UI
-    shows the reason properly instead of "Permission denied with no reason".
-    See anthropics/claude-plugins-official#1375 and #1783. Empty/None
-    means no hookSpecificOutput field is emitted (preserves backward compat
-    for legacy emit-sites that only want metrics).
+    `additional_context` (asyncRewake findings): model-visible guidance text.
+    Delivery channel depends on `hook_event_name` because CC's hook-output
+    contract is NOT symmetric across events:
+
+      - PostToolUse (commit-review, push-sweep): surfaced via the modern
+        hookSpecificOutput.additionalContext protocol. `PostToolUse` is a
+        member of CC's hookSpecificOutput discriminated union
+        (coreSchemas.ts), so the JSON validates and metrics/rewakeSummary
+        are consumed. See #1375 / #1783 for why this replaced the legacy
+        stderr + exit(2) shape for PostToolUse.
+
+      - Stop / SubagentStop: there is NO `Stop` member in that union, so
+        emitting hookSpecificOutput{hookEventName:"Stop"} makes the whole
+        line fail isSyncHookJSONOutput validation — which on the asyncRewake
+        path silently drops metrics AND rewakeSummary, and (because the
+        legacy stderr write was removed) leaks the raw JSON to the model as
+        the rewake body. CC's asyncRewake delivery actually reads
+        `stderr || stdout` for the model-visible body and only scans stdout
+        JSON for metrics+rewakeSummary — it never reads additionalContext
+        on this path. So for Stop we use the documented clean pattern:
+        guidance on stderr, valid JSON (metrics + rewakeSummary +
+        top-level decision/reason) on stdout. The top-level decision:"block"
+        + reason also covers the sync-fallback path (single-shot `claude -p`,
+        where asyncRewake degrades to a sync Stop hook that reads
+        decision/reason). See #2159.
+
+    Empty/None additional_context emits neither channel (back-compat for
+    metrics-only callers).
 
     `system_message` (optional, asyncRewake only): user-visible TUI message,
     distinct from rewakeSummary which is the task-notification one-liner.
@@ -237,10 +256,9 @@ def emit_metrics(
     surface; systemMessage adds a per-fire override when the static
     rewakeMessage isn't specific enough for the finding being shown.
 
-    `hook_event_name` (used only when additional_context is set): which event
-    the hookSpecificOutput attaches to. Defaults to "PostToolUse" since the
-    commit-review and push-sweep handlers are the most common callers;
-    handle_stop_hook explicitly passes "Stop".
+    `hook_event_name` (used only when additional_context is set): selects the
+    delivery channel above. Defaults to "PostToolUse" (commit-review and
+    push-sweep are the most common callers); handle_stop_hook passes "Stop".
     """
     head = {}
     if _PV and "pv" not in metrics:
@@ -252,14 +270,23 @@ def emit_metrics(
     if rewake_summary:
         out["rewakeSummary"] = rewake_summary
     if additional_context:
-        # Wrap in hookSpecificOutput per CC's modern hook-output contract.
-        # Drops the legacy `sys.stderr.write(...) + sys.exit(2)` shape that
-        # left CC's UI showing "denied with no reason" (#1783) and triggered
-        # "json output validation failed" on older CC versions (#1375).
-        out["hookSpecificOutput"] = {
-            "hookEventName": hook_event_name,
-            "additionalContext": additional_context,
-        }
+        if hook_event_name in ("Stop", "SubagentStop"):
+            # Stop is NOT in CC's hookSpecificOutput union — emitting it there
+            # fails schema validation and drops metrics+rewakeSummary (#2159).
+            # Clean pattern: guidance on stderr (the asyncRewake body channel,
+            # delivered via `stderr || stdout`), top-level decision/reason for
+            # the sync-fallback path. stdout JSON stays valid so metrics +
+            # rewakeSummary survive.
+            sys.stderr.write(additional_context)
+            sys.stderr.flush()
+            out["decision"] = "block"
+            out["reason"] = additional_context
+        else:
+            # PostToolUse et al. — valid union member; modern protocol.
+            out["hookSpecificOutput"] = {
+                "hookEventName": hook_event_name,
+                "additionalContext": additional_context,
+            }
     if system_message:
         out["systemMessage"] = system_message
     print(json.dumps(out), flush=True)