diff --git a/plugins/security-guidance/hooks/security_reminder_hook.py b/plugins/security-guidance/hooks/security_reminder_hook.py index ddeb813f..6fab7a66 100755 --- a/plugins/security-guidance/hooks/security_reminder_hook.py +++ b/plugins/security-guidance/hooks/security_reminder_hook.py @@ -190,7 +190,13 @@ CONTINUATION_SUFFIX = ( "response." ) -def emit_metrics(metrics, rewake_summary=None): +def emit_metrics( + metrics, + rewake_summary=None, + additional_context=None, + system_message=None, + hook_event_name="PostToolUse", +): """ Write a SyncHookJSONOutput line to stdout for Claude Code to pick up. For asyncRewake (Stop) hooks, CC scans stdout for the first {-prefixed line @@ -213,6 +219,27 @@ def emit_metrics(metrics, rewake_summary=None): rewakeSummary in hooks.json, shown to the user in the terminal as the 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). + + `system_message` (optional, asyncRewake only): user-visible TUI message, + distinct from rewakeSummary which is the task-notification one-liner. + Use sparingly — the rewakeMessage in hooks.json is the primary user + 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". """ head = {} if _PV and "pv" not in metrics: @@ -223,6 +250,17 @@ def emit_metrics(metrics, rewake_summary=None): out = {"metrics": 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 system_message: + out["systemMessage"] = system_message print(json.dumps(out), flush=True) # ===================================================================== @@ -1379,18 +1417,26 @@ def handle_commit_review_posttooluse(input_data): if s in sev: sev[s] += 1 + # Rebuild guidance from new_vulns only — concrete_guidance from the LLM + # still lists deduped entries. Pass via additional_context so CC surfaces + # the reason via hookSpecificOutput.additionalContext instead of empty + # stdout (#1783) / stderr-only "json output validation failed" (#1375). + _commit_guidance = (PROVENANCE_BANNER + "\n\n" + + _format_vulns_guidance(new_vulns) + + CONTINUATION_SUFFIX + "\n") emit_metrics({ "vulns_found": len(new_vulns), **_base, **_agentic_m, "critical_count": sev["critical"], "high_count": sev["high"], "files_reviewed": len(diff_files), "review_ms": review_ms, **({"deduped": n_deduped} if n_deduped else {}), - }, rewake_summary=_format_vulns_summary(new_vulns, prefix="Commit security review found")) + }, rewake_summary=_format_vulns_summary(new_vulns, prefix="Commit security review found"), + additional_context=_commit_guidance, + hook_event_name="PostToolUse") - # Rebuild guidance from new_vulns only — concrete_guidance from the LLM - # still lists deduped entries. - sys.stderr.write(PROVENANCE_BANNER + "\n\n" - + _format_vulns_guidance(new_vulns) - + CONTINUATION_SUFFIX + "\n") + # exit(2) is preserved per the asyncRewake protocol — it's what CC + # uses as the "force fix" signal that triggers the rewakeMessage flow. + # The stderr.write was removed; additional_context above now carries + # the same text via the modern JSON channel. See #1358/#1375/#1783. sys.exit(2) def handle_push_sweep_posttooluse(input_data): @@ -1647,17 +1693,23 @@ def handle_push_sweep_posttooluse(input_data): # Metrics — keep within the 10-key cap; agentic sub-metrics are dropped # here in favour of the push-sweep funnel keys (telemetry can join on session_id # to the per-commit fires for agentic detail). rewake_summary must ride - # this line (CC reads only the first {-prefixed stdout line); it's a - # no-op when new_vulns is empty since we exit 0 below. - emit_metrics({ + # this line (CC reads only the first {-prefixed stdout line); the emit + # is deferred to the two exit points below so the with-vulns path can + # also pass additional_context in the same JSON line (#1375/#1783) — + # the by-design "CC keeps only the first JSON line" constraint means + # we can't emit twice. Builds the shared metrics dict here; vulns path + # adds additional_context, no-vulns path emits as-is. + _push_metrics = { **_base, "pushed": len(push_range), "unreviewed": len(tail), "prefix_advanced": prefix_advanced, "vulns_found": len(new_vulns), "files_reviewed": len(diff_files), "review_ms": review_ms, **({"deduped": n_deduped} if n_deduped else {}), - }, rewake_summary=_format_vulns_summary(new_vulns, prefix="Push security review found")) + } + _push_rewake_summary = _format_vulns_summary(new_vulns, prefix="Push security review found") if not new_vulns: debug_log("Push sweep: no new findings") + emit_metrics(_push_metrics, rewake_summary=_push_rewake_summary) sys.exit(0) # First-push of a big branch can surface many findings at once across @@ -1710,9 +1762,14 @@ def handle_push_sweep_posttooluse(input_data): guidance = _format_vulns_guidance(reported) or "" else: guidance = concrete_guidance or _format_vulns_guidance(reported) or "" - sys.stderr.write( - PROVENANCE_BANNER + "\n\n" + guidance + CONTINUATION_SUFFIX + "\n" - ) + # Emit metrics + additional_context together — single JSON line is the + # contract CC's hook parser expects. exit(2) preserved as the asyncRewake + # "force fix" trigger (see comment near handle_commit_review_posttooluse). + # See #1358 / #1375 / #1783. + emit_metrics(_push_metrics, rewake_summary=_push_rewake_summary, + additional_context=(PROVENANCE_BANNER + "\n\n" + + guidance + CONTINUATION_SUFFIX + "\n"), + hook_event_name="PostToolUse") sys.exit(2) def handle_stop_hook(input_data): @@ -1945,6 +2002,11 @@ def handle_stop_hook(input_data): # untracked_baseline_n is the signal for whether the UPS-time # untracked-snapshot capture actually ran. sweep_trimmed = {k: v for k, v in sweep.items() if k != "warn_unresolved_mask"} + # Pass guidance via additional_context so CC surfaces the findings via + # hookSpecificOutput.additionalContext instead of stderr-only (which + # was the cause of "json output validation failed" / empty-reason UI in + # #1375 / #1783). exit(2) preserved as the asyncRewake "force fix" + # signal — that's the documented mechanism. See #1358 / #1375 / #1783. emit_metrics({ "vulns_found": len(vulns), "untracked_baseline_n": len(untracked_at_baseline), @@ -1958,10 +2020,10 @@ def handle_stop_hook(input_data): **({"diff_truncated": llm._last_review_truncated_bytes} if llm._last_review_truncated_bytes else {}), **sweep_trimmed, - }, rewake_summary=_format_vulns_summary(vulns)) - - # Exit code 2 with stderr forces Claude to continue and fix - sys.stderr.write(PROVENANCE_BANNER + "\n\n" + concrete_guidance + CONTINUATION_SUFFIX + "\n") + }, rewake_summary=_format_vulns_summary(vulns), + additional_context=(PROVENANCE_BANNER + "\n\n" + + concrete_guidance + CONTINUATION_SUFFIX + "\n"), + hook_event_name="Stop") sys.exit(2) if llm._last_call_claude_http_error is not None: