Skip to content

fix(plugin): restore permission merge order precedence#2806

Merged
code-yeongyu merged 1 commit intodevfrom
fix/b5-permission-merge-order
Mar 24, 2026
Merged

fix(plugin): restore permission merge order precedence#2806
code-yeongyu merged 1 commit intodevfrom
fix/b5-permission-merge-order

Conversation

@code-yeongyu
Copy link
Copy Markdown
Owner

@code-yeongyu code-yeongyu commented Mar 24, 2026

Problem

In tool-config-handler.ts, OmO permissions were placed first in the merge order, meaning webfetch: "allow" and external_directory: "allow" overrode any explicit user/OpenCode deny settings. This widened access silently.

Fix

  • Ensure user/OpenCode explicit deny settings always take precedence over OmO default allows
  • Merge order: OmO defaults first (base), then OpenCode defaults, then user overrides on top
  • Added regression test for explicit deny precedence

Review

All 5 review-work lanes passed (goal, QA, code quality, security, context mining).
Tests passed, typecheck passed, build passed.


Summary by cubic

Fixes permission merge order so explicit user/OpenCode deny values override OmO default allow for webfetch and external_directory, preventing silent permission widening.

  • Bug Fixes
    • Reordered merge in tool-config-handler.ts: OmO defaults → OpenCode defaults → user overrides; keeps task: "deny".
    • Added tests in tool-config-handler.test.ts to ensure explicit deny is preserved and defaults apply when unset.

Written for commit 0b614b7. Summary will update on new commits.

Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

1 issue found across 2 files

Confidence score: 3/5

  • There is a concrete regression risk in src/plugin-handlers/tool-config-handler.ts: setting task: "deny" after spreading user config will override any OpenCode SDK-compatible user-provided task value.
  • Because this issue is severity 7/10 with high confidence (10/10) and directly affects user configuration behavior, this carries moderate merge risk rather than a minor housekeeping concern.
  • Pay close attention to src/plugin-handlers/tool-config-handler.ts - merge order currently forces task and can break expected OpenCode compatibility.
Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/plugin-handlers/tool-config-handler.ts">

<violation number="1" location="src/plugin-handlers/tool-config-handler.ts:121">
P1: Custom agent: **Opencode Compatibility**

Placing `task: "deny"` after the user configuration spread completely overwrites any OpenCode SDK-compatible user settings for `task`.</violation>
</file>

Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.

Comment on lines +121 to 122
...(params.config.permission as Record<string, unknown>),
task: "deny",
Copy link
Copy Markdown

@cubic-dev-ai cubic-dev-ai bot Mar 24, 2026

Choose a reason for hiding this comment

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

P1: Custom agent: Opencode Compatibility

Placing task: "deny" after the user configuration spread completely overwrites any OpenCode SDK-compatible user settings for task.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/plugin-handlers/tool-config-handler.ts, line 121:

<comment>Placing `task: "deny"` after the user configuration spread completely overwrites any OpenCode SDK-compatible user settings for `task`.</comment>

<file context>
@@ -116,9 +116,9 @@ export function applyToolConfig(params: {
-    ...(params.config.permission as Record<string, unknown>),
     webfetch: "allow",
     external_directory: "allow",
+    ...(params.config.permission as Record<string, unknown>),
     task: "deny",
   };
</file context>
Suggested change
...(params.config.permission as Record<string, unknown>),
task: "deny",
task: "deny",
...(params.config.permission as Record<string, unknown>),
Fix with Cubic

@code-yeongyu code-yeongyu merged commit 8b7b1c8 into dev Mar 24, 2026
8 checks passed
@code-yeongyu code-yeongyu deleted the fix/b5-permission-merge-order branch March 24, 2026 15:31
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