Skip to content

fix(agents): harden GitHub mutation guards - #2207

Merged
shunkakinoki merged 1 commit into
mainfrom
codex/harden-agent-github-hooks
Aug 2, 2026
Merged

fix(agents): harden GitHub mutation guards#2207
shunkakinoki merged 1 commit into
mainfrom
codex/harden-agent-github-hooks

Conversation

@shunkakinoki

@shunkakinoki shunkakinoki commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Summary

  • harden protected-branch push detection across explicit, implicit, force, refspec, default-branch, and Git-alias forms
  • block repository control-plane mutations through gh, GraphQL, curl, HTTPie, xh, and wget while preserving ordinary PR, issue, comment, and read operations
  • document shared Codex, Claude, Cursor, Copilot, and Grok hook contracts and structurally assert all five clients remain wired

Validation

  • shellspec spec/block_git_push_spec.sh spec/block_gh_settings_spec.sh spec/agent_github_hook_wiring_spec.sh (83 examples)
  • shellcheck on all changed shell scripts and specs
  • make shell-test-dev (1,805 ShellSpec examples and 422 Fish tests)
  • make shell-check-dev
  • make nix-format-check
  • git diff --check

Security boundary

These local hooks are bypassable defense-in-depth guardrails. Restricted GitHub credentials and server-side rulesets remain authoritative.

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@indent-zero

indent-zero Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor
PR Summary

Hardens the shared agent GitHub guardrails so more push shapes and mutation patterns get caught before an agent runs them. block-git-push.sh and block-gh-settings.sh grow from simple grep filters into tokenizing analyzers, a README documents the security boundary, and ShellSpec coverage plus a wiring spec are added so per-agent configs can't silently drop the guards.

  • Rewrites block-git-push.sh to tokenize the command, follow segments split by ;&|, resolve push.default/branch.*.merge/pushRemote/remote.*.push/cached remote HEAD, handle HEAD, +ref, src:dst, :branch, --delete, --all, --mirror, and follow Git aliases (including nested and shell aliases) up to depth 8.
  • Extends block-gh-settings.sh to block gh repo delete|rename|archive|transfer|edit, gh repo deploy-key add|delete, gh secret|variable set|delete, gh workflow enable|disable, GraphQL calls that mention mutation or read --input, and any explicit or implicit-body mutation against repository control-plane URLs via gh api, curl, httpie, xh, or wget.
  • Adds config/shared/hooks/README.md describing the accepted JSON shapes, protected operations, and the fact that these are advisory guards (server-side rulesets and restricted credentials remain authoritative).
  • Grows spec/block_git_push_spec.sh and spec/block_gh_settings_spec.sh with implicit-destination, alias, direct-HTTP-client, and alternate-JSON-input coverage, and adds spec/agent_github_hook_wiring_spec.sh asserting every agent's config still references both hooks.

Issues

4 potential issues found:

  • git -c <name>=<val> push origin main bypasses the push guard: -c is a valued flag but the subcommand scanner treats it as a zero-arg flag (only -C, --git-dir, --work-tree, --namespace are special-cased), so the "subcommand" becomes the config kv-pair and push is never analyzed. Reproduced locally with git -c push.default=matching push origin main and git -c foo=bar push origin main → both exit 0. Add -c (and -c*) to the valued-flag branches so it consumes its argument like -C. → Autofix
  • Org/user-scoped control-plane calls slip past block-gh-settings.sh: is_control_plane_target only matches /repos/{owner}/{repo}/…, so gh api -X POST /orgs/<org>/repos (repo creation), gh api -X POST /user/repos, and gh api -X PATCH /orgs/<org> all exit 0. The CLI equivalents gh secret set --org are caught, so behavior diverges when an agent reaches for the raw REST form. Confirm scope: if org/user endpoints are intentionally out of scope, note it in config/shared/hooks/README.md; otherwise extend is_control_plane_target (and the direct-HTTP branches) to cover /orgs/[^/]+(/(actions|hooks|secrets|variables|…)|$) and /user/repos. → Autofix
  • The push guard runs its own git remote get-url origin, git remote, git config …, and git show-ref … from the process CWD, so when the command uses git -C /other/repo push … (or --git-dir/--work-tree), the allowlist, cached remote-default branch, upstream, and push.default matching checks all reflect the wrong repository. Triggered any time an agent scripts git -C against a repo other than CWD; will produce silent false-allows (CWD is allowlisted, target is not) or false-blocks (CWD is protected, target is allowlisted). → Autofix
  • spec/agent_github_hook_wiring_spec.sh only asserts substring presence via grep -Fq, so it won't catch structural regressions like a commented-out entry, a hook wired under the wrong trigger, JSON that no longer parses, or a path swapped to a variable that expands elsewhere. Consider validating each config with jq -e (asserting the hook appears inside the expected PreToolUse matcher) so wiring failures are caught before shipping. → Autofix

