Skip to content

Deny direct gradlew calls from agent sessions (PROF-16469) - #937

Merged
gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
rkennke/PROF-16469-gradle-hook
Nov 5, 2026
Merged

gh-worker-dd-mergequeue-cf854d[bot] merged 4 commits into
mainfrom
rkennke/PROF-16469-gradle-hook

Conversation

@rkennke

@rkennke rkennke commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

What does this PR do?:

Enforces the AGENTS.md rule "run Gradle only through .claude/commands/build-and-summarize" in agent sessions.

  • utils/gradle-command-policy.py denies by default. It denies any simple command that names gradlew, unless the command starts with a read-only tool (cat, grep, git, ...) or is gradlew --stop / gradlew --version. Quoted scripts such as bash -c '...' get the same check.
  • The require-gradle-wrapper hook runs the policy in Claude Code, and a Pi adapter runs it in Pi. Subagents are covered too.
  • .claude/settings.json drops the Bash(./gradlew:*) allow rule. The jmh command and the jmh-benchmarks agent now use the wrapper.

Motivation:

AGENTS.md forbade direct Gradle calls, but nothing enforced the rule, and settings.json explicitly 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:

  • Flagged for reviewer acknowledgment: settings.json now allows Bash(./.claude/commands/build-and-summarize:*). Without it, removing the ./gradlew rule would add a permission prompt for every build. This changes every engineer's session, so it needs a second profiling-team reviewer.
  • Trust model: the hook loads the policy from refs/remotes/origin/main with python3 -I, like require-fresh-review. A checked-out branch cannot change the policy. Until this PR merges, the hook allows everything.
  • Why deny by default: an earlier version allowed gradlew unless it followed a known prefix, and reviewers found if, while, env -u X and > log forms that got through. Listing more prefixes would keep leaking.
  • Accepted false blocks:
    • a non-reader command that only names the file, such as mv gradlew gradlew.old;
    • heredoc lines that start with ./gradlew.
  • Not covered: the hook guards against agent mistakes, not adversaries. These still get through:
    • indirection such as G=./gradlew; $G build;
    • a system gradle;
    • a read-only tool that executes its arguments, such as find ... -exec ./gradlew build {} +, an awk -f script that calls system(), or a git alias. utils/README.md lists these too.
  • Pre-PR review: Sphinx (standard), three rounds, 0 Critical/High.
    • Round 1 (full range): 7 Low and 1 Info, all fixed in c6c6e8bae. The fixes close a quoted-argument bypass (./gradlew --stop "build") and add tests.
    • Round 2 (c6c6e8b only): 4 Low and 1 Info. The redesign in the next commit made them obsolete.
    • Round 3 (e71dd3509 only, the deny-by-default redesign that answers the review threads): 7 Low and 1 Info, left as is. Sphinx's fix-loop check recommends closing after three rounds with only Low findings.
      • The executing-reader gap is listed under "Not covered".
      • The rest are missing test cases for single reader-list entries and separators that the existing cases already exercise in other combinations.
    • e6dc23bdc (README note on the executing-reader gap, doc only): /code-review at low effort, no findings.

How to test the change?:

  • bash utils/tests/test_gradle_hook.sh covers 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 in node:22-bookworm.
  • bash utils/tests/test_review_marker.sh still passes with the changed settings.json: 146/146.
  • Test-drive in a scratch clone with refs/remotes/origin/main set to the candidate:
    • Claude Code denied ./gradlew help, and a subagent's attempt was denied too. With a neutral prompt, the agent ran build-and-summarize help.
    • Pi 1.2.0 denied the same command.
  • CI does not run utils/tests/ yet. The follow-up PR for PROF-16469 adds that step.

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a security review (run the dd:platform-security-review
    skill, or file a request via the PSEC review form).
    bewaire also runs automatically on every PR.
  • This PR doesn't touch any of that.
  • JIRA: PROF-16469

🤖 Generated with Claude Code

rkennke and others added 2 commits November 4, 2026 17:35
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>
@rkennke
rkennke requested a review from a team as a code owner November 4, 2026 17:58
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 4, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-04T18:02:38.058152Z c6c6e8b PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@dd-octo-sts

dd-octo-sts Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Scan-Build Report

