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")