WEB-4882: honor CLAUDE_CONFIG_DIR in Claude Code hooks install - #175
WEB-4882: honor CLAUDE_CONFIG_DIR in Claude Code hooks install#175MohamedAklamaash wants to merge 30 commits into
Conversation
Resolve the Claude config dir from --config-dir / CLAUDE_CONFIG_DIR (fallback ~/.claude) in setup.py and at hook runtime in unbound.py, so hooks, settings, the baked command path, audit log, cache, and backfill transcripts all live where Claude reads them when a custom dir is set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
🛡️ Automated Security Review (consensus)3 findings — 3 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks. 🔴 HIGH — Installer/runtime config-dir resolution mismatch
Impact: Fix: Bake the resolved config dir into the hook invocation (e.g. pass Flagged by: Cursor, Claude 🟡 MEDIUM — Whitespace-only
|
- unbound.py: resolve the config dir from the hook's own install location (__file__), so runtime paths always match where the installer wrote them, regardless of how CLAUDE_CONFIG_DIR is propagated into the hook env. - setup.py: strip whitespace-only CLAUDE_CONFIG_DIR, and don't let --config-dir swallow a following flag as its value. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Addressed the review feedback in
|
….json - unbound.py: resolve _CONFIG_DIR from CLAUDE_CONFIG_DIR (stripped) again, not __file__. Deriving from __file__ made SELF_SCRIPT_PATH always equal the running script, defeating the MDM self-update guard that must skip admin-managed installs. Strip the env value so whitespace-only falls back to ~/.claude consistently for both _CONFIG_DIR and _config_dir_is_default. - unbound.py: CLAUDE_MCP_CONFIG_PATH probes $CONFIG_DIR/.claude.json and falls back to ~/.claude.json, so account-identity reads never break if Claude keeps the OAuth config at the home sibling. - setup.py: strip the --config-dir value too, matching the env handling. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Thanks — the re-review caught that my
On the original [High] install-arg / runtime-env mismatch: in the real flow this can't diverge — the CLI computes
|
🛡️ Automated Security Review (consensus)0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks. ✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks) Previously acknowledged (not re-flagged)
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head |
Make setup.py prioritize CLAUDE_CONFIG_DIR (env) over --config-dir, with the CLI arg as fallback. unbound.py resolves runtime paths from the same env, so install-time placement and runtime resolution now agree by the same precedence instead of diverging. The CLI passes --config-dir derived from CLAUDE_CONFIG_DIR, so the gated real flow is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
[High] Hook config dir ignores CLI arg — fixed in |
🛡️ Automated Security Review (consensus)0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks. ✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks) Previously acknowledged (not re-flagged)
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head |
|
[High] Clear misses custom config directory — by design, with the same precedence as install. The narrow gap is purely an ad-hoc-env one (set |
…aude-config-dir # Conflicts: # claude-code/hooks/setup.py # claude-code/hooks/test_setup.py # claude-code/hooks/unbound.py
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)
Previously acknowledged (not re-flagged)
- Install
--config-dirvsCLAUDE_CONFIG_DIR/ runtime mismatch — env-first precedence is deliberate; CLI and Claude runtime both derive fromCLAUDE_CONFIG_DIR, so install and hook paths align in the normal flow. - MDM self-update guard —
__file__-based_CONFIG_DIRwas reverted; env-based resolution restores the managed-install skip for__file__ != SELF_SCRIPT_PATH. .claude.jsonlocation under custom config dir — intentional: probe$CONFIG_DIR/.claude.json, fall back to~/.claude.json.--clear/--backfillwithout ad-hoc env — accepted by design; same resolution as install; persistentCLAUDE_CONFIG_DIRis required (follow-up offered for home-anchored state).- Whitespace-only
CLAUDE_CONFIG_DIR/--config-dir, flag-as-value parsing — fixed via.strip()andnot argv[i+1].startswith("--"). - Semgrep
insecure-file-permissions(0o700/0o755) — pre-existing chmod patterns; 0o700 is owner-only (stricter than 0o644); not introduced by this diff. - Semgrep
dynamic-urllib-usein tests — test-only mock URL fetch; out of scope for this path-threading change.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head d74ba52a · 2026-06-26T06:11Z
🛡️ Automated Security Review (consensus)0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks. ✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks) Previously acknowledged (not re-flagged)
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head |
Mirror the hooks installer: resolve the config dir from CLAUDE_CONFIG_DIR / the --config-dir arg the CLI forwards (else ~/.claude), and thread it through the key-helper writer, settings, install-state detection, and clear. apiKeyHelper keeps the portable ~/.claude form for the default dir and uses the absolute path when relocated so Claude resolves it under the active dir. Adds gateway test_setup. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
🛡️ Automated Security Review (consensus)1 finding — 0 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks. 🟡 TRIAGE — Symlinked config dir breaks hook detection on clear/reinstall
Previously acknowledged (not re-flagged)
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head |
Strip WEB-4882 references and redundant inline comments; behavior unchanged.
🛡️ Automated Security Review (consensus)0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks. ✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks) Previously acknowledged (not re-flagged)
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head |
…lear sweep, portable apiKeyHelper - unbound.py: resolve .claude.json AND plugins/cache the same way — prefer the relocated dir when it has the artifact, else the legacy ~/.claude location — so MCP/plugin policy reads from wherever Claude actually stores them. - setup --clear (hooks + gateway): when the config dir is relocated, also strip enforcement left behind in the default ~/.claude so nothing fires if Claude later runs without CLAUDE_CONFIG_DIR. - gateway apiKeyHelper: compare resolved paths so the portable ~/.claude form is kept even on symlinked HOME / when CLAUDE_CONFIG_DIR equals the default dir. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)
Previously acknowledged (not re-flagged)
- Installer/runtime
--config-dirvsCLAUDE_CONFIG_DIRmismatch — accepted design: env wins; arg-only installs now warn because Claude Code reads the env alone (MohamedAklamaash,AakashVelusamy). --clear/--backfillwithout persisted custom dir — accepted by design: same resolver as install; requires the profile’sCLAUDE_CONFIG_DIR(MohamedAklamaash).- Legacy
~/.claudesweep omitsunbound-setup.py/.last_updated— accepted limitation: inert once settings hooks are removed (MohamedAklamaash,AakashVelusamy, Greptile). .claude.json/ plugin-cache placement assumptions — resolved against Claude Code 2.1.177; deterministic paths now match the shipped resolver (AakashVelusamy).- Semgrep
0o700/0o755“insecure-file-permissions” — false positive: owner-only modes are intentional for hook/key-helper directories, not a regression from this PR.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 4333f590 · 2026-08-24T14:13Z
staging landed WEB-5526 (#259), which makes each mode remove only the artifacts Unbound installed rather than anything sharing the conventional filename. Two conflicts, both the same shape as the unbound-cli side: their "is this ours" guard against our config-dir parameter. Kept both, so the guard now runs against the resolved dir instead of a hardcoded ~/.claude — otherwise a relocated gateway install would clear nothing. Checked the interaction that bit on the CLI side, where an equivalent guard matched on the path and so failed to recognise a helper outside a directory named .claude. This one compares the file body against the text the gateway writer emits, so it is path-agnostic and a relocated install is recognised unchanged. No fix needed. The legacy ~/.claude sweep stays unguarded on purpose: it removes unbound.py and our own settings entries, which are ours by construction, and it does not touch anthropic_key.sh there. Also confirmed while testing Windows semantics that the default-dir comparison in clear_setup needs no case folding — pathlib compares Windows paths case-insensitively already, unlike the JS string compare on the CLI side, which did need it. Tests: hooks 56 passed, gateway 11 passed, with the pre-existing MDM matcher-parity failure unchanged. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)
Previously acknowledged (not re-flagged)
- Gateway legacy sweep deletes
~/.claude/anthropic_key.shwithout ownership guard (claude-code/gateway/setup.py:676) — Greptile; intentional legacy cleanup on--clearwith a relocated config dir (test asserts deletion even for non-Unbound content). - Hooks legacy sweep omits
unbound-setup.py/.last_updated(claude-code/hooks/setup.py:775) — Greptile/Cursor; accepted limitation — files are inert once the settings hook entry is removed (AakashVelusamy). CLAUDE_CONFIG_DIRenv beats--config-dir(claude-code/hooks/setup.py:65) — Greptile; accepted design — Claude Code resolves the config dir from the environment alone, so the arg must not override it (MohamedAklamaash, AakashVelusamy).- Clear/backfill without persistent install path — Cursor; by design —
--clear/--backfilluse the same resolver as install and require the sameCLAUDE_CONFIG_DIRClaude uses for that profile (MohamedAklamaash). - Semgrep
insecure-file-permissions/dynamic-urllib-use-detected— pre-existing patterns unchanged by this diff; not introduced here (Claude).
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 8ad50030 · 2026-08-24T15:25Z
Merging staging's ownership rule left one path uncovered, and it was the destructive one. WEB-5526 stopped clear from removing an anthropic_key.sh we did not write, but this branch's legacy ~/.claude sweep — which only runs when the config dir is relocated — deleted that file unconditionally. Anyone keeping their own helper of that name would have lost it to a --clear under a custom CLAUDE_CONFIG_DIR. The sweep now applies the same content check as the primary path, so it removes only the script the gateway writer emitted. The existing sweep test wrote "echo x" as the legacy helper and asserted it was deleted, which predates the ownership rule and would now be asserting the wrong thing; it writes the real helper body instead. A new test covers the case that matters, that a foreign helper survives while our own install is still cleared. Left the hooks sweep alone deliberately: it removes unbound.py, which nothing else writes, and the primary clear path deletes it unguarded for the same reason. Guarding one and not the other would be the inconsistency, not the fix. Tests: hooks 56 passed, gateway 12 passed, with the pre-existing MDM matcher-parity failure unchanged. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
3 findings — 1 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🔴 HIGH — Legacy clear strips foreign apiKeyHelper from default settings.json
claude-code/gateway/setup.py:676 · Impact: When clearing a relocated install, remove_api_key_helper_setting(default_dir) runs with no ownership check on the setting value, so a user's unrelated apiKeyHelper in ~/.claude/settings.json can be removed even though this install never touched that tree. · Fix: Gate the legacy settings call the same way the primary path guards the helper file (e.g. only remove when _is_unbound_key_helper_setting matches), and add a test with a foreign apiKeyHelper in legacy settings.json. · Flagged by: Greptile, Claude
🟡 TRIAGE — Relative CLAUDE_CONFIG_DIR anchors enforcement state in a cwd-relative, repo-writable tree
claude-code/hooks/unbound.py:25 · Impact: A relative value (e.g. .claude) resolves against the hook's cwd, so policy cache, approval marker, and audit log can land under a project directory; a checked-in .policy_cache.json or .approval_pending may be read as trusted hook state. · Fix: Keep security-sensitive artifacts home-anchored under ~/.unbound/ (as injection-turn guard already is), or reject non-absolute CLAUDE_CONFIG_DIR for cache/approval paths. · Flagged by: Claude
🟡 TRIAGE — POSIX hook command baked without quoting
claude-code/hooks/setup.py:532 · Impact: The hook path written to settings.json is an unquoted bare string on POSIX; a CLAUDE_CONFIG_DIR or --config-dir containing spaces or shell metacharacters can produce a malformed or injectable command string. · Fix: Quote on all platforms (shlex.quote on POSIX, matching the Windows branch) and reject config-dir values with shell metacharacters at resolution time. · Flagged by: Claude
Previously acknowledged (not re-flagged)
- Env-before-arg precedence (
CLAUDE_CONFIG_DIRwins over--config-dir) — accepted design: Claude Code reads the env var alone; arg-only installs now warn instead of silently succeeding. - Legacy sweep omits
unbound-setup.py/.last_updatedin~/.claude— accepted limitation; files are inert once hook settings entries are removed. --clear/--backfillwithout persistentCLAUDE_CONFIG_DIR— accepted by design; same resolution precedence as install; ad-hoc-env gap deferred as follow-up.- Install/runtime config-dir mismatch, whitespace handling, symlink matching,
.claude.jsonplacement, MDM self-update guard — addressed or resolved in prior commits per maintainer thread.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head a116048a · 2026-08-24T15:52Z
Dropped the fourth line, which restated the consequence already implied by the two above it. The rule a maintainer needs — verbatim, no tilde expansion, blank means unset — is unchanged. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
Comment-disposition audit — merge gateEvery review finding on this PR, current and earlier rounds, with where it stands. Head is Greptile
Cursor — all four inline comments, verified against the current code
Consensus / security review0 findings — 0 high-confidence, 0 to triage. Carried over from the June thread
Found by testing, not by a bot
Verificationhooks 56 passed, gateway 12 passed. The single hooks failure ( |
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)
Previously acknowledged (not re-flagged)
- Env-before-
--config-dirprecedence — accepted design: Claude Code keys offCLAUDE_CONFIG_DIRalone; letting the CLI arg override env would install where Claude never reads. - Install/runtime config-dir mismatch (
--config-dirvs env) — accepted: normal CLI flow forwards env into the child; divergence is only reachable by hand-invokingsetup.pywith mismatched values. --clear/--backfillwithoutCLAUDE_CONFIG_DIRon a prior custom-dir install — accepted by design: same resolver precedence as install; persistent env is the supported contract (follow-up state file deferred).- Legacy
~/.claudesweep leavesunbound-setup.py/.last_updated— accepted limitation: inert once hook settings are removed; extra deletion in the default dir is higher blast-radius. .claude.json/.config.jsonplacement under relocated dirs — resolved empirically against Claude Code 2.1.177;.config.jsonpreference intentionally not probed at import (known limitation, no enforcement impact).--config-dirwithoutCLAUDE_CONFIG_DIR— addressed with an explicit install-time warning rather than silent success.- Legacy sweep deleting foreign
anthropic_key.sh— fixed (a116048): legacy path now uses the same ownership/content check as the primary clear path. - Symlinked config dir breaking hook matching on clear — fixed: resolver uses lexical
abspathto match Claude Code / hook-matcher semantics. - Semgrep
insecure-file-permissions(0o700/0o755) — pre-existing, intentional executable/private-dir modes for hook scripts; not introduced or widened by this diff. - Semgrep
dynamic-urllib-usein tests — pre-existing test-only localhost callback exercise; no production attack surface.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head b1bde030 · 2026-08-24T20:21Z
Three predicates still asked "is this ours" against a hardcoded ~/.claude while the install itself had moved. Each is the same bug this branch exists to fix, one level down. The damaging one is the gateway's apiKeyHelper. setup_claude_key_helper writes an absolute path when the dir is relocated, but _is_unbound_key_helper_setting only recognised the ~ form, so clear deleted the script and left the setting pointing at it. Claude Code then holds a dangling apiKeyHelper and every API call fails — a worse state than before clearing. _is_unbound_hook_command was pinned to ~/.claude/hooks/unbound.py, so moving from hooks mode to gateway mode under a custom dir would not strip the existing hooks and both enforcement paths would fire at once. The hooks installer had the same blind spot in reverse: it drops the gateway's apiKeyHelper before installing, and could not see the absolute form either, so the helper would have survived and driven Claude alongside the hooks. All three now take the resolved dir and still accept the portable ~ form, so a default install behaves exactly as before. Tests cover the dangling-setting case and the hooks-to-gateway transition, which are the two that reach a user. Tests: hooks 58 passed, gateway 14 passed, with the pre-existing MDM matcher-parity failure unchanged. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
3 findings — 1 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🔴 HIGH — Unquoted hook command enables shell injection / silent bypass
claude-code/hooks/setup.py:537
Impact: The baked settings.json hook command is the bare POSIX path (str(hook_path)); paths with spaces fail to launch (PreToolUse never fires), and paths containing ;, `, or $(...) inject into the shell Claude Code uses on every tool call.
Fix: Quote the baked path on all platforms (shlex.quote on POSIX; keep existing Windows quoting) and reject config-dir values containing shell metacharacters or newlines in _resolve_claude_config_dir.
Flagged by: Claude, Lead
🟡 TRIAGE — Relative CLAUDE_CONFIG_DIR anchors enforcement state in the repo cwd
claude-code/hooks/unbound.py:25
Impact: A relative env value (e.g. CLAUDE_CONFIG_DIR=.cc) is abspath-ed against the session cwd, so .policy_cache.json, .approval_pending, and agent-audit.log live inside the working tree; repo-controlled cache/marker files could influence policy decisions or leak tool-call history into the tree.
Fix: Reject non-absolute CLAUDE_CONFIG_DIR at install/runtime, or keep enforcement-critical files home-anchored under ~/.unbound/ (as INJECTION_TURN_GUARD_DIR already is).
Flagged by: Claude, Lead
🟡 TRIAGE — Key-helper ownership verified against the wrong file
claude-code/hooks/setup.py:227, claude-code/gateway/setup.py:320
Impact: For a relocated config dir, a settings apiKeyHelper of ~/.claude/anthropic_key.sh is accepted as “ours” when config_dir/anthropic_key.sh is missing, so install/clear can strip a third-party default-dir helper and break API auth.
Fix: Resolve ownership from the path named in the setting (expand ~ if needed) and pass that path to _is_unbound_key_helper_file, not config_dir / "anthropic_key.sh".
Flagged by: Claude, Lead
Previously acknowledged (not re-flagged)
- Empty
CLAUDE_CONFIG_DIRfail-open — installer warns instead of refusing; maintainer chose this deliberately over matching Claude Code’s cwd-relative empty-string behavior. - Env-before-
--config-dirprecedence — accepted design: Claude Code reads the env var alone, so the arg cannot override it without creating an install Claude never loads. --clear/--backfillrequire the sameCLAUDE_CONFIG_DIR— by design; ad-hoc-env-only installs are a known narrow gap, not in WEB-4882 scope.- Legacy
~/.claudesweep leavesunbound-setup.py/.last_updated— accepted limitation; files are inert once hook settings entries are removed. - Semgrep
0o700/0o755chmod findings — pre-existing intentional permissions for executable helpers/scripts, not introduced by this change.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 976ffb92 · 2026-08-24T20:43Z
The sync writes managed skills to <config dir>/skills, which is also where Claude Code reads user-level skills, but _resolve_skill_path only ever looked under ~/.claude/skills. On a relocated install we therefore wrote skills to one directory and searched another, so every Unbound-managed skill failed to resolve and the backend got no path to join against. The search list now includes the resolved skills root alongside the existing roots rather than replacing any of them, so project-scoped and directory- scoped lookups are untouched and the default dir resolves exactly as before — it simply checks the same directory twice there. Tests cover both directions: a skill resolving under a relocated dir, and the default dir still resolving as it did. Tests: hooks 60 passed, gateway 14 passed, with the pre-existing MDM matcher-parity failure unchanged. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
3 findings — 1 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🔴 HIGH — Unquoted config dir baked into shell-executed settings
claude-code/hooks/setup.py:537, claude-code/gateway/setup.py:513
- Impact:
_resolve_claude_config_dir()output is written unquoted intohooks[].commandandapiKeyHelper; aCLAUDE_CONFIG_DIRcontaining shell metacharacters (e.g. from MDM, DockerfileENV, direnv) persists as command execution on every hook/API-key fetch. - Fix: Reject config-dir values outside a conservative path charset in both resolvers, and quote the baked path on POSIX (Windows already quotes).
- Flagged by: Claude, Lead
🟡 TRIAGE — Key-helper ownership probe targets wrong file
claude-code/hooks/setup.py:224, claude-code/gateway/setup.py:318
- Impact:
_is_unbound_key_helper_settingaccepts the portable~/.claude/anthropic_key.shform but probesconfig_dir/anthropic_key.sh; when that file is absent on a relocated install, a third-partyapiKeyHelpercan be misclassified as Unbound-owned and removed. - Fix: Derive the path from the setting value and run the ownership check on that file, not a fixed
config_dirsibling. - Flagged by: Claude
🟡 TRIAGE — Legacy clear deletes foreign unbound.py without ownership check
claude-code/hooks/setup.py:782
- Impact: When clearing a relocated config dir, the legacy sweep unconditionally unlinks
~/.claude/hooks/unbound.py, which can delete a same-named hook the gateway sweep was just hardened to avoid foranthropic_key.sh. - Fix: Gate the legacy delete on a content/ownership check before unlinking (mirror
_is_unbound_key_helper_fileon the gateway path). - Flagged by: Claude, Lead
Previously acknowledged (not re-flagged)
- Install
--config-dirvs runtime env mismatch — Accepted design: runtime resolves fromCLAUDE_CONFIG_DIRto preserve the MDM self-update guard; the supported CLI flow sets env first (MohamedAklamaash). --clear/--backfillwithout persisted install path — Accepted by design: same env precedence as install; clearing a custom profile requires the sameCLAUDE_CONFIG_DIRClaude uses (MohamedAklamaash).- Env beats
--config-dirprecedence — Accepted design choice: Claude Code reads the env var alone, so arg-over-env would install where Claude never loads (AakashVelusamy). - Legacy sweep leaves
unbound-setup.py/.last_updatedin~/.claude— Accepted limitation: inert once settings entries are removed; widening deletion in the default dir is out of scope (AakashVelusamy, Greptile). - Semgrep
0o700/0o755chmod findings — Pre-existing intentional permissions on helper/script files, not introduced by this config-dir routing change.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head a126453a · 2026-08-24T21:12Z
Two from review, both mine. _is_unbound_key_helper_setting checked ownership of the active dir's anthropic_key.sh whatever the setting said. A portable ~ form left over from a default install names ~/.claude, so a script sitting in the relocated dir was vouching for a file it has nothing to do with — it could refuse to clear a stale default-dir setting, or clear one that is not ours. It now picks the target from the candidate and judges that file. Both installers carried the same shape, so both are fixed. The two test classes I appended to test_config_dir.py landed after the unittest.main() guard, so running the file directly defined them too late to be collected — 11 of 15 tests ran, and the four covering this branch's own behaviour were among the silent four. The guard now sits at the end. Tests: hooks 62 passed via unittest and 16 via direct execution, gateway 14 passed, with the pre-existing MDM matcher-parity failure unchanged. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Bugbot needs on-demand usage enabledBugbot uses usage-based billing for this team and requires on-demand usage to be enabled. A team admin can enable on-demand usage in the Cursor dashboard. |
Comment-disposition audit — merge gate, round 3Additions since the last audit. Everything from the earlier rounds still stands as recorded there. Greptile — this round
Earlier rounds — unchangedOwnership predicates following the relocated dir ( Verificationhooks 60 passed / 1 failed of 61 under |
|
@greptileai please re-review — head is now Both findings from your last pass are addressed: Skill discovery under a custom dir —
Also fixed a defect in my own tests that your note surfaced: two classes sat after the Tests: hooks 60 passed of 61 under |
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
1 finding — 0 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🟡 TRIAGE — Relative CLAUDE_CONFIG_DIR relocates trust-bearing hook state into the project tree
claude-code/hooks/unbound.py:24 (also claude-code/hooks/setup.py:79, claude-code/gateway/setup.py:469)
Impact: A non-absolute CLAUDE_CONFIG_DIR (e.g. .claude) is resolved with os.path.abspath against the process cwd, so POLICY_CACHE_FILE, _APPROVAL_MARKER_FILE, audit/error logs, and related paths land inside whatever repo Claude is running in — unlike the prior home-anchored layout. A checked-out tree could ship .claude/hooks/.policy_cache.json or .approval_pending that the hook treats as trusted local state (300s cache TTL; approval-retry short-circuit), weakening policy enforcement or approval gates for that workspace.
Fix: For trust-bearing artifacts only, keep using a home-anchored root (e.g. ~/.unbound/) regardless of config dir, or reject non-absolute CLAUDE_CONFIG_DIR at install/runtime with a warning; alternatively require cryptographic validation of cache/marker contents before trust (confirm whether readers already do this).
Reviewers: Claude
Previously acknowledged (not re-flagged)
- Empty
CLAUDE_CONFIG_DIRdivergence — hook/installer fall back to~/.claudeand warn; Claude Code treats empty as cwd-relative. Accepted as deliberate warn-only behavior (test_config_dir.py,setup.pymain()). - Env-before-
--config-dirprecedence — accepted design: Claude Code reads the environment variable alone; arg-only installs now warn instead of silently succeeding. --clear/--backfillwithout persistentCLAUDE_CONFIG_DIR— accepted: same precedence as install; ad-hoc-env gap deferred to follow-up state persistence.- Legacy
~/.claudesweep leavesunbound-setup.py/.last_updated— accepted limitation; inert once settings entries are removed. - Greptile P1 legacy
anthropic_key.shdeletion without ownership check — fixed ina116048; not re-raised. - Semgrep
0o700/0o755file-permission hits — false positive for this use case (owner-only hook/config dirs are appropriate; not introduced by this change). - Semgrep dynamic
urllibintest_setup.py— test-only localhost callback harness; out of scope for production security.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head de1fad4f · 2026-08-24T21:32Z
staging landed WEB-5563, which matches the Cowork entrypoint by name rather than an exact value. It touches unbound.py, the same file this branch edits, so it merged clean but was worth reading rather than trusting: the change is confined to CLAUDE_CODE_ENTRYPOINT labelling and shares nothing with the config-dir resolution here. Tests: hooks 60 passed of 61 under unittest and 16 under direct execution, gateway 14 passed. The one failure is the pre-existing MDM matcher parity. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment-disposition audit — merge gate, round 4No new findings this round. Head moved only because
Everything from rounds 1–3 stands as recorded: the ownership predicates following the relocated dir, the destructive legacy-sweep guard, the skill-resolution fix, the Verification on this head: hooks 60 passed of 61 under |
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
0 findings — 0 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
✅ Security consensus: no issues found. (reviewers: Cursor, Claude, Semgrep, Gitleaks)
Previously acknowledged (not re-flagged)
- Legacy sweep leaves
unbound-setup.py/.last_updatedin~/.claude— Accepted limitation: files are inert once hook settings entries are removed; widening deletion in the default dir was judged higher blast radius than the cosmetic leftover (Greptile, maintainer). --clear/--backfillwithoutCLAUDE_CONFIG_DIRon a custom-dir install — Accepted by design: same precedence as install (CLAUDE_CONFIG_DIR→--config-dir→ default); clearing a relocated profile requires the same env Claude uses to load it (maintainer).- Env beats
--config-dirprecedence — Accepted design choice: Claude Code keys off the environment variable alone; arg overriding env would install where Claude never reads (maintainer). --config-dirwithout env installs to unreachable tree — Addressed via explicit warning insetup.pyrather than silent success; not a reachable path through unbound-cli (maintainer)..config.jsonvs.claude.jsonunder relocated dir — Known limitation: Claude prefers<configDir>/.config.jsonwhen present; hook reads.claude.jsononly; accepted as no enforcement impact, avoids import-time probe on every hook invocation (maintainer).- Semgrep
insecure-file-permissionson0o700/0o755— Pre-existing restrictive modes, not a widening of access; false positive on Semgrep’s “use 644” heuristic (deterministic scanner).
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 270a4be9 · 2026-08-25T12:31Z
Running them on Windows Server surfaced 13 failures, all in the tests rather than the code — the resolver was right every time and the expectations were POSIX-only. Two causes. The fake home set HOME alone, but expanduser reads USERPROFILE on Windows, so Path.home() kept returning the real profile and every default-dir assertion compared against the wrong directory; _reload now sets both. And a root-relative literal like /opt/cc is stored as C:\opt\cc on Windows, since abspath prepends the current drive — the expectations now go through the same call rather than hardcoding the POSIX spelling. A suite that only passes on POSIX gives false confidence about the platform whose path rules differ most, which is the one this branch had to reason about hardest. It now runs green on macOS, Linux and Windows, under both `python -m unittest` and direct execution. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
2 findings — 2 high-confidence, 0 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🔴 HIGH — Unquoted config-dir paths in shell-executed settings.json commands
claude-code/hooks/setup.py:542, claude-code/gateway/setup.py:517
Impact: Resolved CLAUDE_CONFIG_DIR / --config-dir values are written bare into hook command strings and apiKeyHelper on POSIX; Claude Code executes these via a shell, so paths with spaces break the hook silently and paths with shell metacharacters (e.g. from MDM/Docker ENV) can turn the enforcement line itself into arbitrary command execution.
Fix: Quote POSIX paths the way the Windows branch already does (shlex.quote(str(script_path)) / same for apiKeyHelper), or reject resolved dirs containing shell-unsafe characters in _resolve_claude_config_dir.
Flagged by: Claude, Cursor
🔴 HIGH — Blank CLAUDE_CONFIG_DIR still reports install success to backend
claude-code/hooks/setup.py:1343
Impact: When CLAUDE_CONFIG_DIR is set but whitespace-only, main() warns that Claude Code will not load this install, yet still installs into ~/.claude and calls notify_setup_complete(...) — the fleet backend can record the device as enrolled/protected while no hook ever fires for the active Claude profile.
Fix: Exit non-zero before install/notify in the blank-env case, or pass an explicit broken/unreachable install state to notify_setup_complete so the dashboard does not show a false positive.
Flagged by: Claude, Cursor
Previously acknowledged (not re-flagged)
- Env-before-arg precedence — accepted design: Claude Code keys off the environment variable alone; arg overriding env would install where Claude never reads.
--config-dirwithoutCLAUDE_CONFIG_DIR— addressed by warning (cba0181); hooks cannot fire in that state by construction.- Legacy sweep leaves
unbound-setup.py/.last_updated— accepted limitation; inert once settings entries are removed. .claude.json/ plugin-cache placement — resolved against Claude Code 2.1.177 bundle + live probe; deterministic rules now match.- Literal
~non-expansion, symlink/realpath mismatch, legacyanthropic_key.shdeletion, install/runtime dir divergence, clear-without-env — fixed or accepted in prior maintainer replies.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head a0819d5f · 2026-08-25T14:33Z
staging landed WEB-5524, which collects every suite under tests/ as a pytest package and gates CI on it. That is the first time this repo's tests run in CI at all, which is worth having: the config-dir tests added here were only ever exercised by hand. The one conflict was in the hooks test file, where the restructure rewrote every import to load a tool's module by repo-relative path — several tools ship a setup.py, so a bare `import setup` resolves to whichever directory lands on sys.path first. Took the restructured file whole and re-applied this branch's two classes onto it rather than merging hunk by hunk, since the import style changed on every one. The other two files moved with it. test_config_dir.py needed more than a move: it reloaded the shared module to re-resolve paths under a patched environment, and reload cannot work on a module loaded under a synthetic package name. Each case now imports unbound afresh from its path, which is closer to what the test means anyway — every path resolves at import, so a case wants its own module rather than a mutated shared one. tearDown went away with it, having nothing left to restore. pytest: 1910 passed, 39 skipped. The MDM matcher-parity case that failed throughout this branch's life now passes — staging fixed the drift between the two trees, so the suite is green with nothing left to explain away. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
2 findings — 1 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🔴 HIGH — Gateway install succeeds into a directory Claude Code never reads
claude-code/gateway/setup.py:886
Impact: setup.py --config-dir /opt/cc (with CLAUDE_CONFIG_DIR unset) writes anthropic_key.sh and apiKeyHelper under /opt/cc while Claude Code still reads ~/.claude, so gateway proxying, logging, and DLP inspection are silently bypassed with no operator signal.
Fix: Mirror the hooks installer warning in gateway main(): if --config-dir is set and CLAUDE_CONFIG_DIR is blank/unset, print the same “Claude Code reads only the environment variable” notice (consider non-zero exit).
Flagged by: Claude, Greptile, Lead
🟡 TRIAGE — Relocated config dir not validated for ownership or writability
claude-code/hooks/setup.py:476 (also claude-code/gateway/setup.py:487)
Impact: Installers now create and bake executable hook/key-helper paths under any resolved CLAUDE_CONFIG_DIR/--config-dir (including relative cwd paths, /tmp, or other world-writable parents) without checking ownership or permissions; a local attacker who can write there can replace hooks/unbound.py or anthropic_key.sh and run arbitrary code as the user on every tool call, defeating policy enforcement.
Fix: Before writing, stat the resolved config dir (and parents created); refuse or abort if not owned by the current uid or group/world-writable; create the dir with 0o700 and write hook/helper files with explicit restrictive modes.
Flagged by: Claude, Lead
Previously acknowledged (not re-flagged)
- Env-before-arg precedence (
CLAUDE_CONFIG_DIRwins over--config-dir) — accepted design: Claude Code keys off the environment alone; arg-only installs are warned, not silently treated as authoritative. - Literal
~inCLAUDE_CONFIG_DIRnot expanded — intentional to match Claude Code 2.1.177 verbatim resolution. - Legacy sweep omits
unbound-setup.py/.last_updatedin~/.claude— accepted limitation; files are inert once the settings entry is removed. - Clear/backfill without persisted
CLAUDE_CONFIG_DIR— by design; operating on a relocated profile requires the same env Claude uses. - Legacy sweep deleting foreign
anthropic_key.sh— fixed ina116048with ownership/content guard; not re-flagged.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 73be66dd · 2026-08-25T15:22Z
The hooks installer says so when --config-dir is passed without CLAUDE_CONFIG_DIR in the environment; the gateway did not. Claude Code picks its config dir from the environment alone, so an install aimed elsewhere by the flag writes a key helper Claude never finds, then reports success. The user is left with no working apiKeyHelper and failing API calls, with nothing having said anything. This is the fourth time on this branch a guard landed on one side of a pair, so rather than add the one line I compared every config-dir guard across both installers: resolver, blank-env warning, arg-without-env warning, legacy sweep, and the two ownership predicates. They match now. The one remaining difference is deliberate — the gateway clears the key helper in two places because it owns it, while hooks only removes a leftover one. pytest: 1910 passed, 39 skipped. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
2 findings — 1 high-confidence, 1 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🔴 HIGH — Unquoted config-dir paths written into shell-executed settings.json values
claude-code/gateway/setup.py:517, claude-code/hooks/setup.py:542
Impact: Resolved CLAUDE_CONFIG_DIR / --config-dir is written verbatim into apiKeyHelper (gateway, relocated installs) and the baked hook command (hooks); on POSIX these are unquoted while Windows is quoted. A value containing shell metacharacters (e.g. from MDM/Dockerfile ENV) can turn apiKeyHelper into persistent command execution on every API-key resolution; paths with spaces split the hook command so enforcement never runs.
Fix: shlex.quote() both values on POSIX before writing settings.json, or reject config dirs containing whitespace/shell metacharacters at resolve time; ensure _command_targets_hook tokenizes with the same rules used when writing (e.g. shlex.split) so --clear still matches quoted commands.
Reviewers: Claude, Cursor
🟡 TRIAGE — Legacy ~/.claude sweep deletes unbound.py without an ownership check
claude-code/hooks/setup.py:786
Impact: When clearing a relocated install, _clear_path(default_dir / "hooks" / "unbound.py", …) removes any file with that name in the default tree; a third-party or org-authored hook of the same name is deleted even though the clear targets a different config dir.
Fix: Gate legacy unbound.py removal on the same content/signature check used elsewhere (mirroring the gateway legacy anthropic_key.sh guard added in a116048).
Reviewers: Claude
Previously acknowledged (not re-flagged)
CLAUDE_CONFIG_DIRenv beats--config-dirarg — accepted design: Claude Code keys off the env var alone; arg-only installs now warn instead of silently succeeding (MohamedAklamaash / AakashVelusamy).--clear/--backfillwithout persisted install path — accepted by design: same resolver precedence as install; operating on a relocated profile requires the sameCLAUDE_CONFIG_DIRClaude uses (MohamedAklamaash).- Legacy sweep leaves
unbound-setup.py/.last_updatedin~/.claude— accepted limitation: inert once the settings hook entry is removed (Greptile / AakashVelusamy). - Symlinked config-dir hook matching — fixed via lexical
abspathinstead ofresolve(); residual.resolve()in relocated-vs-default equality branches is a known, low-impact inconsistency (AakashVelusamy). - Semgrep
insecure-file-permissions(0o700/0o755) — pre-existing intentional executable/script permissions, not a regression introduced by this change.
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head 828016c9 · 2026-08-25T15:53Z
The earlier skill-resolution fix appended the config dir's skills root to the search list to avoid disturbing the existing entries. That left ~/.claude/skills ahead of it, and after a move the sync only refreshes the relocated dir — so a copy left behind in the old location would win every lookup and stay frozen at whatever it last held. Being non-destructive about the ordering made the result wrong in the case the fix existed for. The config dir's skills root now stands in for the home-anchored user entry rather than queuing behind it, which is also what Claude Code does: user scope is join(configDir, "skills"), project scope is <cwd>/.claude/skills. Project and directory-scoped lookups are untouched, and with the variable unset the two are the same path, so nothing changes for a default install. The previous test passed only because it never put a competing skill in the old location. There is now one that does, and it fails against the old ordering. pytest: 1911 passed, 39 skipped. py_compile and pyflakes clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
vigneshsubbiah16
left a comment
vigneshsubbiah16
left a comment
There was a problem hiding this comment.
🛡️ Automated Security Review (consensus)
2 findings — 0 high-confidence, 2 to triage. Reviewers: Cursor, Claude, Semgrep, Gitleaks.
🟡 TRIAGE — Relocated --clear may disable an unrelated default profile
claude-code/hooks/setup.py:785 · claude-code/gateway/setup.py:681
- Impact: Clearing with
CLAUDE_CONFIG_DIRpointed at profile B also strips~/.claude/hooks/unbound.pyand Unbound hook entries from~/.claude/settings.json, even when profile A (default dir) is still a live, separate install — silently dropping policy enforcement/telemetry for a profile never named on the command line. - Fix: Gate the legacy sweep on evidence the default tree is a leftover (e.g., no other active hook config, or an explicit
--clear-legacyflag) and log exactly what is removed. - Reviewers: Claude
🟡 TRIAGE — Relative CLAUDE_CONFIG_DIR can drop credential helper into cwd
claude-code/gateway/setup.py:466
- Impact: Unvalidated relative values are absolutized against the installer's cwd, so
CLAUDE_CONFIG_DIR=ccfrom a repo checkout can writeanthropic_key.sh(surfacing the Unbound API key) inside that tree — commit or shared-host exposure risk; empty values warn, but relative paths do not. - Fix: Reject relative paths (or warn as loudly as the empty-env case) and ensure the helper and parent dir are created
0o600/0o700before writing the key. - Reviewers: Claude
Previously acknowledged (not re-flagged)
- Legacy sweep omits
unbound-setup.py/.last_updated— accepted limitation; inert once settings entries are removed (Greptile, Cursor, maintainer). - Env-before-
--config-dirprecedence — accepted design; Claude Code reads the env var alone, so the arg cannot override it without installing where Claude never looks (maintainer). ~/.claude/skillssearch order / stale-skill preference — intentional; relocated installs resolve from<config dir>/skills, not frozen copies under the old tree (Greptile, tests, maintainer).anthropic_key.shlegacy-sweep ownership guard — fixed ina116048; foreign helpers are no longer deleted (Greptile, maintainer).- Tilde / whitespace
CLAUDE_CONFIG_DIRhandling — fixed; values are read verbatim to match Claude Code (maintainer). .claude.jsonplacement under a relocated dir — resolved empirically against Claude Code 2.1.177; rule is unconditional (maintainer).- Install-time vs runtime config-dir mismatch, MDM self-update guard, symlinked hook matching,
--config-dir=parsing, clear without persisted env — addressed, warned, or accepted in prior rounds (Cursor, Greptile, maintainer).
🤖 consensus review · reviewers: Cursor,Claude,Semgrep,Gitleaks · head a1480ca7 · 2026-08-25T16:13Z
4 participants
WEB-4882 — honor
CLAUDE_CONFIG_DIRin Claude Code hooks installThe installer (
setup.py) and the runtime hook (unbound.py) hardcoded~/.claude. With a customCLAUDE_CONFIG_DIR, hooks were written/read where Claude Code never looks → silent loss of policy enforcement + telemetry.Changes
claude-code/hooks/setup.py:_resolve_claude_config_dir(argv)(precedence--config-dirarg >CLAUDE_CONFIG_DIRenv >~/.claude,expanduser().resolve()); the resolved dir is threaded into hooks dir,settings.json, the baked absolute hook command, uninstall/clear, install-state, and--backfilltranscript discovery.claude-code/hooks/unbound.py: module-level_CONFIG_DIRfromCLAUDE_CONFIG_DIRat import; audit log, error log, policy cache, approval marker, and self-update target resolve from it..claude.jsonuses the custom dir when the env var is set, else the~/.claude.jsonsibling.Notes
CLAUDE_CONFIG_DIR, every path equals~/.claudeexactly; the self-update__file__ == SELF_SCRIPT_PATHguard is unaffected for existing installs.~/.unbound/...paths stay home-anchored (independent of the Claude config dir).--config-dir).Tests
python3 -m pytest test_setup.py -q→ 26 passed. Added precedence, install-under-resolved-dir (asserts the baked command is absolute under the dir), backward-compat, and custom-dir backfill tests.🤖 Generated with Claude Code
Note
Medium Risk
Changes where policy hooks and API-key helpers are written and read; default
~/.claudebehavior is preserved when the env var is unset, but mis-resolution could still break enforcement for relocated installs.Overview
Fixes silent policy/telemetry gaps when Claude Code uses a custom config directory by routing all Unbound install and runtime artifacts through a resolved config dir instead of hardcoded
~/.claude.Gateway and hooks installers add
_resolve_claude_config_dir(CLAUDE_CONFIG_DIR→--config-dir→ default), thread that path through hook/key-helper writes,settings.json, install-state,--backfilltranscript discovery, and--clear. Clear also strips leftover enforcement under~/.claudewhen the active dir is relocated. Gateway keeps portable~/.claude/anthropic_key.shinapiKeyHelperonly for the default dir; custom dirs get an absolute helper path.unbound.pyresolves_CONFIG_DIRfromCLAUDE_CONFIG_DIRat import (logs, policy cache, approval marker, self-update target) and uses_relocated_or_legacyfor.claude.json/ plugin cache when Claude’s layout varies by version.New and updated unit tests cover resolver precedence, install paths, legacy sweep on clear, and custom-dir backfill.
Reviewed by Cursor Bugbot for commit d83b1b2. Bugbot is set up for automated code reviews on this repo. Configure here.
Greptile Summary
This PR fixes silent policy and telemetry failures when Claude Code is configured with a non-default
CLAUDE_CONFIG_DIRby threading the resolved config directory through every installer and runtime artifact path that previously hardcoded~/.claude.hooks/setup.pyandgateway/setup.py: Both add_resolve_claude_config_dirwithCLAUDE_CONFIG_DIRenv >--config-dirarg >~/.claudeprecedence; all hook files,settings.json, the baked hook command, key-helper path,--clear, and--backfillstate now resolve under the active config dir. A new legacy sweep inclear_setupalso strips leftover enforcement from~/.claudewhen clearing a relocated install.claude-code/hooks/unbound.py:_CONFIG_DIR,AUDIT_LOG,ERROR_LOG,POLICY_CACHE_FILE,SELF_SCRIPT_PATH,CLAUDE_SKILLS_ROOT,CLAUDE_PLUGIN_CACHE_DIR, and_APPROVAL_MARKER_FILEall resolve fromCLAUDE_CONFIG_DIRat import time;~is intentionally not expanded to match Claude Code's own verbatim treatment of the variable.unbound.pypath constants — though the hooksclear_setuplegacy sweep branch has no test coverage yet.Confidence Score: 5/5
~/.claudebehavior is fully preserved whenCLAUDE_CONFIG_DIRis unset, and the new code paths are covered by a solid suite of unit tests including backward-compat and foreign-file ownership scenarios.os.path.abspath,Path.resolve). The gateway legacy sweep correctly guards ownership before deletion. The two findings flagged are non-blocking quality improvements — one a minor asymmetry between gateway and hooks cleanup, the other missing parity tests for an already-reviewed branch — neither affects correctness in the default or relocated install case.claude-code/hooks/setup.py(legacy sweep ownership check) andtests/claude_code/hooks/test_setup.py(missing coverage for the relocated-dir clear branch).Important Files Changed
_CONFIG_DIRresolved correctly fromCLAUDE_CONFIG_DIRat import time with proper whitespace stripping; all runtime paths (audit log, policy cache, approval marker, self-update, skills root) follow the relocated dir.CLAUDE_MCP_CONFIG_PATHplacement and plugin cache dir logic look correct._resolve_claude_config_dirand threadsconfig_dirthroughsetup_hooks,configure_claude_settings,clear_setup, andrun_backfill. The new legacy sweep inclear_setup(lines 788-792) deletes~/.claude/hooks/unbound.pywithout an ownership check, and there is no test covering this branch._resolve_claude_config_dir, updates all key-helper and hook-detection helpers to acceptconfig_dir, and adds a legacy sweep inclear_setupthat correctly guards~/.claude/anthropic_key.shdeletion with_is_unbound_key_helper_filebefore calling_clear_path.clear_setuplegacy sweep (the relocated-dir branch) has no corresponding test analogous toTestClearSweepsLegacyDirin the gateway test file.detect_install_state, hooks stripping, and the legacy sweep including the foreign-helper ownership guard.unbound.pyfresh per test case to exercise module-level path resolution. Covers default, relocated, blank, tilde-literal, and plugin-cache-dir scenarios, plus skill resolution against the relocated dir.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A["installer invoked\n(setup.py main)"] --> B{CLAUDE_CONFIG_DIR\nenv set & non-blank?} B -- yes --> C["config_dir = abspath(CLAUDE_CONFIG_DIR)"] B -- no --> D{"--config-dir arg\nprovided?"} D -- yes --> E["config_dir = abspath(--config-dir value)"] E --> W1["⚠️ warn: CLAUDE_CONFIG_DIR\nnot set — Claude won't load hooks"] D -- no --> F["config_dir = ~/.claude (default)"] C --> G["install hooks → config_dir/hooks/unbound.py\nbake absolute path into settings.json"] W1 --> G F --> G G --> H{"clear mode?"} H -- no --> Z["done"] H -- yes --> I["delete config_dir/hooks/unbound.py\nremove hook entries from config_dir/settings.json"] I --> J{"config_dir ≠ ~/.claude\n(relocated install)"} J -- yes --> K["legacy sweep:\ndelete ~/.claude/hooks/unbound.py\nstrip hook entries from ~/.claude/settings.json"] J -- no --> Z K --> Z subgraph "unbound.py (runtime hook)" R1["import-time:\n_CONFIG_DIR = abspath(CLAUDE_CONFIG_DIR)\nor ~/.claude if blank/unset"] --> R2["AUDIT_LOG, ERROR_LOG,\nPOLICY_CACHE_FILE, SELF_SCRIPT_PATH,\nCLAUDE_SKILLS_ROOT\nall under _CONFIG_DIR"] endReviews (30): Last reviewed commit: "fix: a stale skill in the old home dir m..." | Re-trigger Greptile