Skip to content

Add Codex and Claude Code push and PR hooks - #620

Merged
romainbrenguier merged 3 commits into
chore/ai-experiment-agent-readinessfrom
romain/codex-hooks
Nov 12, 2026
Merged

romainbrenguier merged 3 commits into
chore/ai-experiment-agent-readinessfrom
romain/codex-hooks

Conversation

@romainbrenguier

@romainbrenguier romainbrenguier commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

Adds repository-owned Codex and Claude Code hooks for git push and turn finalization.

Both clients use the same committed-HEAD checks, saved results, exact-commit Build PR job validation, and review status. Claude's adapter validates CI override quotes against user messages in its session transcript.

Validated with 4 Codex hook tests, 4 Claude hook tests, and mvn clean verify -Pcoverage -DskipTests=false -B against the final committed HEAD.


Summary by Gitar

  • Cleartext protocol exports:
    • Added CleartextProtocolFilterExporter and CleartextProtocolCorpusExporter to generate shared JSON exclusions and corpus for npm/nupkg packages
  • Filter exclusions:
    • Added neverssl.com, osmand.net, and many XML namespace identifier prefixes to CleartextProtocolFilter
  • Secret classifier:
    • Added (bool)True and (bool)False CODESYS boolean value exclusions to SecretClassifier
  • Dependency and CI updates:
    • Bumped sonar plugin API, scanner, build-helper, and mise-action versions, and moved CODEOWNERS and Slack channels to Analysis Platform

This will update automatically on new commits.

Agent workflow

Tool link: https://github.com/SonarSource/languages-experimental-tooling/tree/romain/my-tickets/personal/romain-brenguier
PR updated using uv run update_with_claude.py --prompt "Rebase on origin/master, fixing conflicts" -a "None" -g "codex"

PR updated using uv run update_with_claude.py --prompt "Rebase on origin/master, fixing conflicts" -a "None" -g "codex"

PR updated using `uv run update_with_claude.py --prompt "Implement the action plan described in the document Rebase on chore/ai-experiment-agent-readiness
SonarSource/sonar-analyzer-commons:chore/ai-experiment-agent-readiness
Drop commits from master that shouldn't be part of the PR, after master diverged from the base branch.." -a "Rebase on chore/ai-experiment-agent-readiness
SonarSource/sonar-analyzer-commons:chore/ai-experiment-agent-readiness
Drop commits from master that shouldn't be part ...

PR updated using uv run update_with_claude.py --prompt "Implement the action plan described in the document Code Review: Adds repository-owned Codex and Claude Code hooks that gate git push` and turn finalization on committed-HEAD checks and exact-commit Build PR validation. Changes are requested: the CI-OVERRIDE check accepts any 8+ character substring of a user message, so it can be bypassed by quoting ordinary text, and the Claude PreToolUse hook has no timeout, so a long Maven run could le...

@hashicorp-vault-sonar-prod hashicorp-vault-sonar-prod Bot changed the title Add repository Codex push and PR hooks ACOMMONS-250 Add repository Codex push and PR hooks Nov 12, 2026
@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

ACOMMONS-250

@romainbrenguier
romainbrenguier changed the base branch from master to chore/ai-experiment-agent-readiness November 12, 2026 14:37
@romainbrenguier romainbrenguier changed the title ACOMMONS-250 Add repository Codex push and PR hooks Add Codex and Claude Code push and PR hooks Nov 12, 2026
Comment thread .codex/checks.py Resolved
Comment thread .codex/checks.py Resolved
Comment thread .codex/HOOKS.md Resolved
Comment thread .claude/hooks.py Resolved
Comment thread .claude/settings.json Resolved
Comment thread .claude/hooks.py Resolved
Comment thread .claude/test_hooks.py Resolved
@gitar-bot

gitar-bot Bot commented Nov 12, 2026 •

Copy link
Copy Markdown
CI failed: CI jobs failed with a GitHub API 403 rate-limit error for installation 134737833; this is infrastructure throttling, and it is unclear whether the new PR hooks caused it.

Overview

One unique error template was found across 2 analyzed logs (jobs 131337867864 and 131337868007). The failure is a GitHub API rate-limit response (HTTP 403) rather than a test or build assertion, so the overall assessment is an infrastructure/API throttling issue.

Failures

GitHub API rate limit exceeded (HTTP 403) (confidence: high)

  • Type: infrastructure
  • Affected jobs: 131337867864, 131337868007
  • Related to change: unclear
  • Root cause: GitHub returned 403 because installation ID 134737833 exceeded its API rate limit. The log does not identify which request or CI component triggered the limit. The PR adds hooks that call gh (including .codex/checks.py), so they are a possible source of API calls, but the logs do not show that these hooks ran in the failing jobs.
  • Suggested fix: Retry after the installation's rate limit resets. If it recurs, inspect the full job log to identify the calling request, reduce or cache repeated GitHub API calls, or use an appropriately authorized token with sufficient rate limit.

Summary

  • Change-related failures: None confirmed. The one failure is marked unclear; the new hooks are a possible but unverified source of the API calls.
  • Infrastructure/flaky failures: 1 failure (GitHub API rate limit, 2 jobs), likely transient.
  • Recommended action: Rerun the affected jobs after the rate limit resets. If the failure persists, pull the full logs to find the API caller and check whether the new .codex/checks.py hook made the requests.
Code Review ⚠️ Changes requested 🟡 Medium risk 7 closed / 9 findings

New Codex and Claude hooks gate pushes and turn completion on checks; stale status or hook failures could block legitimate work.

Adds Codex and Claude Code hooks that gate git push and turn finalization on committed-HEAD checks, saved results, and exact-commit Build PR validation. Changes requested: the shell-wrapper push check in .codex/checks.py is bypassed by --norc/--rcfile options or by command prefixes like env, timeout, or cd ... &&, so pushes can skip verify_head. Also, the CI-OVERRIDE quote regex in .claude/hooks.py rejects messages containing inner quotes or newlines, so valid overrides cannot be accepted.

⚠️ Edge Case: Shell-wrapper push check is bypassed by --norc/--rcfile or a prefix

📄 .codex/checks.py:200-214 📄 .claude/HOOKS.md:5

The new wrapper check in is_push only fires when words[0] is sh/bash/zsh. It also breaks on the first option that contains a c. Traced bypasses:

  • bash --norc -c "git push" and bash --rcfile x -c "git push": --norc/--rcfile match "c" in word[1:], so the code recurses on the wrong word (-c or x), gets False, and breaks.
  • cd . && bash -c "git push", env bash -c ... and timeout 60 sh -c ...: words[0] is not a shell, so the wrapper branch is skipped.

In every case the per-segment fallback sees the single token "git push", whose Path(...).name is not git, so is_push returns (False, False). The push then runs without verify_head. This contradicts the new HOOKS.md claim that "Shell wrapped pushes are blocked".

Fix: inside each segment, look for the shell token at any position, skipping env/timeout/exec/command prefixes. Treat only short-option clusters (-[a-zA-Z]*c[a-zA-Z]*, not --…) as -c. Keep scanning instead of breaking.

Detect a shell -c wrapper in every segment and only match short-option clusters
SHELLS = ("sh", "bash", "zsh", "dash", "ksh")

def wrapped_push(tokens):
    for i, token in enumerate(tokens):
        if Path(token).name not in SHELLS:
            continue
        for j in range(i + 1, len(tokens) - 1):
            if re.fullmatch(r"-[a-zA-Z]*c[a-zA-Z]*", tokens[j]) and is_push(tokens[j + 1])[0]:
                return True
    return False
💡 Edge Case: Override quote regex rejects messages containing quotes or newlines

📄 .claude/hooks.py:61-63 📄 .claude/HOOKS.md:7

authorized_override now requires the quoted text to equal the user's entire latest message. The extraction regex is ["\u201c](.+?)["\u201d], which is lazy and has no DOTALL. If the message contains a ", for example CI-OVERRIDE: ignore the "NEXT analysis" failure, the capture stops at the first inner quote. If the message spans more than one line, . cannot match the newline. In both cases the capture never equals latest, so a real user authorization is always rejected, and the documented CI-OVERRIDE: "<their entire latest message>" procedure cannot work.

Fix: capture greedily up to the last closing quote on the reply's final override line, using DOTALL. Alternatively, compare the normalized text after CI-OVERRIDE: without requiring delimiters.

Greedy, DOTALL match anchored at the end of the reply
match = re.search(r'CI-OVERRIDE:\s*["\u201c](.+)["\u201d]\s*$', reply, flags=re.DOTALL)
if match:
    quote = " ".join(match.group(1).split()).casefold()
    if len(quote) >= 8 and quote == latest:
        return match.group(1)
return None
✅ 7 closed
✅ Bug: Shared state dir across worktrees causes false 'fix' stop blocks

📄 .codex/checks.py:18
STATE is keyed only on CONFIG["repository"] (checks.py:18). Every clone and worktree of sonar-analyzer-commons on the machine therefore writes the same results.json, maven_verify.log, committed_diff.log and stop-blocks.json. Example: session A pushes tree TA, then a parallel session in another worktree pushes TB and overwrites results.json. A's Stop hook then sees Saved results are for another tree. append_local_item turns that into failed (stale or missing results), classify maps it to fix, and A's stop is blocked with instructions to investigate a failure that never happened, until the loop guard gives up. Concurrent verify-head runs also overwrite each other's logs, so a reported log path can point at the other build's output. Fix: scope STATE to the checkout, e.g. by including a hash of ROOT (or of git rev-parse --git-dir) in the path.

✅ Edge Case: Push gate skipped when git push is wrapped in sh -c / bash -lc

📄 .codex/checks.py:145-159 📄 .codex/checks.py:190-204
is_push splits the raw command on ;&| and only detects a push when one shell token is literally git followed by push. For bash -c "git push" or sh -lc 'git push origin HEAD', shlex gives ['bash', '-c', 'git push']. Path('git push').name is not git, so is_push returns (False, False), pre_tool_use allows the command, and none of the Maven/diff checks run. That contradicts HOOKS.md's claim that the hook gates pushes. Fix: re-parse the argument of -c/-lc when the first token is a shell (sh, bash, zsh), or block any command whose text matches \bgit\b.*\bpush\b but isn't the exact standalone form.

✅ Quality: HOOKS.md gives the wrong default state path (gettempdir honors TMPDIR)

📄 .codex/HOOKS.md:7 📄 .codex/checks.py:18
HOOKS.md says logs and results live in ${CODEX_CHECK_STATE:-/tmp/codex-repo-checks}/sonar-analyzer-commons/. The code actually uses tempfile.gettempdir(), which honors TMPDIR/TEMP. On macOS, the usual developer machine, that resolves to /var/folders/.../T/, not /tmp, so readers following the doc look in the wrong directory. Fix: describe the default as the system temp directory, or hard-code /tmp in the code so it matches the doc.

✅ Security: CI-OVERRIDE accepts any 8+ char substring of any user message

📄 .claude/hooks.py:56-61 📄 .claude/hooks.py:39-53 📄 .claude/HOOKS.md:7
authorized_override accepts a quote if it is at least 8 characters long and appears as a substring of any user message from any earlier turn in the session. It never checks that the user actually authorized ignoring CI. Example: the user writes "Please fix the failing CI build", the assistant ends with CI-OVERRIDE: "fix the failing CI build", and the hook lets the stop through while Build is red. Even "Don't ignore CI failures" can be quoted as "ignore CI failures". An authorization from an earlier turn or commit also stays valid for every later HEAD in the session. So the gate this PR adds can be bypassed by quoting ordinary instruction text, which is not the "explicitly authorizes" rule HOOKS.md describes. Suggested fix: match the quote against a whole user message, or against a sentence in the latest user turn only. Also require explicit override wording (for example, the user's message contains CI-OVERRIDE or "ignore CI").

✅ Bug: Claude PreToolUse hook lacks the timeout Codex uses for Maven

📄 .claude/settings.json:7 📄 .codex/hooks.json:3
.claude/settings.json gives the PreToolUse hook no timeout, so Claude Code's default of 600 s applies. The Codex config allows 3600 s for the same mvn clean verify -Pcoverage run. Per the Claude Code hooks docs, "On PreToolUse, a timed-out command ... hook doesn't block the tool call". So if the coverage build takes more than 10 minutes, the hook is killed and git push runs without verification. That breaks the documented promise that a failure blocks the push. Killing the hook also skips the finally cleanup, which can leave the isolated worktree and a 'running' results entry behind. Suggested fix: set "timeout": 3600 to match .codex/hooks.json, and consider onFailure: "block" so a timeout fails closed.

...and 2 more closed from earlier reviews

🤖 Prompt for agents
Code Review: Adds Codex and Claude Code hooks that gate `git push` and turn finalization on committed-HEAD checks, saved results, and exact-commit Build PR validation. Changes requested: the shell-wrapper push check in `.codex/checks.py` is bypassed by `--norc`/`--rcfile` options or by command prefixes like `env`, `timeout`, or `cd ... &&`, so pushes can skip `verify_head`. Also, the `CI-OVERRIDE` quote regex in `.claude/hooks.py` rejects messages containing inner quotes or newlines, so valid overrides cannot be accepted.

1. ⚠️ Edge Case: Shell-wrapper push check is bypassed by --norc/--rcfile or a prefix
   Files: .codex/checks.py:200-214, .claude/HOOKS.md:5

   The new wrapper check in `is_push` only fires when `words[0]` is sh/bash/zsh. It also `break`s on the first option that contains a `c`. Traced bypasses:
   
   - `bash --norc -c "git push"` and `bash --rcfile x -c "git push"`: `--norc`/`--rcfile` match `"c" in word[1:]`, so the code recurses on the wrong word (`-c` or `x`), gets False, and breaks.
   - `cd . && bash -c "git push"`, `env bash -c ...` and `timeout 60 sh -c ...`: `words[0]` is not a shell, so the wrapper branch is skipped.
   
   In every case the per-segment fallback sees the single token `"git push"`, whose `Path(...).name` is not `git`, so `is_push` returns `(False, False)`. The push then runs without `verify_head`. This contradicts the new HOOKS.md claim that "Shell wrapped pushes are blocked".
   
   Fix: inside each segment, look for the shell token at any position, skipping `env`/`timeout`/`exec`/`command` prefixes. Treat only short-option clusters (`-[a-zA-Z]*c[a-zA-Z]*`, not `--…`) as `-c`. Keep scanning instead of breaking.

   Fix (Detect a shell -c wrapper in every segment and only match short-option clusters):
   SHELLS = ("sh", "bash", "zsh", "dash", "ksh")
   
   def wrapped_push(tokens):
       for i, token in enumerate(tokens):
           if Path(token).name not in SHELLS:
               continue
           for j in range(i + 1, len(tokens) - 1):
               if re.fullmatch(r"-[a-zA-Z]*c[a-zA-Z]*", tokens[j]) and is_push(tokens[j + 1])[0]:
                   return True
       return False

2. 💡 Edge Case: Override quote regex rejects messages containing quotes or newlines
   Files: .claude/hooks.py:61-63, .claude/HOOKS.md:7

   `authorized_override` now requires the quoted text to equal the user's entire latest message. The extraction regex is `["\u201c](.+?)["\u201d]`, which is lazy and has no DOTALL. If the message contains a `"`, for example `CI-OVERRIDE: ignore the "NEXT analysis" failure`, the capture stops at the first inner quote. If the message spans more than one line, `.` cannot match the newline. In both cases the capture never equals `latest`, so a real user authorization is always rejected, and the documented `CI-OVERRIDE: "<their entire latest message>"` procedure cannot work.
   
   Fix: capture greedily up to the last closing quote on the reply's final override line, using DOTALL. Alternatively, compare the normalized text after `CI-OVERRIDE:` without requiring delimiters.

   Fix (Greedy, DOTALL match anchored at the end of the reply):
   match = re.search(r'CI-OVERRIDE:\s*["\u201c](.+)["\u201d]\s*$', reply, flags=re.DOTALL)
   if match:
       quote = " ".join(match.group(1).split()).casefold()
       if len(quote) >= 8 and quote == latest:
           return match.group(1)
   return None

Verification Rules 3/3 met PRs 2 inspected

Functional validation No results

Rules 3 passed 1 not applicable

✅ Approved GitHub Actions runners
✅ Internal artifact registries
✅ Repository license compliance

Broader impact PRs 2 inspected
Cross-PR 2 PRs inspected 0 open impacts

PR 601: ACOMMONS-243 chore: Improve repository setup and agent instructions Inspected
PR 622: ACOMMONS-251 Prepare Next Development Iteration Inspected

Auto-approval Not enabled

Set up

For more verification details, comment gitar display:verbose.

Tip

Comment Gitar fix CI or enable auto-apply: gitar auto-apply:on

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing compact verification results.
Unblock → Override a blocking verdict and allow merging.

Comment with these commands to change the behavior for this request:

Auto-apply Compact Unblock
gitar auto-apply:on         
gitar display:verbose         
gitar unblock         

Was this helpful? React with 👍 / 👎 | Gitar

@sonarqube-next

Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0 Dependency risks
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Comment thread .codex/checks.py
Comment on lines +200 to +214
if Path(words[0]).name in ("sh", "bash", "zsh"):
for index, word in enumerate(words[1:], 1):
if word.startswith("-") and "c" in word[1:] and index + 1 < len(words):
attempted, _ = is_push(words[index + 1])
if attempted:
return True, False
break
standalone = push_words(command) is not None
for segment in re.split(r"[;&|\n]+", command):
try:
tokens = shlex.split(segment)
except ValueError:
continue
if git_push_tokens(tokens):
return True, standalone

@gitar-bot gitar-bot Bot Nov 12, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Edge Case: Shell-wrapper push check is bypassed by --norc/--rcfile or a prefix

The new wrapper check in is_push only fires when words[0] is sh/bash/zsh. It also breaks on the first option that contains a c. Traced bypasses:

  • bash --norc -c "git push" and bash --rcfile x -c "git push": --norc/--rcfile match "c" in word[1:], so the code recurses on the wrong word (-c or x), gets False, and breaks.
  • cd . && bash -c "git push", env bash -c ... and timeout 60 sh -c ...: words[0] is not a shell, so the wrapper branch is skipped.

In every case the per-segment fallback sees the single token "git push", whose Path(...).name is not git, so is_push returns (False, False). The push then runs without verify_head. This contradicts the new HOOKS.md claim that "Shell wrapped pushes are blocked".

Fix: inside each segment, look for the shell token at any position, skipping env/timeout/exec/command prefixes. Treat only short-option clusters (-[a-zA-Z]*c[a-zA-Z]*, not --…) as -c. Keep scanning instead of breaking.

Detect a shell -c wrapper in every segment and only match short-option clusters:

SHELLS = ("sh", "bash", "zsh", "dash", "ksh")

def wrapped_push(tokens):
    for i, token in enumerate(tokens):
        if Path(token).name not in SHELLS:
            continue
        for j in range(i + 1, len(tokens) - 1):
            if re.fullmatch(r"-[a-zA-Z]*c[a-zA-Z]*", tokens[j]) and is_push(tokens[j + 1])[0]:
                return True
    return False

Was this helpful? React with 👍 / 👎

Comment thread .claude/hooks.py
Comment on lines +61 to +63
for match in re.finditer(r'CI-OVERRIDE:\s*["\u201c](.+?)["\u201d]', reply):
quote = " ".join(match.group(1).split()).casefold()
if len(quote) >= 8 and quote == latest:

@gitar-bot gitar-bot Bot Nov 12, 2026 •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Edge Case: Override quote regex rejects messages containing quotes or newlines

authorized_override now requires the quoted text to equal the user's entire latest message. The extraction regex is ["\u201c](.+?)["\u201d], which is lazy and has no DOTALL. If the message contains a ", for example CI-OVERRIDE: ignore the "NEXT analysis" failure, the capture stops at the first inner quote. If the message spans more than one line, . cannot match the newline. In both cases the capture never equals latest, so a real user authorization is always rejected, and the documented CI-OVERRIDE: "<their entire latest message>" procedure cannot work.

Fix: capture greedily up to the last closing quote on the reply's final override line, using DOTALL. Alternatively, compare the normalized text after CI-OVERRIDE: without requiring delimiters.

Greedy, DOTALL match anchored at the end of the reply:

match = re.search(r'CI-OVERRIDE:\s*["\u201c](.+)["\u201d]\s*$', reply, flags=re.DOTALL)
if match:
    quote = " ".join(match.group(1).split()).casefold()
    if len(quote) >= 8 and quote == latest:
        return match.group(1)
return None

Was this helpful? React with 👍 / 👎

@romainbrenguier
romainbrenguier marked this pull request as ready for review November 12, 2026 18:54
@romainbrenguier
romainbrenguier merged commit bc10516 into chore/ai-experiment-agent-readiness Nov 12, 2026
10 of 12 checks passed
@romainbrenguier
romainbrenguier deleted the romain/codex-hooks branch November 12, 2026 18:54
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