mirror of
https://github.com/anthropics/claude-plugins-official.git
synced 2026-06-13 22:26:03 -03:00
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>
This commit is contained in:
parent
68a700837c
commit
0d22ba3501
@ -10,15 +10,42 @@ import os
|
||||
import threading
|
||||
from datetime import datetime
|
||||
|
||||
def state_dir():
|
||||
"""Return the absolute path of the plugin's state directory.
|
||||
|
||||
Resolution precedence (highest first):
|
||||
1. SECURITY_WARNINGS_STATE_DIR — plugin-specific override (existing)
|
||||
2. CLAUDE_CONFIG_DIR/security — CC's config-dir env var (#1868)
|
||||
3. ~/.claude/security — default fallback
|
||||
|
||||
Empty-string env vars are treated as not-set so a misconfigured shell
|
||||
(`CLAUDE_CONFIG_DIR=` with no value) doesn't silently write to
|
||||
/security at the filesystem root.
|
||||
|
||||
Returns a fully-expanded absolute path (no literal `~`) so subprocess
|
||||
callers can pass it through to code that doesn't re-expand tildes.
|
||||
|
||||
Called per-invocation rather than cached at import time so test
|
||||
monkeypatches of the env vars take effect — the plugin's hooks each
|
||||
run as fresh subprocesses in production, so the per-call cost is
|
||||
negligible compared to subprocess spawn.
|
||||
"""
|
||||
explicit = os.environ.get("SECURITY_WARNINGS_STATE_DIR")
|
||||
if explicit:
|
||||
return os.path.expanduser(explicit)
|
||||
cc_config = os.environ.get("CLAUDE_CONFIG_DIR")
|
||||
if cc_config:
|
||||
return os.path.expanduser(os.path.join(cc_config, "security"))
|
||||
return os.path.expanduser("~/.claude/security")
|
||||
|
||||
|
||||
# Debug log file. Lives under the plugin state dir (default ~/.claude/security/)
|
||||
# rather than /tmp because /tmp is world-writable on multi-user hosts (TOCTOU /
|
||||
# symlink-attack surface, cross-user log leakage). Overridable per-process via
|
||||
# SECURITY_GUIDANCE_DEBUG_LOG, or per-state-dir via SECURITY_WARNINGS_STATE_DIR.
|
||||
_DEFAULT_STATE_DIR = os.path.expanduser(
|
||||
os.environ.get("SECURITY_WARNINGS_STATE_DIR") or "~/.claude/security"
|
||||
)
|
||||
# SECURITY_GUIDANCE_DEBUG_LOG, or per-state-dir via SECURITY_WARNINGS_STATE_DIR
|
||||
# (plugin-specific override) or CLAUDE_CONFIG_DIR (CC-wide config dir, #1868).
|
||||
DEBUG_LOG_FILE = os.environ.get("SECURITY_GUIDANCE_DEBUG_LOG") or os.path.join(
|
||||
_DEFAULT_STATE_DIR, "log.txt"
|
||||
state_dir(), "log.txt"
|
||||
)
|
||||
# Cap the debug log so parallel-worker fleets don't fill disk. When the active
|
||||
# file exceeds this it's atomically rotated to <file>.1 (overwriting any prior
|
||||
|
||||
@ -23,6 +23,12 @@ import sys
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
# Shared state-dir resolver: SECURITY_WARNINGS_STATE_DIR → CLAUDE_CONFIG_DIR/security
|
||||
# → ~/.claude/security. See _base.state_dir for resolution precedence. Re-aliased
|
||||
# here to match the existing local name (state_dir was already a local var in
|
||||
# main() and _maybe_emit_user_notice).
|
||||
from _base import state_dir as _resolve_state_dir
|
||||
|
||||
# Outcome codes for the sdk_bootstrap metric. Values are stable for telemetry.
|
||||
NOOP_SYSTEM = 0 # claude_agent_sdk already importable in system python
|
||||
NOOP_VENV = 1 # venv already built and SDK imports from it
|
||||
@ -90,10 +96,7 @@ def main() -> tuple[int, str, str]:
|
||||
if _sdk_on_syspath():
|
||||
return NOOP_SYSTEM, "", ""
|
||||
|
||||
state_dir = Path(
|
||||
os.environ.get("SECURITY_WARNINGS_STATE_DIR")
|
||||
or os.path.expanduser("~/.claude/security")
|
||||
)
|
||||
state_dir = Path(_resolve_state_dir())
|
||||
venv = state_dir / "agent-sdk-venv"
|
||||
# Windows venvs put the interpreter at Scripts\python.exe; POSIX uses bin/python.
|
||||
if sys.platform == "win32":
|
||||
@ -239,10 +242,7 @@ def _maybe_emit_user_notice(outcome: int, pv: int) -> str | None:
|
||||
if outcome != HOOK_PY_INCOMPATIBLE:
|
||||
return None
|
||||
try:
|
||||
state_dir = Path(
|
||||
os.environ.get("SECURITY_WARNINGS_STATE_DIR")
|
||||
or os.path.expanduser("~/.claude/security")
|
||||
)
|
||||
state_dir = Path(_resolve_state_dir())
|
||||
marker = state_dir / f".agentic_unavailable_notice_v{pv or 0}"
|
||||
if marker.exists():
|
||||
return None
|
||||
|
||||
@ -27,7 +27,7 @@ from typing import Optional, Tuple, Dict, Any, List
|
||||
|
||||
import extensibility
|
||||
import review_api
|
||||
from _base import debug_log, _record_usage, _PV, PROVENANCE_TAG # noqa: F401
|
||||
from _base import debug_log, _record_usage, _PV, PROVENANCE_TAG, state_dir as _resolve_state_dir # noqa: F401
|
||||
from session_state import with_locked_state
|
||||
|
||||
|
||||
@ -355,10 +355,7 @@ def _call_claude_via_sdk(prompt, output_schema, *, max_tokens=16000, model=None)
|
||||
# Try the venv ensure_agent_sdk.py builds. Same fallback logic as
|
||||
# agentic_review() — duplicated here so the 3P path doesn't require
|
||||
# the agentic path to have run first.
|
||||
_state_dir = os.environ.get(
|
||||
"SECURITY_WARNINGS_STATE_DIR",
|
||||
os.path.expanduser("~/.claude/security"),
|
||||
)
|
||||
_state_dir = _resolve_state_dir()
|
||||
_inject_agent_sdk_venv_into_syspath(_state_dir)
|
||||
try:
|
||||
import asyncio as _asyncio # noqa: F811
|
||||
@ -1145,10 +1142,7 @@ def agentic_review(
|
||||
# ~/.claude/security/ with the SDK installed; try that as a fallback
|
||||
# before giving up. The system import is attempted first so users
|
||||
# who DO have it never touch the venv.
|
||||
_state_dir = os.environ.get(
|
||||
"SECURITY_WARNINGS_STATE_DIR",
|
||||
os.path.expanduser("~/.claude/security"),
|
||||
)
|
||||
_state_dir = _resolve_state_dir()
|
||||
_venv_tried = _inject_agent_sdk_venv_into_syspath(_state_dir)
|
||||
try:
|
||||
import asyncio as _asyncio # noqa: F811
|
||||
|
||||
@ -82,6 +82,7 @@ from _base import ( # noqa: E402,F401
|
||||
PROVENANCE_TAG, PROVENANCE_BANNER,
|
||||
_read_plugin_version_int, _PV, _USAGE, _USAGE_LOCK,
|
||||
_PRICE_PER_MTOK, _PRICE_DEFAULT, _record_usage, _usage_metrics,
|
||||
state_dir as _resolve_state_dir,
|
||||
)
|
||||
import extensibility # noqa: E402
|
||||
from patterns import ( # noqa: E402,F401
|
||||
@ -1971,10 +1972,7 @@ def handle_stop_hook(input_data):
|
||||
})
|
||||
sys.exit(0)
|
||||
|
||||
_SDK_BOOTSTRAP_THROTTLE = os.path.join(
|
||||
os.environ.get("SECURITY_WARNINGS_STATE_DIR")
|
||||
or os.path.expanduser("~/.claude/security"),
|
||||
".sdk_bootstrap_spawned")
|
||||
_SDK_BOOTSTRAP_THROTTLE = os.path.join(_resolve_state_dir(), ".sdk_bootstrap_spawned")
|
||||
|
||||
def _maybe_bootstrap_agent_sdk_async():
|
||||
"""Fire-and-forget SDK bootstrap, for remote-pod environments.
|
||||
|
||||
@ -19,7 +19,7 @@ import os
|
||||
import re
|
||||
from datetime import datetime
|
||||
|
||||
from _base import debug_log
|
||||
from _base import debug_log, state_dir as _state_dir
|
||||
|
||||
|
||||
def _state_key(session_id):
|
||||
@ -36,20 +36,20 @@ def _state_key(session_id):
|
||||
|
||||
def get_state_file(session_id):
|
||||
"""Get session-specific state file path."""
|
||||
state_dir = os.environ.get("SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"))
|
||||
state_dir = _state_dir()
|
||||
return os.path.join(state_dir, f"security_warnings_state_{_state_key(session_id)}.json")
|
||||
|
||||
|
||||
def get_lock_file(session_id):
|
||||
"""Get session-specific lock file path."""
|
||||
state_dir = os.environ.get("SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"))
|
||||
state_dir = _state_dir()
|
||||
return os.path.join(state_dir, f"security_warnings_state_{_state_key(session_id)}.lock")
|
||||
|
||||
|
||||
def cleanup_old_state_files():
|
||||
"""Remove state files and lock files older than 30 days."""
|
||||
try:
|
||||
state_dir = os.environ.get("SECURITY_WARNINGS_STATE_DIR", os.path.expanduser("~/.claude/security"))
|
||||
state_dir = _state_dir()
|
||||
if not os.path.exists(state_dir):
|
||||
return
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user