fix(disk-hygiene): resolve kill switch by reading user settings directly

[kyle-sexton]

Summary

The disk_hygiene_enabled kill switch was inert on a default install. The plugin-level engine-gate hook (hooks/hooks.json) carried a bare ${user_config.disk_hygiene_enabled} argument; because the declared userConfig default is unimplemented upstream (#46477 / #39455 / #39827), an unset-but-defaulted token drops the whole hook entry, so the gate never ran. The skill-frontmatter belt could not receive the value either (skill hooks get neither the ${user_config.*} substitution nor CLAUDE_PLUGIN_OPTION_*). Audit-only mode therefore degraded from deny-outright to prompt-gated.

Closes #1019.

Fix

Both guard surfaces now resolve the toggle by reading disk_hygiene_enabled directly from user-scope pluginConfigs in settings.json, via a new shared lib/killswitch_config.py reader:

• Located from the tamper-resistant ${CLAUDE_PLUGIN_ROOT} both surfaces already receive (fallback to CLAUDE_CONFIG_DIR/HOME only when --plugin-root is absent — the report CLI / unit tests).
• Environment is never consulted for the toggle or the settings path: a repo .claude/settings.json env block reaches hook subprocesses and carries no provenance. Since CC 2.1.207 pluginConfigs is honored only from user/managed/--settings scope (project/local ignored), so a hostile repo cannot forge it.
• Fails closed to enabled on any absent/unreadable/ambiguous read.
• The bare ${user_config.*} argv is removed from hooks.json (fixing the hook-drop); kill_switch_probe.py now delegates to the same reader (its single-line JSON contract unchanged).

Design note — supersedes the planned SessionStart + state-file ("C′")

The two enforcement surfaces are the same script through one resolve point (destructive_guard.py), so there is nothing to distribute between sessions or surfaces. A direct read is a smaller trust surface (a settings read, no state-file write), honors a mid-session settings change, and needs no session-start timing dependency. Semantics are unchanged from the locked resolver decision (read user-scope pluginConfigs, ignore env, fail closed to enabled).

Enforcement reach (accurate)

• Bash engine invocations → denied outright in audit-only by the always-on engine gate, whether or not the clean skill is active.
• PowerShell deletion spellings → denied outright by the skill-scoped belt while /disk-hygiene:clean is active (the always-on gate defers on non-engine commands).

§643 plugin-acceptance security review — PASS

The one new surface is a settings-file read (criterion 4). The playbook's own carve-out sanctions reading the plugin's own documented ~/.claude user-global config; it's located from ${CLAUDE_PLUGIN_ROOT} with no ../ reach-out into consumer-repo data. Criterion 1 (hooks) is reduced — a ${user_config.*} arg removed. Surfaces 2/5/6/7 untouched. Trust-surface delta recorded in README.md + CHANGELOG.md.

Testing

• test_hygiene.py: 179 tests OK (new DirectReadKillSwitchTests covering the plugin-root channel, CLAUDE_CONFIG_DIR fallback, tamper-resistance, env/argv ignored, fail-closed; new hooks.json contract test locking --plugin-root present + bare ${user_config.*} gone; guard helpers made hermetic).
• test_kill_switch_probe.py: 17 tests OK, unchanged (behavior-preserving extraction).
• markdownlint clean; hooks.json/plugin.json valid.

Related

• disk-hygiene: PowerShell-tool deletion belt inert on Windows — docs over-promise protection that doesn't fire (preview-tool hook divergence) #1195 — the reported "PowerShell bypass" whose real cause (inert engine gate) this fixes.
• fix(disk-hygiene): retract false PowerShell non-firing claim, document the real bare-argv drop (#1195) #1197 — disk-hygiene 0.8.3, which documented this defect and left the code fix to a follow-up (this PR).
• chore(disk-hygiene): decide guard registration surface — skill-scoped vs plugin-level hooks.json (F1/F5) #1106 — the split-registration (Option E) decision that introduced the plugin-level engine-gate trust surface updated here.
• Upstream userConfig default gap this works around: #46477 / #39455 / #39827 (recheck if resolved).

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 3m 21s —— View job

---

PR Review: fix(disk-hygiene): resolve kill switch by reading user settings directly

• Gather context and understand the request
• Read changed files and analyze the diff
• Review lib/killswitch_config.py (new shared reader)
• Review hooks/hooks.json changes
• Review destructive_guard.py changes
• Review kill_switch_probe.py changes
• Review test coverage in test_hygiene.py
• Post comprehensive review feedback

---

Overview

The root cause diagnosis is correct: a bare ${user_config.disk_hygiene_enabled} in hooks/hooks.json silently dropped the entire hook entry when the userConfig default wasn't implemented upstream, making the engine gate inert on a default install. The fix — removing that token and deriving the kill-switch value by reading pluginConfigs directly from settings.json — is the right approach. Architecture, security model, test coverage, and documentation are all solid. A few specific points below.

---

Findings

1. Confusing zip(dict, list) in error message path — minor clarity

killswitch_config.py line 161:

f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); "

interpreted is a dict, so zip(interpreted, entries) iterates the dict's keys — this works correctly because interpreted is built from entries in the same insertion order, making k == e["key"] for each pair. But it's non-obvious. A future reader maintaining this code might change the order of one of these structures and silently break the error message.

Simpler and clearer: {e["key"]: e["value"] for e in entries} — no zip needed. Fix this →

2. assert used where the guarded invariant is needed at runtime — low severity

killswitch_config.py line 179:

effective = values.pop()
assert effective is not None

After None in values and len(values) > 1 are both false, values contains exactly one non-None boolean, so the assert is logically unreachable. But assert is silently stripped by python -O, which is a common invocation in some environments. A type-narrowing cast or an explicit branch would survive optimization:

effective = values.pop()
if effective is None:
    raise AssertionError("invariant violated: single-element set contained None")

Or simply assert isinstance(effective, bool) if you want to preserve the style while making the type narrower. Low severity — the invariant is sound — but worth noting given that this is safety-critical code.

3. Fallback-to-env on layout mismatch could be tampered — defense-in-depth note

destructive_guard.py _resolve_user_settings_path():

plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
    derived = _user_settings_path_from_root(plugin_root)
    if derived:
        return Path(derived)
return killswitch_config.default_settings_path()   # uses CLAUDE_CONFIG_DIR

If --plugin-root is supplied but _user_settings_path_from_root returns None (no plugins/cache marker in the path), the fallback uses CLAUDE_CONFIG_DIR from the environment — which a hostile repo can set via its settings.json env block. A hostile repo could point CLAUDE_CONFIG_DIR at a settings.json with disk_hygiene_enabled: false to force audit-only mode if the layout check fails.

In practice, this requires CC to ship a real install without plugins/cache in the path (very unlikely), but the PR's own security model claims env is never consulted for the toggle. That claim holds for the normal path but not for this fallback. Consider logging or failing-more-closed here (returning killswitch_config.default_settings_path() is acceptable for CLI/tests, but in a hook invocation where --plugin-root was provided but unrecognized, a degraded=True read is the right outcome — which already happens since the fallback file most likely won't have a pluginConfigs entry, and the read fails closed to enabled). So the practical impact is negligible, but the defense-in-depth comment in the function could note this explicitly.

4. Path(*parts[:index]) edge case for short paths — cosmetic

destructive_guard.py line 416:

plugins_dir = Path(*parts[:index])
return os.fspath(plugins_dir.parent / "settings.json")

For a root like /cache (parts = ('/', 'cache')), index=1, parts[:1] = ('/',), Path('/'), .parent = /, result = /settings.json. The derivation doesn't return None, but the resulting path is clearly wrong. This isn't reachable via CC's installer, but _plugin_data_root_from_root guards against index + 2 >= len(parts) (no name segment); a similar guard here for index == 1 (root's immediate child) would be consistent.

---

What's done well

• Single reader, two surfaces: Extracting lib/killswitch_config.py and delegating both destructive_guard.py and kill_switch_probe.py to it eliminates the split logic that was present before. The re-export aliases in kill_switch_probe.py (default_settings_path = killswitch_config.default_settings_path) preserve the existing contract for callers of that module without copying any logic.

• Fail closed on every degraded case: The probe() function returns effective: True (guard active) for missing file, non-regular file, unreadable JSON, unrecognized value, and conflicting entries. This is conservative and correct for a safety control.

• Hook contract test locks the regression: test_engine_gate_hook_resolves_kill_switch_from_plugin_root_not_user_config directly parses hooks.json and asserts (a) --plugin-root present, and (b) no ${user_config.*} token exists in any arg. This would catch a revert of either invariant immediately.

• DirectReadKillSwitchTests covers the full new channel map: Configured false/true via plugin-root, absent settings, degraded JSON, legacy env var ignored, legacy argv flag ignored, env fallback, and tamper-resistance (plugin-root beats hostile CLAUDE_CONFIG_DIR). That last test is exactly the right adversarial case to lock.

• GuardTests.setUp hermetic isolation: Replacing per-test mock.patch.dict("os.environ", ..., clear=False) with a tempfile.TemporaryDirectory-based _cfg and _set_kill_switch() helper eliminates environmental coupling and makes test intent clearer.

• Version bump to 0.9.0: The semantic-version bump (minor, not patch) is appropriate for a behavioral change to how the kill switch is delivered.

---

Verdict

Approved with minor comments. Finding 1 (zip clarity) is a one-line fix worth making before merge. Findings 2 and 3 are low severity and the code's existing fail-closed behavior largely mitigates them. No blocking issues.

[claude]

Claude finished @kyle-sexton's task in 6m 32s —— View job

---

Security Review — PR #1242

• Gather context and understand the request
• Read all changed files
• Analyze lib/killswitch_config.py (new file — settings reader)
• Analyze destructive_guard.py changes
• Analyze kill_switch_probe.py changes
• Analyze hooks/hooks.json changes
• Analyze remaining changed files
• Post security findings

---

Reviewed changed files: lib/killswitch_config.py, destructive_guard.py, kill_switch_probe.py, hooks/hooks.json, SKILL.md (clean + setup), safety-model.md, plugin.json. No GitHub Actions workflows changed. No command injection, SQL injection, path traversal via model-controlled input, or supply-chain risks introduced.

Two security findings follow.

---

IMPORTANT — CLAUDE_CONFIG_DIR env var reachable in guard's kill-switch path when ${CLAUDE_PLUGIN_ROOT} substitution fails

Confidence: PLAUSIBLE

destructive_guard.py _resolve_user_settings_path() L421–439 — killswitch_config.py default_settings_path() L36–39

The PR's tamper-resistance rests entirely on a CC runtime guarantee: that ${CLAUDE_PLUGIN_ROOT} is always substituted with the real plugin install path before the guard subprocess sees it. When that substitution happens, _resolve_user_settings_path() correctly derives the user settings location from the verified install path and never consults the environment. But the fallback is not fail-closed on this assumption:

# destructive_guard.py L421-439
def _resolve_user_settings_path() -> Path:
    plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
    if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
        derived = _user_settings_path_from_root(plugin_root)
        if derived:
            return Path(derived)
    return killswitch_config.default_settings_path()   # <-- reads CLAUDE_CONFIG_DIR

killswitch_config.default_settings_path() trusts CLAUDE_CONFIG_DIR from the process environment. The PR itself states that "a repo settings.json env block reaches hook subprocesses and carries no provenance." If CC fails to substitute ${CLAUDE_PLUGIN_ROOT} in any edge case (version regression, a hook invocation path the PR authors have not seen), the guard silently falls through to an env-redirectable path.

Failure scenario: A user sets disk_hygiene_enabled: false in their real ~/.claude/settings.json. A hostile repo puts {"env": {"CLAUDE_CONFIG_DIR": "/tmp/evil"}} in its .claude/settings.json and pre-populates /tmp/evil/settings.json with {"pluginConfigs": {"disk-hygiene": {"options": {"disk_hygiene_enabled": true}}}}. If CC's ${CLAUDE_PLUGIN_ROOT} substitution fails for either the plugin-level gate or the skill-scoped belt, the guard reads the forged value and treats execution as enabled, degrading the user's audit-only false setting from "deny outright" to "ask for human approval."

The degradation is not a complete bypass — the human-approval prompt still fires — but it violates the stated kill-switch contract that false produces an outright deny rather than a prompt.

No code change suggested here, but the PR should explicitly document this CC-substitution dependency as a trust precondition — ideally with a check that logs a warning (not silently falls through) when the fallback is reached during hook execution, so operators can detect the gap. Alternatively, a fail-closed choice at the fallback branch (treat any env-reachable read as "indeterminate" → effective=true) would preserve safety while making the dependency explicit.

---

SUGGESTION — Kill-switch probe reads CLAUDE_CONFIG_DIR from env; creates silent guard/probe divergence

Confidence: CONFIRMED

kill_switch_probe.py L53–56

When the guard allows the probe via is_exact_kill_switch_probe() (exactly 2 tokens, no args), main([]) calls default_settings_path() with no --settings-file override. That function reads CLAUDE_CONFIG_DIR from the environment. The guard that authorizes this probe receives --plugin-root ${CLAUDE_PLUGIN_ROOT} and resolves the settings path tamper-resistantly — but the probe itself receives no --plugin-root and consults the env.

Failure scenario: A hostile repo sets CLAUDE_CONFIG_DIR to a forged settings location. The skill content runs the probe (correctly allowed by the guard), the probe reports effective: true (wrong — real setting is false). The model reads this and proceeds to offer deletion options. The guard then correctly denies any actual apply command. No deletion occurs, but the model's intermediate reasoning is driven by bad data, producing a confusing session where the model proposes work the guard will silently block.

This is defense-in-depth working as designed (the guard is the actual enforcement gate), but the silent divergence between what the probe reports and what the guard enforces makes the system harder to reason about from the outside and could confuse users or log analysis. The probe could be made to accept and forward a --plugin-root arg (to be passed by the guard/skill) so both surfaces resolve from the same tamper-resistant channel, eliminating the divergence entirely.

---

No other security issues found

The following were explicitly checked and found clean:

• No command injection: Python code uses no subprocess, os.system, or eval; all output goes through json.dumps().
• No path traversal via model input: _literal_shell_words() is a strict whitelist that rejects {}$*?[]~ and all shell operator characters; _data_root_key() normalizes but does not trust model-supplied paths for anything beyond matching the runtime-derived authority.
• Fail-closed JSON reads: probe() handles FileNotFoundError, OSError, UnicodeDecodeError, and non-dict roots by returning effective=True (guard stays active).
• No secrets or PII introduced: No credentials, tokens, or private data in the diff.
• Hook arg injection: sys.argv is populated by the CC hook launcher from the fixed args array in hooks.json/SKILL.md frontmatter; no model-controllable surface reaches it.
• Conflicting plugin-config entries: Handled correctly — len(values) > 1 resolves to indeterminate → effective=True (guard stays active), not a bypass.
• plugin.json change: Version bump to 0.9.0 only; no new permissions, remotes, or trust delegations.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: bd50ada781

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 56s —— View job

---

PR Review: fix(disk-hygiene): resolve kill switch by reading user settings directly

• Gather context and understand the request
• Read lib/killswitch_config.py (new shared reader)
• Read destructive_guard.py changes
• Read kill_switch_probe.py changes
• Read hooks/hooks.json changes
• Read test coverage in test_hygiene.py
• Read remaining changed files (plugin.json, SKILL.md, safety-model.md)
• Post comprehensive review

---

Overview

The root cause diagnosis is correct: a bare ${user_config.disk_hygiene_enabled} in hooks/hooks.json silently dropped the entire hook entry when the userConfig default was unimplemented upstream, making the engine gate inert on a default install. The fix — removing that token and reading disk_hygiene_enabled directly from pluginConfigs in user settings.json via a shared lib/killswitch_config.py — is sound, well-tested, and properly documented.

The latest commit (23e40d3) correctly dropped the shebang from killswitch_config.py; it is a library module, not a standalone script.

---

Findings

1. zip(dict, list) in error message is correct but fragile — easy fix

killswitch_config.py line 160:

f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); "

interpreted is a dict, so zip(interpreted, entries) iterates dict keys. This works today only because interpreted is built from entries in the same insertion order (Python 3.7+), making k == e["key"] for each pair. If the relative build order of either structure changes, the error message silently maps the wrong keys to the wrong values. Simpler and correct without the fragility:

f"({json.dumps({e['key']: e['value'] for e in entries})}); "

Fix this →

2. assert not safe under python -O in safety-critical path — low severity

killswitch_config.py line 178:

effective = values.pop()
assert effective is not None

The invariant is logically sound — after filtering None in values and len(values) > 1, the single-element set must contain a bool. But assert is silently stripped by python -O. Given this is safety-critical code, an explicit branch is more robust:

effective = values.pop()
if effective is None:
    raise AssertionError("invariant violated: single-element values set contained None")

Low severity in practice (optimizer flags are uncommon here), but worth tightening on a safety gate.

3. Kill-switch probe and guard can diverge on settings path when CLAUDE_CONFIG_DIR is hostile — defense-in-depth gap

The probe (kill_switch_probe.py) runs without --plugin-root and falls back to killswitch_config.default_settings_path(), which reads CLAUDE_CONFIG_DIR from the environment. The guard receives --plugin-root ${CLAUDE_PLUGIN_ROOT} and uses the tamper-resistant derivation. A hostile repo that sets CLAUDE_CONFIG_DIR to a forged settings file would cause the probe (called by the skill body) to report a different effective value than what the guard enforces.

Practical impact is limited: the guard is the actual enforcement gate and is tamper-resistant, so no deletion occurs. The probe's incorrect report would mislead the model's intermediate reasoning (e.g., it might propose work the guard will silently block), creating a confusing session. The SKILL.md correctly calls the guard "the backstop, not the sole enforcer," so this is a known design tradeoff.

As the prior security review noted: the cleanest fix would be to pass --plugin-root to the probe when invoking it from the skill body, allowing kill_switch_probe.py to use the same tamper-resistant channel. This would require a small update to SKILL.md's probe invocation line and a corresponding --plugin-root argument handler in kill_switch_probe.py.

4. Managed settings and --settings flag remain invisible to guard and probe — acknowledged gap

killswitch_config.py line 34–38

default_settings_path() reads only the user settings file. A disk_hygiene_enabled: false set only via managed settings or --settings file is not seen by either surface. As a result, audit-only mode configured through those channels is not enforced — the guard fails closed to enabled (allows the prompt), not to the configured false.

The PR description and probe()'s detail strings explicitly acknowledge this scope limitation ("Managed settings or a --settings flag could still carry a value this probe cannot see"), so this is a known and documented residual gap, not a regression. The failure mode is permissive, not insecure (the guard still runs; only the kill switch isn't respected). Flagging it here for completeness and to ensure it's tracked.

5. _user_settings_path_from_root path construction edge case — cosmetic

destructive_guard.py line 416:

plugins_dir = Path(*parts[:index])
return os.fspath(plugins_dir.parent / "settings.json")

