diff --git a/git-guards/scripts/git-permission-guard.py b/git-guards/scripts/git-permission-guard.py index 22be195..55c8984 100755 --- a/git-guards/scripts/git-permission-guard.py +++ b/git-guards/scripts/git-permission-guard.py @@ -28,16 +28,13 @@ (r"cherry-pick\s+.*--no-verify", "bypasses commit hooks"), (r"rebase\s+.*--no-verify", "bypasses commit hooks"), (r"config\s+.*core\.hooksPath", "changes hook directory"), - (r"push\s+(--force|--force-with-lease|-f)\s+\S+\s+main\b", "force-pushes to the main branch"), + (r"^push\s+.*(--force|--force-with-lease|-f)\b", "force-pushes overwrite remote history"), ] # Commands requiring explicit user confirmation # Ordered from most specific to least specific to avoid false matches ASK_GIT = [ ("commit --amend", "Rewrites the last commit"), - ("push --force-with-lease", "Overwrites remote history"), - ("push --force", "Overwrites remote history"), - ("push -f", "Overwrites remote history"), ("worktree remove --force", "Removes worktree directory, discarding uncommitted changes"), ("worktree remove -f", "Removes worktree directory, discarding uncommitted changes"), ("cherry-pick", "Rewrites commit history"), @@ -305,6 +302,11 @@ def main(): git_config_opts.append(m.group(1).strip("'\"")) rest = m.group(2).strip() continue + # Boolean global options take no argument (different parse from -C/-c) + m = re.match(r'^(-p|-P|--paginate|--no-pager|--no-replace-objects|--bare)\s*(.*)', rest) + if m: + rest = m.group(2).strip() + continue break subcommand = rest else: @@ -341,7 +343,8 @@ def main(): subcmd_tokens = [] for i, tok in enumerate(subcmd_tokens): if tok == "-c" and i + 1 < len(subcmd_tokens): - if re.match(r"core\.hooksPath\s*(?:=|$)", subcmd_tokens[i + 1], re.IGNORECASE): + config_token = subcmd_tokens[i + 1] + if re.match(r"^core\.hooksPath(=|$)", config_token, re.IGNORECASE): deny("This command bypasses configured hooks. Fix the underlying issue instead.") if sub_tokens and sub_tokens[0] in BLOCKED_ON_MAIN and _is_on_main_branch(): diff --git a/git-guards/scripts/test_permission_guard.py b/git-guards/scripts/test_permission_guard.py index fa7a773..ef990ac 100755 --- a/git-guards/scripts/test_permission_guard.py +++ b/git-guards/scripts/test_permission_guard.py @@ -69,11 +69,14 @@ def check(label: str, cmd: str, expected_decision: str) -> bool: # git reset - must ask: can lose uncommitted work all_pass &= check("git reset", "git reset --hard HEAD", "ask") -# git push --force origin main - must DENY (DENY_GIT_ONLY: force-push to protected branch) +# git push --force origin main - must DENY (DENY_GIT_ONLY: any force-push is denied) all_pass &= check("git push --force origin main", "git push --force origin main", "deny") -# git push --force to a feature branch - must ask (non-main target) -all_pass &= check("git push --force feature branch", "git push --force origin feature/my-branch", "ask") +# git push --force to any branch is now denied (DENY_GIT_ONLY, tightened policy) +all_pass &= check("git push --force feature branch", "git push --force origin feature/my-branch", "deny") + +# git global option before push must still deny (--no-pager is stripped by extraction loop) +all_pass &= check("git --no-pager push --force feature branch", "git --no-pager push --force origin feature/my-branch", "deny") # DENY: commit --no-verify all_pass &= check("git commit --no-verify", "git commit -m msg --no-verify", "deny") @@ -91,15 +94,16 @@ def check(label: str, cmd: str, expected_decision: str) -> bool: all_pass &= check("git -C -c core.hooksPath deny", "git -C /some/path -c core.hooksPath=/dev/null commit -m test", "deny") all_pass &= check("git -C restore ask", "git -C /some/path restore file.txt", "ask") -# core.hooksPath precision: value containing the string should not trigger deny -all_pass &= check("hooksPath in value only", "git -c some.key=echo-core.hooksPath commit -m test", "silent_allow") +# core.hooksPath precision: value containing the substring must not trigger deny (uses fetch, not commit) +all_pass &= check("hooksPath in value only", "git -c some.key=echo-core.hooksPath fetch origin", "silent_allow") # Fallback bypass detection: unrecognised global option before -c breaks loop early all_pass &= check("--no-pager before -c hooksPath", "git --no-pager -c core.hooksPath=/dev/null commit -m msg", "deny") # Fallback also fires when loop parsed a prior -c but broke before a second -c hooksPath all_pass &= check("valid -c then --bare then -c hooksPath", "git -c user.name=test --bare -c core.hooksPath=/dev/null commit -m msg", "deny") -# False positive guard: commit message containing the bypass pattern as a substring must not deny -all_pass &= check("hooksPath in commit message", 'git -c user.name=test commit -m "allow -c core.hooksPath bypass example"', "silent_allow") +# False positive guard: tag message containing the bypass pattern as a substring must not deny +# Uses 'git tag' (not in BLOCKED_ON_MAIN) to test the tokenizer false-positive scenario on any branch +all_pass &= check("hooksPath in tag message", 'git -c user.name=test tag v99-test -m "allow -c core.hooksPath bypass example"', "silent_allow") print() print("ALL TESTS PASSED" if all_pass else "SOME TESTS FAILED") diff --git a/git-guards/scripts/test_shlex_valueerror.py b/git-guards/scripts/test_shlex_valueerror.py index ae7c9ac..23a6d33 100755 --- a/git-guards/scripts/test_shlex_valueerror.py +++ b/git-guards/scripts/test_shlex_valueerror.py @@ -45,15 +45,14 @@ def check(label: str, cmd: str, expected_decision: str) -> bool: all_pass = True -# ValueError path: loop breaks on unrecognised global option (--no-pager); -# remaining subcommand has an unclosed double quote → shlex.split() raises -# ValueError → subcmd_tokens = [] → no bypass detected → silent_allow. -# Without the fix, the old str.split() fallback would have tokenised on -# whitespace and incorrectly fired a deny for the -c core.hooksPath token. +# --no-pager is now stripped by the extraction loop before -c is processed. +# The loop extracts -c core.hooksPath=/dev/null directly → deny fires via the +# direct git_config_opts path even though the trailing commit message is +# malformed (unclosed quote). The shlex ValueError path is irrelevant here. all_pass &= check( "ValueError: unclosed double-quote with hooksPath in fallback subcommand", 'git --no-pager -c core.hooksPath=/dev/null commit -m "unclosed', - "silent_allow", + "deny", ) # ValueError path: unclosed single quote with bypass pattern in subcommand text.