User:runner@runnervm48cm3
Working Directory:/home/runner/work/java-profiler/java-profiler/ddprof-lib/src/test/make
Command Line:make -j4 all
Clang Version:Ubuntu clang version 18.1.3 (1ubuntu1)
Date:Wed Nov 4 21:31:17 2026

Bug Summary

Bug TypeQuantityDisplay?
All Bugs2
Logic error
Dereference of null pointer1
Unused code
Dead initialization1

Reports

Bug Group Bug Type ▾ File Function/Method Line Path Length
Unused codeDead initializationreferenceChains.cppapplyOomRamp4971
Logic errorDereference of null pointerperfEvents_linux.cppunregisterThread72026

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 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".

Comment thread utils/gradle-command-policy.py Resolved

@datadog-datadog-prod-us1 datadog-datadog-prod-us1 Bot left a comment

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.

Bits Code Review: FAIL

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.

Open Bits AI session

🤖 Bits Code Review · Commit c6c6e8b

Comment thread utils/gradle-command-policy.py Resolved
@dd-octo-sts

dd-octo-sts Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

CI Test Results

Run: #42041238060 | Commit: 9664770 | Duration: 20m 46s (longest job)

✅ All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - ✅ - -
8-ibm - ✅ - -
8-j9 ✅ ✅ - -
8-librca - - ✅ ✅
8-orcl - ✅ - -
11 - ✅ - -
11-j9 ✅ ✅ - -
11-librca - - ✅ ✅
17 ✅ ✅ - -
17-graal ✅ ✅ - -
17-j9 ✅ ✅ - -
17-librca - - ✅ ✅
21 ✅ ✅ - -
21-graal ✅ ✅ - -
21-librca - - ✅ ✅
25 ✅ ✅ - -
25-graal ✅ ✅ - -
25-librca - - ✅ ✅

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>
@dd-octo-sts

dd-octo-sts Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

✅ All 40 integration tests passed

📊 Dashboard · 👷 Pipeline · 📦 e6dc23bd

…-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>
@rkennke

rkennke commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-11-05 11:48:34 UTC ℹ️ Start processing command /merge


2026-11-05 11:48:38 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 34m (p90).


2026-11-05 12:19:39 UTC ❌ MergeQueue: The build pipeline contains failing jobs for this merge request

Build pipeline has failing jobs for f059391:

⚠️ Do NOT retry failed jobs directly (why?).

What to do next?

  • Investigate the failures and when ready, re-add your pull request to the queue!
  • If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
  • Any question, go check the FAQ.
Details

Since those jobs are not marked as being allowed to fail, the pipeline will most likely fail.
Therefore, and to allow other builds to be processed, this merge request has been rejected and the pipeline got canceled.

@rkennke

rkennke commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-11-05 12:34:20 UTC ℹ️ Start processing command /merge


2026-11-05 12:34:24 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 34m (p90).


2026-11-05 12:49:44 UTC ℹ️ MergeQueue: Readding this merge request to the queue because another merge request processed with yours failed. No action is needed from your side.


2026-11-05 13:05:39 UTC ❌ MergeQueue: The build pipeline contains failing jobs for this merge request

Build pipeline has failing jobs for d3a8ff1:

⚠️ Do NOT retry failed jobs directly (why?).

What to do next?

  • Investigate the failures and when ready, re-add your pull request to the queue!
  • If your PR checks are green, try to rebase/merge. It might be because the CI run is a bit old.
  • Any question, go check the FAQ.
Details

Since those jobs are not marked as being allowed to fail, the pipeline will most likely fail.
Therefore, and to allow other builds to be processed, this merge request has been rejected and the pipeline got canceled.

@rkennke

rkennke commented Nov 5, 2026

Copy link
Copy Markdown
Contributor Author

/merge

@gh-worker-devflow-routing-ef8351

gh-worker-devflow-routing-ef8351 Bot commented Nov 5, 2026 •

Copy link
Copy Markdown

View all feedbacks in Devflow UI.

2026-11-05 13:30:50 UTC ℹ️ Start processing command /merge


2026-11-05 13:30:54 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 34m (p90).


2026-11-05 14:02:57 UTC ℹ️ MergeQueue: This merge request was merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants