Skip to content

fix(opencode): evaluate apply_patch move destination in edit permission ruleset - #29

Open
detail-app[bot] wants to merge 1 commit into
devfrom
detail/bug-fix/fix-opencode-evaluate-apply-patch-move-destination-7b3364
Open

detail-app[bot] wants to merge 1 commit into
devfrom
detail/bug-fix/fix-opencode-evaluate-apply-patch-move-destination-7b3364

Conversation

@detail-app

@detail-app detail-app Bot commented Sep 12, 2026

Copy link
Copy Markdown

Detail bug report: View on Detail

Issue for this PR

Closes #

Type of change

  • Bug fix

What does this PR do?

apply_patch move hunks bypassed destination-scoped edit permission rules. The patterns array passed to ctx.ask was built from only the source path (c.filePath), never the move destination (c.movePath). So a configured rule like { "permission": { "edit": { "secret/*": "deny" } } } was silently ignored for moves into secret/, while direct edit/write calls to the same path were correctly blocked. In --auto mode (auto-approve anything not explicitly denied), this became a silent bypass of the operator's deny rule.

Root cause: apply_patch.ts built relativePaths with fileChanges.map((c) => path.relative(worktree, c.filePath)) — source only. Permission.ask only evaluates rules for patterns in request.patterns, so the destination never reached the ruleset. This is asymmetric with edit.ts/write.ts, which pass the write target in patterns.

Fix: Build relativePaths with flatMap, appending both the source and destination relative paths whenever a change has a movePath. The destination now reaches the edit ruleset just like the write target in edit/write. Out-of-worktree destinations keep their existing external_directory gate unchanged.

How did you verify your code works?

  • Added 3 unit tests in apply_patch.test.ts:
    • move includes destination in permission patterns — asserts both source and destination appear in ctx.ask patterns.
    • destination-scoped edit deny blocks a move (auto mode) — uses a real Permission.fromConfig ruleset driven through a makePermissionCtx mock mirroring --auto mode; asserts the move fails, secret/notes.txt is never created, and the source is intact.
    • move into non-denied destination still succeeds through the ruleset — same secret/*: deny ruleset but moving into allowed/; asserts the move succeeds, proving the deny doesn't over-block.
  • Confirmed all 3 tests fail without the fix and pass with it; all 27 pre-existing tests pass unchanged (no regressions).
  • End-to-end verified (not versioned) by spawning the real opencode run --dangerously-skip-permissions CLI binary against an in-process test LLM server with a gpt-5 model + secret/*: deny config: without the fix secret/notes.txt was created (deny bypassed); with the fix the move was blocked. Repeated through an ACP subprocess with an auto-approving requestPermission handler with the same result.
  • Unit tests, typecheck (tsgo --noEmit), and lint (oxlint) all pass with 0 errors.

Checklist

  • I have tested my changes locally
  • I have not included unrelated changes in this PR

Automatic Fixes PRs can be configured here.

@greptile-apps greptile-apps 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.

Your trial has ended. Reactivate Greptile to resume code reviews.

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.

0 participants