fix(agents): address hook review feedback - #2208
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Warning Review limit reached
Next review available in: 23 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
![indent-zero[bot]](https://avatars.githubusercontent.com/in/1426746?s=60&v=4){kind=small}
| if [[ -z $http_method ]] && | ||
| printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])(http|https|xh)([[:space:]]|$)' && | ||
| printf '%s\n' "$command" | grep -Eq '(^|[[:space:]])[^[:space:]=]+(:=|=)[^[:space:]]+'; then | ||
| printf '%s\n' "$command" | grep -Eq '(^|[[:space:]])[^-[:space:]=?:/][^[:space:]=?/]*(:=|=)[^[:space:]]+'; then |
There was a problem hiding this comment.
Latent: HTTPie query-param syntax still false-blocked.
The new regex correctly excludes URL/option shapes, but HTTPie's URL query-parameter form k==v still matches: the middle class stops before the first =, then (:=|=) consumes the first =, and [^[:space:]]+ greedily eats =value.
Repro on any protected suffix:
printf '{"tool_input":{"command":"http https://api.github.com/repos/o/r/hooks per_page==10"}}' \
| bash config/shared/hooks/block-gh-settings.sh
# exit 2: A direct POST request to a repository control-plane endpoint was requested.This is pre-existing behavior (the old regex had it too), so it's not a regression — but since this PR is loosening HTTPie handling anyway, worth teaching the regex that == is a URL query param, not a body. One option is to require the value not start with =, e.g. (:=|=)[^=[:space:]][^[:space:]]*.
| local config="$1" | ||
| case "$config" in | ||
| config/codex/hooks.json | config/claude/settings.json) | ||
| jq -r '.hooks.PreToolUse[] | select(.matcher == "Bash") | .hooks[]?.command' "$config" |
There was a problem hiding this comment.
Latent: exact-string matcher check will miss compound matchers.
For Codex/Claude you use select(.matcher == "Bash"), while Grok (line 18) already uses the more permissive test("(^|\\|)Bash($|\\|)"). If either Codex or Claude ever adopts a compound matcher such as "Bash|Edit" (Claude-side documentation supports this shape), the extraction returns nothing and the wiring test would fail even though the hook is correctly registered. Applying the same regex form uniformly across all three branches would be forward-compatible.
1 participant
Summary
Follow-up to #2207, which repository automation merged before its review feedback could be incorporated.
Validation
Merge handling
This PR is intentionally a draft so repository automation does not merge it before manual review.
Summary by cubic
Hardened GitHub guardrail hooks to avoid false positives and catch more direct mutation attempts. Improves branch protection handling and cross-platform parsing, with stronger wiring and tests.
http,curl, andxhmutations.jq; tests no longer depend on global default branch and cover new cases.Written for commit 39d347c. Summary will update on new commits.