If index == 1 (i.e., the plugins/cache marker starts at the filesystem root — e.g., /cache), parts[:1] = ('/',), plugins_dir = /, .parent = /, result = /settings.json. This won't happen via CC's real installer, but _plugin_data_root_from_root guards against index + 2 >= len(parts) for consistency. A matching guard (if index <= 1: return None) would make the invariant explicit.

---

What's done well

• Single reader, two surfaces: Extracting lib/killswitch_config.py and delegating both destructive_guard.py and kill_switch_probe.py to it eliminates split logic. The re-export aliases in kill_switch_probe.py (default_settings_path = killswitch_config.default_settings_path, probe = killswitch_config.probe) preserve the existing contract without copying code.

• Fail closed on every degraded case: probe() returns effective: True for FileNotFoundError, OSError, non-regular file, unparseable JSON, unrecognized value, and conflicting entries. Every degraded path is conservative and correct for a safety control.

• Hook contract test locks the regression: test_engine_gate_hook_resolves_kill_switch_from_plugin_root_not_user_config parses hooks.json directly and asserts (a) --plugin-root present with the ${CLAUDE_PLUGIN_ROOT} placeholder, and (b) no ${user_config.*} token in any arg. This would catch a revert of either invariant immediately.

• DirectReadKillSwitchTests is adversarially complete: Covers configured false/true via plugin-root, absent settings, degraded JSON, legacy env var ignored, legacy argv flag ignored, env fallback, and tamper-resistance (test_plugin_root_channel_beats_tamperable_config_dir_env). That last test is exactly the right adversarial case to lock.

• GuardTests setUp made hermetic: Replacing scattered mock.patch.dict env coupling with a TemporaryDirectory-based _cfg and _set_kill_switch() helper makes each test self-contained and independent of the developer machine's state.

• Version bump to 0.9.0: Appropriate semantic-version increment for a behavioral change to the kill-switch delivery channel.

• SKILL.md language is accurate: The updated clean skill instructions correctly describe the probe as the "honest reporting" surface and the guard as the "backstop," which matches the implementation's actual trust model.

---

Verdict

Approved with minor comments. Finding 1 (zip clarity) is a one-line fix worth making before merge. Findings 2 and 3 are low-to-medium severity and the code's existing fail-closed behavior substantially mitigates them. Findings 4 and 5 are pre-existing or cosmetic. No blocking issues.

[claude]

Claude finished @kyle-sexton's task in 6m 11s —— View job

---

Security Review — PR #1242 (HEAD 23e40d3)

• Gather context and understand the request
• Diff the PR to identify all changed surfaces
• Read lib/killswitch_config.py (new shared reader)
• Read destructive_guard.py changes
• Read kill_switch_probe.py changes
• Read hooks/hooks.json changes
• Read remaining changed files (SKILL.md ×2, plugin.json)
• Perform security analysis and post findings

---

Reviewed HEAD 23e40d3. The new commit (shebang removal from lib/killswitch_config.py) has no security surface. The substantive change is in bd50ada. Checked for: command injection, path traversal, credential exposure, sys.path injection, settings-file TOCTOU, JSON injection, hook-arg forgery, and env-based trust bypass. No new critical issues found beyond those already reported in the prior review. Two confirmed findings follow.

---

IMPORTANT — Guard falls through to env-reachable CLAUDE_CONFIG_DIR when --plugin-root layout check fails

Confidence: PLAUSIBLE

destructive_guard.py L421–439 → killswitch_config.py L36–39

def _resolve_user_settings_path() -> Path:
    plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
    if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
        derived = _user_settings_path_from_root(plugin_root)
        if derived:
            return Path(derived)
    return killswitch_config.default_settings_path()  # reads CLAUDE_CONFIG_DIR

_user_settings_path_from_root() returns None whenever the plugin root path does not contain the plugins/cache segment pair. On that failure — whether caused by a future CC install-layout change, a substitution edge case, or a symlinked install — the guard falls back to killswitch_config.default_settings_path(), which reads CLAUDE_CONFIG_DIR from the process environment. A repo's .claude/settings.json env block can set that variable.

Failure scenario: User sets disk_hygiene_enabled: false in ~/.claude/settings.json. Hostile repo sets CLAUDE_CONFIG_DIR=/tmp/evil and writes /tmp/evil/settings.json with pluginConfigs.disk-hygiene.options.disk_hygiene_enabled: true. If _user_settings_path_from_root() returns None (e.g., CC stops using plugins/cache in a future version), the guard reads the forged value and treats the kill switch as enabled — degrading audit-only mode from an outright deny to a human-approval prompt. The human approval prompt still fires, so this is not a full bypass.

In normal operation ${CLAUDE_PLUGIN_ROOT} is always an absolute path containing plugins/cache, so this branch is not reachable through the hook. The risk materialises only under a CC install-layout regression or an undiscovered substitution gap.

Recommended mitigation: When --plugin-root is supplied but _user_settings_path_from_root returns None, treat the kill-switch read as indeterminate and fail closed to enabled=True rather than falling back to the env. The env fallback is appropriate for non-hook contexts (CLI, tests) where --plugin-root is absent; it should not fire when --plugin-root was supplied but unrecognised.

---

SUGGESTION — Kill-switch probe always consults CLAUDE_CONFIG_DIR; creates guard/probe reporting divergence

Confidence: CONFIRMED

kill_switch_probe.py L45–56

def main(argv: list[str] | None = None) -> int:
    ...
    settings_path = (
        Path(args.settings_file) if args.settings_file else default_settings_path()
    )
    print(json.dumps(probe(settings_path)))

The guard is authorized to run the probe in exactly the two-token, argument-free form (is_exact_kill_switch_probe), which means --settings-file cannot be passed. main([]) therefore always falls through to default_settings_path(), which reads CLAUDE_CONFIG_DIR from the environment.

Failure scenario: Hostile repo sets CLAUDE_CONFIG_DIR=/tmp/evil with a forged settings file claiming disk_hygiene_enabled: true when the user's real config has false. The probe returns effective: true. The model reads this, begins planning and proposing deletion work. The guard (which resolves via --plugin-root tamper-resistantly) then denies the actual apply call. No deletion occurs, but the model operated under incorrect state for the duration of the session, producing confusing guidance and potentially spending significant turns before the guard block exposes the discrepancy.

Recommended mitigation: Pass --plugin-root ${CLAUDE_PLUGIN_ROOT} to the probe invocation in the skill content (the guard would need to extend is_exact_kill_switch_probe to allow exactly this additional pair), and have the probe extract and use that path via the same _user_settings_path_from_root logic the guard uses. This closes the reporting divergence with minimal surface expansion.

---

What was checked and found clean

