mirror of
https://github.com/anthropics/claude-plugins-official.git
synced 2026-06-14 14:46:03 -03:00
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Mohamed Hegazy
|
475038edfc
|
security-guidance: emit HTTP error codes + fix sdk_bootstrap phase/err encoding
Fills two failure-visibility gaps in plugin telemetry. ## Gap 1: HTTP errors from _call_claude invisible Before: a 4xx/5xx response from the LLM API caused `_call_claude` to return None and produce ZERO fingerprint in tengu_hook_plugin_metrics. A failed call looked identical to "no review needed". The recent deprecation-400 outage (PR #2105, output_format → output_config.format, #2098) was invisible in aggregate dashboards until a user manually reported errors from their debug log. Cohort-specific or partial outages would never show up in BQ. Fix: add `http_err_last` (most recent status) and `http_err_count` to the existing `_USAGE` accumulator in `_base.py`. `_usage_metrics()` snapshots them whenever count > 0 (skip-path no-pollute contract preserved when count == 0). All `_call_claude` error sites now call the new `_record_http_error()` helper alongside the existing `_last_call_claude_http_error` module-state assignment. Now any future API failure category is queryable in BQ in real time: SELECT DATE(server_timestamp, "America/Los_Angeles") AS d, CAST(JSON_VALUE(additional_metadata, "$.http_err_last") AS INT64) AS code, COUNT(*) AS n FROM ... WHERE event_name = "tengu_hook_plugin_metrics" AND JSON_VALUE(additional_metadata, "$.pluginId") LIKE "%security-guidance%" AND JSON_VALUE(additional_metadata, "$.http_err_count") IS NOT NULL GROUP BY d, code ORDER BY d, n DESC ## Gap 2: sdk_bootstrap_phase / sdk_bootstrap_err always NULL in BQ Before: ensure_agent_sdk.py emitted these as strings (e.g. "pip", "dns_fail"). CC's plugin-metrics pipeline silently drops plugin-emitted string values — only bool|finite-number plugin metrics reach BigQuery. (CC-core fields like `subscription_type` are exempt because they're injected downstream of plugin validation.) Confirmed empirically: ~185K BUILD_FAILED rows in BQ over the past 2 days had `sdk_bootstrap_phase` = NULL and `sdk_bootstrap_err` = NULL despite the Python code emitting them. ~28K BUILD_FAILED sessions/day had no diagnostic split — flying blind on whether the failures are pip-no-match vs dns-fail vs ssl-verify vs proxy-auth etc. Fix: encode phase + err_kind as stable integers via SDK_BOOTSTRAP_PHASE_CODES and SDK_BOOTSTRAP_ERR_CODES. Phase: 1=pre, 2=venv, 3=pip, 4=main. Err: 10 known categories (1-10), 11-98 reserved, 99 = uncategorized catch-all (covers "exc:<X>", "other:<X>", and unmapped strings). APPEND-ONLY for telemetry stability. Also corrects the misleading "CC accepts string metric values" comment in ensure_agent_sdk.py that led to the bug originally. Verified locally on macOS Python 3.13: - py_compile clean. - 32 new tests in test_telemetry_failure_signals.py (added to internal test suite at sg-staging/tests/, not in this PR): * 4 HTTP-error tracking unit tests: _record_http_error increments count + tracks most-recent; handles None/invalid; -1 for network/timeout. * 4 _usage_metrics emission tests: empty when no activity; successful call has no http_err fields; failure-only has http_err and no api_calls; mixed has both. * 1 contract test: every emitted value is bool|finite-number (catches future regression of the string-dropping bug class). * 13 sdk_bootstrap encoding tests (parametrized over the 10 known err_kind categories + 5 catch-all shapes): each maps to the right integer; unknown phase = 0; unknown err = 99. * 1 static-shape regression catcher: every `err_kind = "..."` string in ensure_agent_sdk.main() must be in SDK_BOOTSTRAP_ERR_CODES (otherwise new err_kinds silently collapse to 99). * 2 emit-shape regression catchers: the assignments in main() go through _encode_phase / _encode_err_kind helpers (no raw strings); no literal string values for sdk_bootstrap_phase/err. * 1 comment-accuracy: the misleading "CC accepts string metric values" comment is gone. - Full suite: 437/437 pass + 2 skipped (live API tests, opt-in). NOT verified end-to-end against BQ — would require shipping + observing in production for 24h to confirm the http_err and sdk_bootstrap_phase/err fields actually appear in tengu_hook_plugin_metrics rows. The unit tests pin the contract; if the wire shape is broken, BQ will show NULL for the new fields and we revisit (with the same diagnostic the BUILD_FAILED bug gave us). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Mohamed Hegazy
|
0d22ba3501
|
security-guidance: respect CLAUDE_CONFIG_DIR for plugin state files (#1868)
Fixes #1868 — when CLAUDE_CONFIG_DIR is set to a non-default location (e.g. ~/.config/claude for XDG compliance, or a multi-tenant install path), the plugin still wrote state files to the hardcoded ~/.claude/ path, leaving stale state and breaking CLAUDE_CONFIG_DIR's purpose. Resolution precedence (highest first): 1. SECURITY_WARNINGS_STATE_DIR — plugin-specific override (existing) 2. CLAUDE_CONFIG_DIR/security — CC's config-dir env (new — #1868) 3. ~/.claude/security — default fallback (unchanged) Empty-string env vars (e.g. CLAUDE_CONFIG_DIR= in a misconfigured shell) are treated as not-set so the empty path doesn't collide with os.path.join and silently write to /security at the filesystem root. Implementation: a single state_dir() helper in _base.py is the source of truth for resolution. All five modules that previously had inline SECURITY_WARNINGS_STATE_DIR / ~/.claude/security resolutions (_base.py, session_state.py, ensure_agent_sdk.py, llm.py, and one site in security_reminder_hook.py) now call state_dir() instead. Re-implementing the precedence inline risks drift — one module gets a future fix, others don't. The helper is called per-invocation rather than cached at import time so test monkeypatches of the env vars take effect, and so a long- running test or future shared-process scenario can change the env between calls and have the next call observe the new value. The per-call cost is negligible compared to the subprocess-spawn cost the hooks pay every fire in production. Three hardcoded ~/.claude/security strings remain but are NOT functional resolutions: - _base.py:39: the fallback BRANCH inside state_dir() itself - ensure_agent_sdk.py:6, :11: docstring text describing default location for users Verified locally on macOS Python 3.13: - py_compile clean on all 5 modified files. - Existing 45 smoke + extensibility tests still pass. - 14 new tests in test_claude_config_dir.py (added to internal test suite at sg-staging/tests/, not in this PR): * 7 resolution-semantics: default fallback, CLAUDE_CONFIG_DIR override, SECURITY_WARNINGS_STATE_DIR beats both, tilde expansion, empty-string handling (CLAUDE_CONFIG_DIR= must fall back, NOT join to /security). * 4 static-shape: each of session_state / ensure_agent_sdk / llm / security_reminder_hook either imports state_dir from _base OR has zero resolution patterns. Catches the regression where someone adds a new state-file writer and re-implements resolution inline, missing the CLAUDE_CONFIG_DIR branch. * 3 end-to-end: with CLAUDE_CONFIG_DIR set, get_state_file / get_lock_file return paths under <CLAUDE_CONFIG_DIR>/security/; save_state round-trip writes a file to the redirected path and re-reads the same contents. - 59/59 pass total (45 existing + 14 new) in 2.54s. NOT verified end-to-end with a real CC instance setting CLAUDE_CONFIG_DIR. The shape tests catch the regression class (hardcoded ~/.claude/), and the end-to-end test pins the behavior that user state files actually land at the redirected path. Closes #1868. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Mohamed Hegazy
|
0bde168648
|
Update security-guidance plugin |