enforce read-only PR reviews

[dceoy]

Summary

Fixes #24.

• route /review-pr through a dedicated fail-closed orchestrator and read-only reviewer agents
• isolate review runs from project and inherited OpenCode configuration, refresh the bundled toolkit, and require OpenCode 1.2.14 or newer
• constrain GitHub reads and structured review writes to trusted argument-free helpers with pinned PR context
• revalidate the PR head immediately after App-token verification and before each review write
• preserve command expansion and proposal-only /review-pr simplify behavior from Restore retained command dispatch and review correctness improvements #30

Root cause

Review agents could execute repository-controlled commands that dirtied the checkout, while opencode github run retained an App token capable of committing and pushing those changes. Project-controlled OpenCode configuration and plugins were also still loadable during review runs.

Impact

PR review execution is now analysis-only: repository tools cannot mutate the checkout, project configuration/plugins are excluded, inherited explicit config overrides are cleared, and review submission is limited to the pinned PR head.

Validation

• .agents/skills/local-qa/scripts/qa.sh
• 46 Bats regression tests
• security pattern scan and manual CI/CD trust-boundary review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f1859f7c9d

ℹ️ 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".

[dceoy]

PR Review Summary

No new actionable issues meeting the review’s reporting threshold were identified beyond the existing unresolved review threads.

Checked

• review-only mode detection and slash-command expansion consistency
• read-only orchestrator and reviewer-agent boundaries
• repository binding for gh pr view and gh pr diff reads
• pinned PR context and stale-head checks before writes
• project/inherited OpenCode configuration isolation
• existing unresolved review threads and current-head identity

Notes

Two existing unresolved findings remain applicable and were not duplicated: review-only detection can diverge from actual command expansion, and GitHub CLI PR reads are not explicitly pinned to the event repository. The combined commit-status endpoint returned no status contexts, so current-head CI was not independently confirmed through that endpoint.

Recommended Action

Address the existing unresolved command-routing and repository-binding findings before merge.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6c1824aeea

ℹ️ 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".

[dceoy]

PR Review Summary

No new actionable issues meeting the review’s reporting threshold were identified beyond the existing unresolved review thread.

Checked

• current-head changes to review-only command detection and trusted command expansion
• repository binding for all gh pr view and gh pr diff reads
• pinned repository, PR number, and head-SHA validation before review writes
• token-verification freshness checks and structured review payload validation
• review-only OpenCode configuration isolation and helper installation
• current unresolved review threads and changed documentation

Notes

The previously reported command-routing and repository-binding defects are resolved in the current head. One existing unresolved documentation finding remains and was not duplicated: the README describes the token resolver as the sole external-path exception even though the configured review boundary also permits the review helpers and review-state files. The combined commit-status endpoint returned no status contexts, so current-head CI was not independently confirmed through that endpoint.

Recommended Action

Correct the existing README trust-boundary description before merge. No additional automated blocking concerns were identified.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 011b2146d1

ℹ️ 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".

[opencode-agent]

I'm unable to write the review payload to the state file due to tool permission restrictions. Let me provide the comprehensive review findings directly.

---

OpenCode PR Review: 7 inline finding(s), 3 summary-only finding(s)

Inline Findings

1. README inaccurately describes external_directory allow-listed paths (important · documentation-accuracy-reviewer)

File: README.md line 90

The README claims ~/.config/opencode/scripts/resolve-app-token.sh is "the one narrow exception" and "no other external path is permitted" within the external_directory permission. However, .opencode/opencode.jsonc allow-lists six paths:

• resolve-app-token.sh
• review-pr-submit.sh
• review-pr-gh.sh
• initial.json
• update.json
• context.json

Fix: Update the README to enumerate all six allow-listed external paths so the documented security boundary matches the real configuration.

2. Review-only detection regex does not handle newlines in effective prompt (important · security-code-reviewer)

File: action.yml line 111

The regex ^/review-pr([[:space:]].*)?$ fails when the effective prompt contains newlines after the command because .* in bash ERE does not match newline characters, and $ anchors to end-of-string. A multi-line comment such as /opencode /review-pr security\n<project-controlled content> would not match, leaving review_only=false. This means OPENCODE_DISABLE_PROJECT_CONFIG is not set, inherited config overrides are not cleared, and project commands are included in the resolution path — a bypass of the entire review-only isolation.

Fix: Strip or split on newlines before the regex check, e.g. [[ "${OPENCODE_EFFECTIVE_PROMPT%%$'\n'*}" =~ ^/review-pr([[:space:]].*)?$ ]] to examine only the first line.

3. sort -V is a GNU extension unavailable on macOS (important · silent-failure-hunter)

File: action.yml line 124

sort -V is a GNU extension. On macOS GitHub Actions runners, the version comparison fails with sort: illegal option -- V and the user sees that raw error instead of the intended ::error:: message.

Fix: Use a pure-bash version comparison (split on . and compare numerically) or detect gsort as a fallback.

4. Missing error handling on gh api --method PUT review update (important · silent-failure-hunter)

File: .opencode/scripts/review-pr-submit.sh line 91

The initial-submission path (lines 73-75) captures the response and validates the returned review ID with an explicit fail message, but the update path at line 91 just runs gh api PUT without || fail or any response validation. If the PUT fails after the initial review was already submitted, the script exits with gh's raw error and the review body is left incomplete.

Fix: Add || fail "Failed to submit the review update." to the PUT command, consistent with other failure patterns in the same file.

5. Misleading error message in review-pr-gh.sh (important · comment-analyzer)