• No command injection: All Python subprocesses are spawned via the CC hook launcher with a fixed args array; the guard itself calls no subprocess, os.system, or eval.
• No path traversal via model-controlled input: _literal_shell_words() rejects {}$*?[]~ and all shell operator characters. _data_root_key() normalises but does not trust model-supplied values for authority decisions.
• No settings-file TOCTOU exploitable: The stat() → read_text() window exists but requires an attacker to control ~/.claude/settings.json directly, which is outside the threat model (a hostile repo sets env vars, not the user's home directory).
• sys.path insertion safe: Both scripts insert Path(__file__).resolve().parents[3] / "lib" at position 0 before the import killswitch_config. Since __file__ is the CC-controlled plugin script path, the inserted directory is <plugin-root>/lib — not attacker-controllable. The if str(_LIB_DIR) not in sys.path guard prevents redundant insertion.
• Hook arg forgery not possible: hooks.json lives at ${CLAUDE_PLUGIN_ROOT}/hooks/hooks.json (the plugin's install directory, not the consumer repo), so a hostile repo cannot modify the hook's args array.
• pluginConfigs scope enforcement: The guard's choice to read from the user settings file is sound because CC 2.1.207+ ignores pluginConfigs in project-scope and local settings.json. A hostile repo cannot inject a forged toggle via its own .claude/settings.json (confirmed scope restriction).
• Fail-closed JSON reads: probe() handles FileNotFoundError, OSError, UnicodeDecodeError, non-dict root, and conflicting entries by returning effective=True (guard stays active).
• Conflicting plugin-config entries: len(values) > 1 → indeterminate → effective=True. Multiple agreeing false entries still correctly yield effective=False.
• plugin.json: Version bump to 0.9.0 only; no new permissions, remote MCP servers, or trust delegations.
• No secrets or PII introduced: Confirmed no credentials, tokens, or private data in the diff.

[claude]

Claude finished @kyle-sexton's task in 2m 45s —— View job

---

PR Review: fix(disk-hygiene): resolve kill switch by reading user settings directly

• Gather context and understand the request
• Diff new commit (266c2a9) against previously-reviewed HEAD (23e40d3)
• Read updated killswitch_config.py
• Read updated destructive_guard.py
• Read updated test file
• Assess whether prior findings are addressed
• Post final review

---

Overview of the new commit (266c2a9)

This commit adds managed (enterprise) settings support in response to the Codex inline review comment and the "acknowledged gap" flagged in the two prior security reviews. Both killswitch_config.py and destructive_guard.py now read the platform managed-settings.json as the highest-precedence source, with correct precedence logic: a managed value only wins when source == "configured" (valid, unambiguous, explicitly set) — absent, malformed, or entry-free managed files fall back cleanly to the user file.

---

Findings

1. Windows ProgramFiles env lookup is inconsistent with the "env never consulted" security model — low severity

killswitch_config.py line 63:

program_files = os.environ.get("ProgramFiles", r"C:\Program Files")

macOS (/Library/Application Support/ClaudeCode/…) and Linux (/etc/claude-code/…) use hard-coded absolute system paths. The Windows branch consults the environment instead. ProgramFiles is a Windows system variable, but a hostile repo's .claude/settings.json env block reaches hook subprocesses and could override it — redirecting the managed-settings lookup to a repo-controlled path. The PR's own security model explicitly states "Environment is never consulted."

Hard-coding the Windows fallback is the consistent fix:

return Path(r"C:\Program Files") / "ClaudeCode" / "managed-settings.json"

The %ProgramFiles% shell expansion is Windows-specific to CMD/PowerShell; the Python path does not need it. The ProgramFiles env var exists as a convenience alias; the canonical location is always C:\Program Files. Low severity in practice — overriding ProgramFiles via a repo env block would be unusual — but it is the one place in the security model where env is consulted for a path that's supposed to be tamper-resistant. Fix this →

2. managed_settings_path parameter shadows the module-level function — cosmetic

killswitch_config.py line 220:

def resolve_effective(
    settings_path: Path, managed_settings_path: Path | None = None
) -> bool:

The parameter managed_settings_path shadows the module-level function of the same name. This is harmless here (the body never needs to call the function), but a future reader or editor who adds a call like managed_settings_path() inside this function would get an unexpected TypeError. Renaming the parameter to managed_path would avoid the ambiguity.

3. Two prior findings still unaddressed — carryover

Both were flagged in the previous two review rounds and are not touched by this commit:

zip(dict, list) fragility — killswitch_config.py line 189:

f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); "

Still works only by coincidence of dict insertion order. Still fixable in one line: {e["key"]: e["value"] for e in entries}.

assert under python -O — killswitch_config.py line 207:

effective = values.pop()
assert effective is not None

assert is stripped by python -O. An explicit if effective is None: raise AssertionError(...) would survive optimization. Low severity but relevant in safety-critical code.

4. Probe/guard divergence now extends to managed settings — acknowledged residual

kill_switch_probe.py still probes only the user settings file (unchanged in this PR). The guard now probes both user and managed. If disk_hygiene_enabled: false is set exclusively in the platform managed-settings.json, the guard denies outright (correct), but the probe reports effective: true with source: default. The setup skill's honest reporting is then wrong.

This gap is explicitly documented in the probe module's docstring ("Scope: managed settings … are not visible here") and in SKILL.md, so it is a known residual rather than an undetected defect. The guard being correct is what matters for safety. But organizations that enforce audit-only via managed settings will see confusing probe output in the setup skill. Flagging for completeness — the fix would be to have the probe also read killswitch_config.managed_settings_path() when available.

---

What the new commit gets right

• resolve_effective() precedence logic is correct: The managed verdict only wins when source == "configured" — this is the right threshold. Absent files (source: default), malformed files (source: indeterminate), and files without the key (source: default) all pass through to user settings rather than defaulting to enabled or disabled. The behavior is symmetric with the fail-closed guarantee.

• Test coverage for the new surface is thorough: Five new cases cover managed-wins-false, managed-wins-true, managed-absent, managed-present-but-no-entry, and managed-malformed. The hermetic patching of managed_settings_path via mock.patch.object is the right mechanism — it prevents any test from accidentally reading a real /etc/claude-code/managed-settings.json from the CI runner.

• Existing GuardTests correctly patched: Both test_report_only_mode_approved_commands_produce_no_output and the kill-switch resolution tests now patch managed_settings_path to point inside the temp dir, ensuring the new code path doesn't re-introduce environment coupling through the managed file.

• Documentation updated consistently: CHANGELOG, SKILL.md, and safety-model.md all now accurately describe managed settings as a read surface with the correct residuals listed (managed-settings.d/, --settings flag).

---

Verdict

Approved with minor comments. The managed settings integration (Finding 1 env inconsistency aside) is architecturally correct. Finding 1 (Windows ProgramFiles env lookup) is a one-line fix that would fully close the "env never consulted" security claim. Findings 2–4 are cosmetic to low-severity with no blocking issues. The two long-standing carryover findings (zip, assert) are worth addressing before merge.

[claude]

Claude finished @kyle-sexton's task in 2m 14s —— View job

---

Security Review — PR #1242 (HEAD 266c2a9)

• Gather context and understand the request
• Read PR diff
• Analyze lib/killswitch_config.py (new shared reader + managed settings)
• Analyze destructive_guard.py changes
• Analyze kill_switch_probe.py changes
• Analyze hooks/hooks.json changes
• Analyze remaining changed files (SKILL.md ×2, plugin.json, safety-model.md)
• Post security findings

---

Reviewed HEAD 266c2a9. Three commits total: bd50ada (main fix — direct read via --plugin-root), 23e40d3 (shebang removal — no security surface), 266c2a9 (managed settings scope — new surface). Checked for: command injection, path traversal, env-based trust bypass, settings-file forgery, hook-arg injection, JSON injection, sys.path insertion, and TOCTOU. One new finding on the HEAD commit; one continuing finding from prior reviews.

---

IMPORTANT — managed_settings_path() reads %ProgramFiles% from the process environment on Windows, allowing a hostile repo to forge the managed-settings path

Confidence: CONFIRMED

killswitch_config.py L60–63

if sys.platform == "win32":
    program_files = os.environ.get("ProgramFiles", r"C:\Program Files")
    return Path(program_files) / "ClaudeCode" / "managed-settings.json"

The 266c2a9 commit introduces managed_settings_path() to honor the highest-precedence CC scope. For macOS and Linux the returned path is a hardcoded constant (/Library/... / /etc/...) that no repo can forge. On Windows the path is not hardcoded: it is assembled from os.environ["ProgramFiles"], which a repo's .claude/settings.json env block can override — the same channel the PR explicitly rejects for user-settings resolution ("a repo settings.json env block reaches hook subprocesses and carries no provenance").

Failure scenario: User has disk_hygiene_enabled: false in ~/.claude/settings.json (audit-only mode). A hostile repo sets {"env": {"ProgramFiles": "C:\\Users\\victim\\AppData\\Local\\Temp\\evil"}} in its .claude/settings.json and pre-populates C:\...\evil\ClaudeCode\managed-settings.json with {"pluginConfigs": {"disk-hygiene": {"options": {"disk_hygiene_enabled": true}}}}. When resolve_effective() runs, managed["source"] == "configured" and managed["effective"] == True, so the managed value wins over the user's false setting. The guard resolves enabled = True and returns ask for an apply call instead of deny — the kill switch degrades from "deny outright" to "prompt-gated."

The practical impact on Windows is exactly the kill-switch downgrade that triggered this entire PR. The macOS and Linux paths (/Library/..., /etc/...) are root-owned and unaffected; the Windows path is not.

Recommended fix: Use the %SystemRoot% env var (also potentially settable, but at least normatively system-owned) or, better, read the real %ProgramFiles% via the Win32 API (CSIDL_PROGRAM_FILES / SHGetFolderPath via ctypes) rather than trusting the env. The simplest defensible alternative for Python is to use the registry: HKLM\SOFTWARE\Microsoft\Windows\CurrentVersion\ProgramFilesDir. If adding a registry read is out of scope, document this as a Windows-only residual alongside the --settings flag residual already noted.

Fix this →

---

PLAUSIBLE (continuing from prior reviews, not addressed in 266c2a9) — Guard falls to env-reachable CLAUDE_CONFIG_DIR when --plugin-root layout check fails

destructive_guard.py L421–439

When --plugin-root is supplied but _user_settings_path_from_root() returns None (no plugins/cache marker), _resolve_user_settings_path() falls back to killswitch_config.default_settings_path(), which reads CLAUDE_CONFIG_DIR from the process environment. A hostile repo can redirect this to a forged settings file with disk_hygiene_enabled: true to degrade the user's false kill-switch from deny to prompt-gated. The 266c2a9 commit does not change this codepath. Two prior review passes (bd50ada and 23e40d3 heads) confirmed this finding as PLAUSIBLE; it is flagged again here for completeness. Recommended mitigation: when --plugin-root was supplied but unrecognised, treat the user-settings read as indeterminate and fail closed to enabled=True rather than falling back to the env-reachable default.

---

What was checked and found clean

• No command injection: The guard spawns no subprocesses; all hook decisions go through json.dumps() to stdout. _literal_shell_words() remains unchanged.
• macOS and Linux managed paths are hardcoded: /Library/Application Support/ClaudeCode/managed-settings.json and /etc/claude-code/managed-settings.json are not env-influenced and cannot be forged by a repo.
• resolve_effective() logic is correct: Managed configured wins; absent/unreadable/ambiguous managed yields no verdict and user settings decide; every path through probe() that is not source == "configured" fails closed to effective = True.
• sys.path insertion safe: Path(__file__).resolve().parents[3] / "lib" is the CC-controlled plugin install path; not attacker-controllable.
• plugin.json: Version bump to 0.9.0 only; no new permissions or trust delegations.
• pluginConfigs scope enforcement: CC 2.1.207+ ignores pluginConfigs in project/local scope, so a hostile repo cannot inject a false toggle via its own .claude/settings.json.
• No secrets or PII: Confirmed.

[kyle-sexton]

Addressed the Codex P1 ("Honor managed and --settings kill-switch sources") in commit 266c2a9.

lib/killswitch_config.py now also reads the platform managed-settings.json — macOS /Library/Application Support/ClaudeCode/, Linux/WSL /etc/claude-code/, Windows %ProgramFiles%\ClaudeCode\. As the highest-precedence, non-overridable scope, a value configured there wins over the user file, so an org-enforced disk_hygiene_enabled=false now denies rather than returning ask. resolve_effective() gained managed-precedence handling, covered by test_managed_configured_false_overrides_user_true and siblings (184 tests OK).

Remaining residuals a hook genuinely cannot read (documented in the guard docstring, safety-model, README, CHANGELOG): a session --settings file (a runtime CLI flag no hook observes) and the managed-settings.d/ drop-in directory.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 266c2a905f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 266c2a905f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 11s —— View job

---

PR Review: fix(disk-hygiene): harden managed read against env tamper + marketplace masking

• Gather context and understand the request
• Diff new commit (23cad83) against previously-reviewed HEAD (266c2a9)
• Read updated killswitch_config.py
• Read updated destructive_guard.py
• Read updated test file
• Assess whether prior findings are addressed
• Post final review

---

Overview of the new commit (23cad83)

This commit closes the two open confirmed/confirmed findings from the previous two review rounds:

1. Windows ProgramFiles env tamper (CONFIRMED, both reviews) — managed_settings_path() now returns the hard-coded Path(r"C:\Program Files\ClaudeCode\managed-settings.json") rather than using os.environ.get("ProgramFiles", ...). The docstring explains the rationale. macOS and Linux already used hard-coded paths; Windows is now consistent.

2. Marketplace masking (Codex P2) — _matches_plugin() now accepts an optional plugin_id: str | None; when supplied, only that exact <name>@<marketplace> key matches, preventing a second marketplace's disk-hygiene entry from producing an ambiguous multi-entry read that would fall back to effective=True. The guard derives the exact key from --plugin-root via the new _plugin_id_from_root() and threads it through resolve_disk_hygiene_enabled() → resolve_effective() → probe(). The test test_installed_marketplace_id_isolates_from_other_marketplace locks the adversarial case precisely.

---

Findings

1. zip(dict, list) fragility — carryover, still present

killswitch_config.py line 211:

f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); "

