mirror of
https://github.com/anthropics/claude-plugins-official.git
synced 2026-06-15 07:06:07 -03:00
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Mohamed Hegazy
|
84011d43b1
|
security-guidance: migrate from deprecated output_format to output_config.format (#2098)
Fixes #2098. The Anthropic Messages API moved structured-output schema specification from a top-level `output_format` field to a nested `output_config.format` field, per https://platform.claude.com/docs/en/build-with-claude/structured-outputs. Per docs the old form "will continue working for a transition period" — and indeed for api-key + non-streaming auth it still returns HTTP 200 (verified via live API). But OAuth Bearer users with CLI 2.1.158 hit `invalid_request_error: output_format: This field is deprecated. Use 'output_config.format' instead.` consistently — reporter saw 462 errors in one day. The trigger appears to be auth mode + possibly stream:true (their controlled curl bypass used Bearer + stream=true); api-key + non-streaming was my initial repro attempt and didn't fire. The bug only affected `_call_claude` (the legacy direct-urllib path). The agentic `_agentic_review` path goes through claude_agent_sdk → subprocesses to the `claude` CLI binary, which already uses the new `output_config.format` shape correctly (per src/utils/sideQuery.ts:263 in claude-cli-internal). So this PR only needs to fix the plugin's direct HTTP path. This commit: 1. llm.py: rewrite the payload literal in `_call_claude` to use `output_config: { format: { type: 'json_schema', schema: ... } }` instead of top-level `output_format`. 2. llm.py: in the adaptive-thinking branch, MERGE `effort: "high"` into the existing `output_config` dict instead of reassigning. Reassignment would silently clobber the format schema set in (1). The pre-existing code did `payload["output_config"] = {"effort": "high"}` which was correct WHEN output_format was top-level (and output_config wasn't otherwise used). With the migration the existing dict carries the schema, so we extend it not replace it. Verified locally on macOS Python 3.13: - py_compile clean. - Existing 401 tests still pass — 0 regression. - 6 new tests in test_2098_output_config_format.py (added to internal test suite at sg-staging/tests/, not in this PR): * 2 static-shape: the `_call_claude` source no longer contains top-level `"output_format":` AND uses `output_config`. The adaptive-thinking branch does NOT reassign output_config (and DOES set output_config['effort']). Catches the regression class where a future refactor reintroduces either bug. * 2 payload-shape unit (mocked urllib): both thinking_budget=0 and thinking_budget>0+adaptive code paths produce a payload with the correct `output_config.format` shape AND no `output_format` top-level. The adaptive path verifies both `format` and `effort` coexist in output_config (i.e., the merge fix works). * 2 live-API gating (skip-on-no-key): the new shape returns HTTP 200 against api.anthropic.com; the old shape's current status is recorded for canary purposes (still 200 for api-key today, but reporter shows it's 400 for OAuth). - Full suite: 405/405 pass + 2 skipped (live API tests, opt-in). - The reporter's exact deprecation 400 message reproduces if you swap auth to OAuth Bearer + stream:true (could not test locally without extracting the keychain OAuth token, which was out of scope). The fix shape is API-contract-level so it doesn't depend on which auth mode triggers the 400. NOT verified end-to-end via OAuth-authenticated plugin invocation on my machine (auto-mode classifier correctly declined to extract the keychain token). Reporter's 462 production errors + the docs migration notice + the live-API HTTP 200 on the new form are sufficient evidence to ship. Closes #2098. 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
|
c11244778d
|
Address Windows verification: --prefer-binary + pywin32 bootstrap
The first round of this PR removed SKIP_WIN32, fixed venv_py to use Scripts/python.exe, and added Lib/site-packages to the consumer glob — all necessary. Windows verification (Win11 ARM64, Py 3.13, Git Bash) showed two more blockers, both addressed here. 1. Pip dependency resolver picks unbuildable cryptography on ARM64. Without --prefer-binary, pip picks a cryptography version with no published ARM64 wheel and tries to build it from source. That needs Rust/Cargo, almost never present on user machines → BUILD_FAILED with err_kind=other:cryptography. A binary wheel exists for an adjacent version (cryptography-46.0.3-cp311-abi3-win_arm64.whl); --prefer-binary tells pip to pick it. Cross-platform safe (no-op where the latest version already has a wheel). 2. pywin32 .pth files aren't processed by sys.path.insert(). With the venv built, ensure_agent_sdk.py's post-build probe passes (it runs from venv_py, where Python's site.py at startup processes pywin32.pth and registers win32/, win32/lib/ plus runs pywin32_bootstrap.py to set the DLL search dir). But llm.py runs in the hook's SYSTEM Python and adds the venv via sys.path.insert(), which doesn't trigger site.py at all. Without the bootstrap, the SDK's mcp.client.stdio → mcp.os.win32.utilities chain raises ModuleNotFoundError: pywintypes and the agentic reviewer falls back to single-shot silently — exactly the symptom this PR is trying to fix. The probe says NOOP_VENV; the actual consumer fails. Probe and consumer use different Pythons. Replicate what site.py would do: after inserting site-packages, also insert win32/ and win32/lib/, then exec pywin32_bootstrap.py. Pulled into a shared helper _inject_agent_sdk_venv_into_syspath() so both consumer sites (3P SDK fallback, agentic_review fallback) call the same code — Windows handling stays in one place. Verified on macOS (POSIX path unchanged): - Helper end-to-end test: POSIX-layout venv detected + fake package imports successfully via the injected path - Windows-layout venv also detected; win32 branch correctly skipped via sys.platform check - Both files pass py_compile Credit: @mhegazy verified the previous commit on Win11 ARM64 / Py 3.13 / Git Bash, surfaced both issues end-to-end, and provided the exact fix patterns. This commit applies them with the pywin32 part factored into a shared helper (vs. inlining at both consumer sites). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Mohamed Hegazy
|
4decd2e3b2
|
Enable agentic commit reviewer on Windows
The agentic reviewer is silently no-op on Windows today. SessionStart bootstrap (ensure_agent_sdk.py) short-circuits with SKIP_WIN32 because the consumer glob in llm.py only matches POSIX venv layout (lib/pythonX.Y/site-packages). On Windows, venvs use Lib/site-packages (capital L, no pythonX.Y subdir), so even if a venv got built the glob wouldn't find its contents. Result: Windows users on default installs (no system-wide claude_agent_sdk) get layer 1 (pattern warnings) and layer 2 (single- shot LLM diff review) but not layer 3 — the cross-file agentic review that catches IDOR, auth-bypass, cross-file SSRF, and other things that need to read related files. Plugin description claims layer 3 but it silently doesn't run. Three changes: 1. llm.py — extend the consumer glob (2 sites: 3P SDK fallback at ~L297, agentic_review fallback at ~L1090) to also match the Windows Lib/site-packages layout, so a venv built on Windows is actually discoverable. 2. ensure_agent_sdk.py — remove the sys.platform == 'win32' early-exit so the SessionStart bootstrap builds the venv on Windows too. Outcome code 4 (formerly SKIP_WIN32) is retired but not reused so pre-fix telemetry rows still decode correctly. 3. ensure_agent_sdk.py — venv_py path now branches on sys.platform: Windows venvs put the interpreter at Scripts\python.exe; POSIX uses bin/python. Previously assumed POSIX, so even with the glob fix, the post-build SDK-importability probe would fail on Windows. Verified locally on macOS: - glob test: both layouts now match (POSIX venv detected, simulated Windows venv also detected via the new Lib/site-packages branch) - both files pass py_compile - POSIX path unchanged (sys.platform != 'win32' so old branch runs) Not verified on Windows in this commit — needs an actual Windows runner to confirm the venv build + SDK import + subprocess plumbing all work end-to-end. The SDK spawns a child claude.exe; Windows process plumbing has its own quirks (shell semantics, path escaping) that may surface separately. Worth a controlled rollout (one-week soak under env-var opt-in before flipping default). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> |
||
|
Mohamed Hegazy
|
0bde168648
|
Update security-guidance plugin |