fix(agentic-engineering): classify a flag-shaped word a value flag consumes - #198
Conversation
…nsumes forge-readonly-guard.sh consumed a value-taking flag's next word before checking it against the read-only allowlist, so a denied flag placed right after any value flag was never classified: `gh pr list --repo --web`, `--state --web`, `gh pr view 1 --limit --web`, `gh api … -H --hostname` and `git log --since --work-tree` all read `allow`. The guard's safety there rested on gh's and git's parsers consuming the word as an inert value — a property the guard never asserted and that stops holding the day any value flag grows an optional-value grammar. Classify the consumed word itself: a flag-shaped value is denied by name unless its flag's value grammar legitimately begins with a dash — `--search` expressions, `--jq`/`--template` programs, git `--grep`/`-S`/`-G` patterns — each justified where the set is declared. RED/GREEN: the five swallow shapes above allowed on the unmodified guard and are denied by name after the change; the three dash-leading value shapes stay allowed (383/383). Fixes #181 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lue-flag-classify-181
…main now carries Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Readiness record — tried and evaluated as a user at
|
@coderabbitai review |
|
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🧰 Additional context used📓 Path-based instructions (2)Any change to the plugin set updates both manifests in the same PR Edit both together.📄 CodeRabbit inference engine (AGENTS.md) Files:
README and manifests stay in lockstep.📄 CodeRabbit inference engine (AGENTS.md) Files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughThe forge read-only guard now validates values consumed by GitHub CLI and Git options. It rejects unexpected flag-shaped values, while allowing configured search expressions, templates, regexes, dates, and numeric values. Tests cover both denied and accepted dash-leading values. The plugin version is updated to Merge Risk: ⚪ Minimal · up to The read-only guard now rejects denied flags swallowed as option values while preserving documented dash-leading inputs. Targeted coverage and synchronized release metadata indicate no remaining merge-blocking risk. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files. (5 skipped: 5 unsupported.) 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 |
CodeRabbit refused this head on quota ( @codex review |
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. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dc85a340b7
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…er a flag The flag-shaped-value rule denied `git log --max-count -1`, `-n -1` and `--since -1.day`, all documented read-only git grammars, because it treated any dash-leading word as a flag. No flag name begins with a digit, so admit `-[0-9]*` for every value flag; letters and a second dash stay flag-shaped and denied by name. Pinned in both directions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…lue-flag-classify-181
…main now carries Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== PR diff summary =='
git diff --stat HEAD^ HEAD
git diff --name-status HEAD^ HEAD
printf '%s\n' '== Changed shell/test files: outlines =='
git diff --name-only HEAD^ HEAD | while IFS= read -r file; do
case "$file" in
*.sh)
printf '\n-- %s --\n' "$file"
ast-grep outline "$file" --items all || true
;;
esac
done
printf '%s\n' '== Current-head diff =='
git diff --find-renames --find-copies --unified=80 HEAD^ HEAD -- \
'*.sh' '*.md' '*.json' '*.yaml' '*.yml'Length of output: 14227 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Pull request metadata =='
gh pr view 198 --repo devantler-tech/agent-plugins \
--json baseRefOid,headRefOid,commits \
--jq '{baseRefOid, headRefOid, commits: [.commits[] | {oid, messageHeadline}]}'
base_sha="$(gh pr view 198 --repo devantler-tech/agent-plugins --json baseRefOid --jq '.baseRefOid')"
printf '\n== Full PR diff summary (%s..HEAD) ==\n' "$base_sha"
git diff --stat "$base_sha" HEAD
git diff --name-status "$base_sha" HEAD
printf '%s\n' '== Relevant files =='
fd -a -t f 'forge-readonly-guard|surveyor-forge-readonly' plugins || true
printf '%s\n' '== Full PR guard/test diff =='
git diff --find-renames --unified=100 "$base_sha" HEAD -- \
'*forge-readonly-guard*.sh' '*surveyor-forge-readonly*.sh'Length of output: 53960 🏁 Script executed: #!/bin/bash
set -euo pipefail
guard='plugins/agentic-engineering/scripts/forge-readonly-guard.sh'
state='plugins/agentic-engineering/resources/provider-neutral.desired-state.json'
printf '%s\n' '== Guard function map =='
ast-grep outline "$guard" --items all --type function
printf '%s\n' '== Value-consumption locations =='
rg -n -C 8 \
'split_flag|VALUE_FLAGS|OK_VALUE_FLAGS|check_consumed_value|FLAG_HAS_VALUE|GIT_VALUE_FLAGS' \
"$guard"
printf '%s\n' '== Git classifier and flag splitter =='
start="$(rg -n '^split_flag\(\)|^classify_git\(\)' "$guard" | head -n 1 | cut -d: -f1)"
end="$(rg -n '^classify_filter\(\)' "$guard" | head -n 1 | cut -d: -f1)"
sed -n "${start},${end}p" "$guard"
printf '%s\n' '== Asset digest verification =='
actual="$(sha256sum "$guard" | awk '{print $1}')"
declared="$(jq -r '.spec.source.requiredRuntimeAssets[] | select(.path == "scripts/forge-readonly-guard.sh") | .sha256' "$state")"
printf 'actual=%s\ndeclared=%s\n' "$actual" "$declared"Length of output: 43477
I found no issues in the reviewed change set. The guard checks separated consumed values for I did not execute repository scripts in the sandbox.
|
CodeRabbit refused this head on quota ( @codex review |
Readiness addendum at |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6b9d3a67e6
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… and date flags `--author -bot` is a regex over authors and `--since -yesterday` an approxidate expression; git accepts both separated forms (verified on 2.50), and the flag-shaped-value rule denied them because only `--grep`/`-S`/`-G` were in the git dash-admitting set. Add the pattern flags (`--author`, `--committer`) and the date flags (`--since`, `--until`, `--after`, `--before`) with the reason stated, and move the deny controls to `--max-count`, whose integer grammar no dash-letter word can satisfy. Pinned in both directions. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Second Codex P2 (dash-leading values for git pattern and date flags) fixed at this head and its thread resolved. @coderabbitai review |
Rate Limit Exceeded
|
@coderabbitai review |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='devantler-tech/agent-plugins'
pr='198'
guard='plugins/agentic-engineering/scripts/forge-readonly-guard.sh'
test='plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh'
printf '%s\n' '== Pull request metadata =='
gh pr view "$pr" --repo "$repo" \
--json baseRefOid,headRefOid,commits \
--jq '{baseRefOid,headRefOid,commits:[.commits[]|{oid,messageHeadline}]}'
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
printf '%s\n' '== Changed files =='
git diff --name-status "$base" HEAD
printf '%s\n' '== Full relevant diff =='
git diff --find-renames --unified=100 "$base" HEAD -- \
'plugins/agentic-engineering/scripts/forge-readonly-guard.sh' \
'plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh' \
'plugins/agentic-engineering/scripts/surveyor-forge-readonly.test.sh' \
'plugins/agentic-engineering/resources/provider-neutral.desired-state.json' \
'plugins/agentic-engineering/plugin.json'
printf '%s\n' '== Guard outline =='
ast-grep outline "$guard" --items all --type function || true
printf '%s\n' '== Value parsing and classification code =='
rg -n -C 12 \
'split_flag|classify_(gh|git)|consumed|dash|value flag|VALUE_FLAGS|OK_VALUE|is_.*number|number' \
"$guard"
printf '%s\n' '== Relevant test cases =='
rg -n -C 4 \
'consumed|--web|--work-tree|--since|--grep|--search|--jq|dash|flag-shaped' \
"$test" "$test" 2>/dev/null || true
printf '%s\n' '== Runtime asset declaration =='
actual="$(sha256sum "$guard" | awk '{print $1}')"
declared="$(jq -r '.spec.source.requiredRuntimeAssets[] | select(.path == "scripts/forge-readonly-guard.sh") | .sha256' \
'plugins/agentic-engineering/resources/provider-neutral.desired-state.json')"
printf 'actual=%s\ndeclared=%s\n' "$actual" "$declared"Length of output: 164816 🏁 Script executed: #!/bin/bash
set -euo pipefail
guard='plugins/agentic-engineering/scripts/forge-readonly-guard.sh'
test='plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh'
printf '%s\n' '== Latest commit delta =='
git diff --unified=80 HEAD^ HEAD -- "$guard" "$test"
printf '%s\n' '== split_flag and subcommand discovery =='
sed -n '585,685p' "$guard"
printf '%s\n' '== Full git classifier =='
sed -n '1053,1251p' "$guard"
printf '%s\n' '== Terminator and affected-value test coverage =='
rg -n -C 3 -- \
'(^|[^[:alnum:]])--([^[:alnum:]]|$)|--grep|-S|-G|--author|--committer|--after|--before|--template|-t|--search|--jq|-q' \
"$test"
printf '%s\n' '== Direct guard references outside the changed test =='
rg -n -C 3 -- \
'forge-readonly-guard\.sh|GIT_DASH_VALUE_FLAGS|GH_DASH_VALUE_FLAGS|check_consumed_value' \
plugins .github .claude-pluginLength of output: 98686
I found no issues. The guard classifies separated values for I did not execute repository scripts in the sandbox. You are interacting with an AI system. |
CodeRabbit answered both requests at this head as chat prompts rather than reviews (no review ran), so the round advances to the next lane. Both earlier Codex P2s are fixed at this head and their threads resolved. @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ab96548cb8
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…er values A label or milestone title is a free-form server-side filter string that may itself begin with a dash, so `gh issue list --label -bug` is a valid read the flag-shaped-value rule denied. Add `--label` and `--milestone` to the gh dash-admitting set with the reason stated; the host-choosing, count, state and field flags keep denying a flag-shaped value by name. Pinned in both directions (393/393). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…lue-flag-classify-181
…main now carries Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Third Codex P2 (dash-leading label and milestone filter values) fixed at this head and its thread resolved; merged with @coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
repo='devantler-tech/agent-plugins'
pr='198'
printf '%s\n' '== Pull request metadata =='
gh pr view "$pr" --repo "$repo" \
--json baseRefOid,headRefOid,commits \
--jq '{baseRefOid,headRefOid,commits:[.commits[]|{oid,messageHeadline}]}'
base="$(gh pr view "$pr" --repo "$repo" --json baseRefOid --jq '.baseRefOid')"
printf '%s\n' '== Changed files =='
git diff --name-status "$base" HEAD
printf '%s\n' '== Full PR diff =='
git diff --find-renames --find-copies --unified=100 "$base" HEAD -- \
'plugins/agentic-engineering/scripts/forge-readonly-guard.sh' \
'plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh' \
'plugins/agentic-engineering/scripts/surveyor-forge-readonly.test.sh' \
'plugins/agentic-engineering/resources/provider-neutral.desired-state.json' \
'plugins/agentic-engineering/plugin.json'
printf '%s\n' '== Relevant shell-file maps =='
for file in \
plugins/agentic-engineering/scripts/forge-readonly-guard.sh \
plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh \
plugins/agentic-engineering/scripts/surveyor-forge-readonly.test.sh
do
if [ -f "$file" ]; then
printf '\n-- %s --\n' "$file"
ast-grep outline "$file" --items all --type function || true
fi
done
printf '%s\n' '== Label, milestone, and consumed-value handling =='
rg -n -C 12 -- \
'label|milestone|check_consumed_value|consumed|DASH_VALUE|split_flag|classify_gh|--search|--jq' \
plugins/agentic-engineering/scripts/forge-readonly-guard.sh \
plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh
printf '%s\n' '== Runtime asset digest =='
guard='plugins/agentic-engineering/scripts/forge-readonly-guard.sh'
state='plugins/agentic-engineering/resources/provider-neutral.desired-state.json'
actual="$(sha256sum "$guard" | awk '{print $1}')"
declared="$(jq -r '.spec.source.requiredRuntimeAssets[] | select(.path == "scripts/forge-readonly-guard.sh") | .sha256' "$state")"
printf 'actual=%s\ndeclared=%s\n' "$actual" "$declared"
printf '%s\n' '== Version declarations =='
rg -n -C 2 -- '4\.4\.26|version' \
plugins/agentic-engineering/plugin.json \
plugins/agentic-engineering/resources/provider-neutral.desired-state.jsonLength of output: 184421 🏁 Script executed: #!/bin/bash
set -euo pipefail
base='4cdc4bdc9149acf9c27950ca450371a3c4a7c4e6'
guard='plugins/agentic-engineering/scripts/forge-readonly-guard.sh'
test='plugins/agentic-engineering/scripts/forge-readonly-guard.test.sh'
printf '%s\n' '== Whitespace errors in full PR =='
git diff --check "$base" HEAD
printf '%s\n' '== Latest fix and version-bump deltas =='
git diff --unified=80 ab96548cb8c2256a3292ffad5aa80726b7590506 HEAD -- \
"$guard" "$test" \
.claude-plugin/marketplace.json \
.github/plugin/marketplace.json \
plugins/agentic-engineering/.claude-plugin/plugin.json \
plugins/agentic-engineering/plugin.json \
plugins/agentic-engineering/resources/provider-neutral.desired-state.json
printf '%s\n' '== Argument splitting and subcommand discovery =='
sed -n '587,686p' "$guard"
printf '%s\n' '== Plugin version declarations =='
rg -n -C 3 -- '"agentic-engineering"|"version"[[:space:]]*:[[:space:]]*"4\.4\.26"' \
.claude-plugin/marketplace.json \
.github/plugin/marketplace.json \
plugins/agentic-engineering/.claude-plugin/plugin.json \
plugins/agentic-engineering/plugin.json
printf '%s\n' '== Validate changed JSON files =='
for file in \
.claude-plugin/marketplace.json \
.github/plugin/marketplace.json \
plugins/agentic-engineering/.claude-plugin/plugin.json \
plugins/agentic-engineering/plugin.json \
plugins/agentic-engineering/resources/provider-neutral.desired-state.json
do
printf '%s: ' "$file"
jq -e . "$file" >/dev/null && printf 'valid\n'
done
printf '%s\n' '== Guard references and asset declarations =='
rg -n -C 3 -- \
'forge-readonly-guard\.sh|9591835448efc0d8f08f83da48deacc9a50079f4df79e9db7ed91d9146dd3649|4\.4\.26' \
.claude-plugin .github plugins/agentic-engineeringLength of output: 71480
I found no issues. The guard admits dash-leading values only for The runtime asset SHA-256 matches I did not execute repository scripts in the sandbox. ✅ Action performedReview finished.
|
1 participant
Why
The surveyor's read-only guard is what keeps a survey from opening a browser, retargeting an authenticated request, or reaching a file. It consumed a value-taking flag's next word before checking it, so a denied flag placed right after any value flag was never classified at all —
--repo --webread as allowed. It was not exploitable today only because gh's own parser happens to keep that word inert, a property the guard never asserted.What
The guard now classifies the word it consumes: a flag-shaped value is refused by name unless its flag's value genuinely may begin with a dash (a search expression, a jq or template program, a git grep pattern), each justified where the rule is declared. Proven in both directions with the guard's own self-test — the swallow shapes were allowed before and are denied after, the legitimate dash-leading values stay allowed.
Fixes #181
🤖 Generated with Claude Code