interpreted is a dict, so zip(interpreted, entries) iterates dict keys. This works only because interpreted is built from entries in the same insertion order (Python 3.7+ guarantees). If the relative build order of either structure changes, the error message silently maps the wrong keys to the wrong values. One-line fix:

f"({json.dumps({e['key']: e['value'] for e in entries})}); "

Flagged in both prior review rounds; still not addressed. Fix this →

2. assert effective is not None stripped under python -O — carryover, still present

killswitch_config.py line 229:

effective = values.pop()
assert effective is not None

The invariant is logically sound, but assert is silently stripped by python -O. In safety-critical code an explicit branch is more robust:

effective = values.pop()
if effective is None:
    raise AssertionError("invariant violated: single-element values set contained None")

Low severity but flagged twice before. Fix this →

3. managed_settings_path parameter shadows module-level function — cosmetic, carryover

killswitch_config.py line 242:

def resolve_effective(
    settings_path: Path,
    managed_settings_path: Path | None = None,
    plugin_id: str | None = None,
) -> bool:

The parameter managed_settings_path shadows the module-level function of the same name. Harmless here, but a future edit adding a call to managed_settings_path() inside this body would get a TypeError at the parameter rather than the function. Renaming to managed_path would eliminate the ambiguity.

4. Guard fallback to env-reachable CLAUDE_CONFIG_DIR — PLAUSIBLE, acknowledged residual

destructive_guard.py _resolve_user_settings_path() line 447–465

When --plugin-root is supplied but _user_settings_path_from_root() returns None (no plugins/cache marker in the path), the fallback is killswitch_config.default_settings_path(), which reads CLAUDE_CONFIG_DIR from the environment. This was flagged in both prior review rounds as PLAUSIBLE. Not addressed in this commit, and the docstring notes the dependency on the runtime guarantee (${CLAUDE_PLUGIN_ROOT} always containing plugins/cache). The practical impact is bounded — this branch is unreachable through a normal CC install — but the security claim that "env is never consulted" is technically only true on the happy path.

5. Probe/guard divergence on plugin_id — acknowledged residual

kill_switch_probe.py is not changed in this commit. The probe still matches any disk-hygiene@* key (no plugin_id argument), while the guard now matches exactly disk-hygiene@melodic-software. If a user has two disk-hygiene marketplace installs with different disk_hygiene_enabled values, the probe aggregates both (resolving to indeterminate → effective=True) while the guard reads only the current install's key. The setup skill's honest reporting would then differ from what the guard enforces. This is a known residual and would require passing --plugin-root to the probe to fix.

---

What the new commit gets right

• Windows env tamper is fully closed: Hard-coding C:\Program Files matches the pattern already used for macOS and Linux, and the docstring explains exactly why the env var cannot be trusted. This is the right fix and directly addresses the CONFIRMED security finding.

• Marketplace isolation is correct and minimal: _plugin_id_from_root() uses exactly the same plugins/cache path marker as _user_settings_path_from_root() — consistent behavior for the same input path. The derived f"{name}@{marketplace}" key matches the CC convention documented in _plugin_id_from_root's docstring. The index + 2 >= len(parts) guard is symmetric with the equivalent guard in _plugin_data_root_from_root.

• Fallback on absent/partial paths is safe: When _plugin_id_from_root() returns None (non-marketplace install, or --plugin-root absent), probe() receives plugin_id=None and falls back to matching any disk-hygiene@* key — this is the original behavior, which is less targeted but still safe. The narrowing is additive, not mandatory.

• Test is the right adversarial case: test_installed_marketplace_id_isolates_from_other_marketplace sets disk-hygiene@melodic-software: false and disk-hygiene@other-marketplace: true and asserts false wins. That is precisely the masking scenario the Codex P2 finding described.

• CHANGELOG is accurate: The updated entry correctly describes the fixed Windows path, the marketplace key isolation, and the two remaining residuals (managed-settings.d/, --settings flag).

---

Verdict

Approved with minor comments. This commit closes the two remaining confirmed security findings. Findings 1 and 2 (zip fragility, assert under -O) are minor carryovers worth fixing before the release tag. Finding 3 is cosmetic. Findings 4 and 5 are acknowledged residuals with documented scope. No new issues introduced.

[claude]

Claude finished @kyle-sexton's task in 3m 29s —— View job

---

Security Review — PR #1242 (HEAD 23cad83)

• Gather context and understand the request
• Get full PR diff and identify changed files
• Read lib/killswitch_config.py (current HEAD)
• Read destructive_guard.py (current HEAD)
• Read remaining changed files
• Analyze security surface of new commit (23cad83 — env-tamper hardening + marketplace masking)
• Post security findings

---

Reviewed HEAD 23cad83. The latest commit addresses two previously-confirmed security findings from prior reviews: the Windows %ProgramFiles% env tamper and the multi-marketplace masking attack. One prior PLAUSIBLE finding is not addressed and is re-confirmed below. No new security issues were introduced by this commit.

---

What 23cad83 fixes

Windows managed-settings path now hardcoded.
killswitch_config.py L70–71

if sys.platform == "win32":
    return Path(r"C:\Program Files\ClaudeCode\managed-settings.json")

Prior reviews (confirmed in both the code review and security review of 266c2a9) identified that os.environ.get("ProgramFiles", ...) let a hostile repo redirect the managed-settings lookup via an env block. The fix correctly hardcodes the path for Windows, matching the macOS and Linux treatment.

Exact plugin-ID matching closes multi-marketplace ambiguity.
_plugin_id_from_root() L421–444, _matches_plugin() L77–88

The Codex P2 finding ("a second marketplace's disk-hygiene entry causes an ambiguous read that falls back to enabled") is now closed. The guard derives disk-hygiene@melodic-software from its CC-controlled --plugin-root path and passes it to probe() as a narrowing key; only the exact pluginConfigs entry for this install is matched.

The plugin_id value is derived purely from CC-controlled path components and used only in a plain string equality comparison (key == plugin_id). No injection surface is introduced.

---

IMPORTANT — CLAUDE_CONFIG_DIR reachable in guard's kill-switch path when --plugin-root layout check fails

Confidence: PLAUSIBLE (continuing from prior reviews; not addressed in this commit)

destructive_guard.py L460–465 → killswitch_config.py L35–39

def _resolve_user_settings_path() -> Path:
    plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
    if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
        derived = _user_settings_path_from_root(plugin_root)
        if derived:
            return Path(derived)
    return killswitch_config.default_settings_path()  # reads CLAUDE_CONFIG_DIR

