Harden review-only config isolation and close review submission TOCTOU window#28
Harden review-only config isolation and close review submission TOCTOU window#28dceoy wants to merge 6 commits into
Conversation
…TOCTOU window Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
Found two issues that should be addressed before merge: review-only configuration isolation remains bypassable through additional environment-controlled config paths, and the version comparison introduces a macOS portability regression.
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
Found one remaining configuration-isolation bypass. The previously reported environment-variable and macOS portability issues are resolved, and CI passes, but review-only mode can still load caller-controlled remote configuration through OpenCode's data directory.
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
PR Review Summary
No new actionable issues meeting the review’s reporting threshold were identified beyond the existing unresolved review thread.
Checked
- Current diff for the review-only environment guard, OpenCode version-floor enforcement, and pre-submission PR-head revalidation.
- Regression coverage for redirected configuration, macOS-portable version comparison, and head movement before review POST/PUT operations.
- Existing review threads and current-head CI/CD status.
- Repository ownership, archived status, PR author/state/draft status, and current full head SHA.
Notes
- The existing unresolved critical thread concerning caller-controlled
XDG_DATA_HOMEand OpenCode data-directory configuration remains applicable and was not duplicated. - Current-head CI/CD completed successfully. The separate OpenCode review workflow was still in progress when checked.
Recommended Action
Address the existing unresolved data-directory isolation finding before merge. No additional automated blocking concerns were identified.
Co-authored-by: dceoy <dceoy@users.noreply.github.com>
Triage SummaryMode: Findings
Changes Applied (review #3)
Verification
Platform Action Queue (post-push)
No remaining open items on this PR. |
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
Found one remaining review-only isolation bypass. The latest XDG_DATA_HOME fix is sound and current-head CI passes, but caller-controlled enable-toolkit: false skips installation/cleanup of the trusted global configuration while review-only mode still runs.
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
Found one remaining review-only isolation bypass. The previously reported environment-variable, data-directory, portability, and head-revalidation issues are resolved, and current-head CI passes.
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
The review-only hardening is active, but the actual /review-pr execution path still bypasses the structured-review command and can succeed without producing inline comments. Two issues remain.
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
Found one remaining review-only isolation bypass. Current CI jobs pass, and the previously reported configuration, dispatch, and submission-postcondition issues are resolved.
| if: inputs.enable-toolkit == 'true' | ||
| if: inputs.enable-toolkit == 'true' || steps.review_mode.outputs.enabled == 'true' | ||
| shell: bash -euo pipefail {0} | ||
| env: |
There was a problem hiding this comment.
critical: This now makes the existing fresh-copy branch the mandatory review-only security boundary, but that branch does not handle a symlinked ${HOME}/.opencode. [[ -d ... ]] succeeds for a directory symlink while the following find does not dereference the command-line symlink, so none of the target config, agents, commands, or plugins are removed. OpenCode still scans ${HOME}/.opencode and follows symlinks, allowing a pre-existing malicious tree on a persistent/self-hosted runner to bypass review-only permissions. Reject/remove a symlinked root and recreate a real directory while preserving only the trusted bin path; add a regression test for this case.
![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: 3fc69e08f8
ℹ️ 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".
|
|
||
| if [[ "${prompt}" =~ ^/review-pr([[:space:]]+(.*))?$ ]]; then | ||
| arguments="${BASH_REMATCH[2]:-}" | ||
| elif [[ "${event_comment}" =~ /review-pr([[:space:]]+(.*))?$ ]]; then |
There was a problem hiding this comment.
In issue/PR comments such as /oc can you explain the /review-pr workflow?, review-only detection has already been enabled because /review-pr appears as a whitespace-delimited token, and this unanchored match then treats that mention as the trusted review command and replaces the user's prompt with the full review flow. That makes ordinary OpenCode requests that merely mention /review-pr run a structured PR review or fail the review-only gates; parse only an actual command position, e.g. /review-pr as the prompt or immediately after the configured mention.
Useful? React with 👍 / 👎.
dceoy
left a comment
dceoy
left a comment
There was a problem hiding this comment.
PR Review Summary
No new actionable issues meeting the review’s reporting threshold were identified beyond the existing unresolved review threads.
Checked
- Current-head changes to review-only dispatch, trusted orchestrator selection, OpenCode configuration/data isolation, and PR-head revalidation before review writes.
- Regression coverage for redirected configuration/data paths, command expansion, submission postconditions, and head movement before review POST/PUT operations.
- Existing inline review threads and their current resolution/outdated state.
- Current-head CI/CD status and the separate OpenCode review workflow state.
- Repository ownership and archived status, PR author/state/draft status, base/head refs, changed files, current diff, and full head SHA.
Notes
- Existing unresolved findings concerning a symlinked
${HOME}/.opencodebypass and overly broad/review-prmention detection remain applicable and were not duplicated. - Current-head CI/CD completed successfully. The separate OpenCode review workflow was still in progress when checked.
Recommended Action
Address the existing unresolved review-only trust-boundary and command-detection findings before merge. No additional automated blocking concerns were identified.
|
The structured submission cannot complete because the Edit/Write tools are blocked by the permission rules — the Per the instructions, I am failing the run rather than emitting the findings as a top-level completion comment. Review status: failed at structured-submission step. The findings identified by the reviewers could not be written to the review-state payload file due to a write-permission override in the local environment (the No review was posted to GitHub. To retry, the local permission configuration needs the |
2 participants
Summary
Fixes two post-merge issues from #27 while preserving
opencode github run, non-review behavior, the review-only permission boundary, and proposal-only/review-pr simplify.1. Isolate review-only runs from caller-controlled OpenCode configuration
OPENCODE_DISABLE_PROJECT_CONFIGalone is insufficient. Review-only mode now:OPENCODE_CONFIG,OPENCODE_CONFIG_DIR, andOPENCODE_CONFIG_CONTENT, which upstream honors even when project config discovery is disabled;XDG_CONFIG_HOMEandOPENCODE_TEST_HOME, preventing redirection to alternate global or home.opencode/trees;OPENCODE_PERMISSION, preventing direct permission-policy overrides;x.y.zrelease at or above1.1.29, the first release supportingOPENCODE_DISABLE_PROJECT_CONFIG.The version-floor comparison is implemented with portable Bash numeric comparison and does not depend on GNU
sort -V, so it also works on macOS runners.2. Close the PR review submission TOCTOU window
review-pr-submit.shnow re-reads the live PR head and compares it with the pinned SHA immediately before both the initial reviewPOSTand review updatePUT. A moved or unreadable head aborts the write, while the initial review remains pinned withcommit_id.Tests
Regression coverage verifies that:
sortis unavailable;action.ymlapplies the guard beforeopencode github run.No GitHub permission scope changes are introduced.
🤖 Generated with Claude Code