CI Checks

Waiting for CI checks...


⚡ Autofix All Issues

@coderabbitai

coderabbitai Bot commented Aug 2, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Strengthened safeguards around GitHub repository settings, secrets, variables, deployment keys, workflows, and other control-plane changes.
    • Improved protection for pushes targeting protected branches, including deletions, mirrors, all-branch pushes, aliases, and varied command formats.
    • Continued allowing approved pull-request and issue-comment collaboration actions.
  • Documentation

    • Added guidance on supported commands, protected operations, branch rules, and security limitations.
  • Tests

    • Expanded coverage across supported command formats and configuration integrations.

Walkthrough

The pull request expands shared hooks for GitHub settings and protected Git pushes. It adds command parsing for multiple clients and request forms, broad regression coverage, documentation, and wiring checks across supported agent configurations.

Changes

GitHub guardrails

Layer / File(s) Summary
GitHub settings mutation protection
config/shared/hooks/block-gh-settings.sh, spec/block_gh_settings_spec.sh
The hook blocks protected repository, secret, variable, deploy-key, workflow, GraphQL, gh api, and direct HTTP-client mutations. Tests cover allowed collaboration operations, alternate input formats, and implicit request methods.
Protected branch push parsing
config/shared/hooks/block-git-push.sh, spec/block_git_push_spec.sh
The hook resolves protected branches, remotes, aliases, refspecs, upstreams, and bulk push modes. Tests cover allowed feature pushes, blocked protected destinations, nested commands, and multiple tool payload formats.
Hook wiring and documentation
config/shared/hooks/README.md, spec/agent_github_hook_wiring_spec.sh
The README documents hook behavior and limitations. The wiring spec verifies registration in the Codex, Claude, Cursor, Copilot, and Grok configurations.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ToolClient
  participant block-gh-settings.sh
  participant GitHubControlPlane
  ToolClient->>block-gh-settings.sh: Submit GitHub command
  block-gh-settings.sh->>block-gh-settings.sh: Detect protected mutation
  block-gh-settings.sh-->>ToolClient: Block with exit status 2
  block-gh-settings.sh->>GitHubControlPlane: Permit allowed operation
Loading
sequenceDiagram
  participant ToolClient
  participant block-git-push.sh
  participant GitRemote
  ToolClient->>block-git-push.sh: Submit git push command
  block-git-push.sh->>GitRemote: Resolve remote and default branch
  block-git-push.sh->>block-git-push.sh: Evaluate destination refspec
  block-git-push.sh-->>ToolClient: Block protected destination or allow push
Loading

Possibly related PRs

Poem

A rabbit guards the branch with care,
And checks each GitHub command laid bare.
Secrets stay still, main stays bright,
Safe paths hop through day and night.
Tests thump softly: all is right.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main change: hardened GitHub mutation guards in agent hooks.
Description check ✅ Passed The description directly covers the hook hardening, supported mutation paths, documentation, wiring, tests, and security boundary.
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/harden-agent-github-hooks

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
spec/block_git_push_spec.sh (1)

7-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Pin the initial branch name in both setup functions.

git init uses init.defaultBranch, so later git config calls do not set the initial branch. With init.defaultBranch=trunk, the repository creates trunk instead of main, and bare-push tests that expect a main branch can fail. Use git -C "$TEMP_REPO" init -q -b main in both setup and setup_allowed.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@spec/block_git_push_spec.sh` around lines 7 - 23, Pin the initial branch to
main in both setup and setup_allowed by adding the branch option to each git
init invocation. Leave the remaining repository configuration and test setup
unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@config/shared/hooks/block-gh-settings.sh`:
- Around line 95-99: Update the HTTPie body-detection regex in the http_method
assignment block to match only non-flag request items: exclude tokens beginning
with '-' and any token containing '?' before an '='. Preserve detection of
ordinary `name=value` and `name:=value` body arguments while preventing query
strings and option values from setting http_method=POST.
- Around line 79-108: Normalize http_method to uppercase after all
method-detection branches and before the final ^(POST|PATCH|PUT|DELETE)$
comparison in the command classification flow. Ensure captured lowercase methods
from HTTPie/https/xh and --request options are converted while preserving the
existing blocking behavior.

