| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Mostly a rubber-stamp, particularly the tests I just skimmed. It's also a common fix just with unsafe-options guards, and a regex fix which hopefully is truly better than before. I didn't question it in the interest of time. <!-- agent --> Address GHSA-w8jc-g24h-crhw by applying the existing guard policy to `Git.ls_remote()`, `Repo.merge_base()`, and `IndexFile.move()`. These wrappers previously omitted protocol or option checks already used by sibling APIs. Check flattened positional arguments and split short-option values before `ls_remote()` starts Git, including values that become the repository after option parsing. Add the independent `allow_unsafe_protocols` opt-in and recognize helper selectors even when their address is empty or begins with a newline. Match Git's scheme-character rules at the start of the address so ordinary IPv6 URLs and double colons in repository paths retain their meaning. This addresses the review finding that the broad matcher rejected valid IPv6 remotes. The protocol check is conservative for positional and split option values; long-form `server_option` values remain available without a protocol opt-in. Reuse the revision and pathspec option guards in `merge_base()` and `move()`, with explicit `allow_unsafe_options` opt-ins. Git currently rejects these denylisted options for those subcommands; the checks keep their policy aligned with sibling APIs. Preserve literal move operands behind `--` and validate options before either the dry run or actual move. Only treat exit status 1 from `merge_base()` as no common ancestor. Other failures, including invalid options, now propagate as `GitCommandError` instead of silently returning an empty list. Git reference: `git/git@d38352cd43ab9745686d697872408bc3249a153f`, inspected in `builtin/ls-remote.c`, `transport.c`, `builtin/merge-base.c`, `builtin/mv.c`, `url.c`, and `parse-options.c`. These confirm option parsing before the remote operand, helper selection independently of address contents, long-option abbreviations, and status 1 for unrelated histories. Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
There was a problem hiding this comment.
Unresolved critical release consistency and moderate correctness issues remain.
Review effort: Lite
Findings: 1
This PR adds safety guards to Git command wrappers and improves merge_base() error handling.
Changes:
| File | Summary | Review notes |
|---|---|---|
| test/test_command_guards.py | Adds regression coverage for command guards. | No issues noted. |
| git/repo/base.py | Validates merge-base options and propagates failures. | Moderate issue: status 1 is mishandled with is_ancestor. |
| git/index/base.py | Adds unsafe option validation for moves. | Nit: reference git-mv(1) instead of git-rm(1). |
| git/cmd.py | Adds protocol and option checks to ls_remote(). | Moderate issue: boolean keyword values can cause a TypeError. |
| doc/source/changes.rst | Documents the security release. | Critical version mismatch; nit: add the required release URL. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex GPT-6.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Apply the existing safety guards consistently to Git.ls_remote(), Repo.merge_base(), and IndexFile.move(), retaining explicit unsafe opt-ins. merge_base() now propagates command failures except for Git's status 1 result when no common ancestor exists.
The shared protocol check follows Git's transport-selector syntax, preserving ordinary IPv6 URLs and double colons in repository paths. Protocol-like positional arguments and split short-option values require an opt-in; long-form server options remain available. The option checks for merge_base() and move() align their policy with sibling APIs; current Git versions already reject those denylisted options for these two subcommands.
Advisory summary
GHSA-w8jc-g24h-crhw
The advisory is unpublished. Private reproduction and exploit details are omitted.
Validation
Regression cases failed before the fix. On Python 3.14.7 with Git 2.54.0 (Apple Git-157):
Git behavior was checked against git/git@d38352cd43ab9745686d697872408bc3249a153f, specifically builtin/ls-remote.c, transport.c, builtin/merge-base.c, builtin/mv.c, url.c, and parse-options.c.
Commit: c480bd5 — fix: complete safety checks in Git command wrappers.