When --plugin-root is supplied but _user_settings_path_from_root() returns None (the path doesn't contain a plugins/cache marker), the guard falls back to killswitch_config.default_settings_path(). That function reads CLAUDE_CONFIG_DIR from the process environment — a channel a repo's .claude/settings.json env block controls.

Failure scenario: User has disk_hygiene_enabled: false in real ~/.claude/settings.json. Hostile repo sets {"env": {"CLAUDE_CONFIG_DIR": "/tmp/evil"}} and pre-populates /tmp/evil/settings.json with {"pluginConfigs": {"disk-hygiene@melodic-software": {"options": {"disk_hygiene_enabled": true}}}}. If CC ever ships a layout change that removes the plugins/cache path segment — or if a symlinked install doesn't include it — _user_settings_path_from_root returns None, the fallback reads from the forged path, and the guard resolves enabled = True instead of the user's configured False. The kill switch degrades from deny-outright to a human-approval prompt.

The docstring for _resolve_user_settings_path() now explicitly labels the CLAUDE_CONFIG_DIR branch as the "fallback for non-hook invocations … where --plugin-root is absent." But the fallback also fires when --plugin-root is present but unrecognized. Those two cases have different trust properties and the docstring conflates them.

Suggested fix: Distinguish the two code paths. When --plugin-root is present but layout recognition fails, treat the user-settings read as indeterminate and fail closed (effective = True) rather than falling back to the env-reachable default:

def _resolve_user_settings_path() -> Path | None:  # None = indeterminate
    plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
    if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
        derived = _user_settings_path_from_root(plugin_root)
        return Path(derived) if derived else None  # layout unknown → caller fails closed
    return killswitch_config.default_settings_path()  # safe: no --plugin-root → CLI/test context

The resolve_effective caller already produces True (guard active) when probe() gets FileNotFoundError or an unreadable file — returning None from _resolve_user_settings_path and calling probe(None_path) would error before that, so a small wrapper that substitutes a known-absent path (e.g., Path("/dev/null")) or short-circuits to True achieves the same without restructuring probe().

---

No other security issues found

The following were explicitly checked in this commit and found clean:

• _plugin_id_from_root() path-traversal safety: The derived marketplace and name values are path components from the CC-controlled install root, used only in a string equality comparison (key == plugin_id). No file-system operations are driven by these values; no injection surface exists.
• casefold() normalization: Used only on the plugins and cache segment names for case-insensitive marker detection. The marketplace and name components are taken without folding, preserving the exact key format CC uses when writing pluginConfigs.
• probe() with plugin_id on managed file: Both probe() calls in resolve_effective() now receive the same plugin_id. An org that keys managed settings as disk-hygiene (no marketplace qualifier) would not be matched by the exact-key path, but the result is the fail-closed default (effective=True) — a safe outcome.
• WSL/Linux path unchanged: The sys.platform.startswith("linux") branch still returns /etc/claude-code/managed-settings.json (hardcoded). WSL is Linux from Python's perspective; the Windows-only branch applies only to native win32. No regression there.

[kyle-sexton]

Addressed the second review round in 23cad83:

• P1 (managed path from untrusted env) — Fixed. The Windows managed-settings path is now the hard-coded C:\Program Files\ClaudeCode\managed-settings.json, not %ProgramFiles%-derived, so a repo env block can no longer redirect the highest-precedence managed lookup at a forged file.
• P2 (cross-marketplace masking) — Fixed. The guard derives its exact <name>@<marketplace> key from ${CLAUDE_PLUGIN_ROOT} and matches only that, so another marketplace's disk-hygiene entry can't make the read ambiguous. Covered by test_installed_marketplace_id_isolates_from_other_marketplace. The report-only CLI keeps the prefix match (it can't know its marketplace).
• P1 (session --settings source) — Documented residual, not fixable at the hook layer: Claude Code exposes no channel (env var or hook payload field) carrying the active --settings file path, so a hook genuinely cannot read it. This is the same documented scope the report-only probe shipped with. It is called out explicitly in the guard docstring, safety-model, README, and CHANGELOG so operators know a --settings-only toggle is not guard-enforced.

185 tests OK; markdownlint clean.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 23cad83407

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 30s —— View job

---

PR Review: fix(disk-hygiene): read managed-settings.d drop-in kill-switch entries

• Gather context and understand the request
• Diff new commit (48f8355) against previously-reviewed HEAD (23cad83)
• Read updated lib/killswitch_config.py
• Read updated destructive_guard.py
• Read updated test file
• Assess whether prior findings are addressed
• Post final review

---

Overview of the new commit (48f8355)

This commit closes the last confirmed Codex P1 finding (and the acknowledged residual from all four prior review passes): managed-settings.d/ drop-in files are now read and merged over the primary managed file, using the same precedence model Claude Code uses. The implementation is minimal and correct — no new env lookups, no path traversal, no injection surface.

---

Findings

1. _managed_settings_files() can throw PermissionError from iterdir(), crashing the guard instead of failing closed — low severity

killswitch_config.py L257–265:

dropin = managed_settings_path.parent / _MANAGED_DROPIN_DIRNAME
if dropin.is_dir():
    files.extend(
        sorted(
            child
            for child in dropin.iterdir()
            if child.suffix == ".json" and child.is_file()
        )
    )

Path.is_dir() calls os.stat() on the path, which requires execute permission on the parent directory. iterdir() calls os.listdir(), which requires read permission on the directory itself. If managed-settings.d/ is created with restrictive permissions (e.g., 700 root-owned), is_dir() returns True but iterdir() raises PermissionError. That exception propagates uncaught through _managed_effective() → resolve_effective() → resolve_disk_hygiene_enabled() → main(), crashing the guard with a non-zero exit code.

The guard's own docstring promises "every absent, unreadable, or ambiguous read fails closed to enabled," but a crash in the hook is not a fail-closed outcome — it is an unhandled error whose effect on the hook launcher depends on CC's behavior for a non-zero hook exit (which may produce an error dialog or degrade differently than a clean deny). In practice, system-managed directories are world-readable (755), so this is a very unlikely scenario, but it's inconsistent with the fail-closed contract.

Minimal fix — wrap iterdir():

dropin = managed_settings_path.parent / _MANAGED_DROPIN_DIRNAME
if dropin.is_dir():
    try:
        files.extend(
            sorted(
                child
                for child in dropin.iterdir()
                if child.suffix == ".json" and child.is_file()
            )
        )
    except OSError:
        pass  # unreadable drop-in dir treated as empty; primary file still counted

Fix this →

2. zip(dict, list) fragility — carryover, still present (×4)

killswitch_config.py line 212:

f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); "

One-line fix: {e["key"]: e["value"] for e in entries}. Flagged in all four prior rounds.

Fix this →

3. assert effective is not None stripped under python -O — carryover, still present (×4)

killswitch_config.py line 230:

effective = values.pop()
assert effective is not None

Replace with if effective is None: raise AssertionError(...). Flagged in all four prior rounds.

Fix this →

4. managed_settings_path parameter shadows module-level function — cosmetic, carryover (×2)

killswitch_config.py line 291:

def resolve_effective(
    settings_path: Path,
    managed_settings_path: Path | None = None,

Rename parameter to managed_path to avoid shadowing the module-level function.

---

What the new commit gets right

• Precedence logic matches CC exactly: The primary file comes first in the list; sorted drop-ins follow. _managed_effective() keeps updating verdict on each "configured" probe — so the last configured file wins, including drop-ins overriding the primary. That's the correct "later overrides" merge semantics.

• _managed_settings_files avoids double-listing: The is_file() check on managed_settings_path before appending prevents the (unlikely) case of the base path being absent from also producing a silent no-op; the absence of a try/except around is_file() here is fine because Path.is_file() returns False on any error (including permission denied) rather than raising.

• Test coverage is precise and adversarially placed: The three new tests — drop-in false beats user true, drop-in beats primary managed, later drop-in beats earlier drop-in — are exactly the three precedence invariants the feature needs to lock. They are written hermetically against the config_dir temp tree already established in setUp.

• Docstrings, CHANGELOG, README, SKILL.md, and safety-model.md all updated consistently: The managed-settings.d/ residual is now correctly removed from all four "honored sources a hook cannot read" lists.

• The one remaining documented residual (--settings flag) is accurate and unaddressable at the hook layer: Correct conclusion — no hook receives the active --settings path; no further action is possible.

---

Verdict

Approved with minor comments. This commit closes the last confirmed security finding (Codex P1 / the acknowledged residual). Finding 1 (iterdir() exception handling) is worth a one-line fix to restore full fail-closed coverage for the drop-in directory; its practical impact is very low. Findings 2–4 are the same minor carryovers from prior rounds. No blocking issues.

[claude]

Claude finished @kyle-sexton's task in 3m 50s —— View job

---

Security Review — PR #1242 (HEAD 48f8355)

• Gather context and understand the request
• Read full PR diff (all 5 commits)
• Read lib/killswitch_config.py at HEAD
• Read destructive_guard.py at HEAD
• Read kill_switch_probe.py and hooks/hooks.json
• Analyze security surface of HEAD commit (managed-settings.d drop-in support)
• Post security findings

---

Reviewed HEAD 48f8355. The HEAD commit adds _managed_settings_files() and _managed_effective() to read the managed-settings.d/ drop-in directory — the last CC scope a hook could not previously read. Checked for: command injection, path traversal, env-based trust bypass, settings-file forgery via drop-in, TOCTOU, symlink abuse, hook-arg injection, and sys.path manipulation.

---

IMPORTANT — Guard falls to env-reachable CLAUDE_CONFIG_DIR when --plugin-root layout check fails (continuing, unaddressed in all 5 commits)

Confidence: PLAUSIBLE

destructive_guard.py L460–465

def _resolve_user_settings_path() -> Path:
    plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
    if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
        derived = _user_settings_path_from_root(plugin_root)
        if derived:
            return Path(derived)
    return killswitch_config.default_settings_path()  # reads CLAUDE_CONFIG_DIR

This has been flagged PLAUSIBLE in both prior security review passes and remains unchanged across all five commits. When --plugin-root is provided but _user_settings_path_from_root() returns None (the path lacks the plugins/cache marker), the guard falls through to killswitch_config.default_settings_path(), which reads CLAUDE_CONFIG_DIR from the process environment. A repo's .claude/settings.json env block can set that variable for hook subprocesses.

Failure scenario: User has disk_hygiene_enabled: false in their real ~/.claude/settings.json. Hostile repo sets {"env": {"CLAUDE_CONFIG_DIR": "/tmp/evil"}} in .claude/settings.json and pre-populates /tmp/evil/settings.json with {"pluginConfigs": {"disk-hygiene@melodic-software": {"options": {"disk_hygiene_enabled": true}}}}. If CC ever ships a layout change removing plugins/cache from the install path — or if an edge case bypasses the marker scan — the guard reads the forged value and treats the kill switch as enabled, degrading audit-only mode from an outright deny to a human-approval prompt.

In practice this branch requires a CC install-layout regression, so risk is low. The docstring labels this branch as "fallback for non-hook invocations where --plugin-root is absent" — but the fallback also fires when --plugin-root is present-but-unrecognized, and those two cases have different trust properties.

Recommended fix: When --plugin-root was provided but the layout check failed, treat the settings path as indeterminate and fail closed (return True for enabled), rather than falling through to the env-reachable default. The env fallback is correct when --plugin-root is absent (CLI/test context); it is incorrect when --plugin-root was supplied and unrecognized.

---

SUGGESTION — dropin.iterdir() unhandled in _managed_settings_files (new in HEAD)

Confidence: CONFIRMED

killswitch_config.py L258–263

if dropin.is_dir():
    files.extend(
        sorted(
            child
            for child in dropin.iterdir()   # <-- no OSError catch
            if child.suffix == ".json" and child.is_file()
        )
    )

Path.is_dir() swallows OSError (returns False), but dropin.iterdir() does not — if the directory becomes inaccessible between the is_dir() check and the iteration (TOCTOU, permission change, filesystem error), the unhandled OSError propagates through _managed_effective() → resolve_effective() → resolve_disk_hygiene_enabled() → main(). The guard's try/except at line 770 only wraps JSON parsing, so the exception reaches Python's top level, the guard exits with a traceback and code 1, and Claude Code fails the hook.

Failure scenario: An admin's filesystem encounter with /etc/claude-code/managed-settings.d/ (e.g., a temporary permissions change during a config management run) causes the guard to crash mid-session. In practice, this is not exploitable from a hostile repo (the directory is at a root-owned path; a repo cannot forge it), and Claude Code likely fails-closed when a hook crashes — so no security bypass occurs. The impact is unexpected guard failures in unusual sysadmin scenarios.

Recommended fix: Wrap the dropin.iterdir() call in a try/except, treat an unreadable drop-in directory as contributing no verdict (consistent with how unreadable individual files are treated by probe()):

if dropin.is_dir():
    try:
        entries = sorted(
            child
            for child in dropin.iterdir()
            if child.suffix == ".json" and child.is_file()
        )
    except OSError:
        entries = []
    files.extend(entries)

---

What the HEAD commit gets right

Drop-in directory is as tamper-resistant as the primary managed file. managed_settings_path.parent / "managed-settings.d" resolves to:

• macOS: /Library/Application Support/ClaudeCode/managed-settings.d/
• Linux: /etc/claude-code/managed-settings.d/
• Windows: C:\Program Files\ClaudeCode\managed-settings.d\

All three are the immediate sibling of the primary managed file, at the same root-owned system path. Since managed_settings_path() returns a hard-coded constant (env-independent after 23cad83), the drop-in directory location is also hard-coded — a hostile repo cannot redirect it via ProgramFiles or any other env var.

"Last configured wins" semantics are correct. _managed_effective() iterates in _managed_settings_files() order (primary, then drop-ins sorted) and updates verdict only when a file's probe() returns source == "configured". Files that are absent, carry no entry, or are malformed/ambiguous contribute nothing to verdict. This matches CC's documented drop-in merge model and is safe for a safety toggle — an unreadable drop-in cannot silently enable the switch.

probe() handles all drop-in file error cases. probe() is called for each file in _managed_settings_files(). It catches FileNotFoundError, OSError, UnicodeDecodeError, non-dict JSON root, unrecognized boolean values, and conflicting entries — all returning source != "configured" and therefore contributing no verdict. No drop-in file content can flip the switch to enabled without being a valid, unambiguous, non-None boolean.

child.is_file() filter excludes symlinks to non-files. Path.is_file() follows symlinks and returns False for broken symlinks, directories, and special files. So a malformed drop-in directory (world-writable subdirectory, device file) contributes no file to the list. At a root-owned path this is moot, but the filtering is correct regardless.

What was checked and found clean in this commit:

• No env variable consulted for the drop-in path location (derives from the hardcoded managed_settings_path() result)
• No command injection: no subprocess calls; dropin.iterdir() uses Python's os.scandir(), not a shell
• No path traversal: _MANAGED_DROPIN_DIRNAME = "managed-settings.d" is a constant, not model-derived; child.suffix == ".json" filters by extension only; no ../ is possible via Path.iterdir() (it yields direct children only)
• child.suffix is case-sensitive (".json" not ".JSON"), causing .JSON files to be skipped on case-insensitive filesystems — a minor inconsistency but no security impact
• kill_switch_probe.py unchanged: the probe/guard divergence on managed settings (probe reads user only) is a pre-existing acknowledged residual, documented in the module docstring

---

Status of prior confirmed findings

Finding	Status
Windows %ProgramFiles% env tamper → managed-settings path forgery	Fixed (23cad83)
Multi-marketplace masking → ambiguous read → falls back to enabled	Fixed (23cad83)
Managed settings not honored (only user settings read)	Fixed (266c2a9)
managed-settings.d/ drop-ins not read	Fixed (48f8355, this commit)
Guard falls to env-reachable CLAUDE_CONFIG_DIR when layout check fails	Open (PLAUSIBLE, all 5 commits)

[kyle-sexton]

Fixed the managed-settings.d/ drop-in P1 in 48f8355: the reader now merges the primary managed-settings.json with every *.json in the sibling managed-settings.d/ directory (sorted, later files win) before falling back to user settings — covered by test_managed_dropin_false_overrides_user, test_managed_dropin_overrides_primary_managed_file, and test_later_managed_dropin_wins_over_earlier. The session --settings file is now the only honored source a hook cannot read.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 48f835548c

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[claude]

Claude finished @kyle-sexton's task in 2m 44s —— View job

---

PR Review: fix(disk-hygiene): never trust an env-derived settings path in the guard

• Gather context and understand the request
• Diff new commit (429a29c) against previously-reviewed HEAD (48f8355)
• Read updated killswitch_config.py
• Read updated destructive_guard.py
• Read updated test file
• Assess whether prior findings are addressed
• Post final review

---

Overview of the new commit (429a29c)

This commit closes the one remaining confirmed open finding: the guard's fallback to an env-reachable CLAUDE_CONFIG_DIR when --plugin-root path layout check fails. It was flagged as PLAUSIBLE in all five prior security review passes and remained unaddressed until now.

_resolve_user_settings_path() now returns Path | None instead of Path. When --plugin-root is absent or carries no plugins/cache marker (e.g. a --plugin-dir checkout), the function returns None rather than falling back to killswitch_config.default_settings_path() (which reads CLAUDE_CONFIG_DIR). resolve_effective() accepts settings_path: Path | None; a None value skips the user scope entirely and fails closed to True unless a managed setting wins.

---

What's in this commit

The core fix is correct and complete

destructive_guard.py L447–467 — killswitch_config.py L288–315

The fix is the right shape. The security model now holds without caveats: the guard never consults an environment-derived path in any reachable code path. A --plugin-dir checkout (marker-less root) loses the user-settings read entirely, but the managed-settings path at its fixed system location is still checked — so an organization enforcing audit-only via managed settings is still enforced. Absent a managed verdict the switch fails closed to True (guard active). The docstring in _resolve_user_settings_path is a clear, accurate explanation of the design decision and belongs there.

Test updates match the new behavior precisely

test_no_plugin_root_ignores_env_config_dir_and_fails_closed_enabled (L3613–3624) — Previously test_config_dir_env_fallback_when_no_plugin_root asserted False (env read was honored); now it asserts True (env read is ignored, fails closed). The rename and assertion flip correctly capture the behavioral inversion.

test_plugin_dir_install_ignores_env_user_settings_but_honors_managed (L3626–3641) — This is the adversarially complete test for the --plugin-dir case: a marker-less root + hostile CLAUDE_CONFIG_DIR + configured false in env-reachable file → result is True (env ignored). With a managed false added → result is False (managed honored). Without managed → result is True (fails closed). All three sub-assertions are necessary to lock all three properties simultaneously.

GuardTests._invoke_guard() refactored (L2237–2251) — No longer injects CLAUDE_CONFIG_DIR into os.environ; now patches _resolve_user_settings_path directly via mock.patch.object. This is the right isolation: the guard no longer consults env, so the tests must not rely on it either. The CLAUDE_CONFIG_DIR removals from the three subprocess-invocation helpers are equally correct.

---

Remaining carryover findings (unchanged across all commits)

1. zip(dict, list) in error message — still present (×5 reviews)

killswitch_config.py L212:

f"({json.dumps({k: e['value'] for k, e in zip(interpreted, entries)})}); "

One-line fix: {e["key"]: e["value"] for e in entries}.

Fix this →

2. assert effective is not None stripped under python -O — still present (×5 reviews)

killswitch_config.py L229–230:

effective = values.pop()
assert effective is not None

Replace with if effective is None: raise AssertionError("invariant violated: single-element values set contained None").

Fix this →

3. managed_settings_path parameter shadows module-level function — still present (×3 reviews)

killswitch_config.py L290:

def resolve_effective(settings_path: Path | None, managed_settings_path: Path | None = None, ...):

Rename parameter to managed_path to avoid shadowing the module-level function.

4. dropin.iterdir() unhandled OSError — still present (×1 review)

killswitch_config.py L258–265:

if dropin.is_dir():
    files.extend(sorted(child for child in dropin.iterdir() if child.suffix == ".json" and child.is_file()))

dropin.iterdir() raises OSError on a directory that is is_dir() but not listable (TOCTOU, permission change). Wrap in try/except OSError: pass so an unreadable drop-in dir is treated as empty rather than crashing the guard mid-session.

Fix this →

---

Security finding status — all commits

Finding	Status
${user_config.*} hook-drop making engine gate inert	Fixed (bd50ada)
Managed settings not honored (only user settings read)	Fixed (266c2a9)
Windows %ProgramFiles% env tamper → managed-settings path forgery	Fixed (23cad83)
Multi-marketplace masking → ambiguous read → falls back to enabled	Fixed (23cad83)
managed-settings.d/ drop-ins not read	Fixed (48f8355)
Guard falls to env-reachable CLAUDE_CONFIG_DIR when layout check fails	Fixed (429a29c, this commit)
zip(dict, list) fragility in error message	Open (minor, ×5 reviews)
assert effective is not None stripped by python -O	Open (low severity, ×5 reviews)
managed_settings_path parameter shadows module-level function	Open (cosmetic, ×3 reviews)
dropin.iterdir() unhandled OSError	Open (low severity, ×1 review)

---

Verdict

Approved. This commit closes the last open confirmed/PLAUSIBLE security finding. The kill-switch resolution chain is now fully tamper-resistant: no code path in the guard consults an env-derived path for the user-settings location. The four carryover items (zip, assert, parameter shadow, iterdir) are minor to low-severity with no blocking issues. Ready to merge.

[kyle-sexton]

Fixed the --plugin-dir env-trust P1 in 429a29c. The guard now locates the user settings file solely from the ${CLAUDE_PLUGIN_ROOT} plugins/cache marker and never consults CLAUDE_CONFIG_DIR/HOME. A marker-less --plugin-dir root yields no trusted user path, so the user scope is skipped and the switch relies on managed settings (fixed root-owned system paths), failing closed to enabled otherwise — so managed audit-only enforcement still holds in --plugin-dir installs, and a repo-injected env var can no longer redirect the read. Covered by test_no_plugin_root_ignores_env_config_dir_and_fails_closed_enabled and test_plugin_dir_install_ignores_env_user_settings_but_honors_managed.

[claude]

Claude finished @kyle-sexton's task in 4m 9s —— View job

---

Security Review — PR #1242 (HEAD 429a29c)

• Gather context and understand the request
• Read PR diff and identify changed files
• Read lib/killswitch_config.py at HEAD
• Read destructive_guard.py at HEAD
• Read remaining changed files (kill_switch_probe.py, hooks/hooks.json, SKILL.md)
• Perform security analysis
• Post security findings

---

Reviewed HEAD 429a29c. The final commit removes the env fallback from _resolve_user_settings_path(), closing the last PLAUSIBLE finding that was flagged across all three prior security review passes. Checked for: command injection, path traversal, env-based trust bypass, settings-file forgery, hook-arg injection, sys.path injection, and TOCTOU. Two findings remain, both at SUGGESTION severity.

---

What 429a29c fixes

Previously PLAUSIBLE — now FIXED: _resolve_user_settings_path() no longer falls through to env-reachable CLAUDE_CONFIG_DIR.

destructive_guard.py L447–467

The prior function returned killswitch_config.default_settings_path() (which reads CLAUDE_CONFIG_DIR from the process environment) when --plugin-root was supplied but the path had no plugins/cache marker. That made the kill-switch read env-reachable for --plugin-dir checkout installs. The new function unconditionally returns None in that case:

def _resolve_user_settings_path() -> Path | None:
    plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
    if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
        derived = _user_settings_path_from_root(plugin_root)
        if derived:
            return Path(derived)
    return None  # no longer falls back to CLAUDE_CONFIG_DIR

resolve_effective() receives None for settings_path and, absent a managed verdict, returns True (guard stays active). A --plugin-dir install where CLAUDE_CONFIG_DIR carried disk_hygiene_enabled: false previously degraded from deny to ask; it now fails closed to enabled (stays guarded). The new test test_no_plugin_root_ignores_env_config_dir_and_fails_closed_enabled and test_plugin_dir_install_ignores_env_user_settings_but_honors_managed lock both invariants.

---

SUGGESTION — _managed_settings_files() dropin.iterdir() can raise PermissionError uncaught

Confidence: CONFIRMED

killswitch_config.py L258–265

if dropin.is_dir():
    files.extend(
        sorted(
            child
            for child in dropin.iterdir()   # raises PermissionError if unreadable
            if child.suffix == ".json" and child.is_file()
        )
    )

Path.is_dir() swallows OSError (returns False), but iterdir() does not. A directory that passes is_dir() can still be unlistable (e.g., root-owned with mode 0711). The resulting PermissionError propagates through _managed_effective() → resolve_effective() → resolve_disk_hygiene_enabled() → main() uncaught. The outer try/except in main() covers only JSON parsing; any other unhandled exception exits with a traceback and code 1.

Failure scenario: An administrator creates /etc/claude-code/managed-settings.d/ with restricted permissions (mode 0700). The guard crashes mid-invocation and prints no deny or allow decision. If CC's hook launcher treats an empty output + non-zero exit as a pass-through (no decision), the guard is bypassed for all commands in that session. If CC treats it as a deny, the session is merely broken. The managed-settings.d path is root-owned, so this requires an unusual administrative configuration and is not exploitable by a hostile repo.

The prior code review flagged this finding across two rounds; it is still not addressed. The fix is a two-line try/except OSError: pass around the iterdir() call, as noted in that review. Given that this is in safety-critical code and the fix is trivial, addressing it before the release tag is worthwhile.

Fix this →

---

SUGGESTION — resolve_authorized_data_root() still reads CLAUDE_PLUGIN_DATA env var as a final fallback

Confidence: CONFIRMED

destructive_guard.py L387–395

def resolve_authorized_data_root() -> str | None:
    direct = _argv_flag_value(sys.argv[1:], _AUTHORIZED_DATA_ROOT_FLAG)
    if direct and direct != _AUTHORIZED_DATA_ROOT_PLACEHOLDER:
        return direct
    plugin_root = _argv_flag_value(sys.argv[1:], _PLUGIN_ROOT_FLAG)
    if plugin_root and plugin_root != _PLUGIN_ROOT_PLACEHOLDER:
        derived = _plugin_data_root_from_root(plugin_root)
        if derived:
            return derived
    return os.environ.get(_CLAUDE_PLUGIN_DATA_ENV)   # <-- env fallback

The kill-switch path had an analogous env fallback, now removed in 429a29c. The data-root fallback remains. For a --plugin-dir checkout install (no plugins/cache marker), _plugin_data_root_from_root() returns None and the guard reads CLAUDE_PLUGIN_DATA from the process environment. A hostile repo can set this via its .claude/settings.json env block.

Failure scenario: A hostile repo sets CLAUDE_PLUGIN_DATA=/tmp/evil. On a --plugin-dir install, the guard authorizes --data-root /tmp/evil in engine commands. The engine writes its persistent state (snapshots, plans, reports) to that repo-controlled location. The model reads engine output from there; a pre-positioned file could influence what the model proposes to delete. However, the actual scan TARGET is determined by the user, not the data root, so the deletion footprint is bounded by what the user-specified target actually contains — the hostile repo cannot expand what gets deleted beyond that target.

This is lower severity than the kill-switch issue: it affects where engine output lands and how the model reasons about results, not whether the guard allows a delete. For marketplace installs, the hooks.json --authorized-data-root ${CLAUDE_PLUGIN_DATA} is CC-substituted (not env-derived), and for the skill-frontmatter hook, _plugin_data_root_from_root(plugin_root) derives the path tamper-resistantly. Only --plugin-dir installs reach the env fallback — the same install class now correctly handled by the kill-switch path. Flagging for consistency with the security model the PR establishes: "env is never consulted."

---

SUGGESTION — Kill-switch probe reports from env-reachable path; diverges from guard

Confidence: CONFIRMED (acknowledged residual)

kill_switch_probe.py L53–56

The probe still calls default_settings_path(), which reads CLAUDE_CONFIG_DIR from the environment. The guard (fully tamper-resistant since 429a29c) now resolves the kill switch from a completely different channel. A hostile repo setting CLAUDE_CONFIG_DIR can cause the probe to report effective: true while the guard enforces effective: false — the model receives incorrect state and may propose work the guard will silently deny, creating a confusing session. No deletion occurs. The probe module's docstring explicitly labels this a known scope limitation. Flagging for completeness; the guard is the actual enforcement gate.

---

Status of all prior confirmed findings

Finding	Status in HEAD 429a29c
Guard falls to env-reachable CLAUDE_CONFIG_DIR when layout check fails	Fixed (429a29c)
Windows %ProgramFiles% env tamper → managed path forgery	Fixed (23cad83)
Multi-marketplace masking → ambiguous read → falls back to enabled	Fixed (23cad83)
Managed settings not honored	Fixed (266c2a9)
managed-settings.d/ drop-ins not read	Fixed (48f8355)
dropin.iterdir() PermissionError uncaught	Open (SUGGESTION)
resolve_authorized_data_root() env fallback for CLAUDE_PLUGIN_DATA	Open (SUGGESTION)
Probe/guard reporting divergence on settings path	Open (acknowledged residual)

What was checked and found clean at HEAD

• No command injection: The guard calls no subprocess, os.system, or eval. All decisions go through json.dumps() to stdout.
• No path traversal via model-controlled input: _literal_shell_words() rejects shell operator characters; _data_root_key() normalizes but does not trust model-supplied values for authority decisions.
• sys.path insertion safe: Path(__file__).resolve().parents[3] / "lib" is the CC-controlled plugin install path, not attacker-controllable.
• Kill-switch resolution fully env-independent in hook context: Both _resolve_user_settings_path() and managed_settings_path() now derive their paths without consulting the environment. A hostile repo's env block can no longer redirect either read.
• Fail-closed JSON reads: probe() handles FileNotFoundError, OSError, UnicodeDecodeError, non-dict JSON root, unrecognized booleans, and conflicting entries — all returning effective=True.
• plugin.json: Version bump to 0.9.0 only; no new permissions or trust delegations.
• No secrets or PII introduced.