In `@config/shared/hooks/block-git-push.sh`:
- Around line 62-74: Update check_destination so slash-based splitting only
occurs for remote-qualified destinations or refs/heads/ and heads/ prefixes,
rather than any destination containing “/”. Preserve direct protected-branch
checks for ordinary branch names, including feature/main and release/master, and
continue blocking only when the resulting branch is protected.
- Around line 318-329: Update inspect_command to replace the sed-based command
splitting with the portable tr ';&|' newline conversion, preserving the existing
loop that skips empty segments and invokes inspect_segment for each command
segment.

In `@spec/agent_github_hook_wiring_spec.sh`:
- Around line 14-17: Update the hook validation in the wiring spec to parse each
configuration structurally and verify the command is registered under the
PreToolUse hook entry, rather than searching for the path anywhere in the JSON.
Follow the schema and command-entry contract demonstrated by
activate_config_spec.sh and config/codex/hooks.json, while preserving the
existing missing-hook failure behavior.

---

Nitpick comments:
In `@spec/block_git_push_spec.sh`:
- Around line 7-23: Pin the initial branch to main in both setup and
setup_allowed by adding the branch option to each git init invocation. Leave the
remaining repository configuration and test setup unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cf8741f7-ad9a-4974-b108-6896aa147f9e

📥 Commits

Reviewing files that changed from the base of the PR and between 59481fe and 7007f74.

📒 Files selected for processing (6)
  • config/shared/hooks/README.md
  • config/shared/hooks/block-gh-settings.sh
  • config/shared/hooks/block-git-push.sh
  • spec/agent_github_hook_wiring_spec.sh
  • spec/block_gh_settings_spec.sh
  • spec/block_git_push_spec.sh

Comment on lines +79 to +108
http_method=$(explicit_method "$command")

if printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])(http|https|xh)[[:space:]]+(POST|PATCH|PUT|DELETE)([[:space:]]|$)'; then
http_method=$(printf '%s\n' "$command" | sed -nE 's/.*(^|[;&|[:space:]])(http|https|xh)[[:space:]]+(POST|PATCH|PUT|DELETE)([[:space:]]|$).*/\3/ip' | tail -1)
fi

if [[ -z $http_method ]]; then
http_method=$(printf '%s\n' "$command" | sed -nE 's/.*(^|[[:space:]])--request(=|[[:space:]]+)(POST|PATCH|PUT|DELETE)([[:space:]]|$).*/\3/ip' | tail -1)
fi

if [[ -z $http_method ]] && \
printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])curl([[:space:]]|$)' && \
printf '%s\n' "$command" | grep -Eiq '(^|[[:space:]])(--data[^[:space:]]*|-d|--form|-F|--json|--upload-file|-T)(=|[[:space:]])'; then
http_method=POST
fi

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
http_method=POST
fi

if [[ -z $http_method ]] && \
printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])wget([[:space:]]|$)' && \
printf '%s\n' "$command" | grep -Eiq '(^|[[:space:]])(--post-data|--post-file|--body-data)(=|[[:space:]])'; then
http_method=POST
fi

if [[ $http_method =~ ^(POST|PATCH|PUT|DELETE)$ ]]; then
block_settings "A direct $http_method request to a repository control-plane endpoint was requested."

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🌐 Web query:

Does HTTPie accept a lowercase request method such as http delete https://example.com`?`

💡 Result:

Yes, HTTPie accepts lowercase request methods such as http delete https://example.com [1][2]. While HTTPie documentation typically displays methods in uppercase (e.g., GET, POST, DELETE), the CLI tool is case-insensitive regarding the request method argument and will correctly process lowercase input [1][3].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== locate file =="
git ls-files | grep -Fx 'config/shared/hooks/block-gh-settings.sh' || true

echo "== file outline/stat =="
wc -l config/shared/hooks/block-gh-settings.sh
ast-grep outline config/shared/hooks/block-gh-settings.sh || true

echo "== relevant lines =="
sed -n '1,130p' config/shared/hooks/block-gh-settings.sh

echo "== deterministic guard behavior for lowercase method assignments =="
python3 - <<'PY'
import re

def explicit_method(command, verbose=False):
    method = None
    if verbose and 'http' in command or 'httpie' in command:
        pass
    return method

