Skip to content

feat(gh): allow gh api to POST PR review comments (unprompted)#43

Merged
madarco merged 2 commits into
mainfrom
feat/gh-api-post-comments
Jun 1, 2026
Merged

feat(gh): allow gh api to POST PR review comments (unprompted)#43
madarco merged 2 commits into
mainfrom
feat/gh-api-post-comments

Conversation

@madarco
Copy link
Copy Markdown
Owner

@madarco madarco commented Jun 1, 2026

Summary

Extends the allowlisted gh api proxy (PR #39, read-only) so agents can post inline PR review comments — the one thing gh pr comment can't do (it only posts conversation comments). Per request, comments are low-risk so this runs without a host confirm prompt.

The write policy becomes endpoint-aware: refuseGhApiWrite(args)refuseGhApiCall(endpoint, args).

  • GET — allowed on every allowlisted endpoint (unchanged).
  • POST — allowed (unprompted) only on the PR review-comment endpoints: repos/:o/:r/pulls/:n/comments and .../comments/:id/replies.
  • PATCH/PUT/DELETE — refused.
  • --input — refused: the host gh runs with stdin ignored and a box-side file path wouldn't exist on the host, so it can't cross the relay. Agents pass fields via -f/-F.

Read vs write allowlists are separate consts (GH_API_ALLOWED_ENDPOINTSGH_API_WRITE_ALLOWED_ENDPOINTS) so a future read-only endpoint can be added without widening the POST surface.

Layers

  • packages/relay/src/gh.ts — new GH_API_WRITE_ALLOWED_ENDPOINTS + isWriteAllowedGhApiEndpoint; refuseGhApiCall (reuses the existing robust pflag-spelling parsing to classify the effective method).
  • packages/relay/src/{server,host-actions}.ts — both dispatch handlers call refuseGhApiCall(endpoint, args); no prompt.
  • packages/sandbox-docker/scripts/gh-shimhandle_api forwards field/method flags (the relay is the security boundary), still rejects --input locally, expanded strict_flags allowlist.
  • packages/ctl/src/commands/gh.ts — description wording.

Notes

  • Intentional inconsistency: gh pr comment prompts; gh api … pulls/:n/comments (POST) does not — unprompted was the explicit choice for gh api comments.

Test plan

  • pnpm build green; lint clean.
  • relay 144/144 (gh.test 25 incl. new refuseGhApiCall + isWriteAllowedGhApiEndpoint cases), ctl 184/184 (shim now forwards POST/field flags, still rejects --input). (One unrelated queue.test.ts timing flake under parallel load; passes in isolation.)
  • Live: in a box on a branch with an open PR, gh api repos/<o>/<r>/pulls/<n>/comments -f body=… -f commit_id=… -f path=… -F line=… posts an inline review comment; -X DELETE / issues/:n/comments POST / --input - each refused. Not yet run.

🤖 Generated with Claude Code


Note

Medium Risk
Agents can create GitHub PR review comments without a host prompt on a narrow endpoint allowlist; relay-side parsing is the security boundary against bypass via glued flags or direct ctl calls.

Overview
Extends the allowlisted gh api proxy so agents can POST inline PR review comments (and replies) via GitHub’s REST API—something gh pr comment cannot do—while keeping the host as the only place that holds credentials.

Policy moves from read-only argv checks (refuseGhApiWrite) to endpoint-aware refuseGhApiCall: GET on any proxied path; POST only on repos/.../pulls/:n/comments and .../comments/:id/replies (split GH_API_WRITE_ALLOWED_ENDPOINTS from the overall allowlist so future read-only routes don’t widen writes); PATCH/PUT/DELETE refused; --input refused (bodies must use -f/-F). Comment POSTs run without a host confirm prompt (reads stay silent too).

Layers: relay (gh.ts, docker server.ts, cloud host-actions.ts, exports); box gh-shim forwards method/field flags and only blocks --input locally; ctl help text and tests updated.

Reviewed by Cursor Bugbot for commit 0a7a979. Configure here.

Makes the gh api write policy endpoint-aware: refuseGhApiWrite(args) becomes
refuseGhApiCall(endpoint, args). GET stays allowed on the allowlist; POST is
allowed (no prompt) only on the PR review-comment endpoints
(repos/:o/:r/pulls/:n/comments and .../comments/:id/replies). PATCH/PUT/DELETE
and --input (stdin/file body can't cross the relay) stay refused.

Relax the in-box gh-shim handle_api to forward field/method flags (relay is the
security boundary) while still rejecting --input locally. Both relay dispatch
paths (docker + cloud) updated.
@vercel
Copy link
Copy Markdown

vercel Bot commented Jun 1, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agentbox-web Ready Ready Preview, Comment Jun 1, 2026 5:13pm

Request Review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 0a7a979. Configure here.

Comment thread packages/relay/src/gh.ts
arg.startsWith('--raw-field=')
) {
return refuse(`field flag '${arg}' makes the request mutating`);
hasFieldFlag = true;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Parser doesn't consume field-flag values, enabling method misidentification

Medium Severity

refuseGhApiCall skips the value token for -X/--method (via i++) but not for -f, -F, --field, --raw-field, or other value-taking flags. When gh's pflag parses -f -X=GET, it consumes -X=GET as -f's value and auto-POSTs. But the relay sees -X=GET as a standalone method flag, concluding the method is GET. This mismatch lets a direct agentbox-ctl call trick the relay into treating a POST as a GET, bypassing the write-allowed endpoint check. Currently all allowlisted endpoints permit POST so it's not exploitable, but the code architecture explicitly anticipates future read-only endpoints where this would become a real bypass.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 0a7a979. Configure here.

…wngrade POST to GET

Bugbot: refuseGhApiCall consumed the value of -X/--method but not of the
spaced field flags (-f/-F/--field/--raw-field). pflag binds e.g. `-f -X=GET`'s
`-X=GET` as the field value and POSTs, but the parser misread it as
`--method GET` and classified the call as a read — a write-policy bypass once
a read-only endpoint exists. Consume the value token for spaced field flags.
@madarco madarco merged commit 2a3f6f2 into main Jun 1, 2026
3 checks passed
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