diff --git a/.claude/settings.json b/.claude/settings.json new file mode 100644 index 00000000..bf8c0c21 --- /dev/null +++ b/.claude/settings.json @@ -0,0 +1,16 @@ +{ + "hooks": { + "PreToolUse": [ + { + "matcher": "Bash", + "hooks": [ + { + "type": "command", + "command": "uv run python scripts/block_git_mutations.py", + "timeout": 5 + } + ] + } + ] + } +} diff --git a/.github/code-conventions.md b/.github/code-conventions.md new file mode 100644 index 00000000..c00f16d8 --- /dev/null +++ b/.github/code-conventions.md @@ -0,0 +1,62 @@ +When generating Python code (e.g. when translating TypeScript to Python), +follow these guidelines: + +* When creating a new file, add a copyright header to the top: +``` +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. +``` + +* Assume Python 3.12 + +* Always strip trailing spaces + +* Keep class and type names in `PascalCase` +* Use `python_case` for variable/field and function/method names + +* Use `Literal` for unions of string literals +* Keep union notation (`X | Y`) for other unions +* Use `Protocol` for interfaces whose name starts with `I` followed by a capital letter +* Use `dataclass` for other classes and structured types +* Use `type` for type aliases (`PascalCase` again) +* Use `list`, `tuple`, `dict`, `set` etc., not `List` etc. + +* Translate `foo?: string` to `foo: str | None = None` + +* When writing tests: + - don't mock; use the regular implementation (maybe introduce a fixture to create it) + - assume `pytest`; use `assert` statements + - match the type annotations of the tested functions + - read the code of the tested functions to understand their behavior + - When using fixtures: + - Fully type-annotate the fixture definitions (including return type) + - Fully type-annotate fixture usages + +* Don't put imports inside functions. + Put them at the top of the file with the other imports. + Exception: imports in a `if __name__ == "__main__":` block or a `main()` function. + Another exception: pydantic and logfire. + Final exception: to avoid circular import errors. + +* **Import Architecture Rules**: + - **Never import a symbol from a module that just re-exports it** + - **Always import directly from the module that defines the symbol** + - **Exception**: Package `__init__.py` files that explicitly re-export with `__all__` + - **Exception**: Explicit re-export patterns like `from ... import X as X` or marked with "# For export" + - This prevents circular imports and makes dependencies clear + +* Order imports alphabetically after lowercasing; group them as follows + (with a blank line between groups): + 1. standard library imports + 2. established third-party libraries + 3. experimental third-party libraries (e.g. `typechat`) + 4. local imports (e.g. `from typeagent.knowpro import ...`) + +* **Error Handling**: Don't use `try/except Exception` to catch errors broadly. + Let errors bubble up naturally for proper error handling and debugging at higher levels. + +* **Code Validation**: Don't use `py_compile` for syntax checking. + Use `pyright` or `make check` instead for proper type checking and validation. + +* **Deprecations**: Don't deprecate things -- just delete them and fix the usage sites. + Don't create backward compatibility APIs or exports or whatever. Fix the usage sites. diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e3f4c7cc..9bc968b5 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -1 +1,3 @@ Get your instructions from AGENTS.md in the repo root. + +For code generation and style conventions, see [code-conventions.md](code-conventions.md). diff --git a/.gitignore b/.gitignore index e9431b17..0c9bac43 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ -# Editor settings -.vscode +# Editor settings (but .vscode/settings.json is tracked for Copilot hooks) +.vscode/* +!.vscode/settings.json # Python __pycache__ diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..f7512a18 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,12 @@ +{ + "github.copilot.chat.agent.hooks": { + "PreToolUse": [ + { + "type": "command", + "command": "uv run python scripts/block_git_mutations.py", + "windows": "uv run python scripts\\block_git_mutations.py", + "timeout": 5 + } + ] + } +} diff --git a/AGENTS.md b/AGENTS.md index 981fc974..c592606a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,38 +2,10 @@ **NEVER use TEST_MODEL_NAME or "test" embedding model outside of test files** -Never run git commands that make any changes. (`git status` and `git diff` are fine) -Exceptions: `git push`, `git worktree`, `git branch` (for tracking setup), as instructed below. - -**NEVER COMMIT CODE.** Do not run `git commit` or any other git commands -that make changes to the repository. Exception: Worktrees/Branches below. -`git add` is fine. +Git mutations are blocked by a PreToolUse hook (`scripts/block_git_mutations.py`). When moving, copying or deleting files, use the git commands: `git mv`, `git cp`, `git rm` -## Worktrees and Branches - -- Each session uses its own worktree with a feature branch -- Create worktrees with: `git worktree add ../- -b ` -- Push the branch to the `me` remote: `git push me ` -- Set upstream to `me/`: `git branch --set-upstream-to me/` -- **Never** upstream to `me/main` — that must stay identical to `origin/main` -- The worktree directory name should be `-` (sibling of the main checkout) -- **Work in the worktree directory**, not the main checkout — edit files there, run tests there -- VS Code may show buffers from the main checkout; ignore those when working in a worktree. - When in doubt, verify edits landed on disk with `cat` or `grep` in the terminal. - -## Debugging discipline - -- When a bug seems impossible, suspect stale files or wrong working directory — not exotic causes. -- If you're tempted to blame installed package versions, `__pycache__`, or similar, - **stop and ask the user** before investigating further. You're probably on the wrong track. - -**Whenever the user tells you how to do something, states a preference, or corrects you, -extract a general rule and add it to AGENTS.md** (unless it's already covered -- maybe -reformulate since it apparently didn't work). This applies even without being asked. -In all cases show what you added to AGENTS.md. - - Don't use '!' on the command line, it's some bash magic (even inside single quotes) - When running 'make' commands, do not use the venv (the Makefile uses 'uv run') - To get API keys in ad-hoc code, call `load_dotenv()` @@ -59,67 +31,4 @@ In all cases show what you added to AGENTS.md. **IMPORTANT! YOU ARE NOT DONE UNTIL `make format check test` PASSES** -# Code generation - -When generating Python code (e.g. when translating TypeScript to Python), -please follow these guidelines: - -* When creating a new file, add a copyright header to the top: -``` -# Copyright (c) Microsoft Corporation. -# Licensed under the MIT License. -``` - -* Assume Python 3.12 - -* Always strip trailing spaces - -* Keep class and type names in `PascalCase` -* Use `python_case` for variable/field and function/method names - -* Use `Literal` for unions of string literals -* Keep union notation (`X | Y`) for other unions -* Use `Protocol` for interfaces whose name starts with `I` followed by a capital letter -* Use `dataclass` for other classes and structured types -* Use `type` for type aliases (`PascalCase` again) -* Use `list`, `tuple`, `dict`, `set` etc., not `List` etc. - -* Translate `foo?: string` to `foo: str | None = None` - -* When writing tests: - - don't mock; use the regular implementation (maybe introduce a fixture to create it) - - assume `pytest`; use `assert` statements - - match the type annotations of the tested functions - - read the code of the tested functions to understand their behavior - - When using fixtures: - - Fully type-annotate the fixture definitions (including return type) - - Fully type-annotate fixture usages - -* Don't put imports inside functions. - Put them at the top of the file with the other imports. - Exception: imports in a `if __name__ == "__main__":` block or a `main()` function. - Another exception: pydantic and logfire. - Final exception: to avoid circular import errors. - -* **Import Architecture Rules**: - - **Never import a symbol from a module that just re-exports it** - - **Always import directly from the module that defines the symbol** - - **Exception**: Package `__init__.py` files that explicitly re-export with `__all__` - - **Exception**: Explicit re-export patterns like `from ... import X as X` or marked with "# For export" - - This prevents circular imports and makes dependencies clear - -* Order imports alphabetically after lowercasing; group them as follows - (with a blank line between groups): - 1. standard library imports - 2. established third-party libraries - 3. experimental third-party libraries (e.g. `typechat`) - 4. local imports (e.g. `from typeagent.knowpro import ...`) - -* **Error Handling**: Don't use `try/except Exception` to catch errors broadly. - Let errors bubble up naturally for proper error handling and debugging at higher levels. - -* **Code Validation**: Don't use `py_compile` for syntax checking. - Use `pyright` or `make check` instead for proper type checking and validation. - -* **Deprecations**: Don't deprecate things -- just delete them and fix the usage sites. - Don't create backward compatibility APIs or exports or whatever. Fix the usage sites. +For code generation and style conventions, see [.github/code-conventions.md](.github/code-conventions.md). diff --git a/scripts/block_git_mutations.py b/scripts/block_git_mutations.py new file mode 100644 index 00000000..15122630 --- /dev/null +++ b/scripts/block_git_mutations.py @@ -0,0 +1,201 @@ +#!/usr/bin/env python3 +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +"""PreToolUse hook: block git commands that mutate the repository. + +Works with both Claude Code and VS Code Copilot hooks. +Reads a JSON tool invocation from stdin, checks if it contains a git +mutation, and outputs a deny decision if so. +""" + +import json +import re +import shlex +import subprocess +import sys + +TERMINAL_TOOL_NAMES = frozenset({"Bash", "runTerminalCommand"}) + +ALLOWED_SUBCOMMANDS = frozenset({ + "add", + "blame", + "branch", + "cat-file", + "config", + "count-objects", + "describe", + "diff", + "diff-tree", + "for-each-ref", + "fsck", + "grep", + "log", + "ls-files", + "ls-remote", + "ls-tree", + "name-rev", + "push", + "reflog", + "remote", + "rev-list", + "rev-parse", + "shortlog", + "show", + "show-branch", + "show-ref", + "stash", + "status", + "symbolic-ref", + "tag", + "version", + "worktree", +}) + +BRANCH_WRITE_FLAGS = frozenset({"-d", "-D", "--delete", "-m", "-M", "--move", "-c", "-C", "--copy"}) +TAG_WRITE_FLAGS = frozenset({"-d", "--delete", "-f", "--force", "-a", "-s", "--sign"}) +STASH_ALLOWED_SUBCOMMANDS = frozenset({"list", "show"}) +WORKTREE_ALLOWED_SUBCOMMANDS = frozenset({"list"}) +REMOTE_WRITE_SUBCOMMANDS = frozenset({"add", "remove", "rm", "rename", "set-url", "set-head", "prune", "update"}) +CONFIG_WRITE_INDICATORS = frozenset({"--unset", "--unset-all", "--remove-section", "--rename-section", "--replace-all", "--add"}) + +_GIT_ALIAS_CACHE: dict[str, str] | None = None + + +def _load_git_aliases() -> dict[str, str]: + global _GIT_ALIAS_CACHE + if _GIT_ALIAS_CACHE is not None: + return _GIT_ALIAS_CACHE + try: + result = subprocess.run( + ["git", "config", "--get-regexp", r"^alias\."], + capture_output=True, + text=True, + timeout=5, + ) + aliases: dict[str, str] = {} + for line in result.stdout.strip().splitlines(): + parts = line.split(None, 1) + if len(parts) == 2: + name = parts[0].removeprefix("alias.") + aliases[name] = parts[1].split()[0] + _GIT_ALIAS_CACHE = aliases + except (subprocess.TimeoutExpired, FileNotFoundError): + _GIT_ALIAS_CACHE = {} + return _GIT_ALIAS_CACHE + + +def _resolve_alias(subcommand: str) -> str: + aliases = _load_git_aliases() + return aliases.get(subcommand, subcommand) + + +def _is_readonly_git(tokens: list[str]) -> bool: + """Check if a parsed git command is read-only.""" + if not tokens or tokens[0] != "git": + return True + + args = tokens[1:] + # Skip global git flags (e.g. git -C /path ...) + while args and args[0].startswith("-"): + flag = args.pop(0) + if flag in ("-C", "-c", "--git-dir", "--work-tree", "--namespace"): + if args: + args.pop(0) + if not args: + return True + + subcommand = _resolve_alias(args[0]) + rest = args[1:] + + if subcommand not in ALLOWED_SUBCOMMANDS: + return False + + if subcommand == "branch" and any(f in BRANCH_WRITE_FLAGS for f in rest): + return False + if subcommand == "tag" and any(f in TAG_WRITE_FLAGS for f in rest): + return False + if subcommand == "stash": + stash_sub = rest[0] if rest else "push" + if stash_sub not in STASH_ALLOWED_SUBCOMMANDS: + return False + if subcommand == "worktree": + wt_sub = rest[0] if rest else "" + if wt_sub not in WORKTREE_ALLOWED_SUBCOMMANDS: + return False + if subcommand == "remote": + remote_sub = rest[0] if rest else "" + if remote_sub in REMOTE_WRITE_SUBCOMMANDS: + return False + if subcommand == "config": + if any(f in CONFIG_WRITE_INDICATORS for f in rest): + return False + # `git config key value` (positional set) — 2+ non-flag args means a write + non_flag_args = [a for a in rest if not a.startswith("-")] + if len(non_flag_args) >= 2: + return False + + return True + + +_SHELL_SPLIT = re.compile(r"\s*(?:&&|\|\||[;|])\s*") + + +def _split_shell_commands(command: str) -> list[str]: + return [seg.strip() for seg in _SHELL_SPLIT.split(command) if seg.strip()] + + +def _is_command_safe(command: str) -> bool: + for segment in _split_shell_commands(command): + try: + tokens = shlex.split(segment) + except ValueError: + return False + if tokens and tokens[0] == "git" and not _is_readonly_git(tokens): + return False + return True + + +def _deny(reason: str) -> dict: + return { + "hookSpecificOutput": { + "hookEventName": "PreToolUse", + "permissionDecision": "deny", + "permissionDecisionReason": reason, + } + } + + +def _allow() -> dict: + return {"continue": True} + + +def main() -> None: + try: + payload = json.loads(sys.stdin.read()) + except (json.JSONDecodeError, EOFError): + json.dump(_allow(), sys.stdout) + return + + tool_name = payload.get("tool_name", "") + if tool_name not in TERMINAL_TOOL_NAMES: + json.dump(_allow(), sys.stdout) + return + + tool_input = payload.get("tool_input", {}) + command = tool_input.get("command") or tool_input.get("input") or "" + if not command: + json.dump(_allow(), sys.stdout) + return + + if _is_command_safe(command): + json.dump(_allow(), sys.stdout) + else: + json.dump( + _deny("Blocked: git mutation commands are not allowed in this repository."), + sys.stdout, + ) + + +if __name__ == "__main__": + main() diff --git a/tests/test_hooks.py b/tests/test_hooks.py new file mode 100644 index 00000000..912e6d42 --- /dev/null +++ b/tests/test_hooks.py @@ -0,0 +1,191 @@ +# Copyright (c) Microsoft Corporation. +# Licensed under the MIT License. + +import importlib.util +import json +from pathlib import Path +import sys +from unittest.mock import patch + +import pytest + +_script_path = Path(__file__).parent.parent / "scripts" / "block_git_mutations.py" +_spec = importlib.util.spec_from_file_location("block_git_mutations", _script_path) +assert _spec and _spec.loader +_mod = importlib.util.module_from_spec(_spec) +sys.modules["block_git_mutations"] = _mod +_spec.loader.exec_module(_mod) + +_is_command_safe = _mod._is_command_safe +_is_readonly_git = _mod._is_readonly_git +main = _mod.main + + +class TestIsReadonlyGit: + """Unit tests for _is_readonly_git token-level checks.""" + + @pytest.mark.parametrize( + "tokens", + [ + ["git", "status"], + ["git", "diff"], + ["git", "diff", "--cached"], + ["git", "log", "--oneline"], + ["git", "log", "-n", "5"], + ["git", "show", "HEAD"], + ["git", "branch"], + ["git", "branch", "-v"], + ["git", "branch", "--list"], + ["git", "ls-files"], + ["git", "ls-tree", "HEAD"], + ["git", "rev-parse", "HEAD"], + ["git", "describe", "--tags"], + ["git", "remote", "-v"], + ["git", "tag"], + ["git", "tag", "-l"], + ["git", "stash", "list"], + ["git", "config", "--get", "user.name"], + ["git", "blame", "README.md"], + ["git", "shortlog", "-sn"], + ["git", "worktree", "list"], + ["git", "-C", "/some/path", "status"], + ["git", "add", "."], + ["git", "add", "-A"], + ["git", "push", "origin", "main"], + ], + ) + def test_allows_readonly(self, tokens: list[str]) -> None: + assert _is_readonly_git(tokens) is True + + @pytest.mark.parametrize( + "tokens", + [ + ["git", "commit", "-m", "test"], + ["git", "reset", "--hard", "HEAD~1"], + ["git", "rebase", "main"], + ["git", "merge", "feature-branch"], + ["git", "cherry-pick", "abc123"], + ["git", "revert", "HEAD"], + ["git", "stash"], + ["git", "stash", "push"], + ["git", "stash", "pop"], + ["git", "stash", "drop"], + ["git", "tag", "-a", "v1.0", "-m", "release"], + ["git", "tag", "-d", "v1.0"], + ["git", "branch", "-d", "old-branch"], + ["git", "branch", "-D", "old-branch"], + ["git", "branch", "-m", "new-name"], + ["git", "branch", "--delete", "old-branch"], + ["git", "remote", "add", "upstream", "url"], + ["git", "remote", "remove", "upstream"], + ["git", "config", "user.name", "Somebody"], + ["git", "config", "--unset", "user.name"], + ["git", "am"], + ["git", "apply"], + ["git", "checkout"], + ["git", "switch"], + ["git", "restore"], + ["git", "clean"], + ["git", "rm", "file.py"], + ], + ) + def test_blocks_mutations(self, tokens: list[str]) -> None: + assert _is_readonly_git(tokens) is False + + +class TestIsCommandSafe: + """Tests for full shell command parsing including chained commands and aliases.""" + + def test_non_git_commands_pass(self) -> None: + assert _is_command_safe("python -m pytest tests/") is True + assert _is_command_safe("make check test") is True + assert _is_command_safe("uv add stamina") is True + assert _is_command_safe("pyright src/") is True + + def test_chained_readonly_pass(self) -> None: + assert _is_command_safe("git status && git diff") is True + assert _is_command_safe("git log --oneline | head -5") is True + assert _is_command_safe("git status; git log") is True + + def test_chained_with_mutation_blocked(self) -> None: + assert _is_command_safe("git status && git commit -m 'test'") is False + assert _is_command_safe("git diff; git reset --hard HEAD") is False + assert _is_command_safe("make test && git commit -m 'auto'") is False + + def test_quoted_args_handled(self) -> None: + assert _is_command_safe("git commit -m 'this is a test'") is False + assert _is_command_safe('git log --grep="some pattern"') is True + + def test_git_alias_resolved(self) -> None: + fake_aliases = {"ci": "commit", "st": "status", "co": "checkout"} + with patch.object(_mod, "_load_git_aliases", return_value=fake_aliases): + assert _is_command_safe("git ci -m 'test'") is False + assert _is_command_safe("git st") is True + assert _is_command_safe("git co feature") is False + + def test_empty_command_passes(self) -> None: + assert _is_command_safe("") is True + + +class TestMainHook: + """Integration tests for the main() entry point.""" + + def _run_hook(self, tool_name: str, command: str) -> dict: + payload = json.dumps( + { + "tool_name": tool_name, + "tool_input": {"command": command}, + } + ) + import io + + with ( + patch("sys.stdin", io.StringIO(payload)), + patch("sys.stdout", new_callable=io.StringIO) as mock_stdout, + ): + main() + return json.loads(mock_stdout.getvalue()) + + @pytest.mark.parametrize("tool_name", ["Bash", "runTerminalCommand"]) + @pytest.mark.parametrize( + "command", + [ + "git commit -m 'test'", + "git reset --hard HEAD~1", + "git rebase main", + "git merge feature-branch", + "git cherry-pick abc123", + "git revert HEAD", + "git stash", + "git tag -a v1.0", + ], + ) + def test_blocks_mutations(self, tool_name: str, command: str) -> None: + output = self._run_hook(tool_name, command) + assert output["hookSpecificOutput"]["permissionDecision"] == "deny" + + @pytest.mark.parametrize("tool_name", ["Bash", "runTerminalCommand"]) + @pytest.mark.parametrize( + "command", + [ + "git status", + "git diff", + "git diff --cached", + "git log --oneline", + "git show HEAD", + "git branch -v", + "git add .", + "git push origin main", + ], + ) + def test_allows_readonly(self, tool_name: str, command: str) -> None: + output = self._run_hook(tool_name, command) + assert output.get("continue") is True + + def test_ignores_non_terminal_tools(self) -> None: + output = self._run_hook("editFiles", "git commit -m 'sneaky'") + assert output.get("continue") is True + + def test_allows_non_git_commands(self) -> None: + output = self._run_hook("Bash", "make check test") + assert output.get("continue") is True