def current_block_analysis(command):
    # mirrors the assignment paths enough to show stored value before final regex
    match = re.search(r'(^|[;&|[:space:]])http([[:space:]]+)', command, re.I)
    if match:
        rest = re.split(r'[\s;|&]+', command[match.end():], maxsplit=1)[0] if command[match.end():] else ''
        http_method = rest.upper()
    else:
        http_method = None

    if re.search(r'(^|[;&|[:space:]])https([[:space:]]+)', command, re.I):
        rest = re.split(r'[\s;|&]+', command[match.end():] if match else command, maxsplit=1)[0] if ((match and command[match.end():]) or (not match)) else ''
        http_method = rest.upper()
    else:
        pass

    # lines 82/86 from source if explicit_method returned None and command matches.
    m = re.search(r'.*(^|[;&|[:space:]])(http|https|xh)[[:space:]]+([A-Za-z]+)([[:space:]]|$).*', command, re.I)
    if not http_method and m:
        http_method = m.group(3)
    # line 86 if still empty via curl --request; omit because current examples use implicit lowercase.

    if re.search(r'(^|[;&|[:space:]])curl([[:space:]]|$)', command, re.I) and re.search(r'(^|[[:space:]])(--data[^[:space:]]*|-d|--form|-F|--json|--upload-file|-T)(=|[[:space:]])', command, re.I):
        if not http_method:
            http_method = 'POST'

    if re.search(r'(^|[;&|[:space:]])(http|https|xh)([[:space:]]|$)', command, re.I) and re.search(r'(^|[[:space:]])[^[:space:]=]+(:=|=)[^[:space:]]+', command):
        if not http_method:
            http_method = 'POST'

    if re.search(r'(^|[;&|[:space:]])wget([[:space:]]|$)', command, re.I) and re.search(r'(^|[[:space:]])(--post-data|--post-file|--body-data)(=|[[:space:]])', command, re.I):
        if not http_method:
            http_method = 'POST'

    blocked = False
    if http_method and re.search(r'^(POST|PATCH|PUT|DELETE)$', http_method):
        blocked = True
    return http_method, blocked

for cmd in [
    "http delete https://api.github.com/repos/owner/repo/hooks/1",
    "http DELETE https://api.github.com/repos/owner/repo/hooks/1",
    "curl --request delete https://api.github.com/repos/owner/repo",
    "curl --request delete -d x https://api.github.com/repos/owner/repo",
]:
    print(cmd, current_block_analysis(cmd))
PY

Repository: shunkakinoki/dotfiles

Length of output: 6484


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== deterministic extraction from source =="
python3 - <<'PY'
import re

source = open('config/shared/hooks/block-gh-settings.sh').read()

def explicit_method(command):
    method = None
    for pattern in (
        r'.*(^|[[:space:]])(-X|--method)(=|[[:space:]]+)(GET|POST|PATCH|PUT|DELETE)([[:space:]]|$).*',
        r'.*(^|[[:space:]])-X(GET|POST|PATCH|PUT|DELETE)([[:space:]]|$).*',
    ):
        m = re.search(pattern, command, re.I)
        if m:
            method = m.group(4) or m.group(2)
    return (method or '').upper()

def command_stores_raw_lowercase(command):
    # Returns True if the HTTPie/https/xh capture path at lines 82/86 runs
    # and captures a lowercase method before the final guard.
    if not re.search(r'(^|[;&|[:space:]])http_method=\(explicit_method "$command"\)', command, re.I):
        pass
    m = re.search(r'.*(^|[;&|[:space:]])(http|https|xh)[[:space:]]+([^[:space:]]+)([[:space:]]|$).*', command, re.I)
    if m:
        return (m.group(3) != m.group(3).upper())
    return False

def final_block_stops_http_method(command):
    http_method = explicit_method(command)
    # The guard only runs for lines 82-86 because the script assigns
    # explicit_method first and then the sed capture only when command contains http|https|xh.
    if re.search(r'(^|[;&|[:space:]])http_method=\(explicit_method "$command"\)', source, re.I):
        pass
    if re.search(r'(^|[;&|[:space:]])(http|https|xh)[[:space:]]+([A-Za-z]+)([[:space:]]|$)', command, re.I):
        m = re.search(r'.*(^|[;&|[:space:]])(http|https|xh)[[:space:]]+([A-Za-z]+)([[:space:]]|$).*', command, re.I)
        if m:
            http_method = m.group(3)
    if re.search(r'^(POST|PATCH|PUT|DELETE)$', http_method):
        return True
    return False

for cmd in [
    "http delete https://api.github.com/repos/owner/repo/hooks/1",
    "http DELETE https://api.github.com/repos/owner/repo/hooks/1",
    "curl --request delete https://api.github.com/repos/owner/repo",
    "curl --request delete -d x https://api.github.com/repos/owner/repo",
]:
    print("cmd:", cmd)
    print("explicit_method:", explicit_method(cmd))
    print("sed captures lowercase:", command_stores_raw_lowercase(cmd))
    print("final block matches current source:", final_block_stops_http_method(cmd))
PY

Repository: shunkakinoki/dotfiles

Length of output: 1326


Uppercase the HTTP method before the final comparison.

The HTTPie/https/xh capture path assigns the raw sed capture, so lowercase methods like http delete ..., curl --request delete ..., or --request delete are not matched by the final ^(POST|PATCH|PUT|DELETE)$ guard. Normalize the method once before that comparison.

🛡️ Proposed fix
-  if [[ $http_method =~ ^(POST|PATCH|PUT|DELETE)$ ]]; then
+  http_method=${http_method^^}
+  if [[ $http_method =~ ^(POST|PATCH|PUT|DELETE)$ ]]; then
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
http_method=$(explicit_method "$command")
if printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])(http|https|xh)[[:space:]]+(POST|PATCH|PUT|DELETE)([[:space:]]|$)'; then
http_method=$(printf '%s\n' "$command" | sed -nE 's/.*(^|[;&|[:space:]])(http|https|xh)[[:space:]]+(POST|PATCH|PUT|DELETE)([[:space:]]|$).*/\3/ip' | tail -1)
fi
if [[ -z $http_method ]]; then
http_method=$(printf '%s\n' "$command" | sed -nE 's/.*(^|[[:space:]])--request(=|[[:space:]]+)(POST|PATCH|PUT|DELETE)([[:space:]]|$).*/\3/ip' | tail -1)
fi
if [[ -z $http_method ]] && \
printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])curl([[:space:]]|$)' && \
printf '%s\n' "$command" | grep -Eiq '(^|[[:space:]])(--data[^[:space:]]*|-d|--form|-F|--json|--upload-file|-T)(=|[[:space:]])'; then
http_method=POST
fi
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
http_method=POST
fi
if [[ -z $http_method ]] && \
printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])wget([[:space:]]|$)' && \
printf '%s\n' "$command" | grep -Eiq '(^|[[:space:]])(--post-data|--post-file|--body-data)(=|[[:space:]])'; then
http_method=POST
fi
if [[ $http_method =~ ^(POST|PATCH|PUT|DELETE)$ ]]; then
block_settings "A direct $http_method request to a repository control-plane endpoint was requested."
http_method=$(explicit_method "$command")
if printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])(http|https|xh)[[:space:]]+(POST|PATCH|PUT|DELETE)([[:space:]]|$)'; then
http_method=$(printf '%s\n' "$command" | sed -nE 's/.*(^|[;&|[:space:]])(http|https|xh)[[:space:]]+(POST|PATCH|PUT|DELETE)([[:space:]]|$).*/\3/ip' | tail -1)
fi
if [[ -z $http_method ]]; then
http_method=$(printf '%s\n' "$command" | sed -nE 's/.*(^|[[:space:]])--request(=|[[:space:]]+)(POST|PATCH|PUT|DELETE)([[:space:]]|$).*/\3/ip' | tail -1)
fi
if [[ -z $http_method ]] && \
printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])curl([[:space:]]|$)' && \
printf '%s\n' "$command" | grep -Eiq '(^|[[:space:]])(--data[^[:space:]]*|-d|--form|-F|--json|--upload-file|-T)(=|[[:space:]])'; then
http_method=POST
fi
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
http_method=POST
fi
if [[ -z $http_method ]] && \
printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])wget([[:space:]]|$)' && \
printf '%s\n' "$command" | grep -Eiq '(^|[[:space:]])(--post-data|--post-file|--body-data)(=|[[:space:]])'; then
http_method=POST
fi
http_method=${http_method^^}
if [[ $http_method =~ ^(POST|PATCH|PUT|DELETE)$ ]]; then
block_settings "A direct $http_method request to a repository control-plane endpoint was requested."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/shared/hooks/block-gh-settings.sh` around lines 79 - 108, Normalize
http_method to uppercase after all method-detection branches and before the
final ^(POST|PATCH|PUT|DELETE)$ comparison in the command classification flow.
Ensure captured lowercase methods from HTTPie/https/xh and --request options are
converted while preserving the existing blocking behavior.

Comment on lines +95 to +99
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
http_method=POST
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Restrict the HTTPie body detection to non-flag request items.

The pattern [^[:space:]=]+(:=|=)[^[:space:]]+ matches any token that contains =. A query string or an option value therefore sets http_method=POST. For example, http https://api.github.com/repos/owner/repo/rulesets?per_page=10 is a read, but the hook blocks it. Options such as --auth-type=bearer cause the same result.

Exclude tokens that start with - and tokens that contain ? before the =.

♻️ Proposed fix
-    printf '%s\n' "$command" | grep -Eq '(^|[[:space:]])[^[:space:]=]+(:=|=)[^[:space:]]+'; then
+    printf '%s\n' "$command" | grep -Eq '(^|[[:space:]])[^-[:space:]=?:/][^[:space:]=?/]*(:=|=)[^[:space:]]+'; then
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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
http_method=POST
fi
if [[ -z $http_method ]] && \
printf '%s\n' "$command" | grep -Eiq '(^|[;&|[:space:]])(http|https|xh)([[:space:]]|$)' && \
printf '%s\n' "$command" | grep -Eq '(^|[[:space:]])[^-[:space:]=?:/][^[:space:]=?/]*(:=|=)[^[:space:]]+'; then
http_method=POST
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@config/shared/hooks/block-gh-settings.sh` around lines 95 - 99, Update the
HTTPie body-detection regex in the http_method assignment block to match only
non-flag request items: exclude tokens beginning with '-' and any token
containing '?' before an '='. Preserve detection of ordinary `name=value` and
`name:=value` body arguments while preventing query strings and option values
from setting http_method=POST.

