diff --git a/host-setup/agent-safety/gh-write-guard.py b/host-setup/agent-safety/gh-write-guard.py index 182baceb..2eee93ec 100755 --- a/host-setup/agent-safety/gh-write-guard.py +++ b/host-setup/agent-safety/gh-write-guard.py @@ -23,6 +23,13 @@ is blocked, or an explicit-bypass flag (`gh pr merge --admin`, `git commit/push --no-verify`). The branch's live rules are the judge, so a code-style develop is denied and a config-style develop is allowed with no hardcoded repo list. + 5. a hand-rolled reply/resolve for a review thread: a `resolveReviewThread` mutation via `gh api + graphql`, or a POST to the review-comment replies endpoint, where `scripts/pr_review.py reply ... + --resolve` is the documented one-call path. Splitting the two into separate hand-run acts is what + let a reply sit unresolved across a push and a re-request, reading as untriaged to a maintainer + skimming the pull request (the incident behind this rule). Permitted only under the same + GH_WRITE_GUARD_ALLOW grant rule 3 reads, since the helper refuses a cross-owner pull request outright + and the hand-run GraphQL form is then the documented fallback, not a footgun. Run `gh-write-guard.py --selftest` to verify the decision matrix without Claude Code. """ @@ -50,9 +57,15 @@ re.VERBOSE, ) _GH_API = re.compile(r"\bgh\s+api\b") -_EXPLICIT_WRITE_METHOD = re.compile(r"(?:--method|-X)\s+(?:POST|PUT|PATCH|DELETE)\b", re.IGNORECASE) +# `-X`/`--method` accept a separate value (`-X POST`), an attached one (`-XPOST`), and an equals-attached one (`-X=POST`, `--method=POST`). +# This must match every spelling, matching how `_gh_effective_method` reads it. +_EXPLICIT_WRITE_METHOD = re.compile( + r"(?:--method[= ]|-X[= ]?)\s*(?:POST|PUT|PATCH|DELETE)\b", re.IGNORECASE +) # A gh api call with a field flag defaults to POST even without -X, so it is a write. -_API_FIELD_FLAG = re.compile(r"(?:^|\s)(?:-f|-F|--field|--raw-field|--input)\b") +# `-f`/`-F` also accept an attached value (`-fbody=x`), so no trailing `\b` is required after them. +# It is required after the long-form spellings, where one legitimately separates the flag from the next word. +_API_FIELD_FLAG = re.compile(r"(?:^|\s)(?:-f|-F)|(?:^|\s)(?:--field|--raw-field|--input)\b") _GRAPHQL = re.compile(r"\bgh\s+api\b.*\bgraphql\b", re.DOTALL) _MUTATION = re.compile(r"\bmutation\b") # Loose pre-filter only: matches `git` before `push` even with global options between them @@ -89,18 +102,40 @@ r"""(?:-F|-f|--field|--raw-field)\s+[A-Za-z_][\w]*=(?P'[^']*'|"[^"]*"|\S+)""" ) # Every spelling gh accepts for the target flag, being `--repo x`, `--repo=x`, `-R x`, `-R=x`, and the attached short form `-Rx`. -# A form left out is not a near-miss, it is a silent bypass of the whole repository scope, so the separator is matched rather than assumed to be a space. -# The look-behind requires the flag to start a shell token, meaning whitespace before it or the string start, which is where a real flag always sits. -# A value that opens a quoted span, as in `--title "-Rowner/repo"`, is therefore not read as a target. -# A mention inside prose, as in `--title "use -Rowner/repo"`, is still read as a target and still denies. -# A space precedes it exactly as one precedes a real flag, so no look-behind can separate the two. -# Telling a flag from text needs argv-position parsing, the way _push_targets does it for git push. -_EXPLICIT_REPO = re.compile( - r"(?['\"]?)(?P[^\s'\"]+)(?P=q)" -) -_API_REPO_PATH = re.compile( - r"\bgh\s+api\b[^\n|]*?\brepos/(?P[A-Za-z0-9_.\-]+)/(?P[A-Za-z0-9_.\-]+)" -) +# A form left out is not a near-miss, it is a silent bypass of the whole repository scope, so each is read by argv position below (`_gh_write_targets`) rather than assumed to be a space-separated pair. +_REPO_FLAG_BARE = {"--repo", "-R"} +_REPOS_PATH_TOKEN = re.compile(r"^repos/(?P[A-Za-z0-9_.\-]+)/(?P[A-Za-z0-9_.\-]+)") +# Flags whose own value is opaque text (a PR/issue title or body, a GraphQL field, a jq/template expression, a header), and so is skipped whole rather than pattern-matched for a repo target. +# Without this, a --body describing a `--repo /` doc line, or a commit message quoting the same convention, reads as a real flag. +# The incident this closes denied an ordinary `git commit` whose message body merely quoted the fleet's own `--repo owner/repo` example text. +_GH_TEXT_VALUE_FLAGS = { + "--title", + "-t", + "--body", + "-b", + "--body-file", + "--notes", + "--notes-file", + "--message", + "-m", + "--desc", + "-f", + "-F", + "--field", + "--raw-field", + "--input", + "--jq", + "--template", + "-q", + "-H", + "--header", + "--method", + "-X", + "--cache", + "--hostname", + "-p", + "--preview", +} def _is_gh_write(cmd): @@ -288,6 +323,43 @@ def _is_git_exe(tok): return base in ("git", "git.exe") +def _is_gh_exe(tok): + """True if the token invokes gh, including an absolute/relative path or a .exe suffix, the same + recognition `_is_git_exe` gives git, so an invocation named only inside a quoted --body forms no + such token and is never mistaken for a real gh call. + """ + base = tok.rsplit("/", 1)[-1].rsplit("\\", 1)[-1].lower() + return base in ("gh", "gh.exe") + + +def _collect_arglist(toks, start): + """Collect argv tokens from `start` up to the next shell separator (|, &&, ;, newline), skipping a + redirection operator and the file-descriptor number or target token attached to it. Shared by + `_git_subcommand_arglists` and `_gh_arg_lists` so a command's own argv, not text living inside an + unrelated --body/--title/-f value elsewhere in the line, is what either scans for a target. + + Returns (args, index_after_this_invocation). + """ + n = len(toks) + k = start + args = [] + while k < n: + t = toks[k] + if _is_separator(t): + break # a command separator ends this invocation + if t.isdigit() and k + 1 < n and _is_redir_op(toks[k + 1]): + k += 1 # a file-descriptor number before a redirection is shell syntax, not argv + continue + if _is_redir_op(t): + k += 1 # skip the redirection operator and its target token; args continue after it + if k < n and not _is_shell_op(toks[k]): + k += 1 + continue + args.append(t) + k += 1 + return args, k + + def _git_subcommand_arglists(cmd, sub): """Every `git [global-options] ` in the command, each as the argv up to the next shell operator. @@ -311,22 +383,7 @@ def _git_subcommand_arglists(cmd, sub): else: j += 1 if j < n and toks[j] == sub: - k = j + 1 - args = [] - while k < n: - t = toks[k] - if _is_separator(t): - break # a command separator (|, &&, ;, newline) ends this git invocation - if t.isdigit() and k + 1 < n and _is_redir_op(toks[k + 1]): - k += 1 # a file-descriptor number before a redirection is shell syntax, not git argv - continue - if _is_redir_op(t): - k += 1 # skip the redirection operator and its target token; args continue after it - if k < n and not _is_shell_op(toks[k]): - k += 1 - continue - args.append(t) - k += 1 + args, k = _collect_arglist(toks, j + 1) out.append(args) i = k else: @@ -334,6 +391,228 @@ def _git_subcommand_arglists(cmd, sub): return out +def _gh_arg_lists(cmd): + """Every `gh [args...]` invocation's own argv, from the token after `gh` up to the next shell + separator, in `cmd` itself, not inside any `sh -c`/`bash -c` wrapper (`_all_gh_arg_lists` covers + that). Argv-position parsing, the same as `_git_subcommand_arglists` gives git, so a `--repo`/`-R` + flag, a `repos//` API path, or a GraphQL query field is read only from where a real gh + argument sits, never from text carried inside an unrelated flag value elsewhere in the command. + """ + toks = _shell_tokens(cmd) + n = len(toks) + out = [] + i = 0 + while i < n: + if not _is_gh_exe(toks[i]): + i += 1 + continue + args, k = _collect_arglist(toks, i + 1) + out.append(args) + i = k + return out + + +_SHELL_WRAPPER_EXE = ("sh", "bash", "zsh", "ksh", "dash") + + +def _is_shell_wrapper_exe(tok): + """True if the token invokes a shell that runs a `-c ` argument as a nested command line.""" + base = tok.rsplit("/", 1)[-1].rsplit("\\", 1)[-1].lower().removesuffix(".exe") + return base in _SHELL_WRAPPER_EXE + + +def _embedded_wrapper_commands(cmd, _depth=0): + """Every command string embedded in a `sh -c '...'`/`bash -c "..."`-style wrapper invocation in + `cmd`, recursively, capped at a few levels of nesting. A `gh`/`git` call wrapped this way forms no + standalone `gh`/`git` token of its own, so `_gh_arg_lists` and `_git_subcommand_arglists` would + otherwise miss it entirely, the same bypass `sh -c 'gh issue comment --repo / ...'` + exercises against a plain token scan. + """ + if _depth > 4: + return [] + out = [] + toks = _shell_tokens(cmd) + n = len(toks) + i = 0 + while i < n: + if _is_shell_wrapper_exe(toks[i]): + args, k = _collect_arglist(toks, i + 1) + # `-c` may be clustered with other short options (`bash -lc`, `sh -ec`), the command string still the next argv token. + # A form left out here is a silent bypass of every rule below, the same shape a bare `-c` closes. + ci = next( + ( + x + for x, a in enumerate(args) + if a.startswith("-") and not a.startswith("--") and a.endswith("c") + ), + None, + ) + if ci is not None and ci + 1 < len(args): + inner = args[ci + 1] + out.append(inner) + out.extend(_embedded_wrapper_commands(inner, _depth + 1)) + i = k + else: + i += 1 + return out + + +def _all_gh_arg_lists(cmd): + """`_gh_arg_lists` for `cmd` itself, plus for every command string a `sh -c`/`bash -c`-style wrapper + embeds in it, so a `gh` call hidden behind such a wrapper is scanned exactly like a bare one. + """ + out = list(_gh_arg_lists(cmd)) + for inner in _embedded_wrapper_commands(cmd): + out.extend(_gh_arg_lists(inner)) + return out + + +def _repo_flag_value(tok): + """The value carried by a `--repo=value`/`-R=value`/`-Rvalue` (attached-short-form) token, or None + when tok is not one of those. A bare `--repo`/`-R` is handled separately since its value is the next + token rather than part of this one. + """ + if tok.startswith("--repo="): + return tok[len("--repo=") :] + if tok.startswith("-R="): + return tok[len("-R=") :] + if tok.startswith("-R") and len(tok) > 2 and tok[2] != "=": + return tok[2:] + return None + + +def _gh_write_targets(cmd): + """Every explicit owner/repo target named in an actual `gh` invocation's own argv (including one + embedded in a `sh -c`/`bash -c` wrapper): a `--repo`/`-R` flag value, or a `repos//` API + path token. Argv-position parsing, the way `_push_targets` reads a git push target, so a --repo/repos + mention that is only prose, inside an unrelated --body/--title value or a commit message, is never + read as one. + """ + targets = [] + for args in _all_gh_arg_lists(cmd): + n = len(args) + i = 0 + while i < n: + t = args[i] + if t in _GH_TEXT_VALUE_FLAGS and "=" not in t: + i += 2 # this flag's own value is opaque text, never a repo target + continue + if t in _REPO_FLAG_BARE: + if i + 1 < n: + val = args[i + 1] + if "/" in val and "<" not in val: + o, r = val.split("/", 1) + targets.append((o.lower(), r.lower())) + i += 2 + continue + val = _repo_flag_value(t) + if val is not None: + if "/" in val and "<" not in val: + o, r = val.split("/", 1) + targets.append((o.lower(), r.lower())) + i += 1 + continue + m = _REPOS_PATH_TOKEN.match(t) + if m and "<" not in t: + targets.append((m.group("owner").lower(), m.group("repo").lower())) + i += 1 + return targets + + +def _gh_api_path(args): + """The positional API path argument of a `gh api ...` invocation's own argv, or None. Skips + the invocation's own value-taking flags first (`-X POST`, `-f k=v`, ...) so their values are never + mistaken for the path positional. + """ + if not args or args[0] != "api": + return None + n = len(args) + i = 1 + while i < n: + t = args[i] + if t in _GH_TEXT_VALUE_FLAGS and "=" not in t: + i += 2 + continue + if t.startswith("-"): + i += 1 + continue + return t + return None + + +def _gh_field_value(tok): + """The `name=value` field text carried by one token, in every field-flag spelling `gh` accepts: a + bare `-f`/`-F`/`--field`/`--raw-field` (the caller reads the next token as the value), the + equals-attached long form (`--field=name=value`/`--raw-field=name=value`), the equals-attached short + form (`-f=name=value`/`-F=name=value`), or the fully attached short form (`-fname=value`/ + `-Fname=value`, no separator at all). Returns None for a bare flag, whose value is the next token + rather than part of this one. + """ + for pfx in ("--field=", "--raw-field=", "-f=", "-F="): + if tok.startswith(pfx): + return tok[len(pfx) :] + if tok.startswith(("-f", "-F")) and len(tok) > 2 and tok[2] != "=": + return tok[2:] + return None + + +def _gh_graphql_query(args): + """The GraphQL query text carried by this `gh api graphql` invocation's own `query=...` field + argument, in whichever field-flag spelling carries it (`_gh_field_value`), or None. Reads only that + field token's own content rather than searching the whole command for the mutation's name, so a + --body or PR description merely describing the mutation is not read as one issuing it. + """ + n = len(args) + i = 0 + while i < n: + t = args[i] + if t in ("-f", "-F", "--field", "--raw-field"): + if i + 1 < n and args[i + 1].startswith("query="): + return args[i + 1][len("query=") :] + i += 2 + continue + v = _gh_field_value(t) + if v is not None: + if v.startswith("query="): + return v[len("query=") :] + i += 1 + continue + i += 1 + return None + + +def _gh_effective_method(args): + """The effective HTTP method of a `gh api` invocation's own argv: an explicit `-X`/`--method` value + when present, in every spelling `gh` accepts, else POST when a field flag is present (`gh`'s own + default for a write-shaped call), else GET. + """ + n = len(args) + i = 0 + method = None + has_field = False + while i < n: + t = args[i] + if t in ("-X", "--method"): + if i + 1 < n: + method = args[i + 1].upper() + i += 2 + continue + if t.startswith("--method="): + method = t[len("--method=") :].upper() + i += 1 + continue + if t.startswith("-X") and len(t) > 2: + method = t[3:].upper() if t[2] == "=" else t[2:].upper() + i += 1 + continue + if t in ("-f", "-F", "--field", "--raw-field") or _gh_field_value(t) is not None: + has_field = True + i += 1 + if method: + return method + return "POST" if has_field else "GET" + + def _push_arg_lists(cmd): return _git_subcommand_arglists(cmd, "push") @@ -487,6 +766,66 @@ def _check_push_bypass(cmd, cwd, origin, current_branch=None, rules_lookup=None) return "allow", "" +# The GraphQL mutation resolving a review thread, denied when hand-rolled (see `_check_reply_resolve_helper`). +_RESOLVE_THREAD_MUTATION = re.compile(r"\bresolveReviewThread\b") +# The REST endpoint the incident's reply half hand-rolled: `POST /repos/{owner}/{repo}/pulls/{n}/comments/{id}/replies`. +# Distinct from the `addPullRequestReviewThreadReply` GraphQL mutation, which stays allowed as the documented cross-owner fallback (.github/copilot-instructions.md) and is not matched here. +_REPLY_ENDPOINT_PATH = re.compile(r"\bpulls/\d+/comments/\d+/replies\b") + + +def _check_reply_resolve_helper(cmd, environ): + """Deny a hand-rolled `resolveReviewThread` mutation or a POST to the review-comment replies + endpoint, the two-step shape that let a reply sit unresolved across a push and a re-request, reading + as untriaged to a maintainer skimming the pull request. `scripts/pr_review.py reply ... --resolve` + captures the thread id from a live query and posts the reply and the resolve as one call, the + documented path either way. + + Scoped to the query text or API path an actual `gh api graphql`/`gh api` invocation's own argv + carries, including one embedded in a `sh -c`/`bash -c` wrapper, never a substring search over the + whole command, so a --body or PR description merely describing the mutation or the endpoint is not + misread as a real call. + + A REST reply is permitted when its own URL names a target the maintainer has already granted this + session, since the helper refuses a cross-owner pull request outright and the hand-run form is then + the documented fallback for that specific repository. A `resolveReviewThread` mutation carries no + target in its own text (the thread id is opaque), so the same fallback is permitted there whenever + any grant is active this session, a coarser signal than a REST reply gets, and the residual gap the + module docstring's "precision over recall" already accepts for this class of rule. + """ + granted = _granted_targets(environ) + helper = ( + 'Use `scripts/pr_review.py reply --repo / --match "" ' + '--body "" --resolve` instead, which captures the thread id from a live query and posts ' + "the reply and the resolve as one call. See .github/copilot-instructions.md 'Interacting with " + "GitHub Copilot PR reviews'." + ) + for args in _all_gh_arg_lists(cmd): + path = _gh_api_path(args) + if path == "graphql": + q = _gh_graphql_query(args) + if q and _MUTATION.search(q) and _RESOLVE_THREAD_MUTATION.search(q): + if granted: + continue + return "deny", ( + "This resolves a review thread directly through `gh api graphql` instead of the " + "helper that captures the reply and the resolve in one call, so a reply can be left " + "unresolved across a push and a re-request. " + helper + ) + if path and _REPLY_ENDPOINT_PATH.search(path) and _gh_effective_method(args) == "POST": + m = _REPOS_PATH_TOKEN.match(path) + if m: + target = (m.group("owner").lower(), m.group("repo").lower()) + if target in granted or (target[0], "*") in granted: + continue # this exact target is the maintainer's granted cross-owner exception + elif granted: + continue # path carries no readable owner/repo; fall back to grant presence like the graphql case above + return "deny", ( + "This posts a review-comment reply directly to the REST replies endpoint instead of the " + "helper that captures the reply and the resolve in one call. " + helper + ) + return "allow", "" + + def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, environ=None): """Return (decision, reason). decision is 'allow' or 'deny'. @@ -544,17 +883,9 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, # 3. Explicit target outside the origin's owner if origin is None: origin = _origin_owner_repo(cwd) - targets = [] - # Every occurrence is read rather than the first, since a compound command carries one target per invocation. - # Reading only the first checks the harmless one while the write after `&&` goes unexamined. - for mr in _EXPLICIT_REPO.finditer(cmd): - val = mr.group("r") - if "/" in val and "<" not in val: - o, r = val.split("/", 1) - targets.append((o.lower(), r.lower())) - for m in _API_REPO_PATH.finditer(cmd): - if "<" not in m.group("owner"): - targets.append((m.group("owner").lower(), m.group("repo").lower())) + # `_gh_write_targets` reads argv position within each real `gh` invocation, so a compound command carrying one target per invocation still has every one read (the write after `&&` is not skipped). + # A --repo/repos// mention living inside an unrelated --body/--title/-f value, or in a non-gh command entirely, is not read as a target. + targets = _gh_write_targets(cmd) # This only runs when origin resolves, meaning a git checkout, since with no project context there is nothing to compare an explicit target against, so the check is skipped and rules 1 and 2 still apply. # A node-id target is invisible here regardless, which is what rule 2 guards. if origin: @@ -573,6 +904,11 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, "GOVERNANCE.md." ) + # 5. Hand-rolled reply/resolve for a review thread, bypassing scripts/pr_review.py's one-call helper. + dec, reason = _check_reply_resolve_helper(cmd, environ) + if dec == "deny": + return dec, reason + return "allow", "" @@ -591,8 +927,8 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, ), ( "gh api graphql -f query='mutation($t:ID!){resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"$TID\"", - "allow", - "mutation with captured $TID", + "deny", + "captured $TID still denied: resolve is reserved for the pr_review.py helper (#757)", ), ( 'gh issue comment 5 -R mankatcheung/job-finder --body "hi"', @@ -658,8 +994,86 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, ), ( "gh api graphql -f query='mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"TODO_fixit\"", + "deny", + "not a node id, but still a hand-rolled resolve: denied by rule 5", + ), +] + +# Rule-5 cases, covering the hand-rolled reply/resolve denial and its cross-owner grant escape. +# Each carries the environment the grant is read from, matching the _SCOPE_CASES convention below. +_REPLY_RESOLVE_CASES = [ + # (command, environ, expected_decision, label) + ( + "gh api graphql -f query='mutation($t:ID!){resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"$TID\"", + {}, + "deny", + "hand-rolled resolve with no grant", + ), + ( + 'gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -f body="fixed"', + {}, + "deny", + "hand-rolled REST reply with no grant", + ), + ( + "gh api graphql -f query='mutation($t:ID!){resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"$TID\"", + {_ALLOW_ENV: "esphome/esphome"}, + "allow", + "cross-owner grant present: hand-run resolve is the documented fallback", + ), + ( + 'gh api repos/esphome/esphome/pulls/5/comments/9/replies -f body="fixed"', + {_ALLOW_ENV: "esphome/esphome"}, "allow", - "short all-caps token is not a node id", + "REST reply permitted only for the exact granted target", + ), + ( + 'gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -f body="fixed"', + {_ALLOW_ENV: "esphome/esphome"}, + "deny", + "an unrelated grant does not exempt a same-owner REST reply (#757 review)", + ), + ( + 'gh api graphql -f query=\'mutation($t:ID!,$b:String!){addPullRequestReviewThreadReply(input:{pullRequestReviewThreadId:$t,body:$b}){comment{id}}}\' -F t="$TID" -F b="Fixed in abc123: summary."', + {}, + "allow", + "addPullRequestReviewThreadReply mutation is the documented fallback shape, not denied", + ), + ( + 'gh pr create --title "Guard hand-rolled resolve" --body "Denies a POST to the review-comment replies endpoint and a resolveReviewThread mutation, per #757."', + {}, + "allow", + "a --body merely describing the mutation/endpoint is not read as issuing one", + ), + ( + 'sh -c \'gh api graphql -f query="mutation($t:ID!){resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}" -F t="$TID"\'', + {}, + "deny", + "a resolve hidden behind sh -c is still caught (#757 review)", + ), + ( + 'bash -c "gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -f body=fixed"', + {}, + "deny", + "a REST reply hidden behind bash -c is still caught (#757 review)", + ), + ( + 'gh api graphql --field=query=mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}} -F t="$TID"', + {}, + "deny", + "the equals-attached --field=query=... spelling is still caught (#757 review)", + ), + ( + "gh api graphql -Fquery='mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"$TID\"", + {}, + "deny", + "the attached-short-form -Fquery=... spelling is still caught (#757 review)", + ), + ( + "gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies --method GET -f page=1", + {}, + "allow", + "a GET to the replies endpoint is a read, not the denied POST (#757 review)", ), ] @@ -745,6 +1159,78 @@ def classify(cmd, cwd=None, origin=None, current_branch=None, rules_lookup=None, "allow", "a value opening a quoted span is not a flag", ), + ( + 'git commit -m "Without --repo owner/repo, gh run list/view resolve wrong." && git push origin feature/x', + {}, + "allow", + "the incident: a commit message quoting --repo owner/repo is not a gh invocation at all", + ), + ( + 'gh pr comment 5 --body "See the docs on --repo owner/repo and repos/owner/repo usage"', + {}, + "allow", + "a --body describing --repo/repos path syntax is opaque text, not a real flag or API path", + ), + ( + "sh -c 'gh issue comment 5 --repo esphome/esphome --body hi'", + {}, + "deny", + "a cross-owner target hidden behind sh -c is still caught (#757 review)", + ), + ( + "bash -lc 'gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -f body=fixed'", + {}, + "deny", + "a REST reply behind a clustered bash -lc is still caught (CodeRabbit)", + ), + ( + "gh api --hostname github.com repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -f body=fixed", + {}, + "deny", + "a value-taking flag before the path does not hide the reply endpoint (CodeRabbit)", + ), + ( + "gh api -X POST graphql -f query='mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"$TID\"", + {}, + "deny", + "-X POST preceding graphql does not hide the mutation (CodeRabbit)", + ), + ( + "gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -fbody=fixed", + {}, + "deny", + "the attached -fbody=fixed form still enters the write gate (CodeRabbit)", + ), + ( + "gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -XPOST", + {}, + "deny", + "the attached -XPOST form still enters the write gate (self-found companion to CodeRabbit's -f finding)", + ), + ( + "gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -f=body=fixed", + {}, + "deny", + "the equals-attached -f=body=fixed form is still caught (CodeRabbit)", + ), + ( + "gh api graphql -F=query='mutation{resolveReviewThread(input:{threadId:$t}){thread{isResolved}}}' -F t=\"$TID\"", + {}, + "deny", + "the equals-attached -F=query=... form is still caught (CodeRabbit)", + ), + ( + "gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -X=GET -f page=1", + {}, + "allow", + "the equals-attached -X=GET form is still read as a read (CodeRabbit)", + ), + ( + "gh api repos/ptr727/PlexCleaner/pulls/5/comments/9/replies -X=POST", + {}, + "deny", + "the equals-attached -X=POST form still enters the write gate (CodeRabbit)", + ), ] # Rule-4 cases, covering branch-rule bypass. @@ -1157,6 +1643,18 @@ def _selftest(): if got != want: ok = False print(f" {mark} [{got:5}] want={want:5} {label}") + for cmd, env, want, label in _REPLY_RESOLVE_CASES: + got, _ = classify( + cmd, + origin=origin, + current_branch="feature/x", + rules_lookup=lambda br: set(), + environ=env, + ) + mark = "ok " if got == want else "FAIL" + if got != want: + ok = False + print(f" {mark} [{got:5}] want={want:5} {label}") for cmd, cur, rmap, want, label in _GIT_CASES: got, _ = classify( cmd,