diff --git a/src/autoskillit/hooks/_command_classification.py b/src/autoskillit/hooks/_command_classification.py index 8e32bf662..3b70b4c70 100644 --- a/src/autoskillit/hooks/_command_classification.py +++ b/src/autoskillit/hooks/_command_classification.py @@ -173,7 +173,9 @@ # line (real redirects), so segments carry only executable tokens. _HEREDOC_MARKER_RE = re.compile(r"<<-?\s*['\"]?(\w+)['\"]?([^\n]*)\n\t*\1(?=[ \t]*(?:\n|$))") -_PROTECTED_PATH_METADATA_GIT_SUBCOMMANDS: frozenset[str] = frozenset({"add", "diff", "status"}) +_PROTECTED_PATH_METADATA_GIT_SUBCOMMANDS: frozenset[str] = frozenset( + {"add", "diff", "status", "check-ignore"} +) _GIT_ADD_CONTENT_FLAGS: frozenset[str] = frozenset( { @@ -219,6 +221,7 @@ "--summary", } ) +_GIT_CHECK_IGNORE_SAFE_FLAGS: frozenset[str] = frozenset({"-v", "--verbose", "--no-index"}) _SHELL_SUBSTITUTION_RE = re.compile(r"\$\(|`|[<>]\(") _SHELL_STATE_VAR_RE = re.compile(r"\$(?:_|[A-Za-z][A-Za-z0-9_]*|\{[^}]+\})") _PROTECTED_READ_SHELL_OPS: frozenset[str] = frozenset({"&&", "||", ";", "|", "&"}) @@ -602,6 +605,38 @@ def _is_allowed_wc_flag(token: str) -> bool: return bool(_WC_FLAG_RE.fullmatch(token)) +def _is_safe_check_ignore_segment(segment: list[str]) -> bool: + """Return True for `git check-ignore` restricted to the safe metadata grammar. + + Required shape: ``git check-ignore (-v|--verbose) [--no-index] [--] LITERAL_PATH...`` + No Git global options (``-c``, ``-C``, ``--git-dir``, ``--work-tree``, ``--bare``, ...) + may precede the subcommand — those can redirect repository/config context outside + what the submitted command text shows. ``--stdin`` and any other non-literal path + source are rejected because the classifier cannot see what they would resolve to. + Redirect metacharacters are rejected outright rather than treated as literal paths. + """ + start = _command_start_index(segment) + if start is None or start + 1 >= len(segment) or segment[start + 1] != "check-ignore": + return False + + paths: list[str] = [] + seen_dash_dash = False + for token in segment[start + 2 :]: + if ">" in token or "<" in token: + return False + if not seen_dash_dash and token == "--": + seen_dash_dash = True + continue + if not seen_dash_dash and token.startswith("-"): + if token not in _GIT_CHECK_IGNORE_SAFE_FLAGS: + return False + continue + if not token or _SHELL_VAR_RE.search(token): + return False + paths.append(token) + return bool(paths) + + def is_allowed_protected_path_metadata_command(segment: list[str]) -> bool: """Return True for protected-path commands that inspect metadata or VCS state. @@ -617,6 +652,8 @@ def is_allowed_protected_path_metadata_command(segment: list[str]) -> bool: subcommand, flags = git_parts if subcommand not in _PROTECTED_PATH_METADATA_GIT_SUBCOMMANDS: return False + if subcommand == "check-ignore": + return _is_safe_check_ignore_segment(segment) if subcommand == "add": return not any( flag in _GIT_ADD_CONTENT_FLAGS or flag.startswith("--pathspec-from-file=") diff --git a/tests/hooks/test_command_classification_git.py b/tests/hooks/test_command_classification_git.py index a48ff89e0..3cd1b05fe 100644 --- a/tests/hooks/test_command_classification_git.py +++ b/tests/hooks/test_command_classification_git.py @@ -6,6 +6,7 @@ from autoskillit.hooks._command_classification import ( extract_git_subcommand_and_flags, + is_allowed_protected_path_metadata_command, is_git_command, ) @@ -81,3 +82,117 @@ def test_git_work_tree_skips_value(self): def test_git_bare_flag(self): result = extract_git_subcommand_and_flags(["git", "--bare", "log"]) assert result == ("log", []) + + +class TestCheckIgnoreMetadataClassification: + """Guard-compatibility fixture for the dry-walkthrough prescribed check-ignore form. + + Binds `git check-ignore -v {path}` (SKILL.md:152) to + is_allowed_protected_path_metadata_command so the guard classifier and the + skill's committability contract cannot silently drift apart again. + """ + + def test_prescribed_dry_walkthrough_form(self): + segment = ["git", "check-ignore", "-v", "src/autoskillit/recipes/remediation.yaml"] + assert is_allowed_protected_path_metadata_command(segment) is True + + def test_verbose_long_flag(self): + segment = ["git", "check-ignore", "--verbose", "src/autoskillit/recipes/remediation.yaml"] + assert is_allowed_protected_path_metadata_command(segment) is True + + def test_no_index_flag(self): + segment = [ + "git", + "check-ignore", + "-v", + "--no-index", + "src/autoskillit/recipes/remediation.yaml", + ] + assert is_allowed_protected_path_metadata_command(segment) is True + + def test_double_dash_separator(self): + segment = ["git", "check-ignore", "-v", "--", "src/autoskillit/recipes/remediation.yaml"] + assert is_allowed_protected_path_metadata_command(segment) is True + + def test_multiple_literal_paths(self): + segment = [ + "git", + "check-ignore", + "-v", + "src/autoskillit/recipes/remediation.yaml", + "src/autoskillit/agents/explorer.md", + ] + assert is_allowed_protected_path_metadata_command(segment) is True + + def test_no_paths_is_denied(self): + assert is_allowed_protected_path_metadata_command(["git", "check-ignore", "-v"]) is False + + def test_stdin_flag_denied(self): + segment = ["git", "check-ignore", "-v", "--stdin"] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_quiet_flag_denied(self): + segment = ["git", "check-ignore", "-q", "src/autoskillit/recipes/remediation.yaml"] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_global_c_flag_before_subcommand_denied(self): + segment = [ + "git", + "-c", + "core.excludesFile=src/autoskillit/recipes/remediation.yaml", + "check-ignore", + "-v", + "src/autoskillit/recipes/remediation.yaml", + ] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_global_git_dir_flag_denied(self): + segment = [ + "git", + "--git-dir", + "/tmp/.git", + "check-ignore", + "-v", + "src/autoskillit/recipes/remediation.yaml", + ] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_global_work_tree_flag_denied(self): + segment = [ + "git", + "--work-tree", + "/tmp", + "check-ignore", + "-v", + "src/autoskillit/recipes/remediation.yaml", + ] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_bare_flag_denied(self): + segment = [ + "git", + "--bare", + "check-ignore", + "-v", + "src/autoskillit/recipes/remediation.yaml", + ] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_redirect_target_denied(self): + segment = [ + "git", + "check-ignore", + "-v", + "src/autoskillit/recipes/remediation.yaml", + ">", + "/tmp/out", + ] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_shell_variable_path_denied(self): + segment = ["git", "check-ignore", "-v", "$VAR"] + assert is_allowed_protected_path_metadata_command(segment) is False + + def test_content_bearing_subcommand_still_denied(self): + segment = ["git", "show", "HEAD:src/autoskillit/recipes/remediation.yaml"] + assert is_allowed_protected_path_metadata_command(segment) is False diff --git a/tests/infra/test_recipe_read_guard.py b/tests/infra/test_recipe_read_guard.py index 647077fa8..1fa8f0a9e 100644 --- a/tests/infra/test_recipe_read_guard.py +++ b/tests/infra/test_recipe_read_guard.py @@ -105,6 +105,15 @@ def test_allows_non_recipe_yaml(self): "git diff --name-only -- src/autoskillit/recipes/remediation.yaml", "git status -- src/autoskillit/recipes/remediation.yaml", "wc -l src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git check-ignore --verbose src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v -- src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v --no-index src/autoskillit/recipes/remediation.yaml", + ( + "git check-ignore -v src/autoskillit/recipes/remediation.yaml " + "src/autoskillit/recipes/other.yaml" + ), + "git check-ignore -v src/autoskillit/recipes/remediation.yaml || true", ], ) def test_allows_recipe_path_vcs_and_metadata_commands(self, cmd): @@ -182,6 +191,23 @@ def test_denies_chained_recipe_read_after_allowed_git_add(self): # -A/--all/--force stage content even when restricted to a path. "git add -A -- src/autoskillit/recipes/remediation.yaml", "git add --all -- src/autoskillit/recipes/remediation.yaml", + # git check-ignore bypass fixtures — required negative controls. + "git -c core.excludesFile=src/autoskillit/recipes/remediation.yaml " + "check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git -C /tmp check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git --git-dir=/tmp/.git check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git --work-tree=/tmp check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git --bare check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v --stdin src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml > /tmp/out", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml && " + "cat src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml && cat $_", + 'git check-ignore -v "$(cat src/autoskillit/recipes/remediation.yaml)"', + "git check-ignore -v $VAR src/autoskillit/recipes/remediation.yaml", + "git check-ignore -q src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml" + "&& cat src/autoskillit/recipes/remediation.yaml", ], ) def test_denies_protected_path_content_bypasses(self, cmd): diff --git a/tests/server/test_tools_run_cmd_invariants.py b/tests/server/test_tools_run_cmd_invariants.py index e6523747d..e2919c278 100644 --- a/tests/server/test_tools_run_cmd_invariants.py +++ b/tests/server/test_tools_run_cmd_invariants.py @@ -78,6 +78,15 @@ async def test_allows_non_recipe_python_file(self, tool_ctx_kitchen_open): "git diff --name-only -- src/autoskillit/recipes/remediation.yaml", "git status -- src/autoskillit/recipes/remediation.yaml", "wc -l src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git check-ignore --verbose src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v -- src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v --no-index src/autoskillit/recipes/remediation.yaml", + ( + "git check-ignore -v src/autoskillit/recipes/remediation.yaml " + "src/autoskillit/recipes/other.yaml" + ), + "git check-ignore -v src/autoskillit/recipes/remediation.yaml || true", ], ) async def test_allows_recipe_path_vcs_and_metadata_commands(self, tool_ctx_kitchen_open, cmd): @@ -147,6 +156,23 @@ async def test_denies_chained_recipe_read_after_allowed_git_add(self, tool_ctx_k "git add -- src/autoskillit/recipes/remediation.yaml" "&&cat src/autoskillit/recipes/remediation.yaml" ), + # git check-ignore bypass fixtures — required negative controls. + "git -c core.excludesFile=src/autoskillit/recipes/remediation.yaml " + "check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git -C /tmp check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git --git-dir=/tmp/.git check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git --work-tree=/tmp check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git --bare check-ignore -v src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v --stdin src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml > /tmp/out", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml && " + "cat src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml && cat $_", + 'git check-ignore -v "$(cat src/autoskillit/recipes/remediation.yaml)"', + "git check-ignore -v $VAR src/autoskillit/recipes/remediation.yaml", + "git check-ignore -q src/autoskillit/recipes/remediation.yaml", + "git check-ignore -v src/autoskillit/recipes/remediation.yaml" + "&& cat src/autoskillit/recipes/remediation.yaml", ], ) async def test_denies_protected_path_content_bypasses(self, tool_ctx_kitchen_open, cmd):