From b25569fca8ce062a11093a58ab2caf1a78e4f6be Mon Sep 17 00:00:00 2001 From: raymondginger Date: Sun, 18 Oct 2026 16:57:03 +0800 Subject: [PATCH] permissions: confirm before writing the files hooks are discovered from The three hook-discovery files (user and workspace hooks.json, workspace .claude/settings.json) are not secrets: reads stay free. A written-in handler runs commands in a later session, so a file-tool write that would otherwise be allowed becomes an ask; stricter decisions (deny rules, plan mode, the read-only preset) are never relaxed, and no allow rule can silence the confirmation. Also match sensitive paths through Windows trailing dots and spaces, which the OS drops at open time while normpath keeps them: a stripped probe variant is checked alongside the literal path (the reported path stays as written). --- core/harness/permissions.py | 71 ++++++++++++++++++++ tests/test_harness_permissions.py | 104 ++++++++++++++++++++++++++++++ 2 files changed, 175 insertions(+) diff --git a/core/harness/permissions.py b/core/harness/permissions.py index 93fba1ae..d20900ff 100644 --- a/core/harness/permissions.py +++ b/core/harness/permissions.py @@ -190,14 +190,25 @@ def is_sensitive_path(raw: str, cwd: str | None = None) -> bool: The path is checked as written and with symlinks resolved, so a link in the workspace cannot reach ``~/.ssh/id_rsa``, and without regard to case, since the default macOS and Windows filesystems ignore it. + + Windows also drops trailing dots and spaces from path components at open + time while ``os.path.normpath`` keeps them (the literal name ``.env.`` + opens ``.env`` yet a literal compare misses), so a stripped variant is + checked too. The strip is only a match probe — the reported path stays + as written. """ normalized = _normalize_path(raw, cwd) try: candidates = {normalized, os.path.realpath(normalized)} except (OSError, ValueError): candidates = {normalized} + probes: set[str] = set() for candidate in candidates: probe = candidate.replace(os.sep, "/").casefold() + probes.add(probe) + # Keep "." and ".." components intact (rstrip would blank them). + probes.add("/".join(p.rstrip(" .") or p for p in probe.split("/"))) + for probe in probes: name = probe.rsplit("/", 1)[-1] for pattern in SENSITIVE_PATH_PATTERNS: if fnmatch.fnmatchcase(probe, pattern) or fnmatch.fnmatchcase( @@ -207,6 +218,36 @@ def is_sensitive_path(raw: str, cwd: str | None = None) -> bool: return False +# The files ``core.harness.hooks.discovery`` reads handlers from: the +# user-level and workspace-level ``hooks.json``, plus the workspace +# ``.claude/settings.json``. They are not secrets — reads stay free — but a +# written-in handler runs commands in a later session, so a file-tool write +# to one of them is surfaced for confirmation (see +# ``PermissionEngine._evaluate``). +_HOOK_CONFIG_SUFFIXES = ("/.deepcode/hooks.json", "/.claude/settings.json") + + +def is_hook_config_path(raw: str, cwd: str | None = None) -> bool: + """Whether ``raw`` names one of the hook-discovery files. + + Matched by suffix on the normalized, casefolded path, with symlinks + resolved and the same trailing-dot/space probe as + :func:`is_sensitive_path` (Windows drops those at open time, so + ``.claude/settings.json.`` still names the real file). + """ + normalized = _normalize_path(raw, cwd) + try: + candidates = {normalized, os.path.realpath(normalized)} + except (OSError, ValueError): + candidates = {normalized} + for candidate in candidates: + probe = candidate.replace(os.sep, "/").casefold() + probes = {probe, "/".join(p.rstrip(" .") or p for p in probe.split("/"))} + if any(p.endswith(_HOOK_CONFIG_SUFFIXES) for p in probes): + return True + return False + + def sensitive_locations() -> tuple[str, ...]: """The credential stores on this machine a sandboxed command may not read.""" from core.config import deepcode_home # lazy: core.config is heavy @@ -321,6 +362,13 @@ def hits_sensitive_path(self, arguments: Mapping[str, object]) -> str | None: return raw return None + def hits_hook_config_path(self, arguments: Mapping[str, object]) -> str | None: + """Return the raw path if any argument targets a hook-discovery file.""" + for raw in self._candidate_paths(arguments): + if is_hook_config_path(raw, self.cwd): + return raw + return None + @staticmethod def _argument_string(tool_name: str, arguments: Mapping[str, object]) -> str: """Best-effort string a pattern rule matches against. @@ -429,12 +477,27 @@ def _evaluate( "read-only access preset: mutating tools cannot be enabled by rules", ) + # A write to a file hooks are discovered from installs handlers that + # run commands in a later session. Downgrade every write that would + # otherwise be allowed to a confirmation: reads are never gated here, + # and a decision that is already stricter (deny rule, plan mode, the + # read-only preset above) keeps its strength — an ask only meets allow. + hook_config = None if read_only else self.hits_hook_config_path(arguments) + argument = self._argument_string(tool_name, arguments) matched: PermissionRule | None = None for rule in self.rules: # last match wins if rule.matches(tool_name, argument): matched = rule if matched is not None: + if matched.action is PermissionDecision.ALLOW and hook_config is not None: + return self._apply_approval_policy( + PermissionDecision.ASK, + ( + f"Writing '{hook_config}' requires confirmation: hook " + "handlers run commands in a later session" + ), + ) return self._apply_approval_policy( matched.action, ( @@ -452,6 +515,14 @@ def _evaluate( "exploration is permitted until the plan is approved.", ) if self.mode is PermissionMode.FULL_AUTO: + if hook_config is not None: + return self._apply_approval_policy( + PermissionDecision.ASK, + ( + f"Writing '{hook_config}' requires confirmation: hook " + "handlers run commands in a later session" + ), + ) return PermissionDecision.ALLOW, "full_auto mode: no implicit gate" # DEFAULT if read_only: diff --git a/tests/test_harness_permissions.py b/tests/test_harness_permissions.py index 5abaad64..66120ab7 100644 --- a/tests/test_harness_permissions.py +++ b/tests/test_harness_permissions.py @@ -89,6 +89,110 @@ def test_ordinary_workspace_path_is_not_denylisted(): assert _decide(engine, "write_file", file_path="src/model.py") is ALLOW +def test_writing_a_hook_discovery_file_asks_even_in_full_auto(): + # The three files hooks are discovered from are not secrets, but a + # written-in handler runs commands in a later session: a file-tool + # write to one of them surfaces a confirmation instead of passing + # through full_auto. Reads are checked by the test below. + engine = PermissionEngine(mode=PermissionMode.FULL_AUTO, cwd="/home/u/proj") + for raw in ( + "/home/u/.deepcode/hooks.json", # user level + "/home/u/proj/.deepcode/hooks.json", # workspace + "/home/u/proj/.claude/settings.json", # workspace + ): + decision, reason = engine.evaluate("write", {"file_path": raw}) + assert decision is ASK, raw + assert "hook" in reason + # Other mutating file tools count as writes too. + assert ( + _decide(engine, "edit", file_path="/home/u/proj/.claude/settings.json") is ASK + ) + # Windows drops trailing dots/spaces at open time: the decoration still + # names the real file, so it still asks. + assert ( + _decide(engine, "write", file_path="/home/u/proj/.claude/settings.json.") is ASK + ) + assert _decide(engine, "write", file_path="/home/u/.deepcode/hooks.json .") is ASK + # A blanket allow rule cannot silence the confirmation… + allowed = PermissionEngine( + mode=PermissionMode.FULL_AUTO, + rules=[PermissionRule("*", "*", ALLOW)], + cwd="/home/u/proj", + ) + assert ( + _decide(allowed, "write", file_path="/home/u/proj/.deepcode/hooks.json") is ASK + ) + # …and a stricter decision never relaxes into an ask: deny stays deny. + denied = PermissionEngine( + mode=PermissionMode.FULL_AUTO, + rules=[PermissionRule("*", "*", DENY)], + cwd="/home/u/proj", + ) + assert ( + _decide(denied, "write", file_path="/home/u/proj/.deepcode/hooks.json") is DENY + ) + + +def test_reading_a_hook_discovery_file_is_never_denied_or_asked(): + engine = PermissionEngine(mode=PermissionMode.FULL_AUTO, cwd="/home/u/proj") + for raw in ( + "/home/u/.deepcode/hooks.json", + "/home/u/proj/.deepcode/hooks.json", + "/home/u/proj/.claude/settings.json", + ): + assert _decide(engine, "read_file", file_path=raw) is ALLOW, raw + # Search tools read them with the same freedom. + assert _decide(engine, "grep", path="/home/u/proj/.claude/settings.json") is ALLOW + # Default mode keeps its standing rule: read-only tools are allowed. + default = PermissionEngine(mode=PermissionMode.DEFAULT, cwd="/home/u/proj") + assert ( + _decide(default, "read_file", file_path="/home/u/proj/.claude/settings.json") + is ALLOW + ) + + +def test_hook_write_ask_never_relaxes_plan_or_read_only_modes(): + # Plan mode denies mutating tools outright — the ask check must not + # surface a confirmation where no mutation is permitted at all. + plan = PermissionEngine(mode=PermissionMode.PLAN, cwd="/home/u/proj") + assert _decide(plan, "write", file_path="/home/u/proj/.deepcode/hooks.json") is DENY + # The read-only preset's mutating-tool denial is likewise an upper bound. + read_only = PermissionEngine( + mode=PermissionMode.FULL_AUTO, + enforce_read_only=True, + cwd="/home/u/proj", + ) + assert ( + _decide(read_only, "write", file_path="/home/u/proj/.claude/settings.json") + is DENY + ) + + +def test_denylist_survives_windows_trailing_dot_and_space(): + # Windows drops trailing dots/spaces from path components at open time, + # while os.path.normpath keeps them, so ".env." opens ".env" yet a + # literal match misses. The matcher must see through the decoration on + # every platform. + engine = PermissionEngine(mode=PermissionMode.FULL_AUTO, cwd="/w") + assert _decide(engine, "read_file", file_path="/w/.env.") is DENY + assert _decide(engine, "write", file_path="/w/.env .") is DENY + assert _decide(engine, "read_file", file_path="C:/w/.env.") is DENY + # Existing credential patterns get the same hardening. + assert _decide(engine, "read_file", file_path="/w/.ssh/id_rsa ") is DENY + + +def test_trailing_dot_normalization_does_not_overmatch(): + engine = PermissionEngine(mode=PermissionMode.FULL_AUTO, cwd="/w") + # Editor and ordinary JSON configs are not credentials: no pattern + # matches them, decorated or not. + assert _decide(engine, "write", file_path="/w/.vscode/settings.json") is ALLOW + assert _decide(engine, "write", file_path="/w/settings.json") is ALLOW + assert _decide(engine, "write", file_path="/w/package.json.") is ALLOW + assert _decide(engine, "write", file_path="/w/.envx.") is ALLOW + # Parent-directory references survive normalization untouched. + assert _decide(engine, "write", file_path="/w/../.env") is DENY + + def test_denylist_blocks_a_credential_directory_as_the_argument(): # grep/glob/read take a directory: searching ~/.aws reads what is inside. engine = PermissionEngine(mode=PermissionMode.FULL_AUTO, cwd="/home/u/proj")