From 67b00b827a0b79a120c9776174d2922b8e646287 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 11:13:20 -0500 Subject: [PATCH 01/37] fix: members validate and repair stop rejecting the refs Path creates check_member's reserved-prefix refusal is a join-time admission rule (docs/multi-repo-work.md). Shared with validate/repair it made every locked member fail 'member refs collide with coordinator names' from build start onward -- permanently after a first ship, since the member milestone tag is never deleted. Refuse reserved refs only at add time. Fixes one item of #353. --- scripts/members.py | 20 +++++++++++--------- tests/test_members.py | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/scripts/members.py b/scripts/members.py index d427c6ee4..7f516c846 100644 --- a/scripts/members.py +++ b/scripts/members.py @@ -255,6 +255,7 @@ def check_member( coordinator_name: str, checkout: Path, recorded_remote: Optional[str] = None, + refuse_reserved_refs: bool = True, ) -> str: """Refuse a checkout that cannot join; return its origin URL.""" if checkout.is_symlink() or not checkout.is_dir(): @@ -294,13 +295,14 @@ def check_member( raise MembersError(f"member STATE.md is unreadable: {error}") from error if state.milestone is not None and (state.phase, state.status) != ("shipped", "done"): raise MembersError(f"member has an active milestone: {state.milestone}") - prefixes = member_ref_prefixes(coordinator_name) + tuple( - "refs/remotes/origin/" + branch.format(coordinator_name) for branch in MEMBER_BRANCH_FORMATS - ) - refs = _git(checkout, "for-each-ref", "--format=%(refname)").splitlines() - colliding = [ref for ref in refs if ref.startswith(prefixes)] - if colliding: - raise MembersError("member refs collide with coordinator names: " + ", ".join(colliding)) + if refuse_reserved_refs: + prefixes = member_ref_prefixes(coordinator_name) + tuple( + "refs/remotes/origin/" + branch.format(coordinator_name) for branch in MEMBER_BRANCH_FORMATS + ) + refs = _git(checkout, "for-each-ref", "--format=%(refname)").splitlines() + colliding = [ref for ref in refs if ref.startswith(prefixes)] + if colliding: + raise MembersError("member refs collide with coordinator names: " + ", ".join(colliding)) return remote @@ -620,7 +622,7 @@ def validate_members(repo: Path) -> list[dict[str, str]]: members = read_members(root) for member in members: checkout = Path(member["checkout"]) - check_member(root, state.project, checkout, member["remote"]) + check_member(root, state.project, checkout, member["remote"], refuse_reserved_refs=False) try: role = member_role(checkout) except MembersError as error: @@ -639,7 +641,7 @@ def repair_members(repo: Path) -> list[dict[str, str]]: members = read_members(root) for member in members: checkout = Path(member["checkout"]) - check_member(root, state.project, checkout, member["remote"]) + check_member(root, state.project, checkout, member["remote"], refuse_reserved_refs=False) _refuse_foreign_marker(checkout.resolve(), root, state.project, member["name"]) for member in members: checkout = Path(member["checkout"]).resolve() diff --git a/tests/test_members.py b/tests/test_members.py index 636eacc75..deee74abd 100644 --- a/tests/test_members.py +++ b/tests/test_members.py @@ -225,6 +225,21 @@ def test_add_ignores_member_refs_named_for_other_coordinators(self) -> None: result = self.add("web", member) self.assertEqual(result.returncode, 0, result.stderr) + def test_validate_and_repair_allow_refs_path_created_during_build(self) -> None: + member = self.joined() + git(member, "branch", "gsd-path/acme-M001") + git(member, "branch", "gsd-path-task/acme-T001") + git(member, "tag", "milestone/acme-001-demo") + for command in ("validate", "repair"): + result = self.run_members(command) + self.assertEqual(result.returncode, 0, result.stderr) + + def test_add_still_refuses_refs_path_creates_on_members(self) -> None: + member = self.make_member("web") + git(member, "branch", "gsd-path/acme-M001") + git(member, "tag", "milestone/acme-001-demo") + self.assert_refused(self.add("web", member), "collide with coordinator") + def test_add_refuses_during_build_and_ship(self) -> None: member = self.make_member("web") for phase in ("build", "ship"): From 110fc4583bfe3e982db5d040b53e937165035c81 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 11:13:20 -0500 Subject: [PATCH 02/37] fix: allow same-wave file overlap between dependency-ordered tasks The plan gate rejected any same-wave overlap without consulting the dependency graph it already built, while build_state.ready enforces the real invariant (overlap only among concurrent tasks) and the runtime dispatches same-wave dependencies in order. Skip the rejection when one task transitively depends on the other; unordered overlap, cycles, and later-wave dependencies are unchanged. Fixes one item of #353. --- scripts/check_handoffs.py | 20 +++++++ tests/test_handoffs.py | 112 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 132 insertions(+) diff --git a/scripts/check_handoffs.py b/scripts/check_handoffs.py index f86fa4306..743f7cdf9 100644 --- a/scripts/check_handoffs.py +++ b/scripts/check_handoffs.py @@ -1386,6 +1386,20 @@ def _task_deps(task_text: str, task_id: str) -> List[str]: return _inline_ids(rendered, "T", f"{task_id} deps") +def _dependency_reachable(graph: Dict[str, List[str]], start: str, target: str) -> bool: + seen: Set[str] = set() + pending = list(graph[start]) + while pending: + task_id = pending.pop() + if task_id in seen: + continue + if task_id == target: + return True + seen.add(task_id) + pending.extend(graph[task_id]) + return False + + def _canonical_repo_path(value: str, label: str) -> str: cleaned = value.strip().strip("`\"'") path = PurePosixPath(cleaned) @@ -1509,6 +1523,12 @@ def _validate_task_graph( and _task_scalar(tasks[right], right, "status") == "done" ): continue + # Dependency-ordered tasks serialize safely; only unordered + # same-wave overlap races on a shared file. + if _dependency_reachable(graph, left, right) or _dependency_reachable( + graph, right, left + ): + continue raise HandoffError( f"same-wave file overlap between {left} and {right}: {', '.join(overlap)}" ) diff --git a/tests/test_handoffs.py b/tests/test_handoffs.py index 79eb04b12..0bd6cd3e9 100644 --- a/tests/test_handoffs.py +++ b/tests/test_handoffs.py @@ -1811,6 +1811,118 @@ def test_plan_allows_same_wave_overlap_between_landed_tasks(self) -> None: ) check_handoffs.validate_plan(root) + def test_plan_allows_same_wave_overlap_between_dependency_ordered_tasks(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + self.write_plan_handoff(root) + self.write_plan_coverage( + root, + task_rows=( + "| T001 | Demo task T001 | — | src/app.py |\n" + "| T002 | Demo task T002 | T001 | src/app.py |\n" + ), + ) + self.write_coverage_task(root, "T001", "- SC1", files="src/app.py") + self.write_coverage_task( + root, + "T002", + "- SC2", + acceptance="1. The demo test suite is green.", + deps="[T001]", + files="src/app.py", + ) + + result = check_handoffs.validate_plan(root) + + self.assertEqual(result["tasks"], 2) + + def test_plan_allows_same_wave_overlap_across_a_dependency_chain(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + self.write_plan_handoff(root) + self.write_plan_coverage( + root, + task_rows=( + "| T001 | Demo task T001 | — | src/app.py |\n" + "| T002 | Demo task T002 | T001 | tests/test_app.py |\n" + "| T003 | Demo task T003 | T002 | src/app.py |\n" + ), + ) + self.write_coverage_task(root, "T001", "- SC1", files="src/app.py") + self.write_coverage_task( + root, + "T002", + "- SC2", + acceptance="1. The demo test suite is green.", + deps="[T001]", + ) + self.write_coverage_task( + root, + "T003", + "- None", + deps="[T002]", + files="src/app.py", + ) + + result = check_handoffs.validate_plan(root) + + self.assertEqual(result["tasks"], 3) + + def test_plan_rejects_same_wave_overlap_without_a_dependency_path(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + self.write_plan_handoff(root) + self.write_plan_coverage( + root, + task_rows=( + "| T001 | Demo task T001 | — | src/app.py |\n" + "| T002 | Demo task T002 | T001 | tests/test_app.py |\n" + "| T003 | Demo task T003 | — | src/app.py |\n" + ), + ) + self.write_coverage_task(root, "T001", "- SC1", files="src/app.py") + self.write_coverage_task( + root, + "T002", + "- SC2", + acceptance="1. The demo test suite is green.", + deps="[T001]", + ) + self.write_coverage_task(root, "T003", "- None", files="src/app.py") + + with self.assertRaises(check_handoffs.HandoffError) as failure: + check_handoffs.validate_plan(root) + self.assertIn("same-wave file overlap between T001 and T003", str(failure.exception)) + + def test_plan_rejects_a_later_wave_dependency(self) -> None: + with tempfile.TemporaryDirectory() as directory: + root = Path(directory) + self.write_plan_handoff(root) + self.write_plan_coverage( + root, + task_rows=( + "| T002 | Demo task T002 | T001 | tests/test_app.py |\n" + "\n## Wave 2 — coverage\n\n" + "Goal: Cover the demo behavior.\n" + "Review depth: full\n\n" + "| Task | Title | Deps | Files |\n" + "|------|-------|------|-------|\n" + "| T001 | Demo task T001 | — | src/app.py |" + ), + ) + self.write_coverage_task(root, "T001", "- SC1", wave=2) + self.write_coverage_task( + root, + "T002", + "- SC2", + acceptance="1. The demo test suite is green.", + deps="[T001]", + ) + + with self.assertRaises(check_handoffs.HandoffError) as failure: + check_handoffs.validate_plan(root) + self.assertIn("depends on later-wave T001", str(failure.exception)) + def test_plan_rejects_dependency_cycles(self) -> None: with tempfile.TemporaryDirectory() as directory: root = Path(directory) From f6f20674980da762e4fca9230593dc0abfab5723 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 11:13:20 -0500 Subject: [PATCH 03/37] fix: guard hook precision for read-only commands - git arguments may carry $var when the subcommand is read-only (log/show/diff/status); write-capable subcommands keep the refusal - archive context evaluates read-only-ness per pipeline segment, so cat | wc of allowlisted reads passes; heredoc bodies no longer flip a wrapped payload into archive context - cd targets expand tracked assignments (dir=/x && cd "$dir") - quoted destructive paths may contain ASCII spaces inside one shell unit; injection-relevant characters stay denied - unprovable archive commands (ssh, python -) deny with an honest not-provably-read-only reason instead of claiming a write Fixes one item of #353. --- HOOKS.md | 9 +-- scripts/guard_hook.py | 108 +++++++++++++++++++++++++++++------ tests/test_guard_hook.py | 120 ++++++++++++++++++++++++++++++++++++++- 3 files changed, 213 insertions(+), 24 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 49e1661d8..a63d8ff3d 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -162,10 +162,11 @@ See [UPDATE.md](UPDATE.md). - deletion or move commands outside a single simple segment, including command chains, pipes, newlines, grouping, directory changes, and command substitution - deletion or move commands with any argument outside the literal-path character - set: ASCII letters, digits, `.`, `_`, `-`, and `/` (plus a drive prefix and - backslashes on Windows). One matching pair of surrounding quotes is allowed; - spaces, parameters, wildcards, braces, and tilde paths are denied even when - quoted. This also applies to `find -delete` and supported destructive aliases + set: ASCII letters, digits, `.`, `_`, `-`, `/`, and ASCII spaces inside one + shell unit (plus a drive prefix and backslashes on Windows). One matching + pair of surrounding quotes is allowed; parameters, wildcards, braces, and + tilde paths are denied even when quoted. This also applies to + `find -delete` and supported destructive aliases - destructive Git commands nested in supported shell and command wrappers - archive glob/brace expansions and execution-capable read options such as `rg --pre` diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 286afe31a..245866260 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -121,6 +121,10 @@ "committed GSD Path archives under .project/archive/ are read-only; " "only the bundled pipeline helpers may write there during a ship transaction" ) +ARCHIVE_UNPROVEN_READ_REASON = ( + "commands run in archive context must be provably read-only; only " + "recognized read commands and the bundled pipeline helpers may run there" +) DESTRUCTIVE_SHAPE_REASON = ( "destructive commands must run alone with literal paths; split compound, " "substituted, or expanded destructive commands into single commands" @@ -1176,10 +1180,10 @@ def directory_change_target(arguments): return operands[-1] -def segment_directories(tokens, working_directories): +def segment_directories(tokens, working_directories, contexts=None): """Yield each command segment with the working directories in effect for it.""" current_directories = list(working_directories) - for segment in command_segments(tokens): + for position, segment in enumerate(command_segments(tokens)): yield segment, current_directories invocation = command_invocation(segment) if invocation is None: @@ -1192,16 +1196,27 @@ def segment_directories(tokens, working_directories): if command not in DIRECTORY_CHANGE_COMMANDS: continue target = directory_change_target(arguments) - if SHELL_PARAMETER_SYNTAX.search(target): + assignments = None + if contexts is not None and position < len(contexts): + assignments = contexts[position] + expanded = expand_environment_parameters(target, assignments) + if SHELL_PARAMETER_SYNTAX.search(expanded) or CMD_PARAMETER_SYNTAX.search(expanded): raise ValueError( f"cd target {target} cannot be resolved by the guard; " "run that command first and cd to the literal result" ) - if is_absolute_path(target): - current_directories = [target] + if ( + (SHELL_PARAMETER_SYNTAX.search(target) or CMD_PARAMETER_SYNTAX.search(target)) + and any(character.isspace() or character in "*?[]" for character in expanded) + ): + raise ValueError( + f"cd target {target} may split or glob; pass a literal path" + ) + if is_absolute_path(expanded): + current_directories = [expanded] else: bases = current_directories or ["."] - current_directories = [f"{base}/{target}" for base in bases] + current_directories = [f"{base}/{expanded}" for base in bases] def command_can_destroy_files(invocation): @@ -1217,9 +1232,11 @@ def command_can_destroy_files(invocation): def literal_path(operand): if len(operand) >= 2 and operand[0] in "\"'" and operand[-1] == operand[0]: operand = operand[1:-1] - pattern = r"[A-Za-z0-9._/-]+" + # A space is safe here: the operand is one shlex unit, so it cannot carry a + # second word; injection-relevant characters stay excluded. + pattern = r"[A-Za-z0-9._/ -]+" if os.name == "nt": - pattern = r"(?:[A-Za-z]:(?=[/\\]))?[A-Za-z0-9._/\\-]+" + pattern = r"(?:[A-Za-z]:(?=[/\\]))?[A-Za-z0-9._/\\ -]+" return operand if re.fullmatch(pattern, operand) is not None else None @@ -1235,13 +1252,24 @@ def literal_destructive_operand(operand): def segment_references_archive(segment, directories, assignments=None): invocation = command_invocation(segment) ancestors = command_can_destroy_files(invocation) - operands = [expand_environment_parameters(token, assignments) for token in segment] + # A here-document body survives tokenization only inside a wrapped payload + # (bash -c '...'), where it is data, not commands; keep its text from + # flipping the segment into archive context. + def searchable(token): + return strip_heredoc_bodies(token) if "\n" in token else token + + operands = [ + expand_environment_parameters(searchable(token), assignments) + for token in segment + ] if ancestors: resolved = command_invocation(operands) operands = [literal_destructive_operand(operand) for operand in resolved[1]] if any(path_in_archive(operand, directories, ancestors) for operand in operands): return True - return unresolved_archive_expansion(" ".join(segment), assignments) + return unresolved_archive_expansion( + " ".join(searchable(token) for token in segment), assignments + ) def copy_destinations(arguments, directories, assignments=None): @@ -1701,7 +1729,7 @@ def shell_assignment_contexts(tokens, initial=None): def shell_segment_contexts(tokens, working_directories, initial=None): """Yield each segment with its working directory and prior shell variables.""" contexts, _ = shell_assignment_contexts(tokens, initial) - segments = list(segment_directories(tokens, working_directories)) + segments = list(segment_directories(tokens, working_directories, contexts)) if len(segments) != len(contexts): raise ValueError("shell command segments cannot be aligned") for (segment, directories), assignments in zip(segments, contexts): @@ -2067,7 +2095,14 @@ def xargs_command(arguments): def require_read_command(wrapper, wrapped): - if wrapped and not archive_command_is_read_only(" ".join(wrapped), wrapped, True): + segments = list(command_segments(wrapped)) + if wrapped and ( + not segments + or not all( + archive_command_is_read_only(shlex.join(segment), segment, True) + for segment in segments + ) + ): raise ValueError( f"{wrapper} runs {wrapped[0]} on operands the guard cannot check; " f"only read commands may follow {wrapper}, or pass the paths as literals" @@ -2148,6 +2183,7 @@ def git_command(segment, assignments=None): if executable not in {"git", "git.exe"}: return None resolved_arguments = [] + parameter_arguments = [] argument_index = 0 while argument_index < len(arguments): argument = arguments[argument_index] @@ -2187,10 +2223,7 @@ def git_command(segment, assignments=None): resolved_arguments.append(f"-C{expanded}") else: if SHELL_PARAMETER_SYNTAX.search(argument) or CMD_PARAMETER_SYNTAX.search(argument): - raise ValueError( - f"git argument {argument} cannot be resolved by the guard; pass a " - "literal value (for commit messages use -F )" - ) + parameter_arguments.append(argument) resolved_arguments.append(expand_environment_parameters(argument, assignments)) argument_index += 1 arguments = resolved_arguments @@ -2225,7 +2258,16 @@ def git_command(segment, assignments=None): git_options.extend((option, str(value))) if index >= len(arguments): return None - return arguments[index].casefold(), arguments[index + 1:], git_options + subcommand = arguments[index].casefold() + read_only_subcommands = ARCHIVE_READ_GIT_COMMANDS | CLOSED_READ_GIT_COMMANDS + if parameter_arguments and subcommand not in read_only_subcommands: + # The scan exists for write-capable subcommands (commit messages, ref + # operands); read-only ones cannot mutate what the guard protects. + raise ValueError( + f"git argument {parameter_arguments[0]} cannot be resolved by the guard; " + "pass a literal value (for commit messages use -F )" + ) + return subcommand, arguments[index + 1:], git_options def wrapped_command_tokens(segment, expand_parameters=True): @@ -2566,6 +2608,34 @@ def archive_command_is_read_only( ) +def archive_write_attempt(segment): + """Whether a segment denied in archive context attempts a write there.""" + if any( + token and set(token) <= SHELL_WRITE_REDIRECTION_CHARS for token in segment + ): + return True + invocation = command_invocation(segment) + if invocation is None: + return False + executable, arguments = invocation + executable = executable.removesuffix(".exe") + if executable in SHELL_WRITE_COMMANDS: + return True + if executable in IN_PLACE_EDITORS and ( + has_short_option(arguments, "i") + or any(argument.startswith("--in-place") for argument in arguments) + ): + return True + git = git_command(segment) + if git is None: + return False + subcommand, git_arguments, _options = git + return subcommand not in ARCHIVE_READ_GIT_COMMANDS or any( + token in GIT_READ_WRITE_OPTIONS or token.startswith("--output=") + for token in git_arguments + ) + + def deny(reason): # One denial object per documented host schema: decision/reason (Grok, # Antigravity), permission + user/agentMessage (Cursor), permissionDecision @@ -2795,7 +2865,9 @@ def command_denial(command, working_directories, allow_destructive=True): archive_context and bundled_helper ): - return ARCHIVE_REASON + if archive_write_attempt(resolved_segment): + return ARCHIVE_REASON + return ARCHIVE_UNPROVEN_READ_REASON reason = destructive_git_reason(tokens) or protected_shell_write_reason( tokens, working_directories ) diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 7051f4ea6..4f1980442 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -1309,6 +1309,74 @@ def test_allows_read_tool_from_archive_working_directory(self): } ) + def test_allows_pipelines_of_read_commands_in_archive_context(self): + for command in ( + "cat NOTE.md | wc -l", + "cat NOTE.md | head -3 | tail -1", + "cat NOTE.md | grep -c x", + ): + with self.subTest(command=command): + self.assert_allowed( + { + "tool_name": "Bash", + "tool_input": { + "command": command, + "cwd": ".project/archive/001-mvp", + }, + } + ) + for command in ( + "cat NOTE.md | tee out.txt", + "python3 -", + "python3 - <<'EOF'\nprint(1)\nEOF", + ): + with self.subTest(command=command): + self.assert_denied( + { + "tool_name": "Bash", + "tool_input": { + "command": command, + "cwd": ".project/archive/001-mvp", + }, + } + ) + + def test_heredoc_body_does_not_flip_archive_context(self): + for command in ( + "python3 - <<'EOF'\n# mention of .project/archive/001-mvp\nprint(1)\nEOF", + "bash -c 'cat < .project/archive/001-mvp/NOTE.md'") + ) + + def test_ssh_from_archive_cwd_is_denied_as_unproven_read(self): + status, _, error = run_guard( + { + "tool_name": "Bash", + "tool_input": { + "command": "ssh host 'cat /etc/hosts'", + "cwd": ".project/archive/001-mvp", + }, + } + ) + self.assertEqual(status, 2, error) + self.assertIn(guard_hook.ARCHIVE_UNPROVEN_READ_REASON, error) + self.assertNotIn(guard_hook.ARCHIVE_REASON, error) + status, _, error = run_guard( + { + "tool_name": "Bash", + "tool_input": { + "command": "touch NOTE.md", + "cwd": ".project/archive/001-mvp", + }, + } + ) + self.assertEqual(status, 2, error) + self.assertIn(guard_hook.ARCHIVE_REASON, error) + def test_denies_ambiguous_archive_mutations(self): for command in ( "rm .project/$(printf archive)/001-mvp/NOTE.md", @@ -1509,7 +1577,6 @@ def test_denies_expansion_in_destructive_operands(self): 'rm -rf .{project,unused}', 'rm -rf ~/x', 'rm -rf ~root/x', - 'rm -rf "scratch folder"', 'rm -rf "scratch;other"', 'rm -rf ""', "Remove-Item -Recurse -Force '.proj*'", @@ -1532,6 +1599,7 @@ def test_denies_expansion_in_destructive_operands(self): "rm -rf './scratch/old-file.txt'", 'rm -rf scratch_123', 'rm -rf "scratch"', + 'rm -rf "scratch folder"', ) # Windows needs PATH (to find git) and SYSTEMROOT to start any process. kept = {name: os.environ[name] for name in ("PATH", "SYSTEMROOT") if os.name == "nt" and name in os.environ} @@ -1545,6 +1613,24 @@ def test_denies_expansion_in_destructive_operands(self): "cwd": str(repository), }) + def test_allows_quoted_spaces_in_destructive_paths(self): + with tempfile.TemporaryDirectory() as temporary: + repository = Path(temporary).resolve() + (repository / ".project" / "archive" / "001-mvp").mkdir(parents=True) + (repository / "scratch folder").mkdir() + for command, assertion in ( + ('rm -rf "scratch folder"', self.assert_allowed), + ('rm "$var/whatever"', self.assert_denied), + ('rm "scratch;other"', self.assert_denied), + ): + with self.subTest(command=command): + assertion( + { + "tool_name": "Bash", + "tool_input": {"command": command, "cwd": str(repository)}, + } + ) + def test_destructive_commands_require_a_single_simple_segment(self): with tempfile.TemporaryDirectory() as temporary: repository = Path(temporary).resolve() / "repo" @@ -1597,7 +1683,28 @@ def test_destructive_commands_require_a_single_simple_segment(self): }) def test_denies_protected_write_after_parameter_directory_change(self): - self.assert_denied(self.bash("TARGET=.project; cd $TARGET; printf broken > STATE.md")) + # The expanded cd target is tracked, so the later write is still caught + # as a protected control write rather than by refusing the cd itself. + with tempfile.TemporaryDirectory() as temporary: + root = Path(temporary) / "repo" + (root / ".project").mkdir(parents=True) + (root / ".project" / "STATE.md").write_bytes("owned\n".encode("utf-8")) + with ( + mock.patch.object(guard_hook.os, "getcwd", return_value=str(root)), + mock.patch.object(guard_hook, "repository_root", return_value=root), + ): + self.assert_denied( + self.bash("TARGET=.project; cd $TARGET; printf broken > STATE.md") + ) + + def test_allows_tracked_assignment_in_cd_target(self): + with tempfile.TemporaryDirectory() as temporary: + directory = shlex.quote(str(Path(temporary).resolve())) + self.assert_allowed( + self.bash(f'dir={directory} && cd "$dir" && git status') + ) + self.assert_denied(self.bash('dir="a b"; cd "$dir" && git status')) + self.assert_denied(self.bash('cd "$unassigned" && git status')) def test_denies_deleting_archive_ancestor_on_windows(self): previous = Path.cwd() @@ -2060,6 +2167,15 @@ def test_denies_word_split_git_subcommands_from_variables(self): with self.subTest(command=command): self.assert_denied(self.bash(command)) + def test_allows_parameters_in_read_only_git_arguments(self): + for command in ( + 'git log --format="$t %H"', + "git show HEAD:$f", + ): + with self.subTest(command=command): + self.assert_allowed(self.bash(command)) + self.assert_denied(self.bash('git commit -m "$msg"')) + def test_assignment_state_is_segment_local_and_last_value_wins(self): self.assert_denied( self.bash('P=.project/archive; echo x > "$P/NOTE.md"; P=/tmp') From 84b97f4ac2360768e3d0a929b015f6e23ea19ae8 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 11:13:26 -0500 Subject: [PATCH 04/37] fix: dashboard probe resolves the pinned runtime via the project launcher _runtime_status still executed the legacy per-project .gsd-path/runtime/pipeline_state.py that current installs no longer write and migration removes, so every declared-runtime project -- stock or hotfixed -- rendered Unverified amber. Invoke the project launcher first and keep the legacy exec as the fallback. Fixes one item of #353. --- daemon/gsd_daemon/probe.py | 22 ++++++--- tests/test_daemon_probe.py | 92 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 5 deletions(-) diff --git a/daemon/gsd_daemon/probe.py b/daemon/gsd_daemon/probe.py index 2c80a95eb..b30898c3d 100644 --- a/daemon/gsd_daemon/probe.py +++ b/daemon/gsd_daemon/probe.py @@ -623,15 +623,12 @@ def tree_mtime(path: Union[str, Path]) -> Optional[float]: return latest -def _runtime_status(root: str) -> Optional[dict]: - runtime = Path(root) / ".gsd-path" / "runtime" / "pipeline_state.py" - if not runtime.is_file(): - return None +def _run_status_command(command: List[str]) -> Optional[dict]: try: # The daemon's own interpreter: python3 is usually absent on Windows, # where process start and git are also slow enough to need the margin. result = subprocess_platform.run( - [sys.executable, "-B", str(runtime), "status", "--repo", root], + command, capture_output=True, encoding="utf-8", errors="replace", @@ -651,6 +648,21 @@ def _runtime_status(root: str) -> Optional[dict]: return payload +def _runtime_status(root: str) -> Optional[dict]: + # Current installs pin a shared runtime and proxy status through the + # project launcher, which resolves the declaration exactly like the + # status command does. Unmigrated projects keep the per-project runtime. + launcher = Path(root) / ".gsd-path" / "status_runtime.py" + if launcher.is_file(): + payload = _run_status_command([sys.executable, "-B", str(launcher), "--repo", root]) + if payload is not None: + return payload + runtime = Path(root) / ".gsd-path" / "runtime" / "pipeline_state.py" + if not runtime.is_file(): + return None + return _run_status_command([sys.executable, "-B", str(runtime), "status", "--repo", root]) + + def _activity_time(status: ProjectStatus) -> Optional[datetime]: parsed = _parse_iso(status.last_activity_iso) if parsed is not None: diff --git a/tests/test_daemon_probe.py b/tests/test_daemon_probe.py index bdb5f6afe..fd2cac56f 100644 --- a/tests/test_daemon_probe.py +++ b/tests/test_daemon_probe.py @@ -1,5 +1,7 @@ +import hashlib import json import os +import shutil import sys import tempfile import unittest @@ -10,6 +12,8 @@ sys.path.insert(0, str(Path(__file__).resolve().parents[1] / "daemon")) +SOURCE = Path(__file__).resolve().parents[1] + from gsd_daemon import probe from gsd_daemon.model import ProjectStatus @@ -101,6 +105,18 @@ })) """ +LAUNCHER_RUNTIME_STUB = """import json +print(json.dumps({ + "schema": "gsd-path/status/v1", + "state": {"project": "demo", "milestone": "demo-ms", "phase": "build", + "status": "active", "branch": "gsd-path/M001"}, + "route": {}, + "git": {"branch": "gsd-path/M001", "head": "def456", "dirty": False}, + "pending_answers": [{"id": "Q001"}], + "next_skill": "gsd-path-ship", +})) +""" + def make_task(project_dir: Path, task_id: str, slug: str, wave: int, status: str, title: str = None, files=("src/a.py",)) -> Path: @@ -147,6 +163,31 @@ def make_project(root: Path, state: str = STATE_FULL, with_next: bool = True) -> return root +def make_declared_runtime(root: Path, home: Path) -> None: + """Pinned-runtime install shape: shared store + declaration + real launcher. + + The store's pipeline_state is a payload stub; everything between it and + the daemon (runtime.json resolution, manifest validation, the subprocess + hop) is the real installed launcher from scripts/status_runtime.py. + """ + stub = LAUNCHER_RUNTIME_STUB.encode("utf-8") + manifest = {"version": "test", + "files": {"pipeline_state.py": hashlib.sha256(stub).hexdigest()}} + digest = hashlib.sha256( + json.dumps(manifest, sort_keys=True, separators=(",", ":")).encode("utf-8")).hexdigest() + runtime = home / ".gsd-path" / "runtimes" / digest + runtime.mkdir(parents=True) + (runtime / "pipeline_state.py").write_bytes(stub) + (runtime / "manifest.json").write_bytes( + (json.dumps(manifest, indent=2, sort_keys=True) + "\n").encode("utf-8")) + gsd = root / ".gsd-path" + gsd.mkdir(parents=True, exist_ok=True) + (gsd / "runtime.json").write_bytes( + (json.dumps({"schema": "gsd-path/runtime/v1", "version": "test", "digest": digest}, + sort_keys=True) + "\n").encode("utf-8")) + shutil.copyfile(SOURCE / "scripts" / "status_runtime.py", gsd / "status_runtime.py") + + class ParseStateTests(unittest.TestCase): def setUp(self) -> None: self.tmp = tempfile.TemporaryDirectory() @@ -458,6 +499,57 @@ def test_round_trip(self) -> None: self.assertEqual(restored.to_dict(), status.to_dict()) +class DeclaredRuntimeTests(unittest.TestCase): + """The project launcher is the probe's first hop for pinned-runtime projects.""" + + def setUp(self) -> None: + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + base = Path(self.tmp.name).resolve() + self.root = base / "demo" + self.home = base / "home" + self.home.mkdir() + make_project(self.root) + environment = mock.patch.dict(os.environ, {"HOME": str(self.home), + "USERPROFILE": str(self.home)}) + environment.start() + self.addCleanup(environment.stop) + + def test_launcher_payload_enriches_projection(self) -> None: + make_declared_runtime(self.root, self.home) + status = probe.probe_project(self.root) + self.assertEqual(status.status_source, "runtime") + self.assertEqual(status.workflow, {"state": "active", "label": "In build"}) + self.assertEqual(status.project, "demo") + self.assertEqual(status.branch, "gsd-path/M001") + self.assertEqual(status.git["head"], "def456") + self.assertEqual(status.next_skill, "gsd-path-ship") + + def test_unavailable_runtime_store_fails_soft(self) -> None: + make_declared_runtime(self.root, self.home) + shutil.rmtree(self.home / ".gsd-path" / "runtimes") + status = probe.probe_project(self.root) + self.assertEqual(status.status_source, "parse-only") + self.assertEqual(status.workflow["state"], "unverified") + self.assertEqual(status.health, "amber") + + def test_failing_launcher_falls_back_to_legacy_runtime(self) -> None: + gsd = self.root / ".gsd-path" + (gsd / "runtime").mkdir(parents=True) + (gsd / "runtime" / "pipeline_state.py").write_bytes(RUNTIME_STUB.encode("utf-8")) + (gsd / "status_runtime.py").write_bytes("raise SystemExit(2)\n".encode("utf-8")) + status = probe.probe_project(self.root) + self.assertEqual(status.status_source, "runtime") + + def test_failing_launcher_without_legacy_fails_soft(self) -> None: + gsd = self.root / ".gsd-path" + gsd.mkdir(parents=True) + (gsd / "status_runtime.py").write_bytes("raise SystemExit(2)\n".encode("utf-8")) + status = probe.probe_project(self.root) + self.assertEqual(status.status_source, "parse-only") + self.assertEqual(status.workflow["state"], "unverified") + + WAVE_REVIEW = """# Review — wave 1, cycle 1 ## Surface contract diff --git a/skills/gsd-path-research/scripts/isolation.py b/skills/gsd-path-research/scripts/isolation.py index fd524fa3c..3805f809a 100644 --- a/skills/gsd-path-research/scripts/isolation.py +++ b/skills/gsd-path-research/scripts/isolation.py @@ -484,6 +484,14 @@ def _same_directory(path: Path, expected: os.stat_result) -> bool: return stat.S_ISDIR(current.st_mode) and os.path.samestat(current, expected) +def _long_path(absolute: str) -> str: + if absolute.startswith("\\\\?\\"): + return absolute + if absolute.startswith("\\\\"): + return "\\\\?\\UNC\\" + absolute[2:] + return "\\\\?\\" + absolute + + def _force_rmtree(path: Path) -> None: """Delete a sidecar tree directly, tolerating read-only files and long paths.""" def _writable(function, target, _error): @@ -491,7 +499,7 @@ def _writable(function, target, _error): function(target) # The \\?\ prefix sidesteps Windows MAX_PATH on deep sidecar trees. - target = Path(f"\\\\?\\{os.path.abspath(path)}") if os.name == "nt" else path + target = Path(_long_path(os.path.abspath(path))) if os.name == "nt" else path try: shutil.rmtree(target, onerror=_writable) except OSError as error: diff --git a/skills/gsd-path-roadmap/scripts/isolation.py b/skills/gsd-path-roadmap/scripts/isolation.py index fd524fa3c..3805f809a 100644 --- a/skills/gsd-path-roadmap/scripts/isolation.py +++ b/skills/gsd-path-roadmap/scripts/isolation.py @@ -484,6 +484,14 @@ def _same_directory(path: Path, expected: os.stat_result) -> bool: return stat.S_ISDIR(current.st_mode) and os.path.samestat(current, expected) +def _long_path(absolute: str) -> str: + if absolute.startswith("\\\\?\\"): + return absolute + if absolute.startswith("\\\\"): + return "\\\\?\\UNC\\" + absolute[2:] + return "\\\\?\\" + absolute + + def _force_rmtree(path: Path) -> None: """Delete a sidecar tree directly, tolerating read-only files and long paths.""" def _writable(function, target, _error): @@ -491,7 +499,7 @@ def _writable(function, target, _error): function(target) # The \\?\ prefix sidesteps Windows MAX_PATH on deep sidecar trees. - target = Path(f"\\\\?\\{os.path.abspath(path)}") if os.name == "nt" else path + target = Path(_long_path(os.path.abspath(path))) if os.name == "nt" else path try: shutil.rmtree(target, onerror=_writable) except OSError as error: diff --git a/skills/gsd-path-ship/scripts/isolation.py b/skills/gsd-path-ship/scripts/isolation.py index fd524fa3c..3805f809a 100644 --- a/skills/gsd-path-ship/scripts/isolation.py +++ b/skills/gsd-path-ship/scripts/isolation.py @@ -484,6 +484,14 @@ def _same_directory(path: Path, expected: os.stat_result) -> bool: return stat.S_ISDIR(current.st_mode) and os.path.samestat(current, expected) +def _long_path(absolute: str) -> str: + if absolute.startswith("\\\\?\\"): + return absolute + if absolute.startswith("\\\\"): + return "\\\\?\\UNC\\" + absolute[2:] + return "\\\\?\\" + absolute + + def _force_rmtree(path: Path) -> None: """Delete a sidecar tree directly, tolerating read-only files and long paths.""" def _writable(function, target, _error): @@ -491,7 +499,7 @@ def _writable(function, target, _error): function(target) # The \\?\ prefix sidesteps Windows MAX_PATH on deep sidecar trees. - target = Path(f"\\\\?\\{os.path.abspath(path)}") if os.name == "nt" else path + target = Path(_long_path(os.path.abspath(path))) if os.name == "nt" else path try: shutil.rmtree(target, onerror=_writable) except OSError as error: diff --git a/skills/gsd-path/PLAN.md b/skills/gsd-path/PLAN.md index 654eacd1a..a87ea5149 100644 --- a/skills/gsd-path/PLAN.md +++ b/skills/gsd-path/PLAN.md @@ -115,9 +115,10 @@ as stated in Lookahead mode. In patch mode, tasks already `done` keep their landed metadata; only the appended patch tasks must start clean. - Require every dependency id to exist, forbid later-wave dependencies, - detect cycles, and forbid file overlap between planned tasks in the same - wave. Same-wave dependency chains are allowed only when their files do - not overlap; the build executes them in dependency layers. + detect cycles, and forbid unordered file overlap between planned tasks + in the same wave. Same-wave tasks may share files only when one + transitively depends on the other through `deps`; the build executes + them in dependency layers. - Reject a declared dependency that carries neither data nor a prerequisite effect. A `deps` edge is valid only when the dependent consumes a symbol, signature, schema, endpoint, file format, or path named in that diff --git a/skills/gsd-path/references/planner.md b/skills/gsd-path/references/planner.md index 5f4421976..138eb1dfd 100644 --- a/skills/gsd-path/references/planner.md +++ b/skills/gsd-path/references/planner.md @@ -38,8 +38,9 @@ the how. security-critical; `verify-only` suits polish and low-risk feature waves whose Verify commands meaningfully cover the criteria. -Every dependency must be in an earlier wave or in the same wave with no file -overlap. Produce an acyclic graph; same-wave chains execute in layers. +Every dependency must be in an earlier wave or in the same wave. Same-wave +tasks may share files only when one transitively depends on the other. Produce +an acyclic graph; same-wave chains execute in layers. Declare a dependency only when something real crosses it: data — the dependent consumes a symbol, signature, schema, endpoint, file format, or path named in diff --git a/skills/gsd-path/scripts/dispatch_driver.py b/skills/gsd-path/scripts/dispatch_driver.py index 99eb84900..c18b1e008 100644 --- a/skills/gsd-path/scripts/dispatch_driver.py +++ b/skills/gsd-path/scripts/dispatch_driver.py @@ -2036,7 +2036,7 @@ def fix_tasks(primary: Path, options: argparse.Namespace) -> Dict[str, object]: texts = contracts._task_texts(primary, options.project_dir) groups = {group["locator"]: group for group in findings["groups"]} today = now()[:10] - # Same-wave file overlap is forbidden, so repairs land in a newly appended wave. + # Unordered same-wave file overlap is rejected, so repairs land in a newly appended wave. plan_path = project / "plan/PLAN.md" plan_text = plan_path.read_text(encoding="utf-8") depths, waves = contracts._plan_waves(plan_text) diff --git a/skills/gsd-path/scripts/isolation.py b/skills/gsd-path/scripts/isolation.py index fd524fa3c..3805f809a 100644 --- a/skills/gsd-path/scripts/isolation.py +++ b/skills/gsd-path/scripts/isolation.py @@ -484,6 +484,14 @@ def _same_directory(path: Path, expected: os.stat_result) -> bool: return stat.S_ISDIR(current.st_mode) and os.path.samestat(current, expected) +def _long_path(absolute: str) -> str: + if absolute.startswith("\\\\?\\"): + return absolute + if absolute.startswith("\\\\"): + return "\\\\?\\UNC\\" + absolute[2:] + return "\\\\?\\" + absolute + + def _force_rmtree(path: Path) -> None: """Delete a sidecar tree directly, tolerating read-only files and long paths.""" def _writable(function, target, _error): @@ -491,7 +499,7 @@ def _writable(function, target, _error): function(target) # The \\?\ prefix sidesteps Windows MAX_PATH on deep sidecar trees. - target = Path(f"\\\\?\\{os.path.abspath(path)}") if os.name == "nt" else path + target = Path(_long_path(os.path.abspath(path))) if os.name == "nt" else path try: shutil.rmtree(target, onerror=_writable) except OSError as error: diff --git a/skills/gsd-path/templates/plan.md b/skills/gsd-path/templates/plan.md index a2c8ea8ec..2b49a23f4 100644 --- a/skills/gsd-path/templates/plan.md +++ b/skills/gsd-path/templates/plan.md @@ -47,8 +47,8 @@ Goal: Review depth: ## Surface contract diff --git a/skills/path/PLAN.md b/skills/path/PLAN.md index 654eacd1a..a87ea5149 100644 --- a/skills/path/PLAN.md +++ b/skills/path/PLAN.md @@ -115,9 +115,10 @@ as stated in Lookahead mode. In patch mode, tasks already `done` keep their landed metadata; only the appended patch tasks must start clean. - Require every dependency id to exist, forbid later-wave dependencies, - detect cycles, and forbid file overlap between planned tasks in the same - wave. Same-wave dependency chains are allowed only when their files do - not overlap; the build executes them in dependency layers. + detect cycles, and forbid unordered file overlap between planned tasks + in the same wave. Same-wave tasks may share files only when one + transitively depends on the other through `deps`; the build executes + them in dependency layers. - Reject a declared dependency that carries neither data nor a prerequisite effect. A `deps` edge is valid only when the dependent consumes a symbol, signature, schema, endpoint, file format, or path named in that diff --git a/skills/path/references/planner.md b/skills/path/references/planner.md index 5f4421976..138eb1dfd 100644 --- a/skills/path/references/planner.md +++ b/skills/path/references/planner.md @@ -38,8 +38,9 @@ the how. security-critical; `verify-only` suits polish and low-risk feature waves whose Verify commands meaningfully cover the criteria. -Every dependency must be in an earlier wave or in the same wave with no file -overlap. Produce an acyclic graph; same-wave chains execute in layers. +Every dependency must be in an earlier wave or in the same wave. Same-wave +tasks may share files only when one transitively depends on the other. Produce +an acyclic graph; same-wave chains execute in layers. Declare a dependency only when something real crosses it: data — the dependent consumes a symbol, signature, schema, endpoint, file format, or path named in diff --git a/skills/path/scripts/dispatch_driver.py b/skills/path/scripts/dispatch_driver.py index 99eb84900..c18b1e008 100644 --- a/skills/path/scripts/dispatch_driver.py +++ b/skills/path/scripts/dispatch_driver.py @@ -2036,7 +2036,7 @@ def fix_tasks(primary: Path, options: argparse.Namespace) -> Dict[str, object]: texts = contracts._task_texts(primary, options.project_dir) groups = {group["locator"]: group for group in findings["groups"]} today = now()[:10] - # Same-wave file overlap is forbidden, so repairs land in a newly appended wave. + # Unordered same-wave file overlap is rejected, so repairs land in a newly appended wave. plan_path = project / "plan/PLAN.md" plan_text = plan_path.read_text(encoding="utf-8") depths, waves = contracts._plan_waves(plan_text) diff --git a/skills/path/scripts/isolation.py b/skills/path/scripts/isolation.py index fd524fa3c..3805f809a 100644 --- a/skills/path/scripts/isolation.py +++ b/skills/path/scripts/isolation.py @@ -484,6 +484,14 @@ def _same_directory(path: Path, expected: os.stat_result) -> bool: return stat.S_ISDIR(current.st_mode) and os.path.samestat(current, expected) +def _long_path(absolute: str) -> str: + if absolute.startswith("\\\\?\\"): + return absolute + if absolute.startswith("\\\\"): + return "\\\\?\\UNC\\" + absolute[2:] + return "\\\\?\\" + absolute + + def _force_rmtree(path: Path) -> None: """Delete a sidecar tree directly, tolerating read-only files and long paths.""" def _writable(function, target, _error): @@ -491,7 +499,7 @@ def _writable(function, target, _error): function(target) # The \\?\ prefix sidesteps Windows MAX_PATH on deep sidecar trees. - target = Path(f"\\\\?\\{os.path.abspath(path)}") if os.name == "nt" else path + target = Path(_long_path(os.path.abspath(path))) if os.name == "nt" else path try: shutil.rmtree(target, onerror=_writable) except OSError as error: diff --git a/skills/path/templates/plan.md b/skills/path/templates/plan.md index a2c8ea8ec..2b49a23f4 100644 --- a/skills/path/templates/plan.md +++ b/skills/path/templates/plan.md @@ -47,8 +47,8 @@ Goal: Review depth: ## Surface contract diff --git a/tests/test_daemon_probe.py b/tests/test_daemon_probe.py index fd2cac56f..c6a1e4ff5 100644 --- a/tests/test_daemon_probe.py +++ b/tests/test_daemon_probe.py @@ -533,13 +533,14 @@ def test_unavailable_runtime_store_fails_soft(self) -> None: self.assertEqual(status.workflow["state"], "unverified") self.assertEqual(status.health, "amber") - def test_failing_launcher_falls_back_to_legacy_runtime(self) -> None: + def test_failing_launcher_does_not_run_legacy_runtime(self) -> None: gsd = self.root / ".gsd-path" (gsd / "runtime").mkdir(parents=True) (gsd / "runtime" / "pipeline_state.py").write_bytes(RUNTIME_STUB.encode("utf-8")) (gsd / "status_runtime.py").write_bytes("raise SystemExit(2)\n".encode("utf-8")) status = probe.probe_project(self.root) - self.assertEqual(status.status_source, "runtime") + self.assertEqual(status.status_source, "parse-only") + self.assertEqual(status.workflow["state"], "unverified") def test_failing_launcher_without_legacy_fails_soft(self) -> None: gsd = self.root / ".gsd-path" diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 4f1980442..b01b75679 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -1706,6 +1706,15 @@ def test_allows_tracked_assignment_in_cd_target(self): self.assert_denied(self.bash('dir="a b"; cd "$dir" && git status')) self.assert_denied(self.bash('cd "$unassigned" && git status')) + def test_denies_cd_to_shell_maintained_directory_variables(self): + for command in ( + 'cd .project/archive/001-mvp && cd "$PWD" && touch NOTE.md', + "cd .project/archive/001-mvp && cd / && cd $OLDPWD && touch NOTE.md", + 'PWD=/tmp; cd .project/archive/001-mvp && cd "$PWD" && touch NOTE.md', + ): + with self.subTest(command=command): + self.assert_denied(self.bash(command)) + def test_denies_deleting_archive_ancestor_on_windows(self): previous = Path.cwd() with tempfile.TemporaryDirectory() as temporary: @@ -2175,6 +2184,14 @@ def test_allows_parameters_in_read_only_git_arguments(self): with self.subTest(command=command): self.assert_allowed(self.bash(command)) self.assert_denied(self.bash('git commit -m "$msg"')) + for command in ( + "git diff --output=$f", + "git diff --output $f", + "git diff $o", + "git log -$o", + ): + with self.subTest(command=command): + self.assert_denied(self.bash(command)) def test_assignment_state_is_segment_local_and_last_value_wins(self): self.assert_denied( diff --git a/tests/test_isolation.py b/tests/test_isolation.py index 00947f9e0..b10f0c153 100644 --- a/tests/test_isolation.py +++ b/tests/test_isolation.py @@ -44,6 +44,13 @@ def git(root: Path, *arguments: str) -> str: class IsolationTests(unittest.TestCase): + def test_long_path_prefix_handles_drive_unc_and_prefixed_roots(self): + self.assertEqual(isolation._long_path("C:\\work\\tree"), "\\\\?\\C:\\work\\tree") + self.assertEqual( + isolation._long_path("\\\\server\\share\\tree"), "\\\\?\\UNC\\server\\share\\tree" + ) + self.assertEqual(isolation._long_path("\\\\?\\C:\\work\\tree"), "\\\\?\\C:\\work\\tree") + def write(self, root: Path, relative: str, content: str) -> None: path = root / relative path.parent.mkdir(parents=True, exist_ok=True) diff --git a/tests/test_runtime_lifecycle.py b/tests/test_runtime_lifecycle.py index f93c4930d..8417d45f5 100644 --- a/tests/test_runtime_lifecycle.py +++ b/tests/test_runtime_lifecycle.py @@ -735,6 +735,17 @@ def test_upgrade_records_provenance_without_changing_digest(self): self.assertEqual(result.returncode, 0, result.stderr) self.assertEqual(self.pin()["digest"], stock["digest"]) self.assertEqual(self.pin()["provenance"], {"source": "npm @opengsd/gsd-path@1.4.1"}) + # A plain source nested in an unrelated repository never records that repository's HEAD. + enclosing = self.root / "enclosing" + nested = enclosing / "node_modules" / "gsd-path" + shutil.copytree(plain, nested) + self.git("init", "-q", repo=enclosing) + self.git("-c", "user.name=t", "-c", "user.email=t@example.com", "commit", "-q", + "--allow-empty", "-m", "unrelated", repo=enclosing) + result = self.runtime_command("--runtime-upgrade", nested, extra=[ + "--runtime-provenance-source", "npm @opengsd/gsd-path@1.4.1"]) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(self.pin()["provenance"], {"source": "npm @opengsd/gsd-path@1.4.1"}) def test_reupgrade_without_provenance_flags_preserves_provenance(self): self.provision() From bcd10089754d54fddd29d7f2e91cc1e3ec10fa2c Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 13:29:40 -0500 Subject: [PATCH 10/37] no-mistakes(review): deny bare and substituted parameters in read-only git arguments --- HOOKS.md | 7 +++++++ scripts/guard_hook.py | 16 +++++++++++----- tests/test_guard_hook.py | 14 +++++++++++++- 3 files changed, 31 insertions(+), 6 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index a63d8ff3d..898201071 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -168,6 +168,13 @@ See [UPDATE.md](UPDATE.md). tilde paths are denied even when quoted. This also applies to `find -delete` and supported destructive aliases - destructive Git commands nested in supported shell and command wrappers +- Git arguments that contain a shell parameter. The one exception is a + read-only subcommand (`status`, `log`, `diff`, `show`, and the other + inspection commands) with the parameter inside a quote-grouped argument + that contains whitespace, such as `git log --format="$t %H"`. A bare + parameter operand (`$x`, `HEAD:$f`, `--format=$x`) is denied because its + expansion can split into a write option such as `--output`; command + substitution output and `--output` values are denied in every form - archive glob/brace expansions and execution-capable read options such as `rg --pre` - direct write, edit, and patch tool calls targeting `.project/STATE.md`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index fe027b32a..a224a7408 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2192,7 +2192,7 @@ def git_command(segment, assignments=None): return None resolved_arguments = [] parameter_arguments = [] - write_capable_parameters = [] + unproven_parameters = [] argument_index = 0 while argument_index < len(arguments): argument = arguments[argument_index] @@ -2235,13 +2235,17 @@ def git_command(segment, assignments=None): if parameter: parameter_arguments.append(argument) literal = re.split(r"[$%!]", argument, maxsplit=1)[0] + expanded = expand_environment_parameters(argument, assignments) if ( not literal or (literal.startswith("-") and "=" not in literal) or literal.split("=", 1)[0] in GIT_READ_WRITE_OPTIONS or arguments[argument_index - 1] in GIT_READ_WRITE_OPTIONS + or not any(character.isspace() for character in argument) + or SUBSTITUTION_PLACEHOLDER in argument + or SUBSTITUTION_PLACEHOLDER in expanded ): - write_capable_parameters.append(argument) + unproven_parameters.append(argument) resolved_arguments.append(expand_environment_parameters(argument, assignments)) argument_index += 1 arguments = resolved_arguments @@ -2279,9 +2283,11 @@ def git_command(segment, assignments=None): subcommand = arguments[index].casefold() read_only_subcommands = ARCHIVE_READ_GIT_COMMANDS | CLOSED_READ_GIT_COMMANDS if subcommand in read_only_subcommands: - # Read-only subcommands still write through --output, so a parameter - # that can form an option or its value keeps the refusal. - parameter_arguments = write_capable_parameters + # Read-only subcommands still write through --output. A parameter + # passes only inside a whitespace-bearing token, which the shell must + # have quote-grouped into one word; a bare one can word-split into an + # option, and command substitution output is never known. + parameter_arguments = unproven_parameters if parameter_arguments: raise ValueError( f"git argument {parameter_arguments[0]} cannot be resolved by the guard; " diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index b01b75679..4109409fd 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2179,16 +2179,28 @@ def test_denies_word_split_git_subcommands_from_variables(self): def test_allows_parameters_in_read_only_git_arguments(self): for command in ( 'git log --format="$t %H"', - "git show HEAD:$f", + 't=abc; git log --format="$t %H"', ): with self.subTest(command=command): self.assert_allowed(self.bash(command)) self.assert_denied(self.bash('git commit -m "$msg"')) + # Deliberate fail-closed choice: an unquoted bare parameter operand + # stays denied, because its expansion word-splits and can add a write + # option such as --output. for command in ( "git diff --output=$f", "git diff --output $f", + 'git diff "--output=$f x"', "git diff $o", "git log -$o", + "git show HEAD:$f", + "git diff --format=$x", + "x='a --output=NOTE.md'; git diff --format=$x", + "cd .project/archive/001-mvp && x='a --output=NOTE.md'; git diff --format=$x", + "x='a --output=.project/archive/001-mvp/NOTE.md'; git diff --format=$x", + 'x=$(cat /tmp/v); git diff --format=$x', + 'x=$(cat /tmp/v); git log --format="$x %H"', + 'git log --format="$(cat /tmp/v) %H"', ): with self.subTest(command=command): self.assert_denied(self.bash(command)) From dea895285122011732461176c59e75cff7f6fc5c Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 13:35:22 -0500 Subject: [PATCH 11/37] no-mistakes(review): allow git parameters only inside double-quoted spans --- HOOKS.md | 16 ++++---- scripts/guard_hook.py | 79 +++++++++++++++++++++++++++++++++++++--- tests/test_guard_hook.py | 22 ++++++++++- 3 files changed, 104 insertions(+), 13 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 898201071..8505977c3 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -168,13 +168,15 @@ See [UPDATE.md](UPDATE.md). tilde paths are denied even when quoted. This also applies to `find -delete` and supported destructive aliases - destructive Git commands nested in supported shell and command wrappers -- Git arguments that contain a shell parameter. The one exception is a - read-only subcommand (`status`, `log`, `diff`, `show`, and the other - inspection commands) with the parameter inside a quote-grouped argument - that contains whitespace, such as `git log --format="$t %H"`. A bare - parameter operand (`$x`, `HEAD:$f`, `--format=$x`) is denied because its - expansion can split into a write option such as `--output`; command - substitution output and `--output` values are denied in every form +- Git arguments that contain a shell parameter. Parameters are accepted only + inside double-quoted spans of read-only subcommands (`status`, `log`, + `diff`, `show`, and the other inspection commands), such as + `git log --format="$t %H"` or `git show "HEAD:$f"`, because only double + quoting prevents word-splitting into options. A parameter outside double + quotes (`$x`, `HEAD:$f`, `--format=$x`, `"a b"$x`) is denied because its + expansion can split into a write option such as `--output`; `"$@"`, + command substitution output, a parameter that starts an argument, and + `--output` values are denied in every form - archive glob/brace expansions and execution-capable read options such as `rg --pre` - direct write, edit, and patch tool calls targeting `.project/STATE.md`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index a224a7408..bac27a47a 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2028,6 +2028,64 @@ def decode_patch_path(raw_path, strip_prefix): return path +PARAMETER_MARKERS = frozenset("$%!") + + +class ShellToken(str): + """A token that keeps, per parameter marker character, whether double quotes enclosed it.""" + + double_quoted = () + + +def double_quoted_markers(command): + """Return, for each parameter marker in order, whether double quotes enclose it.""" + flags = [] + quote = None + comment = False + previous = " " + index = 0 + while index < len(command): + character = command[index] + if quote == "'": + if character == "'": + quote = None + elif character == "\\": + index += 1 + if index < len(command) and command[index] in PARAMETER_MARKERS: + flags.append(quote == '"' and not comment) + previous = character + index += 1 + continue + elif character == '"': + quote = None if quote else '"' + elif quote is None and character == "'": + quote = "'" + elif quote is None and character == "#" and previous in " \t\r\n;&|()": + # The lexer reads past a shell comment, so quotes after it prove nothing. + comment = True + if character in PARAMETER_MARKERS: + flags.append(quote == '"' and not comment) + previous = character + index += 1 + return flags + + +def parameters_double_quoted(token): + """Whether every parameter in the token expands inside double quotes to one word.""" + flags = getattr(token, "double_quoted", ()) + positions = [ + index for index, character in enumerate(token) if character in PARAMETER_MARKERS + ] + if len(flags) != len(positions): + return False + quoted = dict(zip(positions, flags)) + return all( + quoted[match.start()] and quoted.get(match.end() - 1, True) and "@" not in match.group(0) + for syntax in (SHELL_PARAMETER_SYNTAX, CMD_PARAMETER_SYNTAX) + for match in syntax.finditer(token) + ) + + def shell_tokens(command): command = strip_heredoc_bodies(LINE_CONTINUATION.sub(r"\1 ", command)) lexer = shlex.shlex(command, posix=True, punctuation_chars="|;&()<>\n\r") @@ -2043,7 +2101,18 @@ def shell_tokens(command): ) from None if not tokens: raise ValueError("shell command is empty") - return tokens + flags = double_quoted_markers(command) + if len(flags) != sum( + character in PARAMETER_MARKERS for token in tokens for character in token + ): + return tokens + marked = [] + for token in tokens: + count = sum(character in PARAMETER_MARKERS for character in token) + token = ShellToken(token) + token.double_quoted, flags = tuple(flags[:count]), flags[count:] + marked.append(token) + return marked def command_segments(tokens): @@ -2241,7 +2310,7 @@ def git_command(segment, assignments=None): or (literal.startswith("-") and "=" not in literal) or literal.split("=", 1)[0] in GIT_READ_WRITE_OPTIONS or arguments[argument_index - 1] in GIT_READ_WRITE_OPTIONS - or not any(character.isspace() for character in argument) + or not parameters_double_quoted(argument) or SUBSTITUTION_PLACEHOLDER in argument or SUBSTITUTION_PLACEHOLDER in expanded ): @@ -2284,9 +2353,9 @@ def git_command(segment, assignments=None): read_only_subcommands = ARCHIVE_READ_GIT_COMMANDS | CLOSED_READ_GIT_COMMANDS if subcommand in read_only_subcommands: # Read-only subcommands still write through --output. A parameter - # passes only inside a whitespace-bearing token, which the shell must - # have quote-grouped into one word; a bare one can word-split into an - # option, and command substitution output is never known. + # passes only inside double quotes, where its expansion stays one + # word; an unquoted one can word-split into an option, and command + # substitution output is never known. parameter_arguments = unproven_parameters if parameter_arguments: raise ValueError( diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 4109409fd..365a44f9b 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2180,13 +2180,33 @@ def test_allows_parameters_in_read_only_git_arguments(self): for command in ( 'git log --format="$t %H"', 't=abc; git log --format="$t %H"', + 'git show "HEAD:$f"', + 'git log --format="%H%n%s"', ): with self.subTest(command=command): self.assert_allowed(self.bash(command)) self.assert_denied(self.bash('git commit -m "$msg"')) - # Deliberate fail-closed choice: an unquoted bare parameter operand + # Deliberate fail-closed choice: a parameter outside double quotes # stays denied, because its expansion word-splits and can add a write # option such as --output. + unquoted = ( + "git show HEAD:$f", + "git diff --format=$x", + "x='a --output=NOTE.md'; git diff --format=$x", + "x='c --output=.project/archive/001-mvp/NOTE.md'; git diff --format=\"a b\"$x", + 'git diff --format="a b"$x', + 'git diff "a b"$x', + 'git log --format="a b"$1', + "git log --format='$t %H'", + 'git log --format=$t\\ %H', + 'git log "--format=$@ %H"', + 'git log # "\n git diff --format=$x "', + "git diff --output=$f", + ) + for command in unquoted: + for prefix in ("", "cd .project/archive/001-mvp && "): + with self.subTest(command=prefix + command): + self.assert_denied(self.bash(prefix + command)) for command in ( "git diff --output=$f", "git diff --output $f", From 711beb60d46a98b8cda584179b4b26aa85f65a2e Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 13:54:50 -0500 Subject: [PATCH 12/37] no-mistakes(review): fail closed on unrecognized quoting in git parameter exception --- HOOKS.md | 9 ++++-- scripts/guard_hook.py | 70 +++++++++++++++++++++++++++------------- tests/test_guard_hook.py | 14 ++++++++ 3 files changed, 68 insertions(+), 25 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 8505977c3..c2f4962bb 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -168,15 +168,18 @@ See [UPDATE.md](UPDATE.md). tilde paths are denied even when quoted. This also applies to `find -delete` and supported destructive aliases - destructive Git commands nested in supported shell and command wrappers -- Git arguments that contain a shell parameter. Parameters are accepted only - inside double-quoted spans of read-only subcommands (`status`, `log`, +- Git arguments that contain a shell parameter. Parameters pass only inside + plain double-quoted spans of read-only subcommands (`status`, `log`, `diff`, `show`, and the other inspection commands), such as `git log --format="$t %H"` or `git show "HEAD:$f"`, because only double quoting prevents word-splitting into options. A parameter outside double quotes (`$x`, `HEAD:$f`, `--format=$x`, `"a b"$x`) is denied because its expansion can split into a write option such as `--output`; `"$@"`, command substitution output, a parameter that starts an argument, and - `--output` values are denied in every form + `--output` values are denied in every form. Any other quoting construct in + the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or + backslashes in a comment, here-documents and here-strings) disables the + exception, so every parameter in a git argument of that command is denied - archive glob/brace expansions and execution-capable read options such as `rg --pre` - direct write, edit, and patch tool calls targeting `.project/STATE.md`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index bac27a47a..3bf593bbf 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2038,36 +2038,59 @@ class ShellToken(str): def double_quoted_markers(command): - """Return, for each parameter marker in order, whether double quotes enclose it.""" + """Return, per parameter marker in order, whether plain double quotes enclose it. + + None means the command uses quoting outside the plain constructs this scan + models (plain single quotes, plain double quotes, unquoted text), so no + marker in it can be proven quoted. + """ flags = [] quote = None comment = False + braces = 0 previous = " " index = 0 while index < len(command): character = command[index] - if quote == "'": + following = command[index + 1 : index + 2] + if comment: + if character in "\r\n": + comment = False + elif character in "'\"\\`": + return None + elif quote == "'": if character == "'": quote = None elif character == "\\": - index += 1 - if index < len(command) and command[index] in PARAMETER_MARKERS: - flags.append(quote == '"' and not comment) + if following and following in PARAMETER_MARKERS: + flags.append(quote == '"') previous = character - index += 1 + index += 2 continue + elif character == "`" or (character == "$" and following == "("): + return None + elif character == "$" and following == "{": + braces += 1 + elif character == "}" and braces: + braces -= 1 + elif character in "'\"" and braces: + return None elif character == '"': quote = None if quote else '"' - elif quote is None and character == "'": - quote = "'" - elif quote is None and character == "#" and previous in " \t\r\n;&|()": - # The lexer reads past a shell comment, so quotes after it prove nothing. - comment = True + elif quote is None: + if character == "'": + quote = "'" + elif character == "$" and following and following in "'\"": + return None + elif character == "<" and following == "<": + return None + elif character == "#" and previous in " \t\r\n;&|()<>": + comment = True if character in PARAMETER_MARKERS: flags.append(quote == '"' and not comment) previous = character index += 1 - return flags + return flags if quote is None and not braces else None def parameters_double_quoted(token): @@ -2102,7 +2125,7 @@ def shell_tokens(command): if not tokens: raise ValueError("shell command is empty") flags = double_quoted_markers(command) - if len(flags) != sum( + if flags is None or len(flags) != sum( character in PARAMETER_MARKERS for token in tokens for character in token ): return tokens @@ -2353,9 +2376,9 @@ def git_command(segment, assignments=None): read_only_subcommands = ARCHIVE_READ_GIT_COMMANDS | CLOSED_READ_GIT_COMMANDS if subcommand in read_only_subcommands: # Read-only subcommands still write through --output. A parameter - # passes only inside double quotes, where its expansion stays one - # word; an unquoted one can word-split into an option, and command - # substitution output is never known. + # passes only inside plain double quotes, where its expansion stays + # one word; an unquoted one can word-split into an option, and + # command substitution output is never known. parameter_arguments = unproven_parameters if parameter_arguments: raise ValueError( @@ -2662,8 +2685,10 @@ def bundled_helper_invocation(command, tokens, working_directories, helpers=PIPE def archive_command_is_read_only( - command, tokens, archive_context=False, git_commands=ARCHIVE_READ_GIT_COMMANDS + command, tokens, archive_context=False, git_commands=ARCHIVE_READ_GIT_COMMANDS, + source=None, ): + """`source` is the unexpanded (segment, assignments) whose tokens keep their quote flags.""" if archive_context and AMBIGUOUS_SHELL_SYNTAX.search(command): return False if not archive_context: @@ -2691,7 +2716,7 @@ def archive_command_is_read_only( return True if executable != "git": return False - git = git_command(tokens) + git = git_command(*(source or (tokens,))) if git is None: return False subcommand, git_arguments, _options = git @@ -2703,7 +2728,7 @@ def archive_command_is_read_only( ) -def archive_write_attempt(segment): +def archive_write_attempt(segment, source=None): """Whether a segment denied in archive context attempts a write there.""" if any( token @@ -2724,7 +2749,7 @@ def archive_write_attempt(segment): or any(argument.startswith("--in-place") for argument in arguments) ): return True - git = git_command(segment) + git = git_command(*(source or (segment,))) if git is None: return False subcommand, git_arguments, _options = git @@ -2958,12 +2983,13 @@ def command_denial(command, working_directories, allow_destructive=True): ) ) if not archive_command_is_read_only( - shlex.join(resolved_segment), resolved_segment, archive_context + shlex.join(resolved_segment), resolved_segment, archive_context, + source=(segment, assignments), ) and not ( archive_context and bundled_helper ): - if archive_write_attempt(resolved_segment): + if archive_write_attempt(resolved_segment, (segment, assignments)): return ARCHIVE_REASON return ARCHIVE_UNPROVEN_READ_REASON reason = destructive_git_reason(tokens) or protected_shell_write_reason( diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 365a44f9b..563455a8f 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2182,6 +2182,10 @@ def test_allows_parameters_in_read_only_git_arguments(self): 't=abc; git log --format="$t %H"', 'git show "HEAD:$f"', 'git log --format="%H%n%s"', + '# note\ngit log --format="$t %H"', + 'x=abc; cd .project/archive/001-mvp && git log --format="%H $x"', + 'cd .project/archive/001-mvp && git log --format="%H $x"', + 'cd .project/archive/001-mvp && git show "HEAD:$f"', ): with self.subTest(command=command): self.assert_allowed(self.bash(command)) @@ -2201,6 +2205,16 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git log --format=$t\\ %H', 'git log "--format=$@ %H"', 'git log # "\n git diff --format=$x "', + # Quoting outside plain single and double quotes disables the + # exception for the whole command, quoted parameters included. + "x='c --output=.project/archive/001-mvp/NOTE.md'; " + "git diff --no-index --src-prefix=a$'\\'''\"'$x\"'\"\\''' /dev/null f", + "git log --format=a$'\\'''\"'$x\"'\"\\'''", + "echo $'a'; git log --format=\"$t %H\"", + 'echo $"a"; git log --format="$t %H"', + 'git log --format="${t:-"a"} $x %H"', + "# it's\ngit log --format=\"$t %H\"", + 'git log --format="$t %H" <<< "x"', "git diff --output=$f", ) for command in unquoted: From afc02527a2c6440ff4f4a3887e36cbb06312cc71 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 14:02:08 -0500 Subject: [PATCH 13/37] no-mistakes(review): deny git parameters inside wrapped shell strings --- HOOKS.md | 5 ++++- scripts/guard_hook.py | 19 ++++++++++++------- tests/test_guard_hook.py | 9 +++++++++ 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index c2f4962bb..224799cb2 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -179,7 +179,10 @@ See [UPDATE.md](UPDATE.md). `--output` values are denied in every form. Any other quoting construct in the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or backslashes in a comment, here-documents and here-strings) disables the - exception, so every parameter in a git argument of that command is denied + exception, so every parameter in a git argument of that command is denied. + The exception applies only to commands the guard parses directly, never + inside wrapped shell strings (`eval`, `bash -c`, `sh -c`, PowerShell or + `cmd` command strings), where the outer shell expands the parameter first - archive glob/brace expansions and execution-capable read options such as `rg --pre` - direct write, edit, and patch tool calls targeting `.project/STATE.md`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 3bf593bbf..e03950716 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2109,7 +2109,12 @@ def parameters_double_quoted(token): ) -def shell_tokens(command): +def shell_tokens(command, direct=True): + """Tokenize a command; `direct` is False for a string that a second shell parses. + + Only a directly parsed command keeps quote flags: an outer shell expands + parameters before the inner parse, so inner quotes prove nothing. + """ command = strip_heredoc_bodies(LINE_CONTINUATION.sub(r"\1 ", command)) lexer = shlex.shlex(command, posix=True, punctuation_chars="|;&()<>\n\r") lexer.whitespace = " \t" @@ -2124,7 +2129,7 @@ def shell_tokens(command): ) from None if not tokens: raise ValueError("shell command is empty") - flags = double_quoted_markers(command) + flags = double_quoted_markers(command) if direct else None if flags is None or len(flags) != sum( character in PARAMETER_MARKERS for token in tokens for character in token ): @@ -2394,7 +2399,7 @@ def wrapped_command_tokens(segment, expand_parameters=True): return None executable, arguments = invocation if executable == "eval": - return shell_tokens(" ".join(arguments)) if arguments else None + return shell_tokens(" ".join(arguments), direct=False) if arguments else None if executable in COMMAND_WRAPPERS: if executable == "command" and arguments[:1] in (["-v"], ["-V"]): return None @@ -2419,7 +2424,7 @@ def wrapped_command_tokens(segment, expand_parameters=True): executable=executable, option=argument ) ) - return shell_tokens(arguments[index + 1]) + return shell_tokens(arguments[index + 1], direct=False) raise ValueError(SHELL_FILE_REASON.format(executable=executable)) if executable in POWERSHELL_WRAPPERS or executable in {"cmd", "cmd.exe"}: switches = {"-c", "-command", "/c", "/k"} @@ -2438,8 +2443,8 @@ def wrapped_command_tokens(segment, expand_parameters=True): for argument in payload ] if len(payload) == 1: - return shell_tokens(payload[0]) - return payload + return shell_tokens(payload[0], direct=False) + return [str(argument) for argument in payload] raise ValueError(SHELL_FILE_REASON.format(executable=executable)) return None @@ -2528,7 +2533,7 @@ def destructive_git_reason( "git command directly" ) reason = destructive_git_reason( - ["git", *git_options, *shell_tokens(alias), *arguments], + ["git", *git_options, *shell_tokens(alias, direct=False), *arguments], resolved_aliases | {command}, assignments, ) diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 563455a8f..df9b44e9e 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2215,6 +2215,15 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git log --format="${t:-"a"} $x %H"', "# it's\ngit log --format=\"$t %H\"", 'git log --format="$t %H" <<< "x"', + # A wrapped string is parsed by a second shell after the outer + # one expands parameters, so its inner quotes prove nothing. + 'read x < v.txt; eval "git diff --src-prefix=\\"a $x\\""', + 'read x < v.txt; bash -c "git diff --src-prefix=\\"a $x\\""', + 'read x < v.txt; sh -c "git log --format=\\"a $x\\""', + 'eval "git status \\"a $x\\""', + 'eval "git log --format=\\"a $1\\""', + "bash -c 'git log --format=\"$t %H\"'", + 'powershell -Command git log "--format=$t %H"', "git diff --output=$f", ) for command in unquoted: From f3b1e025f239357fd954ed332c9dfe5d860b37e2 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 14:08:05 -0500 Subject: [PATCH 14/37] no-mistakes(review): accept only plain name expansions in git parameter exception --- HOOKS.md | 13 ++++++++----- scripts/guard_hook.py | 15 +++++++++++++-- tests/test_guard_hook.py | 13 +++++++++++++ 3 files changed, 34 insertions(+), 7 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 224799cb2..921c03e5a 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -168,14 +168,17 @@ See [UPDATE.md](UPDATE.md). tilde paths are denied even when quoted. This also applies to `find -delete` and supported destructive aliases - destructive Git commands nested in supported shell and command wrappers -- Git arguments that contain a shell parameter. Parameters pass only inside - plain double-quoted spans of read-only subcommands (`status`, `log`, - `diff`, `show`, and the other inspection commands), such as +- Git arguments that contain a shell parameter. Only plain `$name` and + `${name}` expansions pass, and only inside plain double-quoted spans of + read-only subcommands (`status`, `log`, `diff`, `show`, and the other + inspection commands) that the guard parses directly, such as `git log --format="$t %H"` or `git show "HEAD:$f"`, because only double quoting prevents word-splitting into options. A parameter outside double quotes (`$x`, `HEAD:$f`, `--format=$x`, `"a b"$x`) is denied because its - expansion can split into a write option such as `--output`; `"$@"`, - command substitution output, a parameter that starts an argument, and + expansion can split into a write option such as `--output`. Every other + parameter form (`${=x}`, `${(z)x}`, `${x:-y}`, `$x[@]`, `$1`, `$@`, `$*`, + `$#`) is denied even inside double quotes, because it can expand to more + than one word. Command substitution output, a parameter that starts an argument, and `--output` values are denied in every form. Any other quoting construct in the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or backslashes in a comment, here-documents and here-strings) disables the diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index e03950716..9e20f3ea2 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2094,7 +2094,11 @@ def double_quoted_markers(command): def parameters_double_quoted(token): - """Whether every parameter in the token expands inside double quotes to one word.""" + """Whether every parameter in the token expands inside double quotes to one word. + + Only plain $name and ${name} qualify: flags, operators, subscripts, + positional and special parameters can split even inside double quotes. + """ flags = getattr(token, "double_quoted", ()) positions = [ index for index, character in enumerate(token) if character in PARAMETER_MARKERS @@ -2102,8 +2106,15 @@ def parameters_double_quoted(token): if len(flags) != len(positions): return False quoted = dict(zip(positions, flags)) + plain = { + match.start(): match.end() for match in NAMED_SHELL_PARAMETER_SYNTAX.finditer(token) + } + if any( + character == "$" and index not in plain for index, character in enumerate(token) + ) or any(token[end : end + 1] == "[" for end in plain.values()): + return False return all( - quoted[match.start()] and quoted.get(match.end() - 1, True) and "@" not in match.group(0) + quoted[match.start()] and quoted.get(match.end() - 1, True) for syntax in (SHELL_PARAMETER_SYNTAX, CMD_PARAMETER_SYNTAX) for match in syntax.finditer(token) ) diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index df9b44e9e..747d716b3 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2181,6 +2181,7 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git log --format="$t %H"', 't=abc; git log --format="$t %H"', 'git show "HEAD:$f"', + 'git show "HEAD:${f}"', 'git log --format="%H%n%s"', '# note\ngit log --format="$t %H"', 'x=abc; cd .project/archive/001-mvp && git log --format="%H $x"', @@ -2224,6 +2225,18 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'eval "git log --format=\\"a $1\\""', "bash -c 'git log --format=\"$t %H\"'", 'powershell -Command git log "--format=$t %H"', + # Only plain $name and ${name} are single-word: zsh flags and + # array or positional forms split even inside double quotes. + *( + "x='c --output=.project/archive/001-mvp/NOTE.md'; " + f'git diff --no-index "--src-prefix=a {form}" /dev/null f' + for form in ( + "${=x}", "${(s: :)x}", "${(z)x}", "${(f)x}", "$x[@]", + "$argv[@]", "${x[@]}", "${x:-b}", "$1", "$*", "$#", "$t $=x", + ) + ), + 'git log --format="a ${=x}"', + 'git log --format="$x[@] %H"', "git diff --output=$f", ) for command in unquoted: From 336d266aafa66b6698f7e6a795969a0fa09c3761 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 14:14:16 -0500 Subject: [PATCH 15/37] no-mistakes(review): deny parameters in command-valued git options --- HOOKS.md | 7 +++++-- scripts/guard_hook.py | 11 +++++++++-- tests/test_guard_hook.py | 11 +++++++++++ 3 files changed, 25 insertions(+), 4 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 921c03e5a..48a276b44 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -178,8 +178,11 @@ See [UPDATE.md](UPDATE.md). expansion can split into a write option such as `--output`. Every other parameter form (`${=x}`, `${(z)x}`, `${x:-y}`, `$x[@]`, `$1`, `$@`, `$*`, `$#`) is denied even inside double quotes, because it can expand to more - than one word. Command substitution output, a parameter that starts an argument, and - `--output` values are denied in every form. Any other quoting construct in + than one word. Command substitution output, a parameter that starts an + argument, and a parameter in the value of an option that git writes to or + runs as a command (`--output`, `--upload-pack`, `--receive-pack`, `--exec`, + `-c`, `--config-env`, or a `::
` remote) are denied in + every form. Any other quoting construct in the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or backslashes in a comment, here-documents and here-strings) disables the exception, so every parameter in a git argument of that command is denied. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 9e20f3ea2..c1f04e5a4 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -238,6 +238,11 @@ "symbolic-ref": frozenset({"-q", "--quiet", "--short"}), } GIT_READ_WRITE_OPTIONS = frozenset({"--output", "--ext-diff", "--textconv"}) +# Options whose value git writes to or runs as a command: one unknown word is +# already enough, so a parameter in or after them is never accepted. +GIT_PARAMETER_DENIED_OPTIONS = GIT_READ_WRITE_OPTIONS | { + "--upload-pack", "--receive-pack", "--exec", "-c", "--config-env", +} ARCHIVE_READ_EXECUTION_OPTIONS = {"rg": frozenset({"--pre"})} AMBIGUOUS_SHELL_SYNTAX = re.compile(r"[\r\n|;&<>`]|\$\(|@\(") SUBSTITUTION_PLACEHOLDER = "COMMAND_SUBSTITUTION_" @@ -2347,8 +2352,10 @@ def git_command(segment, assignments=None): if ( not literal or (literal.startswith("-") and "=" not in literal) - or literal.split("=", 1)[0] in GIT_READ_WRITE_OPTIONS - or arguments[argument_index - 1] in GIT_READ_WRITE_OPTIONS + or literal.split("=", 1)[0] in GIT_PARAMETER_DENIED_OPTIONS + or arguments[argument_index - 1] in GIT_PARAMETER_DENIED_OPTIONS + or (literal.startswith("-c") and literal != "-c") + or "::" in literal or not parameters_double_quoted(argument) or SUBSTITUTION_PLACEHOLDER in argument or SUBSTITUTION_PLACEHOLDER in expanded diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 747d716b3..259f5692c 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2236,6 +2236,17 @@ def test_allows_parameters_in_read_only_git_arguments(self): ) ), 'git log --format="a ${=x}"', + # One word is enough where git runs the value as a command. + 'read x < v.txt; git ls-remote --upload-pack="$x" .', + 'read x < v.txt; git ls-remote --upload-pack "a $x" .', + 'read x < v.txt; git ls-remote "--exec=$x" .', + 'read x < v.txt; git ls-remote --exec "a $x" .', + 'read x < v.txt; git ls-remote --receive-pack="$x" .', + 'read x < v.txt; git ls-remote --receive-pack "a $x" .', + 'read x < v.txt; git ls-remote "ext::$x"', + 'read x < v.txt; git -c "diff.external=$x" diff', + 'read x < v.txt; git "-cdiff.external=$x" diff', + 'read x < v.txt; git -c "core.fsmonitor=$x" status', 'git log --format="$x[@] %H"', "git diff --output=$f", ) From 38e91abc1ae69371721247bc6b5e749ad2880df5 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 14:17:27 -0500 Subject: [PATCH 16/37] no-mistakes(review): match denied git options by abbreviated prefix --- HOOKS.md | 5 ++++- scripts/guard_hook.py | 22 ++++++++++++++++------ tests/test_guard_hook.py | 17 +++++++++++++++++ 3 files changed, 37 insertions(+), 7 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 48a276b44..e13faf154 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -182,7 +182,10 @@ See [UPDATE.md](UPDATE.md). argument, and a parameter in the value of an option that git writes to or runs as a command (`--output`, `--upload-pack`, `--receive-pack`, `--exec`, `-c`, `--config-env`, or a `::
` remote) are denied in - every form. Any other quoting construct in + every form. These option names match by prefix (`--upload-pa`, `--exe`), + because git accepts abbreviated long options; in archive context the same + prefix match applies to `--output`, `--ext-diff`, and `--textconv`. Any + other quoting construct in the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or backslashes in a comment, here-documents and here-strings) disables the exception, so every parameter in a git argument of that command is denied. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index c1f04e5a4..b72449596 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -243,6 +243,18 @@ GIT_PARAMETER_DENIED_OPTIONS = GIT_READ_WRITE_OPTIONS | { "--upload-pack", "--receive-pack", "--exec", "-c", "--config-env", } + + +def git_option_in(token, options): + """Whether a token names one of the options, by full name or any long-option prefix. + + git accepts each unique prefix of a long option; matching every prefix is + stricter than git and so only fails closed. + """ + name = token.split("=", 1)[0] + if name.startswith("--") and len(name) > 2: + return any(option.startswith(name) for option in options) + return name in options ARCHIVE_READ_EXECUTION_OPTIONS = {"rg": frozenset({"--pre"})} AMBIGUOUS_SHELL_SYNTAX = re.compile(r"[\r\n|;&<>`]|\$\(|@\(") SUBSTITUTION_PLACEHOLDER = "COMMAND_SUBSTITUTION_" @@ -2352,8 +2364,8 @@ def git_command(segment, assignments=None): if ( not literal or (literal.startswith("-") and "=" not in literal) - or literal.split("=", 1)[0] in GIT_PARAMETER_DENIED_OPTIONS - or arguments[argument_index - 1] in GIT_PARAMETER_DENIED_OPTIONS + or git_option_in(literal, GIT_PARAMETER_DENIED_OPTIONS) + or git_option_in(arguments[argument_index - 1], GIT_PARAMETER_DENIED_OPTIONS) or (literal.startswith("-c") and literal != "-c") or "::" in literal or not parameters_double_quoted(argument) @@ -2746,8 +2758,7 @@ def archive_command_is_read_only( if subcommand not in git_commands: return False return not any( - token in GIT_READ_WRITE_OPTIONS or token.startswith("--output=") - for token in git_arguments + git_option_in(token, GIT_READ_WRITE_OPTIONS) for token in git_arguments ) @@ -2777,8 +2788,7 @@ def archive_write_attempt(segment, source=None): return False subcommand, git_arguments, _options = git return subcommand not in ARCHIVE_READ_GIT_COMMANDS or any( - token in GIT_READ_WRITE_OPTIONS or token.startswith("--output=") - for token in git_arguments + git_option_in(token, GIT_READ_WRITE_OPTIONS) for token in git_arguments ) diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 259f5692c..810126ef0 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2244,6 +2244,15 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'read x < v.txt; git ls-remote --receive-pack="$x" .', 'read x < v.txt; git ls-remote --receive-pack "a $x" .', 'read x < v.txt; git ls-remote "ext::$x"', + # git accepts each unique prefix of a long option name. + 'read x < v.txt; git ls-remote --upload-pa="a $x" .', + "read x < v.txt; git ls-remote --upl=$x .", + 'read x < v.txt; git ls-remote "--upl=$x" .', + 'read x < v.txt; git ls-remote --upload-pac "a $x" .', + "read x < v.txt; git ls-remote --exe=$x .", + 'read x < v.txt; git ls-remote "--exe=a $x" .', + 'read x < v.txt; git ls-remote "--receive-p=a $x" .', + 'git diff "--outpu=a $f"', 'read x < v.txt; git -c "diff.external=$x" diff', 'read x < v.txt; git "-cdiff.external=$x" diff', 'read x < v.txt; git -c "core.fsmonitor=$x" status', @@ -2272,6 +2281,14 @@ def test_allows_parameters_in_read_only_git_arguments(self): with self.subTest(command=command): self.assert_denied(self.bash(command)) + def test_denies_abbreviated_git_write_options_in_archive_context(self): + for option in ("--ext-diff", "--ext-d", "--textc", "--output=NOTE.md"): + with self.subTest(option=option): + self.assert_denied( + self.bash(f"cd .project/archive/001-mvp && git diff {option}") + ) + self.assert_allowed(self.bash("cd .project/archive/001-mvp && git diff --stat")) + def test_assignment_state_is_segment_local_and_last_value_wins(self): self.assert_denied( self.bash('P=.project/archive; echo x > "$P/NOTE.md"; P=/tmp') From e2fa0106ba48e8b1d04c2392772f468325d945c4 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 14:20:43 -0500 Subject: [PATCH 17/37] no-mistakes(review): keep exact --text allowed under git option prefix rule --- HOOKS.md | 3 ++- scripts/guard_hook.py | 9 ++++++++- tests/test_guard_hook.py | 16 ++++++++++++++-- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index e13faf154..af873009d 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -184,7 +184,8 @@ See [UPDATE.md](UPDATE.md). `-c`, `--config-env`, or a `::
` remote) are denied in every form. These option names match by prefix (`--upload-pa`, `--exe`), because git accepts abbreviated long options; in archive context the same - prefix match applies to `--output`, `--ext-diff`, and `--textconv`. Any + prefix match applies to `--output`, `--ext-diff`, and `--textconv`. The + exact read-only option `--text` is not an abbreviation and stays allowed. Any other quoting construct in the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or backslashes in a comment, here-documents and here-strings) disables the diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index b72449596..fc05a09e0 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -245,13 +245,20 @@ } +# Real read-only git options that are a strict prefix of a denied option name. +GIT_SAFE_PREFIX_OPTIONS = frozenset({"--text"}) + + def git_option_in(token, options): """Whether a token names one of the options, by full name or any long-option prefix. git accepts each unique prefix of a long option; matching every prefix is - stricter than git and so only fails closed. + stricter than git and so only fails closed. A prefix that is itself the + exact name of a read-only option is that option, not an abbreviation. """ name = token.split("=", 1)[0] + if name in GIT_SAFE_PREFIX_OPTIONS and name not in options: + return False if name.startswith("--") and len(name) > 2: return any(option.startswith(name) for option in options) return name in options diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 810126ef0..07b91a7f4 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2282,12 +2282,24 @@ def test_allows_parameters_in_read_only_git_arguments(self): self.assert_denied(self.bash(command)) def test_denies_abbreviated_git_write_options_in_archive_context(self): - for option in ("--ext-diff", "--ext-d", "--textc", "--output=NOTE.md"): + for option in ( + "--ext-diff", "--ext-d", "--textconv", "--textc", "--tex", "--te", + "--output=NOTE.md", + ): with self.subTest(option=option): self.assert_denied( self.bash(f"cd .project/archive/001-mvp && git diff {option}") ) - self.assert_allowed(self.bash("cd .project/archive/001-mvp && git diff --stat")) + for command in ( + "cd .project/archive/001-mvp && git diff --stat", + "cd .project/archive/001-mvp && git diff --text", + "cd .project/archive/001-mvp && git log --text -1", + "cd .project/archive/001-mvp && git show --text HEAD", + "git diff --text .project/archive/001-mvp/plan/PLAN.md", + "git log --text -- .project/archive/001-mvp/plan/PLAN.md", + ): + with self.subTest(command=command): + self.assert_allowed(self.bash(command)) def test_assignment_state_is_segment_local_and_last_value_wins(self): self.assert_denied( From f7540e80bb860a560efb2329ff54474a2cd63993 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 14:28:35 -0500 Subject: [PATCH 18/37] no-mistakes(document): docs: align guard, worktree pin, provenance, planner docs --- DOCS.md | 14 ++++++++++++-- HOOKS.md | 6 +++--- skills/gsd-path-plan/references/planner.md | 3 ++- skills/gsd-path/references/planner.md | 3 ++- skills/path/references/planner.md | 3 ++- 5 files changed, 21 insertions(+), 8 deletions(-) diff --git a/DOCS.md b/DOCS.md index 6937f1457..debc3a037 100644 --- a/DOCS.md +++ b/DOCS.md @@ -247,8 +247,10 @@ resolved Git common directory and primary path, so separate clones and primary folders do not share a location. The first allocation pins the workspace location in -`/gsd-path/workspaces/.json`. Later environment -changes do not move it. Existing sibling worktrees remain at their original +`/gsd-path/workspaces/.json`. Environment +changes do not move it while a linked worktree remains. When a sidecar is +retired and no linked worktree remains, Path removes the pin, and the next +allocation reads `GSD_PATH_WORKTREE_ROOT` again. Existing sibling worktrees remain at their original paths and retain the same recovery and cleanup checks. These folders contain active work and evidence; they are not a disposable cache. Relocation needs a separate migration; do not edit placement records or move folders manually. @@ -397,6 +399,14 @@ the supplied package's runtime and its compatible status launcher. Review those configuration changes before committing them. Existing versions remain available for other projects and branches. +To record where a patched runtime came from, add `--runtime-provenance-source`, +`--runtime-provenance-patch`, or `--runtime-provenance-note` to +`--runtime-upgrade`. The values are written to the `provenance` object +(`source`, `patch_ref`, `note`) in `.gsd-path/runtime.json`. Without a patch +value, a source root that is a Git checkout records its `HEAD` commit. A later +upgrade without these flags keeps the object only when the digest is unchanged. +Provenance is a record only; it does not change the digest or runtime validation. + A new machine needs the selected runtime installed before status and guards can run. `gsd-path --runtime-restore --project /absolute/project --source-root /path/to/matching/package` restores that exact version without changing project diff --git a/HOOKS.md b/HOOKS.md index af873009d..3679d1a02 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -146,7 +146,8 @@ See [UPDATE.md](UPDATE.md). source trees containing directory symlinks are denied; use literal directory paths and copy directory links separately - shell commands that reference the archive unless the whole command is a - recognized standalone read or a single-command invocation of the bundled + recognized standalone read, a pipeline in which each segment is a recognized + read, or a single-command invocation of the bundled `pipeline_state.py` / `archive_milestone.py` helper, resolved to a regular file inside the verified runtime selected by `.gsd-path/runtime.json` (legacy projects use `.gsd-path/runtime/` beside `guard_hook.py` in the repository layout), using exactly `python` or `python3` with optional @@ -186,8 +187,7 @@ See [UPDATE.md](UPDATE.md). because git accepts abbreviated long options; in archive context the same prefix match applies to `--output`, `--ext-diff`, and `--textconv`. The exact read-only option `--text` is not an abbreviation and stays allowed. Any - other quoting construct in - the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or + other quoting construct in the command (`$'...'`, `$"..."`, quotes inside `${...}`, quotes or backslashes in a comment, here-documents and here-strings) disables the exception, so every parameter in a git argument of that command is denied. The exception applies only to commands the guard parses directly, never diff --git a/skills/gsd-path-plan/references/planner.md b/skills/gsd-path-plan/references/planner.md index 138eb1dfd..e98a5c613 100644 --- a/skills/gsd-path-plan/references/planner.md +++ b/skills/gsd-path-plan/references/planner.md @@ -82,7 +82,8 @@ plan order above, which still keeps plan-invalidating risk in wave 1. held-out. Put each `judgment` prohibition's Must not and Detail in the owning task's Approach. - Make `files` exhaustive, including imports, routes, generated artifacts, - tests, and wiring. Require disjoint files for same-wave tasks. + tests, and wiring. Require disjoint files for same-wave tasks that are not + dependency-ordered. - Write an Interface contract in every task: `None` for independent tasks; for any tasks that exchange a symbol, signature, schema, endpoint, file format, or path, the exact shapes both sides code against — identical text diff --git a/skills/gsd-path/references/planner.md b/skills/gsd-path/references/planner.md index 138eb1dfd..e98a5c613 100644 --- a/skills/gsd-path/references/planner.md +++ b/skills/gsd-path/references/planner.md @@ -82,7 +82,8 @@ plan order above, which still keeps plan-invalidating risk in wave 1. held-out. Put each `judgment` prohibition's Must not and Detail in the owning task's Approach. - Make `files` exhaustive, including imports, routes, generated artifacts, - tests, and wiring. Require disjoint files for same-wave tasks. + tests, and wiring. Require disjoint files for same-wave tasks that are not + dependency-ordered. - Write an Interface contract in every task: `None` for independent tasks; for any tasks that exchange a symbol, signature, schema, endpoint, file format, or path, the exact shapes both sides code against — identical text diff --git a/skills/path/references/planner.md b/skills/path/references/planner.md index 138eb1dfd..e98a5c613 100644 --- a/skills/path/references/planner.md +++ b/skills/path/references/planner.md @@ -82,7 +82,8 @@ plan order above, which still keeps plan-invalidating risk in wave 1. held-out. Put each `judgment` prohibition's Must not and Detail in the owning task's Approach. - Make `files` exhaustive, including imports, routes, generated artifacts, - tests, and wiring. Require disjoint files for same-wave tasks. + tests, and wiring. Require disjoint files for same-wave tasks that are not + dependency-ordered. - Write an Interface contract in every task: `None` for independent tasks; for any tasks that exchange a symbol, signature, schema, endpoint, file format, or path, the exact shapes both sides code against — identical text From 87f028de3f336687b72ac1dfa91ceaaf5a6213c3 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 15:40:18 -0500 Subject: [PATCH 19/37] fix: close zsh flag and escaped-marker gaps in the git parameter exception Unbraced zsh expansion flags ($=x, $~x, $^x, $+x) matched no parameter syntax, so they never entered the parameter proof while zsh word-splits them exactly like the braced forms the proof denies. A backslash-escaped marker kept a proven-quoted flag although the shell executes the literal text and the token keeps the bare marker. Both now deny: any $-position covered by no recognized form routes into the proof chain, and an escaped marker is never a proven parameter. --- HOOKS.md | 7 +++++-- scripts/guard_hook.py | 27 +++++++++++++++++++++++++-- tests/test_guard_hook.py | 13 +++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 3679d1a02..50c4a5c37 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -178,8 +178,11 @@ See [UPDATE.md](UPDATE.md). quotes (`$x`, `HEAD:$f`, `--format=$x`, `"a b"$x`) is denied because its expansion can split into a write option such as `--output`. Every other parameter form (`${=x}`, `${(z)x}`, `${x:-y}`, `$x[@]`, `$1`, `$@`, `$*`, - `$#`) is denied even inside double quotes, because it can expand to more - than one word. Command substitution output, a parameter that starts an + `$#`, and the unbraced zsh flags `$=x`, `$~x`, `$^x`, `$+x`) is denied even + inside double quotes, because it can expand to more than one word; a + backslash-escaped marker (`"\$t"`) is likewise denied, because the shell + executes the literal text while the argument still shows the bare marker. + Command substitution output, a parameter that starts an argument, and a parameter in the value of an option that git writes to or runs as a command (`--output`, `--upload-pack`, `--receive-pack`, `--exec`, `-c`, `--config-env`, or a `::
` remote) are denied in diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index fc05a09e0..f8dbd652f 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2087,7 +2087,9 @@ def double_quoted_markers(command): quote = None elif character == "\\": if following and following in PARAMETER_MARKERS: - flags.append(quote == '"') + # The shell executes the escaped marker as literal text; the + # token keeps the bare marker, so the pair is never provable. + flags.append(False) previous = character index += 2 continue @@ -2117,11 +2119,28 @@ def double_quoted_markers(command): return flags if quote is None and not braces else None +def unrecognized_dollar(text): + """Whether a $-position is covered by no recognized parameter form. + + Unbraced zsh expansion flags ($=x, $~x, $^x, $+x) match no parameter + syntax, so without this check they never enter the parameter proof at all + — while zsh word-splits them exactly like the forms the proof denies. + """ + covered = {match.start() for match in SHELL_PARAMETER_SYNTAX.finditer(text)} + return any( + character == "$" and index not in covered + for index, character in enumerate(text) + ) + + def parameters_double_quoted(token): """Whether every parameter in the token expands inside double quotes to one word. Only plain $name and ${name} qualify: flags, operators, subscripts, positional and special parameters can split even inside double quotes. + A backslash-escaped marker stays one flag short of proven: the shell + executes the literal text while the guard's token still shows the bare + marker, so nothing about its expansion is provable. """ flags = getattr(token, "double_quoted", ()) positions = [ @@ -2363,7 +2382,11 @@ def git_command(segment, assignments=None): ) resolved_arguments.append(f"-C{expanded}") else: - parameter = SHELL_PARAMETER_SYNTAX.search(argument) or CMD_PARAMETER_SYNTAX.search(argument) + parameter = ( + SHELL_PARAMETER_SYNTAX.search(argument) + or CMD_PARAMETER_SYNTAX.search(argument) + or unrecognized_dollar(argument) + ) if parameter: parameter_arguments.append(argument) literal = re.split(r"[$%!]", argument, maxsplit=1)[0] diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 07b91a7f4..0c0a22bef 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2258,6 +2258,19 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'read x < v.txt; git -c "core.fsmonitor=$x" status', 'git log --format="$x[@] %H"', "git diff --output=$f", + # Unbraced zsh expansion flags match no parameter syntax, so an + # argument carrying one must still enter the parameter proof; + # zsh splits $=x exactly like the braced forms above. + *( + f'git log --format="{flag}"' + for flag in ("$=x", "$~x", "$^x", "$+x") + ), + 'x="a --output=.project/archive/001-mvp/NOTE.md"; git log --format="$=x"', + 'x="a --output=NOTE.md"; cd .project/archive/001-mvp && git log --format="$=x"', + # A backslash-escaped marker executes as literal text while the + # token keeps the bare marker, so the pair is never provable. + 'git log --format="\\$t %H"', + 't=hi; git log --format="\\$t %H"', ) for command in unquoted: for prefix in ("", "cd .project/archive/001-mvp && "): From 49482a9c180f7dcdf282327512bd2fd1cdc92b77 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 15:56:40 -0500 Subject: [PATCH 20/37] no-mistakes(review): narrow zsh flag detector and cover git -C paths --- HOOKS.md | 2 ++ scripts/guard_hook.py | 31 ++++++++++++++++++------------- tests/test_guard_hook.py | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 13 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 50c4a5c37..9990fabfa 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -182,6 +182,8 @@ See [UPDATE.md](UPDATE.md). inside double quotes, because it can expand to more than one word; a backslash-escaped marker (`"\$t"`) is likewise denied, because the shell executes the literal text while the argument still shows the bare marker. + The same four zsh flags are denied in a `git -C` directory. A literal `$` + that starts no expansion (a regex anchor such as `--grep='fix$'`) passes. Command substitution output, a parameter that starts an argument, and a parameter in the value of an option that git writes to or runs as a command (`--output`, `--upload-pack`, `--receive-pack`, `--exec`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index f8dbd652f..dabd9514f 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -276,6 +276,7 @@ def git_option_in(token, options): NAMED_SHELL_PARAMETER_SYNTAX = re.compile( r"\$(?:([A-Za-z_][A-Za-z0-9_]*)|\{([A-Za-z_][A-Za-z0-9_]*)\})" ) +ZSH_EXPANSION_FLAG_SYNTAX = re.compile(r"\$[=~^+]") CMD_PARAMETER_SYNTAX = re.compile( r"%([A-Za-z_][A-Za-z0-9_]*)%|!([A-Za-z_][A-Za-z0-9_]*)!" ) @@ -2119,18 +2120,14 @@ def double_quoted_markers(command): return flags if quote is None and not braces else None -def unrecognized_dollar(text): - """Whether a $-position is covered by no recognized parameter form. +def zsh_expansion_flag(text): + """Whether the text holds an unbraced zsh expansion flag ($=x, $~x, $^x, $+x). - Unbraced zsh expansion flags ($=x, $~x, $^x, $+x) match no parameter - syntax, so without this check they never enter the parameter proof at all - — while zsh word-splits them exactly like the forms the proof denies. + These match no parameter syntax, so without this check they never enter + the parameter proof at all — while zsh word-splits them exactly like the + forms the proof denies. Any other unmatched $ is inert literal text. """ - covered = {match.start() for match in SHELL_PARAMETER_SYNTAX.finditer(text)} - return any( - character == "$" and index not in covered - for index, character in enumerate(text) - ) + return ZSH_EXPANSION_FLAG_SYNTAX.search(text) def parameters_double_quoted(token): @@ -2352,7 +2349,11 @@ def git_command(segment, assignments=None): argument_index += 1 directory = arguments[argument_index] expanded = expand_environment_parameters(directory, assignments) - if SHELL_PARAMETER_SYNTAX.search(expanded) or CMD_PARAMETER_SYNTAX.search(expanded): + if ( + SHELL_PARAMETER_SYNTAX.search(expanded) + or CMD_PARAMETER_SYNTAX.search(expanded) + or zsh_expansion_flag(expanded) + ): raise ValueError( f"git argument {directory} cannot be resolved by the guard; " "pass a literal path" @@ -2368,7 +2369,11 @@ def git_command(segment, assignments=None): elif attached_directory: directory = argument[2:] expanded = expand_environment_parameters(directory, assignments) - if SHELL_PARAMETER_SYNTAX.search(expanded) or CMD_PARAMETER_SYNTAX.search(expanded): + if ( + SHELL_PARAMETER_SYNTAX.search(expanded) + or CMD_PARAMETER_SYNTAX.search(expanded) + or zsh_expansion_flag(expanded) + ): raise ValueError( f"git argument {directory} cannot be resolved by the guard; " "pass a literal path" @@ -2385,7 +2390,7 @@ def git_command(segment, assignments=None): parameter = ( SHELL_PARAMETER_SYNTAX.search(argument) or CMD_PARAMETER_SYNTAX.search(argument) - or unrecognized_dollar(argument) + or zsh_expansion_flag(argument) ) if parameter: parameter_arguments.append(argument) diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 0c0a22bef..608776aaf 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2271,6 +2271,13 @@ def test_allows_parameters_in_read_only_git_arguments(self): # token keeps the bare marker, so the pair is never provable. 'git log --format="\\$t %H"', 't=hi; git log --format="\\$t %H"', + # The -C directory operand takes the same zsh flag check: zsh + # splits the value, which can carry a subcommand and --output. + *( + prefix + form + for prefix in ("", "x='. diff --output=.project/archive/001-mvp/N.md'; ") + for form in ("git -C $=x", "git -C$=x", 'git -C "$=x"') + ), ) for command in unquoted: for prefix in ("", "cd .project/archive/001-mvp && "): @@ -2294,6 +2301,39 @@ def test_allows_parameters_in_read_only_git_arguments(self): with self.subTest(command=command): self.assert_denied(self.bash(command)) + def test_allows_literal_dollar_in_git_arguments(self): + # A $ that starts no expansion (a regex anchor, a price) is inert text. + for command in ( + "git log --grep='fix$'", + "git grep -n 'foo$'", + "git log -G'foo$' --oneline", + "git commit -m 'costs 5$'", + "git branch --list 'feat$'", + "git config --get-regexp 'user\\..*$'", + "git log --format='%H$'", + ): + with self.subTest(command=command): + self.assert_allowed(self.bash(command)) + # In archive cwd only the archive read commands pass; grep, commit, + # branch and config are denied there with or without a $. + for command in ( + "git log --grep='fix$'", + "git log -G'foo$' --oneline", + "git log --format='%H$'", + ): + command = "cd .project/archive/001-mvp && " + command + with self.subTest(command=command): + self.assert_allowed(self.bash(command)) + for command in ( + "git grep -n foo", + "git commit -m costs", + "git branch --list feat", + "git config --get-regexp user", + ): + command = "cd .project/archive/001-mvp && " + command + with self.subTest(command=command): + self.assert_denied(self.bash(command)) + def test_denies_abbreviated_git_write_options_in_archive_context(self): for option in ( "--ext-diff", "--ext-d", "--textconv", "--textc", "--tex", "--te", From f1d56192501a5103f14aa25c21d2b5e50745f09a Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 16:56:04 -0500 Subject: [PATCH 21/37] fix: deny git -c keys whose value git executes The guard scanned -c/--config values only for parameters, so a literal core.fsmonitor path passed while git executes the configured program during an otherwise read-only subcommand (demonstrated archive write via git -c core.fsmonitor= status .project/archive). Keys whose value git runs as a program -- fsmonitor, hooksPath, editors, pagers, sshCommand, askPass, filter/diff/merge drivers -- are now denied with literal values as with parameters; ordinary settings keep working. --- HOOKS.md | 6 ++++++ scripts/guard_hook.py | 28 ++++++++++++++++++++++++++++ tests/test_guard_hook.py | 23 +++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/HOOKS.md b/HOOKS.md index 9990fabfa..147292d95 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -184,6 +184,12 @@ See [UPDATE.md](UPDATE.md). executes the literal text while the argument still shows the bare marker. The same four zsh flags are denied in a `git -C` directory. A literal `$` that starts no expansion (a regex anchor such as `--grep='fix$'`) passes. + A `-c`/`--config` key whose value git executes as a program — + `core.fsmonitor`, `core.hooksPath`, editors and pagers, `core.sshCommand`, + `core.askPass`, `filter.*` clean/smudge/process, `diff.*` + textconv/external/command, `merge.*` drivers — is denied with a literal + value as with a parameter, because git runs it during an otherwise + read-only subcommand. Command substitution output, a parameter that starts an argument, and a parameter in the value of an option that git writes to or runs as a command (`--output`, `--upload-pack`, `--receive-pack`, `--exec`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index dabd9514f..2249e9b4c 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -245,6 +245,29 @@ } +# -c/--config keys whose value git executes as a program. A literal value is +# as dangerous as a parameter here: git -c core.fsmonitor= status runs +# the configured program while the subcommand stays nominally read-only. +GIT_EXECUTED_CONFIG_KEYS = frozenset({ + "core.fsmonitor", "core.hookspath", "core.editor", "sequence.editor", + "core.sshcommand", "core.askpass", "core.pager", "interactive.difffilter", +}) +GIT_EXECUTED_CONFIG_SECTIONS = { + "filter": (".clean", ".smudge", ".process", ".command"), + "diff": (".command", ".textconv", ".external"), + "merge": (".driver",), +} + + +def executed_config_key(key): + """Whether a -c/--config key's value is a program git runs.""" + if key in GIT_EXECUTED_CONFIG_KEYS: + return True + section, _, remainder = key.partition(".") + suffixes = GIT_EXECUTED_CONFIG_SECTIONS.get(section, ()) + return bool(remainder) and key.endswith(tuple(suffixes)) + + # Real read-only git options that are a strict prefix of a denied option name. GIT_SAFE_PREFIX_OPTIONS = frozenset({"--text"}) @@ -2433,6 +2456,11 @@ def git_command(segment, assignments=None): f"git -c {config_key} defines an alias the guard cannot inspect; " "run the underlying git command directly" ) + if executed_config_key(config_key): + raise ValueError( + f"git -c {config_key} configures a program git executes; " + "the guard cannot allow it" + ) if config_key == "clean.requireforce": raise ValueError( "git -c clean.requireForce disables the git clean safety check; " diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 608776aaf..9fc4d448c 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2256,6 +2256,18 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'read x < v.txt; git -c "diff.external=$x" diff', 'read x < v.txt; git "-cdiff.external=$x" diff', 'read x < v.txt; git -c "core.fsmonitor=$x" status', + # A literal value is just as dangerous: git executes the + # configured program while the subcommand stays read-only. + 'git -c core.fsmonitor=/tmp/fsm-hook status .project/archive', + 'cd .project/archive/001-mvp && git -c core.fsmonitor=/tmp/fsm-hook status', + 'git -c core.hooksPath=/tmp/hooks status', + 'git -c core.editor=/tmp/editor.sh log -1', + 'git -c core.sshCommand=/tmp/ssh.sh ls-remote origin', + 'git -c core.pager=/tmp/pager.sh log', + 'git -c diff.my.textconv=/tmp/tc.sh log', + 'git -c filter.driver.clean=/tmp/clean.sh show HEAD:file', + 'git -c merge.rewriter.driver=/tmp/merge.sh merge topic', + 'git --config-env=core.fsmonitor=FSM status', 'git log --format="$x[@] %H"', "git diff --output=$f", # Unbraced zsh expansion flags match no parameter syntax, so an @@ -2301,6 +2313,17 @@ def test_allows_parameters_in_read_only_git_arguments(self): with self.subTest(command=command): self.assert_denied(self.bash(command)) + def test_denies_executed_config_keys_but_allows_plain_settings(self): + # Only -c keys whose value git runs as a program are denied; ordinary + # settings keep working (see the literal-value cases in the test above). + for command in ( + "git -c user.name=Test log -1", + "git -c core.autocrlf=false status", + 'git -c "commit.gpgsign=false" log -1', + ): + with self.subTest(command=command): + self.assert_allowed(self.bash(command)) + def test_allows_literal_dollar_in_git_arguments(self): # A $ that starts no expansion (a regex anchor, a price) is inert text. for command in ( From e1c03e2bef6a4fd117efd784f91e71f63daca385 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 17:07:58 -0500 Subject: [PATCH 22/37] no-mistakes(review): close executed git config keys, persisted form, disabling values --- HOOKS.md | 12 +++++++--- scripts/guard_hook.py | 52 +++++++++++++++++++++++++++++++++------- tests/test_guard_hook.py | 27 +++++++++++++++++++++ 3 files changed, 79 insertions(+), 12 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 147292d95..b6be0b9b4 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -185,11 +185,17 @@ See [UPDATE.md](UPDATE.md). The same four zsh flags are denied in a `git -C` directory. A literal `$` that starts no expansion (a regex anchor such as `--grep='fix$'`) passes. A `-c`/`--config` key whose value git executes as a program — - `core.fsmonitor`, `core.hooksPath`, editors and pagers, `core.sshCommand`, + `core.fsmonitor`, editors and pagers, `core.sshCommand`, `core.askPass`, `filter.*` clean/smudge/process, `diff.*` - textconv/external/command, `merge.*` drivers — is denied with a literal + textconv/external/command, `merge.*` drivers, `core.gitProxy`, + `remote.*` uploadpack/receivepack — is denied with a literal value as with a parameter, because git runs it during an otherwise - read-only subcommand. + read-only subcommand. A disabling form of such a key passes: the bare key, + an empty value, or `false`. A `--config-env` form is always denied, because + its value comes from the environment. A key through which git loads code — + `core.hooksPath`, `include.path`, `includeIf.*.path`, `url.*.insteadOf`, + `url.*.pushInsteadOf` — is denied with each value. `git config` is denied + when it sets one of these keys; to read or unset the key stays allowed. Command substitution output, a parameter that starts an argument, and a parameter in the value of an option that git writes to or runs as a command (`--output`, `--upload-pack`, `--receive-pack`, `--exec`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 2249e9b4c..f30048320 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -249,23 +249,43 @@ # as dangerous as a parameter here: git -c core.fsmonitor= status runs # the configured program while the subcommand stays nominally read-only. GIT_EXECUTED_CONFIG_KEYS = frozenset({ - "core.fsmonitor", "core.hookspath", "core.editor", "sequence.editor", - "core.sshcommand", "core.askpass", "core.pager", "interactive.difffilter", + "core.fsmonitor", "core.editor", "sequence.editor", "core.sshcommand", + "core.askpass", "core.pager", "core.gitproxy", "interactive.difffilter", }) GIT_EXECUTED_CONFIG_SECTIONS = { "filter": (".clean", ".smudge", ".process", ".command"), "diff": (".command", ".textconv", ".external"), "merge": (".driver",), + "remote": (".uploadpack", ".receivepack"), } +# Keys whose value is a path or URL git loads code through: an included file +# can set every key above, and a rewritten URL can retarget the transport. +# No value of these is a disabling form. +GIT_LOADED_CONFIG_KEYS = frozenset({"core.hookspath", "include.path"}) +GIT_LOADED_CONFIG_SECTIONS = { + "includeif": (".path",), + "url": (".insteadof", ".pushinsteadof"), +} +GIT_CONFIG_DISABLING_VALUES = frozenset({"", "false"}) -def executed_config_key(key): - """Whether a -c/--config key's value is a program git runs.""" - if key in GIT_EXECUTED_CONFIG_KEYS: +def config_key_in(key, keys, sections): + if key in keys: return True section, _, remainder = key.partition(".") - suffixes = GIT_EXECUTED_CONFIG_SECTIONS.get(section, ()) - return bool(remainder) and key.endswith(tuple(suffixes)) + return bool(remainder) and key.endswith(tuple(sections.get(section, ()))) + + +def loaded_config_key(key): + """Whether a config key's value is a path or URL git loads code through.""" + return config_key_in(key, GIT_LOADED_CONFIG_KEYS, GIT_LOADED_CONFIG_SECTIONS) + + +def executed_config_key(key): + """Whether a config key's value is a program git runs or code it loads.""" + return loaded_config_key(key) or config_key_in( + key, GIT_EXECUTED_CONFIG_KEYS, GIT_EXECUTED_CONFIG_SECTIONS + ) # Real read-only git options that are a strict prefix of a denied option name. @@ -2450,13 +2470,18 @@ def git_command(segment, assignments=None): value = arguments[index] index += 1 if option in {"-c", "--config-env"}: - config_key = str(value).split("=", 1)[0].casefold() + config_key, _, config_value = str(value).partition("=") + config_key = config_key.casefold() if config_key.startswith("alias."): raise ValueError( f"git -c {config_key} defines an alias the guard cannot inspect; " "run the underlying git command directly" ) - if executed_config_key(config_key): + if executed_config_key(config_key) and not ( + option == "-c" + and not loaded_config_key(config_key) + and config_value.casefold() in GIT_CONFIG_DISABLING_VALUES + ): raise ValueError( f"git -c {config_key} configures a program git executes; " "the guard cannot allow it" @@ -2471,6 +2496,15 @@ def git_command(segment, assignments=None): if index >= len(arguments): return None subcommand = arguments[index].casefold() + if subcommand == "config": + # A key followed by another argument is a set form; a key in last + # position only reads or unsets it. + for argument in arguments[index + 1:-1]: + if executed_config_key(argument.casefold()): + raise ValueError( + f"git config {argument} persists a program git executes; " + "the guard cannot allow it" + ) read_only_subcommands = ARCHIVE_READ_GIT_COMMANDS | CLOSED_READ_GIT_COMMANDS if subcommand in read_only_subcommands: # Read-only subcommands still write through --output. A parameter diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 9fc4d448c..a0ff0cea7 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2268,6 +2268,23 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git -c filter.driver.clean=/tmp/clean.sh show HEAD:file', 'git -c merge.rewriter.driver=/tmp/merge.sh merge topic', 'git --config-env=core.fsmonitor=FSM status', + 'git -c core.fsmonitor=true status', + 'git --config-env=core.fsmonitor=FALSE_VALUE status', + # Keys that load code through a path or URL have no disabling value. + 'git -c include.path=/tmp/cfg status', + 'git -c includeIf.gitdir:/tmp/.path=/tmp/cfg status', + 'git -c core.hooksPath= status', + 'git -c remote.o.uploadpack=/tmp/h.sh ls-remote o', + 'git -c remote.o.receivepack=/tmp/h.sh ls-remote o', + 'git -c core.gitProxy=/tmp/h.sh ls-remote git://example.invalid/x', + 'git -c url.x.insteadOf=y ls-remote x', + 'git -c url.x.pushInsteadOf=y ls-remote x', + # The persisted form runs the program on each later plain git. + 'git config core.fsmonitor /tmp/h.sh', + 'git config core.fsmonitor /tmp/h.sh && git status', + 'git config --local core.hooksPath /tmp/hooks', + 'git config --add include.path /tmp/cfg', + 'git config set core.fsmonitor /tmp/h.sh', 'git log --format="$x[@] %H"', "git diff --output=$f", # Unbraced zsh expansion flags match no parameter syntax, so an @@ -2320,6 +2337,16 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "git -c user.name=Test log -1", "git -c core.autocrlf=false status", 'git -c "commit.gpgsign=false" log -1', + # Disabling forms of a program key run no program. + "git -c core.fsmonitor=false status", + "git -c core.fsmonitor=FALSE status", + "git -c core.fsmonitor status", + "git -c filter.lfs.smudge= -c filter.lfs.process= checkout main", + # Reading or unsetting a persisted key writes no program. + "git config --unset core.fsmonitor", + "git config unset core.fsmonitor", + "git config --get core.fsmonitor", + "git config user.name Test", ): with self.subTest(command=command): self.assert_allowed(self.bash(command)) From f5dc8739e7f5d62a5721c3dde91945ebccff3c72 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 17:23:03 -0500 Subject: [PATCH 23/37] no-mistakes(review): parse git config modes; .git/config append stays open --- HOOKS.md | 3 ++ scripts/guard_hook.py | 83 ++++++++++++++++++++++++++++++++++++---- tests/test_guard_hook.py | 33 ++++++++++++++++ 3 files changed, 111 insertions(+), 8 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index b6be0b9b4..74f2b6013 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -196,6 +196,9 @@ See [UPDATE.md](UPDATE.md). `core.hooksPath`, `include.path`, `includeIf.*.path`, `url.*.insteadOf`, `url.*.pushInsteadOf` — is denied with each value. `git config` is denied when it sets one of these keys; to read or unset the key stays allowed. + `git config` section renames and removals (`--rename-section`, + `--remove-section`) and `--edit` are denied, because a rename can move a + key into an executed section and `--edit` starts an editor. Command substitution output, a parameter that starts an argument, and a parameter in the value of an option that git writes to or runs as a command (`--output`, `--upload-pack`, `--receive-pack`, `--exec`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index f30048320..b907ade8f 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -305,6 +305,75 @@ def git_option_in(token, options): if name.startswith("--") and len(name) > 2: return any(option.startswith(name) for option in options) return name in options +GIT_CONFIG_READ_MODES = frozenset({ + "--get", "--get-all", "--get-regexp", "--get-urlmatch", "--get-color", + "--get-colorbool", "-l", "--list", "--unset", "--unset-all", +}) +GIT_CONFIG_READ_SUBCOMMANDS = frozenset({"get", "unset", "list"}) +GIT_CONFIG_DENIED_MODES = frozenset({"--rename-section", "--remove-section", "--edit"}) +GIT_CONFIG_DENIED_SUBCOMMANDS = frozenset({"rename-section", "remove-section", "edit"}) +GIT_CONFIG_VALUE_OPTIONS = frozenset({ + "--file", "--blob", "--type", "--default", "--comment", "--value", "--url", +}) +GIT_CONFIG_EDIT_SHORT_OPTION = re.compile(r"-[A-Za-z]*e[A-Za-z]*") + + +def denied_git_config_argument(arguments): + """The git config argument that persists or runs a program git executes. + + A section rename can carry a key into an executed section and --edit runs + the editor, so both are denied outright. Otherwise only a set denies: an + executed key with a value operand after it and no read or unset mode. + Redirections are dropped; a key hidden in one fails closed. + """ + kept, redirected = [], [] + index = 0 + while index < len(arguments): + token = arguments[index] + if ( + set(token) <= SHELL_WRITE_REDIRECTION_CHARS + and ("<" in token or ">" in token) + ): + if kept and kept[-1].isdigit(): + redirected.append(kept.pop()) + redirected.extend(arguments[index + 1:index + 2]) + index += 2 + continue + kept.append(token) + index += 1 + separator = kept.index("--") if "--" in kept else len(kept) + options = kept[:separator] + for token in options: + if ( + git_option_in(token, GIT_CONFIG_DENIED_MODES) + or GIT_CONFIG_EDIT_SHORT_OPTION.fullmatch(token) + or token.casefold() in GIT_CONFIG_DENIED_SUBCOMMANDS + ): + return token + for token in redirected: + if executed_config_key(token.casefold()): + return token + if options[:1] and options[0].casefold() in GIT_CONFIG_READ_SUBCOMMANDS: + return None + for position, token in enumerate(options): + # A mode word that is the value of the option before it is no mode. + if token in GIT_CONFIG_READ_MODES and not ( + position + and ( + options[position - 1] == "-f" + or git_option_in(options[position - 1], GIT_CONFIG_VALUE_OPTIONS) + ) + ): + return None + operands = [ + token for token in options if not token.startswith("-") + ] + kept[separator + 1:] + for token in operands[:-1]: + if executed_config_key(token.casefold()): + return token + return None + + ARCHIVE_READ_EXECUTION_OPTIONS = {"rg": frozenset({"--pre"})} AMBIGUOUS_SHELL_SYNTAX = re.compile(r"[\r\n|;&<>`]|\$\(|@\(") SUBSTITUTION_PLACEHOLDER = "COMMAND_SUBSTITUTION_" @@ -2497,14 +2566,12 @@ def git_command(segment, assignments=None): return None subcommand = arguments[index].casefold() if subcommand == "config": - # A key followed by another argument is a set form; a key in last - # position only reads or unsets it. - for argument in arguments[index + 1:-1]: - if executed_config_key(argument.casefold()): - raise ValueError( - f"git config {argument} persists a program git executes; " - "the guard cannot allow it" - ) + denied = denied_git_config_argument(arguments[index + 1:]) + if denied is not None: + raise ValueError( + f"git config {denied} persists a program git executes; " + "the guard cannot allow it" + ) read_only_subcommands = ARCHIVE_READ_GIT_COMMANDS | CLOSED_READ_GIT_COMMANDS if subcommand in read_only_subcommands: # Read-only subcommands still write through --output. A parameter diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index a0ff0cea7..1ecb790bb 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2285,6 +2285,25 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git config --local core.hooksPath /tmp/hooks', 'git config --add include.path /tmp/cfg', 'git config set core.fsmonitor /tmp/h.sh', + 'git config core.fsmonitor /tmp/h.sh >/dev/null', + 'git config >/dev/null core.fsmonitor /tmp/h.sh', + 'git config --replace-all core.fsmonitor /tmp/h.sh', + 'git config --type path core.fsmonitor /tmp/h.sh', + # A mode word that git reads as a value or operand is no read mode. + 'git config --file --get core.fsmonitor /tmp/h.sh', + 'git config --global set core.fsmonitor /tmp/h.sh', + 'git config core.fsmonitor /tmp/h.sh -- --get', + # A section rename can carry a key into an executed section, and + # --edit launches the editor. + 'git config foo.fsmonitor /tmp/h.sh && git config --rename-section foo core', + 'git config --rename-section foo core', + 'git config --rename-sec foo core', + 'git config rename-section foo core', + 'git config --remove-section foo', + 'git config remove-section foo', + 'git config --edit', + 'git config --global -e', + 'git config edit', 'git log --format="$x[@] %H"', "git diff --output=$f", # Unbraced zsh expansion flags match no parameter syntax, so an @@ -2346,6 +2365,20 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "git config --unset core.fsmonitor", "git config unset core.fsmonitor", "git config --get core.fsmonitor", + "git config core.fsmonitor", + "git config --get core.fsmonitor 2>/dev/null", + "git config --get core.hooksPath 2>/dev/null || true", + "git config --get-all core.hooksPath 2>&1", + "git config get core.editor 2>/dev/null", + "git config --get core.fsmonitor --show-origin", + "git config --get-urlmatch core.fsmonitor https://x", + "git config --global --get core.editor", + "git config --unset core.fsmonitor 2>/dev/null", + "git config --unset-all core.fsmonitor", + "git config unset core.fsmonitor 2>/dev/null", + "git config --list", + "git config -l > config.txt", + "git config list", "git config user.name Test", ): with self.subTest(command=command): From b65eabe119a49261e71a11622ee52ff8fa7c90f5 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 17:28:52 -0500 Subject: [PATCH 24/37] no-mistakes(review): deny gpg program keys and redirected git config sets --- HOOKS.md | 4 +++- scripts/guard_hook.py | 28 ++++++---------------------- tests/test_guard_hook.py | 12 ++++++++++++ 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 74f2b6013..dde858835 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -188,7 +188,7 @@ See [UPDATE.md](UPDATE.md). `core.fsmonitor`, editors and pagers, `core.sshCommand`, `core.askPass`, `filter.*` clean/smudge/process, `diff.*` textconv/external/command, `merge.*` drivers, `core.gitProxy`, - `remote.*` uploadpack/receivepack — is denied with a literal + `remote.*` uploadpack/receivepack, `gpg.program` and `gpg.*.program` — is denied with a literal value as with a parameter, because git runs it during an otherwise read-only subcommand. A disabling form of such a key passes: the bare key, an empty value, or `false`. A `--config-env` form is always denied, because @@ -196,6 +196,8 @@ See [UPDATE.md](UPDATE.md). `core.hooksPath`, `include.path`, `includeIf.*.path`, `url.*.insteadOf`, `url.*.pushInsteadOf` — is denied with each value. `git config` is denied when it sets one of these keys; to read or unset the key stays allowed. + A redirection after the key counts as a value, so use an explicit mode + (`git config --get 2>/dev/null`) when you redirect a read. `git config` section renames and removals (`--rename-section`, `--remove-section`) and `--edit` are denied, because a rename can move a key into an executed section and `--edit` starts an editor. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index b907ade8f..00eb812b5 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -257,6 +257,7 @@ "diff": (".command", ".textconv", ".external"), "merge": (".driver",), "remote": (".uploadpack", ".receivepack"), + "gpg": (".program",), } # Keys whose value is a path or URL git loads code through: an included file # can set every key above, and a rewritten URL can retarget the transport. @@ -324,25 +325,11 @@ def denied_git_config_argument(arguments): A section rename can carry a key into an executed section and --edit runs the editor, so both are denied outright. Otherwise only a set denies: an executed key with a value operand after it and no read or unset mode. - Redirections are dropped; a key hidden in one fails closed. + The tokens carry no quoting or adjacency, so a redirection is not told + apart from an operand: each token counts, which only fails closed. """ - kept, redirected = [], [] - index = 0 - while index < len(arguments): - token = arguments[index] - if ( - set(token) <= SHELL_WRITE_REDIRECTION_CHARS - and ("<" in token or ">" in token) - ): - if kept and kept[-1].isdigit(): - redirected.append(kept.pop()) - redirected.extend(arguments[index + 1:index + 2]) - index += 2 - continue - kept.append(token) - index += 1 - separator = kept.index("--") if "--" in kept else len(kept) - options = kept[:separator] + separator = arguments.index("--") if "--" in arguments else len(arguments) + options = arguments[:separator] for token in options: if ( git_option_in(token, GIT_CONFIG_DENIED_MODES) @@ -350,9 +337,6 @@ def denied_git_config_argument(arguments): or token.casefold() in GIT_CONFIG_DENIED_SUBCOMMANDS ): return token - for token in redirected: - if executed_config_key(token.casefold()): - return token if options[:1] and options[0].casefold() in GIT_CONFIG_READ_SUBCOMMANDS: return None for position, token in enumerate(options): @@ -367,7 +351,7 @@ def denied_git_config_argument(arguments): return None operands = [ token for token in options if not token.startswith("-") - ] + kept[separator + 1:] + ] + arguments[separator + 1:] for token in operands[:-1]: if executed_config_key(token.casefold()): return token diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 1ecb790bb..570b7ad69 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2286,6 +2286,18 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git config --add include.path /tmp/cfg', 'git config set core.fsmonitor /tmp/h.sh', 'git config core.fsmonitor /tmp/h.sh >/dev/null', + # A digit before a redirection is a real operand, and a quoted + # operator is a real value: both are a set. + 'git config core.hooksPath 5 >/dev/null', + 'git config core.hooksPath 5 >> log.txt', + 'git config include.path 5 > /dev/null && git status', + "git config core.hooksPath '>' x", + # Signature verification runs the configured program in a read. + 'git -c gpg.program=/tmp/h.sh log --show-signature', + 'git -c gpg.ssh.program=/tmp/h.sh log --show-signature', + 'git -c gpg.x509.program=/tmp/h.sh log --show-signature', + 'git -c log.showSignature=true -c gpg.program=/tmp/h.sh log', + 'git config gpg.program /tmp/h.sh', 'git config >/dev/null core.fsmonitor /tmp/h.sh', 'git config --replace-all core.fsmonitor /tmp/h.sh', 'git config --type path core.fsmonitor /tmp/h.sh', From 24c1c2b02ff1784308d581b95c164ab3ce8eb4b9 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 17:34:40 -0500 Subject: [PATCH 25/37] no-mistakes(review): allow redirected git config reads, keep digit sets denied --- HOOKS.md | 5 ++-- scripts/guard_hook.py | 53 ++++++++++++++++++++++++++++++++++++---- tests/test_guard_hook.py | 13 ++++++++++ 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index dde858835..659aadad4 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -196,8 +196,9 @@ See [UPDATE.md](UPDATE.md). `core.hooksPath`, `include.path`, `includeIf.*.path`, `url.*.insteadOf`, `url.*.pushInsteadOf` — is denied with each value. `git config` is denied when it sets one of these keys; to read or unset the key stays allowed. - A redirection after the key counts as a value, so use an explicit mode - (`git config --get 2>/dev/null`) when you redirect a read. + A redirection at the end of a read (`git config 2>/dev/null`) is not + a value. A separate digit before a redirection (`5 >/dev/null`) and a quoted + operator are values, so they make a set. `git config` section renames and removals (`--rename-section`, `--remove-section`) and `--edit` are denied, because a rename can move a key into an executed section and `--edit` starts an editor. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 00eb812b5..46dedda8f 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -325,9 +325,23 @@ def denied_git_config_argument(arguments): A section rename can carry a key into an executed section and --edit runs the editor, so both are denied outright. Otherwise only a set denies: an executed key with a value operand after it and no read or unset mode. - The tokens carry no quoting or adjacency, so a redirection is not told - apart from an operand: each token counts, which only fails closed. + Only a trailing redirection is dropped: the operator, its target and a + file descriptor digit written against it. A separate digit (5 >file) and a + quoted operator are operands. """ + arguments = list(arguments) + while ( + len(arguments) >= 2 + and getattr(arguments[-2], "redirection", False) + and not arguments[-2].isdigit() + ): + del arguments[-2:] + if ( + arguments + and getattr(arguments[-1], "redirection", False) + and arguments[-1].isdigit() + ): + del arguments[-1] separator = arguments.index("--") if "--" in arguments else len(arguments) options = arguments[:separator] for token in options: @@ -2153,9 +2167,14 @@ def decode_patch_path(raw_path, strip_prefix): class ShellToken(str): - """A token that keeps, per parameter marker character, whether double quotes enclosed it.""" + """A token that keeps, per parameter marker character, whether double quotes enclosed it. + + `redirection` is true for an unquoted redirection operator and for a file + descriptor digit written against one (the 2 of 2>file, not of 2 >file). + """ double_quoted = () + redirection = False def double_quoted_markers(command): @@ -2267,8 +2286,29 @@ def shell_tokens(command, direct=True): lexer.whitespace = " \t" lexer.whitespace_split = True lexer.commenters = "" + tokens = [] try: - tokens = list(lexer) + for token in lexer: + end = lexer.instream.tell() - len(lexer._pushback_chars) + if command[end - 1:end] in (" ", "\t"): + end -= 1 + start = end - len(token) + before = command[start - 1:start] if start > 0 else "" + if command[start:end] == token and ( + ( + set(token) <= SHELL_WRITE_REDIRECTION_CHARS + and ("<" in token or ">" in token) + and before != "\\" + ) + or ( + token.isdigit() + and command[end:end + 1] in ("<", ">") + and (not before or before in " \t|;&()<>\n\r") + ) + ): + token = ShellToken(token) + token.redirection = True + tokens.append(token) except ValueError as error: raise ValueError( f"shell command cannot be tokenized ({error}); balance the quotes " @@ -2284,7 +2324,9 @@ def shell_tokens(command, direct=True): marked = [] for token in tokens: count = sum(character in PARAMETER_MARKERS for character in token) + redirection = getattr(token, "redirection", False) token = ShellToken(token) + token.redirection = redirection token.double_quoted, flags = tuple(flags[:count]), flags[count:] marked.append(token) return marked @@ -2504,7 +2546,8 @@ def git_command(segment, assignments=None): or SUBSTITUTION_PLACEHOLDER in expanded ): unproven_parameters.append(argument) - resolved_arguments.append(expand_environment_parameters(argument, assignments)) + expanded = expand_environment_parameters(argument, assignments) + resolved_arguments.append(argument if expanded == argument else expanded) argument_index += 1 arguments = resolved_arguments index = 0 diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 570b7ad69..bcecae2f6 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2290,6 +2290,10 @@ def test_allows_parameters_in_read_only_git_arguments(self): # operator is a real value: both are a set. 'git config core.hooksPath 5 >/dev/null', 'git config core.hooksPath 5 >> log.txt', + 'git config core.fsmonitor 2 >/dev/null', + 'git config core.hooksPath 5 2>/dev/null', + "git config core.hooksPath '2'>/dev/null", + 'git config core.hooksPath \\> x', 'git config include.path 5 > /dev/null && git status', "git config core.hooksPath '>' x", # Signature verification runs the configured program in a read. @@ -2378,6 +2382,15 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "git config unset core.fsmonitor", "git config --get core.fsmonitor", "git config core.fsmonitor", + # A redirection after a bare read is no value. + "git config core.fsmonitor 2>/dev/null", + "git config core.hooksPath 2>/dev/null || true", + "git config --global core.editor 2>/dev/null", + "git config gpg.program 2>/dev/null", + "git config core.fsmonitor >/dev/null", + "git config core.fsmonitor > out.txt", + "git config core.fsmonitor < /dev/null", + "git config core.fsmonitor >/dev/null 2>&1", "git config --get core.fsmonitor 2>/dev/null", "git config --get core.hooksPath 2>/dev/null || true", "git config --get-all core.hooksPath 2>&1", From 30e8bddf91573ae1c19e49138d4981461fcfd148 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 17:50:12 -0500 Subject: [PATCH 26/37] no-mistakes(review): treat only one ASCII digit as redirection file descriptor --- HOOKS.md | 5 +++-- scripts/guard_hook.py | 8 +++++--- tests/test_guard_hook.py | 6 ++++++ 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 659aadad4..54688310f 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -197,8 +197,9 @@ See [UPDATE.md](UPDATE.md). `url.*.pushInsteadOf` — is denied with each value. `git config` is denied when it sets one of these keys; to read or unset the key stays allowed. A redirection at the end of a read (`git config 2>/dev/null`) is not - a value. A separate digit before a redirection (`5 >/dev/null`) and a quoted - operator are values, so they make a set. + a value. A separate digit before a redirection (`5 >/dev/null`), a word of two + or more digits (`12>/dev/null`) and a quoted operator are values, so they + make a set. `git config` section renames and removals (`--rename-section`, `--remove-section`) and `--edit` are denied, because a rename can move a key into an executed section and `--edit` starts an editor. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 46dedda8f..7f6d05804 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2169,8 +2169,9 @@ def decode_patch_path(raw_path, strip_prefix): class ShellToken(str): """A token that keeps, per parameter marker character, whether double quotes enclosed it. - `redirection` is true for an unquoted redirection operator and for a file - descriptor digit written against one (the 2 of 2>file, not of 2 >file). + `redirection` is true for an unquoted redirection operator and for one + ASCII file descriptor digit written against one (the 2 of 2>file, not of + 2 >file). zsh passes a longer or non-ASCII digit word on as an argument. """ double_quoted = () @@ -2301,7 +2302,8 @@ def shell_tokens(command, direct=True): and before != "\\" ) or ( - token.isdigit() + len(token) == 1 + and token in "0123456789" and command[end:end + 1] in ("<", ">") and (not before or before in " \t|;&()<>\n\r") ) diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index bcecae2f6..30aba4365 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2291,6 +2291,12 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git config core.hooksPath 5 >/dev/null', 'git config core.hooksPath 5 >> log.txt', 'git config core.fsmonitor 2 >/dev/null', + # zsh reads one ASCII digit as a file descriptor; a longer or + # non-ASCII digit word is the value. + 'git config core.hooksPath 12>/dev/null', + 'git config core.hooksPath 12>/dev/null && git commit -m x', + 'git config core.hooksPath \u0663>/dev/null', + 'git config core.hooksPath \u00b2>/dev/null', 'git config core.hooksPath 5 2>/dev/null', "git config core.hooksPath '2'>/dev/null", 'git config core.hooksPath \\> x', From 0697288b2801cddccb521bbc9a8b9209c25191d4 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 18:00:59 -0500 Subject: [PATCH 27/37] no-mistakes(review): deny non-literal git config arguments --- HOOKS.md | 5 +++++ scripts/guard_hook.py | 47 +++++++++++++++++++++++++++++++++++----- tests/test_guard_hook.py | 23 ++++++++++++++++++++ 3 files changed, 69 insertions(+), 6 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 54688310f..81186d183 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -200,6 +200,11 @@ See [UPDATE.md](UPDATE.md). a value. A separate digit before a redirection (`5 >/dev/null`), a word of two or more digits (`12>/dev/null`) and a quoted operator are values, so they make a set. + Each `git config` argument must be a plain literal word, bare or in one + pair of quotes: a backslash, mixed quotes, an unquoted glob (`*`, `?`, `[`), + an unquoted brace list or range (`{a,b}`, `{1..3}`) or a line continuation + that joins two words is denied, because the shell can make a different + argument list from it. `git config` section renames and removals (`--rename-section`, `--remove-section`) and `--edit` are denied, because a rename can move a key into an executed section and `--edit` starts an editor. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 7f6d05804..b19c0726e 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -317,6 +317,9 @@ def git_option_in(token, options): "--file", "--blob", "--type", "--default", "--comment", "--value", "--url", }) GIT_CONFIG_EDIT_SHORT_OPTION = re.compile(r"-[A-Za-z]*e[A-Za-z]*") +# An unquoted word the shell can turn into several arguments: a glob, or a +# brace list or range. +GIT_CONFIG_EXPANDING_WORD = re.compile(r"[*?\[]|\{.*(?:,|\.\.)") def denied_git_config_argument(arguments): @@ -2172,10 +2175,15 @@ class ShellToken(str): `redirection` is true for an unquoted redirection operator and for one ASCII file descriptor digit written against one (the 2 of 2>file, not of 2 >file). zsh passes a longer or non-ASCII digit word on as an argument. + `literal` is "bare" for a word written with no quote or escape, "quoted" + for a word inside one pair of plain quotes, and None when the shell may + build the word in another way (escapes, mixed quotes, a line continuation + that joins words). """ double_quoted = () redirection = False + literal = None def double_quoted_markers(command): @@ -2282,6 +2290,11 @@ def shell_tokens(command, direct=True): Only a directly parsed command keeps quote flags: an outer shell expands parameters before the inner parse, so inner quotes prove nothing. """ + joined = any( + (match.group(1) or command[max(match.start() - 1, 0):match.start()].strip()) + and command[match.end():match.end() + 1].strip() + for match in LINE_CONTINUATION.finditer(command) + ) command = strip_heredoc_bodies(LINE_CONTINUATION.sub(r"\1 ", command)) lexer = shlex.shlex(command, posix=True, punctuation_chars="|;&()<>\n\r") lexer.whitespace = " \t" @@ -2293,9 +2306,20 @@ def shell_tokens(command, direct=True): end = lexer.instream.tell() - len(lexer._pushback_chars) if command[end - 1:end] in (" ", "\t"): end -= 1 + literal = None + for quote in ("", "'", '"'): + start = end - len(token) - 2 * len(quote) + if ( + not joined + and start >= 0 + and command[start:end] == quote + token + quote + and (start == 0 or command[start - 1] in " \t|;&()<>\n\r") + ): + literal = "quoted" if quote else "bare" + break start = end - len(token) before = command[start - 1:start] if start > 0 else "" - if command[start:end] == token and ( + redirection = command[start:end] == token and ( ( set(token) <= SHELL_WRITE_REDIRECTION_CHARS and ("<" in token or ">" in token) @@ -2307,9 +2331,10 @@ def shell_tokens(command, direct=True): and command[end:end + 1] in ("<", ">") and (not before or before in " \t|;&()<>\n\r") ) - ): - token = ShellToken(token) - token.redirection = True + ) + token = ShellToken(token) + token.redirection = redirection + token.literal = literal tokens.append(token) except ValueError as error: raise ValueError( @@ -2326,9 +2351,9 @@ def shell_tokens(command, direct=True): marked = [] for token in tokens: count = sum(character in PARAMETER_MARKERS for character in token) - redirection = getattr(token, "redirection", False) + redirection, literal = token.redirection, token.literal token = ShellToken(token) - token.redirection = redirection + token.redirection, token.literal = redirection, literal token.double_quoted, flags = tuple(flags[:count]), flags[count:] marked.append(token) return marked @@ -2595,6 +2620,16 @@ def git_command(segment, assignments=None): return None subcommand = arguments[index].casefold() if subcommand == "config": + for argument in arguments[index + 1:]: + literal = getattr(argument, "literal", None) + if not getattr(argument, "redirection", False) and ( + literal is None + or (literal == "bare" and GIT_CONFIG_EXPANDING_WORD.search(argument)) + ): + raise ValueError( + f"git argument {argument} cannot be resolved by the guard; " + "pass a literal value" + ) denied = denied_git_config_argument(arguments[index + 1:]) if denied is not None: raise ValueError( diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 30aba4365..1e3d5dd7e 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2291,6 +2291,21 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git config core.hooksPath 5 >/dev/null', 'git config core.hooksPath 5 >> log.txt', 'git config core.fsmonitor 2 >/dev/null', + # One token can become several git arguments, or two tokens + # one: only plain literal words prove the read or set form. + 'git config {core.hooksPath,/tmp/hooks}', + 'git config core.hooksPath{,/tmp/hooks}', + 'git config {core.fsmonitor,/abs/h.sh}', + 'git config {--add,core.fsmonitor,/abs/h.sh}', + 'git config core.fsmoni\\\ntor /tmp/h.sh', + 'git config "core.fsmoni\\\ntor" /tmp/h.sh', + 'git config core.hooksPath\\ x', + 'git config user.name Jeremy\\ M', + "git config --get core.fsmoni''tor", + 'git config --get-regexp core.*', + 'git config --get core.[f]smonitor', + 'git config user.{name,email}', + 'git config branch.{1..3}.merge', # zsh reads one ASCII digit as a file descriptor; a longer or # non-ASCII digit word is the value. 'git config core.hooksPath 12>/dev/null', @@ -2411,6 +2426,14 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "git config -l > config.txt", "git config list", "git config user.name Test", + # Plain quoted words are literal, with the characters in them. + 'git config user.name "Test User"', + "git config user.name 'Test {a,b} *'", + "git config --get 'core.fsmonitor'", + 'git config --get "core.hooksPath" 2>/dev/null', + 'git config user.name ""', + "git config --get-regexp '^user\\.'", + "git config \\\n --get core.fsmonitor", ): with self.subTest(command=command): self.assert_allowed(self.bash(command)) From afc43faf72887eee97d69f74fdcffd131d175978 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 18:06:43 -0500 Subject: [PATCH 28/37] no-mistakes(review): require literal git subcommand, deny comments in git config --- HOOKS.md | 6 ++++- scripts/guard_hook.py | 47 ++++++++++++++++++++++++++++++++-------- tests/test_guard_hook.py | 14 ++++++++++++ 3 files changed, 57 insertions(+), 10 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 81186d183..483d3a33c 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -203,8 +203,12 @@ See [UPDATE.md](UPDATE.md). Each `git config` argument must be a plain literal word, bare or in one pair of quotes: a backslash, mixed quotes, an unquoted glob (`*`, `?`, `[`), an unquoted brace list or range (`{a,b}`, `{1..3}`) or a line continuation - that joins two words is denied, because the shell can make a different + in the word or one that joins it to the next word is denied, because the shell can make a different argument list from it. + An unquoted word that starts with `#` among the `git config` arguments + is denied, because the shell reads it and the words after it as a comment. + The git subcommand word must be a plain literal word in each git command: + `git {config,...}` and a subcommand split by a line continuation are denied. `git config` section renames and removals (`--rename-section`, `--remove-section`) and `--edit` are denied, because a rename can move a key into an executed section and `--edit` starts an editor. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index b19c0726e..5315c5692 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -2178,7 +2178,7 @@ class ShellToken(str): `literal` is "bare" for a word written with no quote or escape, "quoted" for a word inside one pair of plain quotes, and None when the shell may build the word in another way (escapes, mixed quotes, a line continuation - that joins words). + in the word or one that joins it to the next word). """ double_quoted = () @@ -2290,12 +2290,27 @@ def shell_tokens(command, direct=True): Only a directly parsed command keeps quote flags: an outer shell expands parameters before the inner parse, so inner quotes prove nothing. """ - joined = any( - (match.group(1) or command[max(match.start() - 1, 0):match.start()].strip()) - and command[match.end():match.end() + 1].strip() - for match in LINE_CONTINUATION.finditer(command) - ) - command = strip_heredoc_bodies(LINE_CONTINUATION.sub(r"\1 ", command)) + breaks = set() + if LINE_CONTINUATION.search(command): + mark = next( + character for character in map(chr, range(0xE000, 0xF900)) + if character not in command + ) + marked = strip_heredoc_bodies( + LINE_CONTINUATION.sub(lambda match: match.group(1) + mark, command) + ) + command = strip_heredoc_bodies(LINE_CONTINUATION.sub(r"\1 ", command)) + breaks = ( + {index for index, character in enumerate(marked) if character == mark} + if marked.replace(mark, " ") == command + else None + ) + else: + command = strip_heredoc_bodies(command) + joins = { + index for index in breaks or () + if command[max(index - 1, 0):index].strip() and command[index + 1:index + 2].strip() + } lexer = shlex.shlex(command, posix=True, punctuation_chars="|;&()<>\n\r") lexer.whitespace = " \t" lexer.whitespace_split = True @@ -2310,10 +2325,13 @@ def shell_tokens(command, direct=True): for quote in ("", "'", '"'): start = end - len(token) - 2 * len(quote) if ( - not joined + breaks is not None and start >= 0 and command[start:end] == quote + token + quote and (start == 0 or command[start - 1] in " \t|;&()<>\n\r") + and breaks.isdisjoint(range(start, end)) + and end not in joins + and start - 1 not in joins ): literal = "quoted" if quote else "bare" break @@ -2618,13 +2636,24 @@ def git_command(segment, assignments=None): git_options.extend((option, str(value))) if index >= len(arguments): return None + subcommand_literal = getattr(arguments[index], "literal", "bare") + if subcommand_literal is None or ( + subcommand_literal == "bare" and GIT_CONFIG_EXPANDING_WORD.search(arguments[index]) + ): + raise ValueError( + f"git argument {arguments[index]} cannot be resolved by the guard; " + "pass a literal subcommand" + ) subcommand = arguments[index].casefold() if subcommand == "config": for argument in arguments[index + 1:]: literal = getattr(argument, "literal", None) if not getattr(argument, "redirection", False) and ( literal is None - or (literal == "bare" and GIT_CONFIG_EXPANDING_WORD.search(argument)) + or ( + literal == "bare" + and (argument.startswith("#") or GIT_CONFIG_EXPANDING_WORD.search(argument)) + ) ): raise ValueError( f"git argument {argument} cannot be resolved by the guard; " diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 1e3d5dd7e..fe6685724 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2293,6 +2293,15 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git config core.fsmonitor 2 >/dev/null', # One token can become several git arguments, or two tokens # one: only plain literal words prove the read or set form. + # The subcommand word must be one literal word too. + 'git {config,core.fsmonitor,/abs/h.sh}', + 'git con\\\nfig core.fsmonitor /abs/h.sh', + 'git --no-pager {config,core.fsmonitor,/abs/h.sh}', + "git con''fig core.fsmonitor /abs/h.sh", + # Comment text is no argument: a mode word in it proves nothing. + 'git config core.fsmonitor /tmp/h.sh # --get', + 'git config core.fsmonitor /tmp/h.sh #--get', + 'git config --get core.fsmonitor # note', 'git config {core.hooksPath,/tmp/hooks}', 'git config core.hooksPath{,/tmp/hooks}', 'git config {core.fsmonitor,/abs/h.sh}', @@ -2434,6 +2443,11 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): 'git config user.name ""', "git config --get-regexp '^user\\.'", "git config \\\n --get core.fsmonitor", + # A joined word in another command leaves this one literal. + "echo a\\\nb; git config user.name x", + "git config user.name '#x'", + "git \\\n config --get user.name", + 'git "config" --get user.name', ): with self.subTest(command=command): self.assert_allowed(self.bash(command)) From cc2620beecb7c41ff23b461d2168a714c5b6d9ce Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 18:16:18 -0500 Subject: [PATCH 29/37] no-mistakes(review): deny joined continuations and expanding git option values --- HOOKS.md | 5 +++ scripts/guard_hook.py | 75 +++++++++++++++++++++++++++++++--------- tests/test_guard_hook.py | 25 ++++++++++++++ 3 files changed, 88 insertions(+), 17 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 483d3a33c..5978d781d 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -209,6 +209,11 @@ See [UPDATE.md](UPDATE.md). is denied, because the shell reads it and the words after it as a comment. The git subcommand word must be a plain literal word in each git command: `git {config,...}` and a subcommand split by a line continuation are denied. + The value word of a git global option (`-c`, `-C`, `--git-dir`, + `--work-tree`, `--namespace`, `--config-env`) must resolve to one word: a + backslash, an unquoted glob or an unquoted brace list or range in it is + denied (`git -c {core.fsmonitor=/abs/h.sh,status}`). Plain quoted parts + pass (`git -c user.name='A B' log`). `git config` section renames and removals (`--rename-section`, `--remove-section`) and `--edit` are denied, because a rename can move a key into an executed section and `--edit` starts an editor. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 5315c5692..3930e97a3 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -319,6 +319,8 @@ def git_option_in(token, options): GIT_CONFIG_EDIT_SHORT_OPTION = re.compile(r"-[A-Za-z]*e[A-Za-z]*") # An unquoted word the shell can turn into several arguments: a glob, or a # brace list or range. +PLAIN_QUOTED_SEGMENT = re.compile(r"'[^']*'|\"[^\"]*\"") +ANSI_QUOTE_START = re.compile(r"\$['\"]") GIT_CONFIG_EXPANDING_WORD = re.compile(r"[*?\[]|\{.*(?:,|\.\.)") @@ -2176,9 +2178,10 @@ class ShellToken(str): ASCII file descriptor digit written against one (the 2 of 2>file, not of 2 >file). zsh passes a longer or non-ASCII digit word on as an argument. `literal` is "bare" for a word written with no quote or escape, "quoted" - for a word inside one pair of plain quotes, and None when the shell may - build the word in another way (escapes, mixed quotes, a line continuation - in the word or one that joins it to the next word). + for a word inside one pair of plain quotes, "mixed" for plain quoted and + unquoted parts where no unquoted part can expand, and None when the shell + may build the word in another way (escapes, an expanding unquoted part, a + line continuation in the word or one that joins it to the next word). """ double_quoted = () @@ -2307,21 +2310,48 @@ def shell_tokens(command, direct=True): ) else: command = strip_heredoc_bodies(command) - joins = { - index for index in breaks or () - if command[max(index - 1, 0):index].strip() and command[index + 1:index + 2].strip() - } + joins = set() + for index in breaks or (): + left, right = index - 1, index + 1 + while left >= 0 and marked[left] == mark: + left -= 1 + while right < len(marked) and marked[right] == mark: + right += 1 + if left >= 0 and marked[left].strip() and marked[right:right + 1].strip(): + joins.add(index) lexer = shlex.shlex(command, posix=True, punctuation_chars="|;&()<>\n\r") lexer.whitespace = " \t" lexer.whitespace_split = True lexer.commenters = "" tokens = [] + previous_end = 0 try: for token in lexer: end = lexer.instream.tell() - len(lexer._pushback_chars) if command[end - 1:end] in (" ", "\t"): end -= 1 + word_start = previous_end + while word_start < end and command[word_start] in " \t": + word_start += 1 + previous_end = end + word = command[word_start:end] literal = None + if ( + breaks is not None + and "\\" not in word + and not ANSI_QUOTE_START.search(word) + and breaks.isdisjoint(range(word_start, end)) + and end not in joins + and word_start - 1 not in joins + and PLAIN_QUOTED_SEGMENT.sub(lambda match: match.group(0)[1:-1], word) == token + ): + unquoted = PLAIN_QUOTED_SEGMENT.sub("", word) + if ( + "'" not in unquoted + and '"' not in unquoted + and not GIT_CONFIG_EXPANDING_WORD.search(unquoted) + ): + literal = "mixed" for quote in ("", "'", '"'): start = end - len(token) - 2 * len(quote) if ( @@ -2514,6 +2544,20 @@ def command_invocation(segment): return executable, arguments +def require_resolved_git_word(word, mixed=True): + """Deny a git word the shell can turn into other or several arguments.""" + literal = getattr(word, "literal", "bare") + if ( + literal is None + or (literal == "mixed" and not mixed) + or (literal == "bare" and GIT_CONFIG_EXPANDING_WORD.search(word)) + ): + raise ValueError( + f"git argument {word} cannot be resolved by the guard; " + "pass a literal value" + ) + + def git_command(segment, assignments=None): invocation = command_invocation(segment) if invocation is None: @@ -2531,6 +2575,7 @@ def git_command(segment, assignments=None): if argument == "-C" and argument_index + 1 < len(arguments): argument_index += 1 directory = arguments[argument_index] + require_resolved_git_word(directory) expanded = expand_environment_parameters(directory, assignments) if ( SHELL_PARAMETER_SYNTAX.search(expanded) @@ -2550,6 +2595,7 @@ def git_command(segment, assignments=None): ) resolved_arguments.extend((argument, expanded)) elif attached_directory: + require_resolved_git_word(argument) directory = argument[2:] expanded = expand_environment_parameters(directory, assignments) if ( @@ -2608,8 +2654,10 @@ def git_command(segment, assignments=None): if option in GIT_GLOBAL_OPTIONS_WITH_VALUES and value is None: if index >= len(arguments): raise ValueError(f"git option {option} lacks a value") - value = arguments[index] + value = token = arguments[index] index += 1 + if option in GIT_GLOBAL_OPTIONS_WITH_VALUES and option != "-C": + require_resolved_git_word(token) if option in {"-c", "--config-env"}: config_key, _, config_value = str(value).partition("=") config_key = config_key.casefold() @@ -2636,20 +2684,13 @@ def git_command(segment, assignments=None): git_options.extend((option, str(value))) if index >= len(arguments): return None - subcommand_literal = getattr(arguments[index], "literal", "bare") - if subcommand_literal is None or ( - subcommand_literal == "bare" and GIT_CONFIG_EXPANDING_WORD.search(arguments[index]) - ): - raise ValueError( - f"git argument {arguments[index]} cannot be resolved by the guard; " - "pass a literal subcommand" - ) + require_resolved_git_word(arguments[index], mixed=False) subcommand = arguments[index].casefold() if subcommand == "config": for argument in arguments[index + 1:]: literal = getattr(argument, "literal", None) if not getattr(argument, "redirection", False) and ( - literal is None + literal in (None, "mixed") or ( literal == "bare" and (argument.startswith("#") or GIT_CONFIG_EXPANDING_WORD.search(argument)) diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index fe6685724..c4bdf2a88 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2293,6 +2293,25 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git config core.fsmonitor 2 >/dev/null', # One token can become several git arguments, or two tokens # one: only plain literal words prove the read or set form. + # A run of continuations joins the words on its two sides. + 'git config core.fsmoni\\\n\\\ntor /abs/h.sh', + 'git c\\\n\\\nonfig core.fsmonitor /abs/h.sh', + 'git con\\\n\\\n\\\nfig core.fsmonitor /abs/h.sh', + 'git con\\\r\nfig core.fsmonitor /abs/h.sh', + 'git con\\\r\n\\\r\nfig core.fsmonitor /abs/h.sh', + # The value word of a git global option must be one word. + 'git -c {core.fsmonitor=/abs/h.sh,status}', + 'git -c{core.fsmonitor=/abs/h.sh,status}', + 'git -c {a.b=c,config,core.fsmonitor,/abs/h.sh}', + 'git -C {.,config,core.fsmonitor,/abs/h.sh}', + 'git -C{.,config,core.fsmonitor,/abs/h.sh}', + 'git --git-dir {.git,config,core.fsmonitor,/abs/h.sh}', + 'git --git-dir={.git,config}', + 'git --work-tree {.,config,core.fsmonitor,/abs/h.sh}', + 'git --namespace {a,config,core.fsmonitor,/abs/h.sh}', + "git -c 'a.b=c'{,config,core.fsmonitor,/abs/h.sh}", + 'git -c user.name=A\\ B log -1', + 'git -C sub\\ dir status', # The subcommand word must be one literal word too. 'git {config,core.fsmonitor,/abs/h.sh}', 'git con\\\nfig core.fsmonitor /abs/h.sh', @@ -2400,6 +2419,12 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): # settings keep working (see the literal-value cases in the test above). for command in ( "git -c user.name=Test log -1", + "git -c user.name='A B' log -1", + 'git -c "user.name=A B" log -1', + "git -c user.name='{a,b} *' log -1", + 'git -C . status', + 'git -C "." status', + "git --git-dir=.git status", "git -c core.autocrlf=false status", 'git -c "commit.gpgsign=false" log -1', # Disabling forms of a program key run no program. From bfcd6dcc348c22ba382abcf55fff2edeb3ffd96e Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 18:26:40 -0500 Subject: [PATCH 30/37] no-mistakes(review): deny unresolved git option words and credential helpers --- HOOKS.md | 13 ++++++++----- scripts/guard_hook.py | 14 +++++++++++--- tests/test_guard_hook.py | 18 ++++++++++++++++++ 3 files changed, 37 insertions(+), 8 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 5978d781d..f3796580a 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -188,7 +188,8 @@ See [UPDATE.md](UPDATE.md). `core.fsmonitor`, editors and pagers, `core.sshCommand`, `core.askPass`, `filter.*` clean/smudge/process, `diff.*` textconv/external/command, `merge.*` drivers, `core.gitProxy`, - `remote.*` uploadpack/receivepack, `gpg.program` and `gpg.*.program` — is denied with a literal + `remote.*` uploadpack/receivepack, `gpg.program` and `gpg.*.program`, `credential.helper` and + `credential.*.helper` — is denied with a literal value as with a parameter, because git runs it during an otherwise read-only subcommand. A disabling form of such a key passes: the bare key, an empty value, or `false`. A `--config-env` form is always denied, because @@ -209,10 +210,12 @@ See [UPDATE.md](UPDATE.md). is denied, because the shell reads it and the words after it as a comment. The git subcommand word must be a plain literal word in each git command: `git {config,...}` and a subcommand split by a line continuation are denied. - The value word of a git global option (`-c`, `-C`, `--git-dir`, - `--work-tree`, `--namespace`, `--config-env`) must resolve to one word: a - backslash, an unquoted glob or an unquoted brace list or range in it is - denied (`git -c {core.fsmonitor=/abs/h.sh,status}`). Plain quoted parts + Each git global option word, and the value word of `-c`, `-C`, + `--git-dir`, `--work-tree`, `--namespace` and `--config-env`, must resolve + to one word: a backslash, an unquoted glob or an unquoted brace list or + range in it is denied (`git -c {core.fsmonitor=/abs/h.sh,status}`, + `git -{p,c} core.fsmonitor=/abs/h.sh status`). A parameter in such a value + is denied also when no subcommand follows (`git -c $x`). Plain quoted parts pass (`git -c user.name='A B' log`). `git config` section renames and removals (`--rename-section`, `--remove-section`) and `--edit` are denied, because a rename can move a diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 3930e97a3..e3b47bd1c 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -258,6 +258,7 @@ "merge": (".driver",), "remote": (".uploadpack", ".receivepack"), "gpg": (".program",), + "credential": (".helper",), } # Keys whose value is a path or URL git loads code through: an included file # can set every key above, and a rewritten URL can retarget the transport. @@ -2651,13 +2652,15 @@ def git_command(segment, assignments=None): option = token.split("=", 1)[0] value = token.split("=", 1)[1] if "=" in token else None index += 1 + if option != "-C": + require_resolved_git_word(token) if option in GIT_GLOBAL_OPTIONS_WITH_VALUES and value is None: if index >= len(arguments): raise ValueError(f"git option {option} lacks a value") - value = token = arguments[index] + value = arguments[index] + if option != "-C": + require_resolved_git_word(value) index += 1 - if option in GIT_GLOBAL_OPTIONS_WITH_VALUES and option != "-C": - require_resolved_git_word(token) if option in {"-c", "--config-env"}: config_key, _, config_value = str(value).partition("=") config_key = config_key.casefold() @@ -2683,6 +2686,11 @@ def git_command(segment, assignments=None): if option in GIT_GLOBAL_OPTIONS_WITH_VALUES: git_options.extend((option, str(value))) if index >= len(arguments): + if parameter_arguments: + raise ValueError( + f"git argument {parameter_arguments[0]} cannot be resolved by the guard; " + "pass a literal value (for commit messages use -F )" + ) return None require_resolved_git_word(arguments[index], mixed=False) subcommand = arguments[index].casefold() diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index c4bdf2a88..0e41db2f9 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2299,6 +2299,21 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git con\\\n\\\n\\\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\n\\\r\nfig core.fsmonitor /abs/h.sh', + # A parameter in a global option value is unresolved with or + # without a visible subcommand: bash splits it into both. + 'read x < v.txt; git -c $x', + 'git -c $x', + 'git --git-dir $x', + 'git --git-dir=$x', + # Each global option word must be one word. + 'git -{p,c} core.fsmonitor=/abs/h.sh status', + 'X=/abs/h.sh git --{no-pager,config-env=core.fsmonitor=X} status', + 'git --no-pager{,} -c core.fsmonitor=/abs/h.sh status', + 'git --no-\\\npager {config,core.fsmonitor,/abs/h.sh}', + # git runs a credential helper when a remote asks for credentials. + 'git -c credential.helper=/abs/h.sh ls-remote http://127.0.0.1:8080/x.git', + 'git -c credential.https://x.helper=/abs/h.sh ls-remote origin', + 'git config credential.helper /abs/h.sh', # The value word of a git global option must be one word. 'git -c {core.fsmonitor=/abs/h.sh,status}', 'git -c{core.fsmonitor=/abs/h.sh,status}', @@ -2423,6 +2438,9 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): 'git -c "user.name=A B" log -1', "git -c user.name='{a,b} *' log -1", 'git -C . status', + "git --no-pager log -1", + "git -c credential.helper= ls-remote origin", + "git config --get credential.helper", 'git -C "." status', "git --git-dir=.git status", "git -c core.autocrlf=false status", From 27cc609ca52369990b1187c9eae7850a8f3cf9ba Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 18:33:49 -0500 Subject: [PATCH 31/37] no-mistakes(review): deny git behind programs the guard does not model --- HOOKS.md | 6 ++++++ scripts/guard_hook.py | 30 ++++++++++++++++++++++++++++++ tests/test_guard_hook.py | 18 ++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/HOOKS.md b/HOOKS.md index f3796580a..27362b22a 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -210,6 +210,12 @@ See [UPDATE.md](UPDATE.md). is denied, because the shell reads it and the words after it as a comment. The git subcommand word must be a plain literal word in each git command: `git {config,...}` and a subcommand split by a line continuation are denied. + A program that the guard does not model (`nice`, `nohup`, `timeout`, + `sudo`, `noglob` and each other unknown program) is denied when one of its + arguments is the word `git`, because the guard cannot apply its git checks + behind it: run git directly. `command`, `builtin`, `exec`, `env`, the + shell wrappers and the read and write commands that the guard knows keep + their handling. Each git global option word, and the value word of `-c`, `-C`, `--git-dir`, `--work-tree`, `--namespace` and `--config-env`, must resolve to one word: a backslash, an unquoted glob or an unquoted brace list or diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index e3b47bd1c..ec3079881 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -537,6 +537,23 @@ def denied_git_config_argument(arguments): } ) FIND_EXECUTION_ACTIONS = frozenset({"-exec", "-execdir", "-ok", "-okdir"}) +GIT_PROGRAMS = frozenset({"git", "git.exe"}) +# Programs the guard models: it knows whether and how each one runs a command +# from its arguments. Any other program with a git word among its arguments +# may run git in a way the guard cannot inspect. +MODELED_PROGRAMS = ( + GIT_PROGRAMS + | COMMAND_WRAPPERS + | POSIX_SHELL_WRAPPERS + | POWERSHELL_WRAPPERS + | XARGS_COMMANDS + | VARIABLE_COMMANDS + | ARCHIVE_READ_COMMANDS + | SHELL_WRITE_COMMANDS + | IN_PLACE_EDITORS + | DIRECTORY_CHANGE_COMMANDS + | {"cmd", "echo", "eval", "find", "printf"} +) SHELL_CONTROL_WORDS = frozenset( { "!", @@ -2542,6 +2559,19 @@ def command_invocation(segment): executable=segment[index], argument=first_argument(arguments) ) ) + if ( + executable not in MODELED_PROGRAMS + and executable.removesuffix(".exe") not in MODELED_PROGRAMS + and any( + argument.replace("\\", "/").rsplit("/", 1)[-1].casefold() in GIT_PROGRAMS + for argument in arguments + ) + ): + raise ValueError( + f"executable {segment[index]} cannot be resolved by the guard: it can " + f"run git in a way the guard cannot inspect; run git directly, without " + f"{segment[index]}" + ) return executable, arguments diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 0e41db2f9..ed5483ddc 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2299,6 +2299,17 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git con\\\n\\\n\\\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\n\\\r\nfig core.fsmonitor /abs/h.sh', + # A program the guard does not model can run the git word in + # its arguments, so no git check would apply. + *( + f"{wrapper} git -c core.fsmonitor=/abs/h.sh status" + for wrapper in ("nice", "nohup", "timeout 5", "sudo", "noglob") + ), + 'nice git status', + 'nice git config core.fsmonitor /abs/h.sh', + 'nice /usr/bin/git -c core.fsmonitor=/abs/h.sh status', + 'timeout 5 git -c $x', + 'command nice git status', # A parameter in a global option value is unresolved with or # without a visible subcommand: bash splits it into both. 'read x < v.txt; git -c $x', @@ -2439,6 +2450,13 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "git -c user.name='{a,b} *' log -1", 'git -C . status', "git --no-pager log -1", + "nice make", + "sudo systemctl status", + "git status", + "command git status", + "env git status", + "echo git", + "grep -r git README.md", "git -c credential.helper= ls-remote origin", "git config --get credential.helper", 'git -C "." status', From 37ab0725516494f0a2305fd67d29f0ed937d8b91 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 18:37:43 -0500 Subject: [PATCH 32/37] no-mistakes(review): scope git wrapper denial to exec-prefix programs --- HOOKS.md | 13 +++++++------ scripts/guard_hook.py | 29 ++++++++--------------------- tests/test_guard_hook.py | 27 ++++++++++++++++++++++++--- 3 files changed, 39 insertions(+), 30 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 27362b22a..d37f9e431 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -210,12 +210,13 @@ See [UPDATE.md](UPDATE.md). is denied, because the shell reads it and the words after it as a comment. The git subcommand word must be a plain literal word in each git command: `git {config,...}` and a subcommand split by a line continuation are denied. - A program that the guard does not model (`nice`, `nohup`, `timeout`, - `sudo`, `noglob` and each other unknown program) is denied when one of its - arguments is the word `git`, because the guard cannot apply its git checks - behind it: run git directly. `command`, `builtin`, `exec`, `env`, the - shell wrappers and the read and write commands that the guard knows keep - their handling. + A program that runs the command in its arguments (`nice`, `nohup`, + `timeout`, `sudo`, `watch`, `time` with options, `stdbuf`, `setsid`) is + denied when one of its arguments is the word `git`, because the guard + cannot apply its git checks behind it: run git directly. Other programs + that only name git (`which git`, `brew install git`) pass. The check reads + literal words only: `nice sh -c 'git ...'`, a parameter or a glob in place + of the `git` word, and other command runners are not covered. Each git global option word, and the value word of `-c`, `-C`, `--git-dir`, `--work-tree`, `--namespace` and `--config-env`, must resolve to one word: a backslash, an unquoted glob or an unquoted brace list or diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index ec3079881..7daa01035 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -538,21 +538,10 @@ def denied_git_config_argument(arguments): ) FIND_EXECUTION_ACTIONS = frozenset({"-exec", "-execdir", "-ok", "-okdir"}) GIT_PROGRAMS = frozenset({"git", "git.exe"}) -# Programs the guard models: it knows whether and how each one runs a command -# from its arguments. Any other program with a git word among its arguments -# may run git in a way the guard cannot inspect. -MODELED_PROGRAMS = ( - GIT_PROGRAMS - | COMMAND_WRAPPERS - | POSIX_SHELL_WRAPPERS - | POWERSHELL_WRAPPERS - | XARGS_COMMANDS - | VARIABLE_COMMANDS - | ARCHIVE_READ_COMMANDS - | SHELL_WRITE_COMMANDS - | IN_PLACE_EDITORS - | DIRECTORY_CHANGE_COMMANDS - | {"cmd", "echo", "eval", "find", "printf"} +# Programs whose purpose is to run the command in their arguments. The guard +# does not unwrap them, so its git checks cannot apply behind one. +EXEC_PREFIX_PROGRAMS = frozenset( + {"nice", "nohup", "timeout", "sudo", "watch", "time", "env", "stdbuf", "setsid"} ) SHELL_CONTROL_WORDS = frozenset( { @@ -2560,12 +2549,10 @@ def command_invocation(segment): ) ) if ( - executable not in MODELED_PROGRAMS - and executable.removesuffix(".exe") not in MODELED_PROGRAMS - and any( - argument.replace("\\", "/").rsplit("/", 1)[-1].casefold() in GIT_PROGRAMS - for argument in arguments - ) + executable in EXEC_PREFIX_PROGRAMS or executable.startswith("-") + ) and any( + argument.replace("\\", "/").rsplit("/", 1)[-1].casefold() in GIT_PROGRAMS + for argument in arguments ): raise ValueError( f"executable {segment[index]} cannot be resolved by the guard: it can " diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index ed5483ddc..73bc5d319 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2299,12 +2299,19 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git con\\\n\\\n\\\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\n\\\r\nfig core.fsmonitor /abs/h.sh', - # A program the guard does not model can run the git word in - # its arguments, so no git check would apply. + # A program that runs the command in its arguments hides git + # from the git checks. *( f"{wrapper} git -c core.fsmonitor=/abs/h.sh status" - for wrapper in ("nice", "nohup", "timeout 5", "sudo", "noglob") + for wrapper in ( + "nice", "nohup", "timeout 5", "sudo", "sudo -u root", "watch", + "stdbuf -o0", "setsid", "time -p", "env nice", "nice env", + "nice command", + ) ), + 'nice -n 5 git status', + 'nice ./git status', + 'watch git status', 'nice git status', 'nice git config core.fsmonitor /abs/h.sh', 'nice /usr/bin/git -c core.fsmonitor=/abs/h.sh status', @@ -2457,6 +2464,20 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "env git status", "echo git", "grep -r git README.md", + # A program that only names git runs no git the guard could check. + "which git", + "type git", + "hash git", + "man git", + "whereis git", + "pgrep git", + "brew install git", + "apt-get install -y git", + "gh pr create --title git", + "gh repo clone cli/cli git", + "pytest tests/git", + "make git", + "time git status", "git -c credential.helper= ls-remote origin", "git config --get credential.helper", 'git -C "." status', From 71e75f844d1b7277744c46255e6466d3162fcec5 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 18:47:25 -0500 Subject: [PATCH 33/37] no-mistakes(review): unwrap command runners so git checks reach payload --- HOOKS.md | 12 ++-- scripts/guard_hook.py | 130 +++++++++++++++++++++++---------------- tests/test_guard_hook.py | 38 ++++++++---- 3 files changed, 111 insertions(+), 69 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index d37f9e431..52d8cae8f 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -211,12 +211,12 @@ See [UPDATE.md](UPDATE.md). The git subcommand word must be a plain literal word in each git command: `git {config,...}` and a subcommand split by a line continuation are denied. A program that runs the command in its arguments (`nice`, `nohup`, - `timeout`, `sudo`, `watch`, `time` with options, `stdbuf`, `setsid`) is - denied when one of its arguments is the word `git`, because the guard - cannot apply its git checks behind it: run git directly. Other programs - that only name git (`which git`, `brew install git`) pass. The check reads - literal words only: `nice sh -c 'git ...'`, a parameter or a glob in place - of the `git` word, and other command runners are not covered. + `timeout`, `sudo`, `watch`, `time`, `env`, `stdbuf`, `setsid`) is + unwrapped: the guard skips the options of the program and checks the + command that it runs, so `nice git -c core.fsmonitor=/abs/h.sh status` is + denied by the git rule and `sudo apt install git` passes. Other command + runners (`doas`, `ionice`, `noglob`) are not unwrapped, and `watch` with + the command in one quoted word is not checked. Each git global option word, and the value word of `-c`, `-C`, `--git-dir`, `--work-tree`, `--namespace` and `--config-env`, must resolve to one word: a backslash, an unquoted glob or an unquoted brace list or diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 7daa01035..3d665a675 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -537,12 +537,23 @@ def denied_git_config_argument(arguments): } ) FIND_EXECUTION_ACTIONS = frozenset({"-exec", "-execdir", "-ok", "-okdir"}) -GIT_PROGRAMS = frozenset({"git", "git.exe"}) -# Programs whose purpose is to run the command in their arguments. The guard -# does not unwrap them, so its git checks cannot apply behind one. -EXEC_PREFIX_PROGRAMS = frozenset( - {"nice", "nohup", "timeout", "sudo", "watch", "time", "env", "stdbuf", "setsid"} -) +# Programs that run the command in their arguments, with the options of each +# that take a separate value. The guard checks the command they run. +EXEC_PREFIX_VALUE_OPTIONS = { + "nice": frozenset({"-n", "--adjustment"}), + "nohup": frozenset(), + "setsid": frozenset(), + "stdbuf": frozenset({"-i", "-o", "-e", "--input", "--output", "--error"}), + "sudo": frozenset({ + "-u", "-g", "-h", "-p", "-C", "-D", "-R", "-T", "-U", "-r", "-t", + "--user", "--group", "--host", "--prompt", "--close-from", "--chdir", + "--chroot", "--command-timeout", "--other-user", "--role", "--type", + }), + "time": frozenset({"-o", "-f", "--output", "--format"}), + "timeout": frozenset({"-s", "-k", "--signal", "--kill-after"}), + "watch": frozenset({"-n", "--interval"}), +} +TIMEOUT_DURATION = re.compile(r"\d+(?:\.\d+)?[smhd]?") SHELL_CONTROL_WORDS = frozenset( { "!", @@ -2487,47 +2498,73 @@ def require_read_command(wrapper, wrapped): def command_invocation(segment): index = 0 - while index < len(segment) and SHELL_ASSIGNMENT_PATTERN.match(segment[index]): - validate_shell_assignment(segment[index]) + # The shell word `time` is already removed; its only option can remain. + while index < len(segment) and segment[index] == "-p": index += 1 - if index >= len(segment): - return None - executable = segment[index].replace("\\", "/").rsplit("/", 1)[-1].casefold() - if executable == "env": - index += 1 - while index < len(segment): - token = segment[index] - if token == "--": - index += 1 - break - if SHELL_ASSIGNMENT_PATTERN.match(token): - validate_shell_assignment(token) - index += 1 - continue - option = token.split("=", 1)[0] - if option in ENV_OPTIONS_WITHOUT_VALUES: - index += 1 - continue - if option in ENV_OPTIONS_WITH_VALUES: - if option in {"-C", "--chdir"}: + while True: + while index < len(segment) and SHELL_ASSIGNMENT_PATTERN.match(segment[index]): + validate_shell_assignment(segment[index]) + index += 1 + if index >= len(segment): + return None + executable = segment[index].replace("\\", "/").rsplit("/", 1)[-1].casefold() + prefix = executable.removesuffix(".exe") + if executable == "env": + index += 1 + while index < len(segment): + token = segment[index] + if token == "--": + index += 1 + break + if SHELL_ASSIGNMENT_PATTERN.match(token): + validate_shell_assignment(token) + index += 1 + continue + option = token.split("=", 1)[0] + if option in ENV_OPTIONS_WITHOUT_VALUES: + index += 1 + continue + if option in ENV_OPTIONS_WITH_VALUES: + if option in {"-C", "--chdir"}: + raise ValueError( + f"env {option} changes the working directory the guard " + "tracks; use cd with a literal path instead" + ) + index += 1 + if "=" not in token: + if index >= len(segment): + raise ValueError(f"env {option} lacks a value") + index += 1 + continue + if token.startswith("-"): raise ValueError( - f"env {option} changes the working directory the guard " - "tracks; use cd with a literal path instead" + f"env {token} cannot be validated; use env NAME=VALUE " ) + break + elif prefix in EXEC_PREFIX_VALUE_OPTIONS: + value_options = EXEC_PREFIX_VALUE_OPTIONS[prefix] + index += 1 + while index < len(segment): + token = segment[index] + if token == "--": + index += 1 + break + if not token.startswith("-") or token == "-": + break index += 1 - if "=" not in token: - if index >= len(segment): - raise ValueError(f"env {option} lacks a value") + if "=" not in token and ( + token in value_options + or (not token.startswith("--") and f"-{token[-1]}" in value_options) + ): index += 1 - continue - if token.startswith("-"): - raise ValueError( - f"env {token} cannot be validated; use env NAME=VALUE " - ) + if ( + prefix == "timeout" + and index < len(segment) + and TIMEOUT_DURATION.fullmatch(segment[index]) + ): + index += 1 + else: break - if index >= len(segment): - return None - executable = segment[index].replace("\\", "/").rsplit("/", 1)[-1].casefold() arguments = segment[index + 1 :] if SHELL_PARAMETER_SYNTAX.search(executable): raise ValueError( @@ -2548,17 +2585,6 @@ def command_invocation(segment): executable=segment[index], argument=first_argument(arguments) ) ) - if ( - executable in EXEC_PREFIX_PROGRAMS or executable.startswith("-") - ) and any( - argument.replace("\\", "/").rsplit("/", 1)[-1].casefold() in GIT_PROGRAMS - for argument in arguments - ): - raise ValueError( - f"executable {segment[index]} cannot be resolved by the guard: it can " - f"run git in a way the guard cannot inspect; run git directly, without " - f"{segment[index]}" - ) return executable, arguments diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 73bc5d319..e02a17895 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2299,24 +2299,24 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'git con\\\n\\\n\\\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\nfig core.fsmonitor /abs/h.sh', 'git con\\\r\n\\\r\nfig core.fsmonitor /abs/h.sh', - # A program that runs the command in its arguments hides git - # from the git checks. + # The guard checks the command that a command runner runs. *( f"{wrapper} git -c core.fsmonitor=/abs/h.sh status" for wrapper in ( - "nice", "nohup", "timeout 5", "sudo", "sudo -u root", "watch", - "stdbuf -o0", "setsid", "time -p", "env nice", "nice env", - "nice command", + "nice", "nice -n 5", "nice -n5", "nice --adjustment=5", "nohup", + "timeout 5", "timeout -s KILL 5", "timeout --signal=KILL 2.5s", + "sudo", "sudo -u root", "sudo -Eu root", "sudo --user root", + "sudo FOO=1", "watch", "watch -n 5", "stdbuf -o0", "stdbuf -o L", + "setsid", "time -p", "env nice", "nice env", "nice command", + "nice nohup", "/usr/bin/nice", "sudo.exe", "nice --", ) ), - 'nice -n 5 git status', - 'nice ./git status', - 'watch git status', - 'nice git status', - 'nice git config core.fsmonitor /abs/h.sh', + "nice sh -c 'git -c core.fsmonitor=/abs/h.sh status'", 'nice /usr/bin/git -c core.fsmonitor=/abs/h.sh status', + 'nice git config core.fsmonitor /abs/h.sh', 'timeout 5 git -c $x', - 'command nice git status', + 'timeout $t git status', + 'sudo GIT_CONFIG_COUNT=1 git status', # A parameter in a global option value is unresolved with or # without a visible subcommand: bash splits it into both. 'read x < v.txt; git -c $x', @@ -2464,6 +2464,22 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "env git status", "echo git", "grep -r git README.md", + # A command runner denies nothing itself: its command is checked. + "nice git status", + "nice -n 5 git status", + "watch git status", + "command nice git status", + "timeout 5 git log -1", + "sudo apt install git", + "sudo apt-get install -y git", + "timeout 60 brew install git", + "sudo -u git psql", + "nice grep -r git README.md", + "sudo ls vendor/git", + "nohup npm run git", + "watch -n 5 which git", + "time -p grep git README.md", + "time -p make git", # A program that only names git runs no git the guard could check. "which git", "type git", From 181c3570119307be9e28ccd1de7156f7c86b184c Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 19:08:46 -0500 Subject: [PATCH 34/37] no-mistakes(review): deny file-valued runner options and non-literal runner operands --- HOOKS.md | 8 +++- scripts/guard_hook.py | 101 +++++++++++++++++++++++++++++++-------- tests/test_guard_hook.py | 36 ++++++++++++++ 3 files changed, 125 insertions(+), 20 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index 52d8cae8f..af65bd26a 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -214,7 +214,13 @@ See [UPDATE.md](UPDATE.md). `timeout`, `sudo`, `watch`, `time`, `env`, `stdbuf`, `setsid`) is unwrapped: the guard skips the options of the program and checks the command that it runs, so `nice git -c core.fsmonitor=/abs/h.sh status` is - denied by the git rule and `sudo apt install git` passes. Other command + denied by the git rule and `sudo apt install git` passes. A runner + option that writes, edits or enters a file or directory by itself is + denied (`time -o`, `sudo -e`, `sudo -D`, `sudo -R`, `watch -s`). The + `timeout` duration and the `watch` interval must be literal numbers, and + each other runner option must be a literal word or a parameter in double + quotes. With an archive working directory a command runner is not + unwrapped and is denied as an unknown program. Other command runners (`doas`, `ionice`, `noglob`) are not unwrapped, and `watch` with the command in one quoted word is not checked. Each git global option word, and the value word of `-c`, `-C`, diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 3d665a675..be76a0654 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -545,15 +545,22 @@ def denied_git_config_argument(arguments): "setsid": frozenset(), "stdbuf": frozenset({"-i", "-o", "-e", "--input", "--output", "--error"}), "sudo": frozenset({ - "-u", "-g", "-h", "-p", "-C", "-D", "-R", "-T", "-U", "-r", "-t", - "--user", "--group", "--host", "--prompt", "--close-from", "--chdir", - "--chroot", "--command-timeout", "--other-user", "--role", "--type", + "-u", "-g", "-h", "-p", "-C", "-T", "-U", "-r", "-t", + "--user", "--group", "--host", "--prompt", "--close-from", + "--command-timeout", "--other-user", "--role", "--type", }), - "time": frozenset({"-o", "-f", "--output", "--format"}), + "time": frozenset({"-f", "--format"}), "timeout": frozenset({"-s", "-k", "--signal", "--kill-after"}), - "watch": frozenset({"-n", "--interval"}), + "watch": frozenset({"-n", "-q", "--interval", "--equexit"}), } -TIMEOUT_DURATION = re.compile(r"\d+(?:\.\d+)?[smhd]?") +# Runner options that write, edit or enter a file or directory by themselves. +EXEC_PREFIX_FILE_OPTIONS = { + "sudo": frozenset({"-e", "-D", "-R", "--edit", "--chdir", "--chroot"}), + "time": frozenset({"-o", "--output"}), + "watch": frozenset({"-s", "--shotsdir"}), +} +EXEC_PREFIX_INTERVAL_OPTIONS = frozenset({"-n", "--interval"}) +RUNNER_DURATION = re.compile(r"(?:\d+(?:\.\d*)?|\.\d+)[smhd]?") SHELL_CONTROL_WORDS = frozenset( { "!", @@ -2496,10 +2503,31 @@ def require_read_command(wrapper, wrapped): ) -def command_invocation(segment): +def runner_option_in(token, options): + """Whether a runner option word names one of the options. + + A long option matches by each prefix, as getopt accepts it; a short + cluster matches when its last letter is the option. + """ + name = token.split("=", 1)[0] + if name.startswith("--"): + return any(option.startswith(name) for option in options) + return f"-{name[-1]}" in options + + +def unresolved_runner_word(token): + return ( + SHELL_PARAMETER_SYNTAX.search(token) + or CMD_PARAMETER_SYNTAX.search(token) + or zsh_expansion_flag(token) + ) and not parameters_double_quoted(token) + + +def command_invocation(segment, unwrap_runners=True): + """`unwrap_runners` is False where a command runner itself must be judged.""" index = 0 # The shell word `time` is already removed; its only option can remain. - while index < len(segment) and segment[index] == "-p": + while unwrap_runners and index < len(segment) and segment[index] == "-p": index += 1 while True: while index < len(segment) and SHELL_ASSIGNMENT_PATTERN.match(segment[index]): @@ -2540,9 +2568,16 @@ def command_invocation(segment): raise ValueError( f"env {token} cannot be validated; use env NAME=VALUE " ) + if SHELL_PARAMETER_SYNTAX.search(token) or zsh_expansion_flag(token): + raise ValueError( + f"env word {token} cannot be resolved by the guard; env " + "assignments must be literal NAME=VALUE words and the " + "program a literal name" + ) break - elif prefix in EXEC_PREFIX_VALUE_OPTIONS: + elif unwrap_runners and prefix in EXEC_PREFIX_VALUE_OPTIONS: value_options = EXEC_PREFIX_VALUE_OPTIONS[prefix] + file_options = EXEC_PREFIX_FILE_OPTIONS.get(prefix, frozenset()) index += 1 while index < len(segment): token = segment[index] @@ -2552,16 +2587,44 @@ def command_invocation(segment): if not token.startswith("-") or token == "-": break index += 1 - if "=" not in token and ( - token in value_options - or (not token.startswith("--") and f"-{token[-1]}" in value_options) + short_cluster = not token.startswith("--") + if unresolved_runner_word(token): + raise ValueError( + f"{prefix} option {token} cannot be resolved by the guard; " + f"{prefix} options must be literal words" + ) + if runner_option_in(token, file_options) or ( + short_cluster + and any(f"-{letter}" in file_options for letter in token[1:]) ): - index += 1 - if ( - prefix == "timeout" - and index < len(segment) - and TIMEOUT_DURATION.fullmatch(segment[index]) - ): + raise ValueError( + f"{prefix} {token} writes, edits or enters a file or directory " + "that the guard cannot check; run the command without this " + "option and redirect its output instead" + ) + if "=" in token or not runner_option_in(token, value_options): + continue + if index >= len(segment): + raise ValueError(f"{prefix} option {token} lacks a value") + value = segment[index] + index += 1 + if runner_option_in(token, EXEC_PREFIX_INTERVAL_OPTIONS) and prefix == "watch": + if not RUNNER_DURATION.fullmatch(value): + raise ValueError( + f"watch interval {value} cannot be resolved by the guard; " + "the interval must be a literal number" + ) + elif unresolved_runner_word(value): + raise ValueError( + f"{prefix} option value {value} cannot be resolved by the " + f"guard; {prefix} option values must be literal words" + ) + if prefix == "timeout" and index < len(segment): + if not RUNNER_DURATION.fullmatch(segment[index]): + raise ValueError( + f"timeout duration {segment[index]} cannot be resolved by the " + "guard; the duration must be a literal number" + ) index += 1 else: break @@ -3077,7 +3140,7 @@ def archive_command_is_read_only( return False if not archive_context: return True - invocation = command_invocation(tokens) + invocation = command_invocation(tokens, unwrap_runners=False) if invocation is None: return True executable, arguments = invocation diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index e02a17895..e2893efc6 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2311,6 +2311,31 @@ def test_allows_parameters_in_read_only_git_arguments(self): "nice nohup", "/usr/bin/nice", "sudo.exe", "nice --", ) ), + # A runner option that writes or edits a file is not skipped. + 'nice time -o .project/archive/001-mvp/N ls', + 'env time -o .project/archive/001-mvp/N ls', + '/usr/bin/time -o .project/archive/001-mvp/N ls', + 'timeout 5 time -o .project/archive/001-mvp/N ls', + 'nice time --output=.project/archive/001-mvp/N ls', + 'nice time --outp .project/archive/001-mvp/N ls', + 'nice time -ao .project/archive/001-mvp/N ls', + 'nice time -o .project/archive/001-mvp/N git status', + 'sudo -e cat .project/archive/001-mvp/plan/PLAN.md', + 'sudo --edit .project/archive/001-mvp/plan/PLAN.md', + 'sudo -D .project/archive/001-mvp touch N', + 'watch -s .project/archive/001-mvp ls', + # A runner operand must be a literal word. + 'timeout "$T" make', + 'timeout ${T:-5} make', + 'timeout five make', + 'watch -n $t ls', + 'nice -n $n git status', + 'sudo -u $u ls', + 'env $A=1 git status', + 'nice $CMD', + 'timeout .5 git -c core.fsmonitor=/abs/h.sh status', + 'watch -q 3 git -c core.fsmonitor=/abs/h.sh status', + 'nice --adj 5 git -c core.fsmonitor=/abs/h.sh status', "nice sh -c 'git -c core.fsmonitor=/abs/h.sh status'", 'nice /usr/bin/git -c core.fsmonitor=/abs/h.sh status', 'nice git config core.fsmonitor /abs/h.sh', @@ -2465,6 +2490,17 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "echo git", "grep -r git README.md", # A command runner denies nothing itself: its command is checked. + "nice make", + "timeout 5 npm test", + "timeout .5 make", + "sudo -E -u root ls", + 'sudo -u "$USER" ls', + "nohup npm run build", + "time -p make", + "nice time -p make", + "sudo --preserve-env=PATH make install", + "stdbuf -oL -eL python3 x.py", + "watch -n 2 ls", "nice git status", "nice -n 5 git status", "watch git status", From 8e793c8aba43e482ace0d79a4eb189571ee4f4de Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 19:16:01 -0500 Subject: [PATCH 35/37] no-mistakes(review): parse runner short options as getopt, deny non-literal option words --- HOOKS.md | 6 ++- scripts/guard_hook.py | 83 +++++++++++++++++++++++++++------------- tests/test_guard_hook.py | 26 +++++++++++++ 3 files changed, 87 insertions(+), 28 deletions(-) diff --git a/HOOKS.md b/HOOKS.md index af65bd26a..e327ddbec 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -218,8 +218,10 @@ See [UPDATE.md](UPDATE.md). option that writes, edits or enters a file or directory by itself is denied (`time -o`, `sudo -e`, `sudo -D`, `sudo -R`, `watch -s`). The `timeout` duration and the `watch` interval must be literal numbers, and - each other runner option must be a literal word or a parameter in double - quotes. With an archive working directory a command runner is not + each runner option word must be a literal word; only a separate option + value can be a parameter in double quotes (`sudo -u "$USER" ls`). A short + option word is read as getopt reads it, so the attached value in + `sudo -upostgres psql` is not taken as more options. With an archive working directory a command runner is not unwrapped and is denied as an unknown program. Other command runners (`doas`, `ionice`, `noglob`) are not unwrapped, and `watch` with the command in one quoted word is not checked. diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index be76a0654..54ed28ba7 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -559,8 +559,21 @@ def denied_git_config_argument(arguments): "time": frozenset({"-o", "--output"}), "watch": frozenset({"-s", "--shotsdir"}), } +# The short option letters of each runner that take no value. +EXEC_PREFIX_FLAG_LETTERS = { + "nice": "", + "nohup": "", + "setsid": "cfw", + "stdbuf": "", + "sudo": "AbBEHiKklnNPSsVv", + "time": "ahlpqvV", + "timeout": "fpv", + "watch": "bcCdegprtwx", +} EXEC_PREFIX_INTERVAL_OPTIONS = frozenset({"-n", "--interval"}) -RUNNER_DURATION = re.compile(r"(?:\d+(?:\.\d*)?|\.\d+)[smhd]?") +RUNNER_DURATION = re.compile( + r"(?:(?:\d+(?:\.\d*)?|\.\d+)(?:e[+-]?\d+)?|inf(?:inity)?)[smhd]?", re.IGNORECASE +) SHELL_CONTROL_WORDS = frozenset( { "!", @@ -2503,16 +2516,47 @@ def require_read_command(wrapper, wrapped): ) -def runner_option_in(token, options): - """Whether a runner option word names one of the options. +def runner_value_option(prefix, token): + """The option in a runner option word whose value is the next word, if any. - A long option matches by each prefix, as getopt accepts it; a short - cluster matches when its last letter is the option. + Raises for an option that writes, edits or enters a file or directory. A + long option matches by each prefix, as getopt accepts it. A short word is + read as getopt reads it: flag letters, then the first value option letter, + whose value is the rest of the word or, with no rest, the next word. """ - name = token.split("=", 1)[0] - if name.startswith("--"): - return any(option.startswith(name) for option in options) - return f"-{name[-1]}" in options + value_options = EXEC_PREFIX_VALUE_OPTIONS[prefix] + file_options = EXEC_PREFIX_FILE_OPTIONS.get(prefix, frozenset()) + denial = ValueError( + f"{prefix} {token} writes, edits or enters a file or directory that the " + "guard cannot check; run the command without this option and redirect " + "its output instead" + ) + if token.startswith("--"): + name = token.split("=", 1)[0] + if any(option.startswith(name) for option in file_options): + raise denial + option = next((option for option in value_options if option.startswith(name)), None) + return option if "=" not in token else None + for position, letter in enumerate(token[1:], start=1): + option = f"-{letter}" + if option in file_options: + raise denial + if option in value_options: + return option if position == len(token) - 1 else None + if letter not in EXEC_PREFIX_FLAG_LETTERS[prefix]: + return None + return None + + +def nonliteral_runner_option(token): + literal = getattr(token, "literal", "bare") + return ( + SHELL_PARAMETER_SYNTAX.search(token) + or CMD_PARAMETER_SYNTAX.search(token) + or zsh_expansion_flag(token) + or literal is None + or (literal == "bare" and GIT_CONFIG_EXPANDING_WORD.search(token)) + ) def unresolved_runner_word(token): @@ -2576,8 +2620,6 @@ def command_invocation(segment, unwrap_runners=True): ) break elif unwrap_runners and prefix in EXEC_PREFIX_VALUE_OPTIONS: - value_options = EXEC_PREFIX_VALUE_OPTIONS[prefix] - file_options = EXEC_PREFIX_FILE_OPTIONS.get(prefix, frozenset()) index += 1 while index < len(segment): token = segment[index] @@ -2587,28 +2629,17 @@ def command_invocation(segment, unwrap_runners=True): if not token.startswith("-") or token == "-": break index += 1 - short_cluster = not token.startswith("--") - if unresolved_runner_word(token): + if nonliteral_runner_option(token): raise ValueError( f"{prefix} option {token} cannot be resolved by the guard; " f"{prefix} options must be literal words" ) - if runner_option_in(token, file_options) or ( - short_cluster - and any(f"-{letter}" in file_options for letter in token[1:]) - ): - raise ValueError( - f"{prefix} {token} writes, edits or enters a file or directory " - "that the guard cannot check; run the command without this " - "option and redirect its output instead" - ) - if "=" in token or not runner_option_in(token, value_options): + option = runner_value_option(prefix, token) + if option is None or index >= len(segment): continue - if index >= len(segment): - raise ValueError(f"{prefix} option {token} lacks a value") value = segment[index] index += 1 - if runner_option_in(token, EXEC_PREFIX_INTERVAL_OPTIONS) and prefix == "watch": + if prefix == "watch" and option in EXEC_PREFIX_INTERVAL_OPTIONS: if not RUNNER_DURATION.fullmatch(value): raise ValueError( f"watch interval {value} cannot be resolved by the guard; " diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index e2893efc6..1d09f771b 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2324,6 +2324,23 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'sudo --edit .project/archive/001-mvp/plan/PLAN.md', 'sudo -D .project/archive/001-mvp touch N', 'watch -s .project/archive/001-mvp ls', + 'nice time -oN ls', + 'nice time -pao N ls', + # A runner option word that is not literal can spell a file option. + 'read x < v.txt; nice /usr/bin/time "-$x" ls', + 'read x < v.txt; nice time "-$x" git status', + 'read x < v.txt; sudo "-$x" touch N', + 'watch --interval="$t" ls', + 'nice time -\\o N ls', + 'sudo -{u,e} root ls', + # An attached short option value is no more options, and the next + # word is the command. + 'sudo -uroot git -c core.fsmonitor=/abs/h.sh status', + 'sudo -hlocalhost git -c core.fsmonitor=/abs/h.sh status', + 'sudo -uroot git config core.fsmonitor /abs/h.sh', + 'sudo -Eu root git -c core.fsmonitor=/abs/h.sh status', + 'nice -n5 git -c core.fsmonitor=/abs/h.sh status', + 'timeout infinity git -c core.fsmonitor=/abs/h.sh status', # A runner operand must be a literal word. 'timeout "$T" make', 'timeout ${T:-5} make', @@ -2495,6 +2512,15 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "timeout .5 make", "sudo -E -u root ls", 'sudo -u "$USER" ls', + "sudo -upostgres psql", + "sudo -u postgres psql", + "sudo -udeploy ls", + "sudo -ujenkins make", + "sudo -uDeploy ls", + "sudo -h", + "nice -n", + "timeout infinity make", + "timeout 1e3 make", "nohup npm run build", "time -p make", "nice time -p make", From 9b36e3ab423f1d9de7e893ba6cfcc559e94db1c6 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 19:22:29 -0500 Subject: [PATCH 36/37] no-mistakes(review): deny pager. config keys that git executes --- HOOKS.md | 2 +- scripts/guard_hook.py | 1 + tests/test_guard_hook.py | 12 ++++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/HOOKS.md b/HOOKS.md index e327ddbec..df70b78c4 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -189,7 +189,7 @@ See [UPDATE.md](UPDATE.md). `core.askPass`, `filter.*` clean/smudge/process, `diff.*` textconv/external/command, `merge.*` drivers, `core.gitProxy`, `remote.*` uploadpack/receivepack, `gpg.program` and `gpg.*.program`, `credential.helper` and - `credential.*.helper` — is denied with a literal + `credential.*.helper`, each `pager.` key — is denied with a literal value as with a parameter, because git runs it during an otherwise read-only subcommand. A disabling form of such a key passes: the bare key, an empty value, or `false`. A `--config-env` form is always denied, because diff --git a/scripts/guard_hook.py b/scripts/guard_hook.py index 54ed28ba7..2dabda185 100644 --- a/scripts/guard_hook.py +++ b/scripts/guard_hook.py @@ -259,6 +259,7 @@ "remote": (".uploadpack", ".receivepack"), "gpg": (".program",), "credential": (".helper",), + "pager": ("",), } # Keys whose value is a path or URL git loads code through: an included file # can set every key above, and a rewritten URL can retarget the transport. diff --git a/tests/test_guard_hook.py b/tests/test_guard_hook.py index 1d09f771b..9c4e3b7c6 100644 --- a/tests/test_guard_hook.py +++ b/tests/test_guard_hook.py @@ -2370,6 +2370,12 @@ def test_allows_parameters_in_read_only_git_arguments(self): 'X=/abs/h.sh git --{no-pager,config-env=core.fsmonitor=X} status', 'git --no-pager{,} -c core.fsmonitor=/abs/h.sh status', 'git --no-\\\npager {config,core.fsmonitor,/abs/h.sh}', + # git runs a pager. value that is no boolean as the pager. + 'git -c "pager.status=touch .project/archive/001-mvp/N" status', + 'git -c pager.log=/abs/h.sh log', + 'git -c PAGER.LOG=/abs/h.sh log', + 'git -c pager.log=cat log', + 'git config pager.log /abs/h.sh', # git runs a credential helper when a remote asks for credentials. 'git -c credential.helper=/abs/h.sh ls-remote http://127.0.0.1:8080/x.git', 'git -c credential.https://x.helper=/abs/h.sh ls-remote origin', @@ -2557,6 +2563,12 @@ def test_denies_executed_config_keys_but_allows_plain_settings(self): "make git", "time git status", "git -c credential.helper= ls-remote origin", + "git -c pager.status=false status", + "git -c pager.log=false log -1", + "git -c pager.diff=FALSE diff", + "git -c pager.log= log -1", + "git config pager.log", + "git -c core.pagerx=1 status", "git config --get credential.helper", 'git -C "." status', "git --git-dir=.git status", From b764036070e2781e1cec3d815471cdb6590348f3 Mon Sep 17 00:00:00 2001 From: Jeremy McSpadden Date: Sun, 4 Oct 2026 19:29:26 -0500 Subject: [PATCH 37/37] no-mistakes(document): Name interactive.diffFilter in HOOKS.md executed-key list --- HOOKS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/HOOKS.md b/HOOKS.md index df70b78c4..1dfb83b62 100644 --- a/HOOKS.md +++ b/HOOKS.md @@ -188,6 +188,7 @@ See [UPDATE.md](UPDATE.md). `core.fsmonitor`, editors and pagers, `core.sshCommand`, `core.askPass`, `filter.*` clean/smudge/process, `diff.*` textconv/external/command, `merge.*` drivers, `core.gitProxy`, + `interactive.diffFilter`, `remote.*` uploadpack/receivepack, `gpg.program` and `gpg.*.program`, `credential.helper` and `credential.*.helper`, each `pager.` key — is denied with a literal value as with a parameter, because git runs it during an otherwise