Comment thread config/shared/hooks/block-git-push.sh
Comment thread config/shared/hooks/block-git-push.sh
Comment on lines +14 to +17
if ! grep -Fq "config/shared/hooks/$hook" "$config"; then
printf 'missing %s in %s\n' "$hook" "$config" >&2
return 1
fi

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

Validate the active hook registration, not only the path text.

grep -Fq searches the entire JSON file. A stale value or unrelated field can satisfy the check while the hook is not registered under PreToolUse. Parse each configuration according to its schema and assert the command entry. The existing spec/activate_config_spec.sh:160-170 and config/codex/hooks.json:55-61 show the structural contract.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@spec/agent_github_hook_wiring_spec.sh` around lines 14 - 17, Update the hook
validation in the wiring spec to parse each configuration structurally and
verify the command is registered under the PreToolUse hook entry, rather than
searching for the path anywhere in the JSON. Follow the schema and command-entry
contract demonstrated by activate_config_spec.sh and config/codex/hooks.json,
while preserving the existing missing-hook failure behavior.

@shunkakinoki
shunkakinoki force-pushed the codex/harden-agent-github-hooks branch from 7007f74 to 3684767 Compare August 2, 2026 18:04
Comment thread config/shared/hooks/block-git-push.sh Outdated
index=$((index + 2))
continue
;;
-C* | --git-dir=* | --work-tree=* | --namespace=* | --no-pager | --paginate | --literal-pathspecs | --no-literal-pathspecs | --glob-pathspecs | --noglob-pathspecs | --icase-pathspecs)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

git -c foo=bar push bypasses this guard. -c is a valued flag (like -C, --git-dir, --work-tree, --namespace — all special-cased above), but it isn't enumerated here, so it falls through to the generic -*) branch on line 277 which only skips one token. That makes the next token — the name=value config pair — the detected "subcommand", so push is never seen and analyze_push_words never runs. Reproduced:

$ echo '{"tool_input": {"command": "git -c push.default=matching push origin main"}}' \
  | bash config/shared/hooks/block-git-push.sh; echo "exit: $?"
exit: 0
$ echo '{"tool_input": {"command": "git -c foo=bar push origin main"}}' \
  | bash config/shared/hooks/block-git-push.sh; echo "exit: $?"
exit: 0

Fix: add -c to the first case arm (consumes 2 tokens) and -c* to the second (single-token -cKEY=VAL form).

@shunkakinoki
shunkakinoki merged commit d28a486 into main Aug 2, 2026
18 of 24 checks passed
@shunkakinoki
shunkakinoki deleted the codex/harden-agent-github-hooks branch August 2, 2026 18:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant