Repository navigation
Deny direct gradlew calls from agent sessions (PROF-16469) - #937
Conversation
AGENTS.md requires Gradle to run through .claude/commands/build-and-summarize, but nothing enforced it and .claude/settings.json allowed ./gradlew outright. - utils/gradle-command-policy.py denies gradlew in command position, except --stop and --version. - The require-gradle-wrapper hook runs it in Claude Code and, through a Pi adapter, in Pi. Like require-fresh-review, it loads the policy from refs/remotes/origin/main with python3 -I and allows everything until the policy is there. - settings.json drops the ./gradlew allow rule and allows the wrapper instead; the jmh command and jmh-benchmarks agent use the wrapper. - utils/tests/test_gradle_hook.sh covers the policy, the hook, the Pi adapter and the agent configuration. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The policy's argument capture stopped at the first quote, so `./gradlew --stop "build"` passed as `--stop`. Arguments now run to the next separator; an unbalanced quote makes shlex fail, which denies. A bare redirection operator now skips its target, so `./gradlew --stop > /dev/null` is allowed like `2>/dev/null`. The Pi adapter skips the python3 spawn for commands without gradlew. New policy cases pin the redirect, continuation, trailing-backslash and lookalike-name branches. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Scan-Build Report
Bug Summary
Reports
|
|||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c6c6e8baea
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
Ordinary direct Gradle builds inside conditionals, behind env options, or after leading redirections bypass the policy, allowing both hooks to skip the required build wrapper.
🤖 Bits Code Review · Commit c6c6e8b
CI Test ResultsRun: #42041238060 | Commit:
Status Overview
Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled Summary: Total: 32 | Passed: 32 | Failed: 0 Updated: 2026-11-05 14:26:07 UTC |
…F-16469) The policy recognized gradlew only behind a fixed prefix list, so `if ./gradlew test`, `while`/`until`, `env -u X ./gradlew build` and `> log ./gradlew build` ran the build directly. It now splits the command into simple commands and denies any that names gradlew unless it starts with a read-only tool (cat, grep, git, ...); quoted scripts such as `bash -c '...'` are checked recursively. As a side effect, `bash -c './gradlew --stop'` and `echo "./gradlew build"` now pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…-16469) find -exec, an awk -f script calling system() and git aliases start with an allowed tool, so the policy lets them through. The README now says the hook stops accidental direct runs, not deliberate ones. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for f059391: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
Build pipeline has failing jobs for d3a8ff1: What to do next?
DetailsSince those jobs are not marked as being allowed to fail, the pipeline will most likely fail. |
|
/merge |
|
View all feedbacks in Devflow UI.
The expected merge time in
|
9664770
into
main
What does this PR do?:
Enforces the
AGENTS.mdrule "run Gradle only through.claude/commands/build-and-summarize" in agent sessions.utils/gradle-command-policy.pydenies by default. It denies any simple command that namesgradlew, unless the command starts with a read-only tool (cat,grep,git, ...) or isgradlew --stop/gradlew --version. Quoted scripts such asbash -c '...'get the same check.require-gradle-wrapperhook runs the policy in Claude Code, and a Pi adapter runs it in Pi. Subagents are covered too..claude/settings.jsondrops theBash(./gradlew:*)allow rule. Thejmhcommand and thejmh-benchmarksagent now use the wrapper.Motivation:
AGENTS.mdforbade direct Gradle calls, but nothing enforced the rule, andsettings.jsonexplicitly allowed them. This is step 1 of aligning the agent configuration with the AI Native SDLC Playbook: rules that a machine can check get a check.Additional Notes:
settings.jsonnow allowsBash(./.claude/commands/build-and-summarize:*). Without it, removing the./gradlewrule would add a permission prompt for every build. This changes every engineer's session, so it needs a second profiling-team reviewer.refs/remotes/origin/mainwithpython3 -I, likerequire-fresh-review. A checked-out branch cannot change the policy. Until this PR merges, the hook allows everything.gradlewunless it followed a known prefix, and reviewers foundif,while,env -u Xand> logforms that got through. Listing more prefixes would keep leaking.mv gradlew gradlew.old;./gradlew.G=./gradlew; $G build;gradle;find ... -exec ./gradlew build {} +, anawk -fscript that callssystem(), or agitalias.utils/README.mdlists these too../gradlew --stop "build") and add tests./code-reviewat low effort, no findings.How to test the change?:
bash utils/tests/test_gradle_hook.shcovers the policy, the hook, the Pi adapter and the agent configuration. Its Pi cases need Node.js 22.6 or newer (--experimental-strip-types) and skip on older Node.js. Result: 78/78 innode:22-bookworm.bash utils/tests/test_review_marker.shstill passes with the changedsettings.json: 146/146.refs/remotes/origin/mainset to the candidate:./gradlew help, and a subagent's attempt was denied too. With a neutral prompt, the agent ranbuild-and-summarize help.utils/tests/yet. The follow-up PR for PROF-16469 adds that step.For Datadog employees:
credentials of any kind, I've requested a security review (run the
dd:platform-security-reviewskill, or file a request via the PSEC review form).
bewairealso runs automatically on every PR.🤖 Generated with Claude Code