File: .opencode/scripts/review-pr-gh.sh line 39

The error message says "Review helper operations take no arguments" but the script enforces exactly one argument (the operation name: context|metadata|diff). A future maintainer debugging a failed invocation could be misled by this contradiction.

Fix: Reword to "Review helper operations take exactly one argument (the operation name) and no additional arguments."

6. Same misleading error message in review-pr-submit.sh (important · comment-analyzer)

File: .opencode/scripts/review-pr-submit.sh line 34

Same issue as finding #5: says "take no arguments" but requires exactly one (prepare|submit-initial|update).

Fix: Update to "Review helper operations take exactly one argument (the operation name) and no additional arguments."

7. Test payload missing required "side" key passes only incidentally (important · code-reviewer)

File: .agents/skills/local-qa/scripts/test-review-pr-read-only.bats line 107

The test "submission fails if the PR head changes after context" writes a payload {"path":"x","line":1,"body":"finding"} with no "side":"RIGHT". The test passes only incidentally because the trusted_context head-change check exits before the JSON schema validation is reached. If the trusted_context flow is refactored, this test will fail with "Invalid initial review payload." instead of the intended "PR head changed" error.

Fix: Add "side":"RIGHT" to the JSON payload on line 107.

---

Summary-only Findings

1. Test coverage gaps (test-coverage-reviewer, pr-test-analyzer)

Multiple behavioral test gaps were identified across critical paths:

• No negative tests for the complex jq payload validation in submit-initial (missing body, empty comments, invalid side, non-positive line, start_line > line)
• The entire update operation has zero test coverage
• The metadata and diff operations in review-pr-gh.sh have no behavioral tests
• trusted_context is only tested for head-SHA mismatch; repository mismatch and PR number mismatch are untested
• The OpenCode version gate (>= 1.2.14) has no behavioral test — only a static grep for the error string exists
• The malicious-project isolation test uses only static grep checks; no runtime execution test confirms that OPENCODE_DISABLE_PROJECT_CONFIG or the config cleanup actually prevents plugin execution

2. review-pr.md conflates prepare and context operations (comment-analyzer, suggestion)

Line 65 states "the single prepare operation in section 1 has already created the empty payload files and pinned context." The prepare operation creates the empty payload files, but it is the context operation (invoked immediately after prepare) that actually pins the repository, PR number, and head SHA. Conflating the two could confuse a reader reasoning about which operation sets which state.

Fix: Reword to "the prepare and context operations in section 1 have already created the empty payload files and pinned the review context."

3. Missing explanatory comment for review-only cleanup (comment-analyzer, suggestion)

Action.yml line 141's find command strips all non-bin contents from ~/.opencode during review-only mode but has no inline comment explaining why this cleanup is necessary. Future maintainers may not understand that this removes pre-existing project-level plugins, agents, and config that could interfere with the isolated review run.

Fix: Add a comment such as # Strip any pre-existing project-level plugins, agents, or config from the home directory that could interfere with the isolated review run.

---

Note: The structured review submission via the constrained helper could not be completed due to tool permission restrictions preventing writing to the review state file. The findings above constitute the complete analysis.

github run

[dceoy]

Reviewed the current head. I found one additional configuration-isolation gap and left it inline. The three existing unresolved findings also remain applicable: the README overstates the external-path boundary, pinned PR read failures can fall back to local mode, and no-finding/summary-only exits do not revalidate the pinned head. CI/CD and the OpenCode workflow are currently green, but these trust-boundary issues should be addressed before merge.

[dceoy]

Addressed the concrete findings in 29345df, including the trust-boundary, stale-head, portability, helper-error, documentation, and focused regression-test fixes; the broader test-coverage inventory remains follow-up work.

[dceoy]

Reviewed the current head. I found two remaining configuration-boundary regressions in action.yml: the minimum-version check admits prereleases below the required stable floor, and the non-review branch clears a caller-provided project-config opt-out. Existing review threads are resolved and CI/CD run 206 is green, but these two issues should be fixed before merge.

[dceoy]

PR Review Summary

No actionable issues meeting the review’s reporting threshold were identified.

Checked

• Current-head review-only mode detection and slash-command expansion
• Isolation from project and inherited OpenCode configuration and plugins
• Pinned repository, PR number, and head-SHA handling for GitHub reads and review writes
• App-token verification and stale-head checks before initial submission and review updates
• Structured inline-comment payload validation and read-only agent/tool boundaries
• Added Bats trust-boundary regressions and existing review discussion

Notes

The combined commit-status endpoint returned no status contexts, so current-head CI was not independently confirmed through that endpoint.

Recommended Action

No automated blocking concerns were identified. Merge remains a maintainer decision after required checks pass.

[dceoy]

Reviewed the current head. The read-only boundary, pinned-head checks, configuration isolation, and structured-review write path are materially improved, and CI/CD run 207 is green. I found one remaining control-flow inconsistency: the documented local review mode is unreachable when the event has no PR context because the command first requires the context helper to succeed and stop on any failure.

[dceoy]

Reviewed the current head. No additional actionable issues meeting the reporting threshold were identified.

Verified:

• all existing review threads are resolved
• review-only command detection matches slash-command expansion
• project and inherited OpenCode configuration are isolated
• trusted PR context and head SHA are revalidated before reads and writes
• structured inline-review payloads and helper commands remain constrained
• local review fallback is limited to events without a trusted PR number

CI/CD run 208 completed successfully, including Bats, ShellCheck, actionlint/security scans, YAML lint, and CodeQL. No blocking concerns remain from this review.