Skip to content

fix(core): confirm command substitutions consistently - #4523

Closed
Jerry2003826 wants to merge 1 commit into
QwenLM:mainfrom
Jerry2003826:Jiarui/fix-command-substitution-permission
Closed

fix(core): confirm command substitutions consistently#4523
Jerry2003826 wants to merge 1 commit into
QwenLM:mainfrom
Jerry2003826:Jiarui/fix-command-substitution-permission

Conversation

@Jerry2003826

Copy link
Copy Markdown
Contributor

Fixes #4093

Summary

  • Make PermissionManager resolve command substitution defaults to ask instead of hard-denying only when relevant permission rules exist.
  • Surface Contains command_substitution in shell execution confirmation details and render it in the CLI confirmation prompt.
  • Add regression coverage for the compound-command permission path, shell confirmation details, and CLI warning rendering.

Root Cause

Command substitution handling was split across permission layers. ShellToolInvocation's default permission path treated command substitutions as normal non-read-only commands (ask), but PermissionManager's shell default fallback returned deny when it was invoked for unmatched sub-commands. That made behavior depend on whether existing permission rules were considered relevant.

Validation

  • npm run test --workspace=@qwen-code/qwen-code-core -- src/permissions/permission-manager.test.ts
  • npm run test --workspace=@qwen-code/qwen-code-core -- src/tools/shell.test.ts
  • npm run test --workspace=packages/cli -- src/ui/components/messages/ToolConfirmationMessage.test.tsx
  • npm run lint --workspace=@qwen-code/qwen-code-core
  • npm run lint --workspace=packages/cli
  • npm run typecheck --workspace=@qwen-code/qwen-code-core
  • npm run typecheck --workspace=packages/cli
  • npx prettier --check packages/core/src/permissions/permission-manager.ts packages/core/src/tools/shell.ts packages/core/src/tools/tools.ts packages/core/src/confirmation-bus/types.ts packages/core/src/permissions/permission-manager.test.ts packages/core/src/tools/shell.test.ts packages/cli/src/ui/components/messages/ToolConfirmationMessage.tsx packages/cli/src/ui/components/messages/ToolConfirmationMessage.test.tsx

@Jerry2003826
Jerry2003826 marked this pull request as ready for review May 25, 2026 23:50
@Jerry2003826

Copy link
Copy Markdown
Contributor Author

I noticed that #4386 is an earlier open PR for the same #4093 issue and already has maintainer review/E2E coverage. Closing this duplicate to avoid adding noise. Thanks!

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.

Bug: Command substitution denial is inconsistently applied and opaque

1 participant