Open
Conversation
- Add Workspace tab to PermissionsDialog with full directory management UI
- Directory list view: initial (non-removable) dirs shown inline,
runtime-added dirs selectable; "Add directory…" always first
- Add directory input view: filesystem autocomplete with ↑/↓ navigation
and Tab-to-complete; path validation (existence, type, duplicate,
subdirectory checks)
- Remove directory confirmation view
- Save directly to project settings (SettingScope.Workspace), no scope
selection step
- Add onTab/onUp/onDown props to TextInput to intercept keys before buffer
- Add removeDirectory() and isInitialDirectory() to WorkspaceContext
- Add --add-dir CLI alias for --include-directories
- Add /add-dir slash command (alias for /directory add)
- Add permissions.additionalDirectories settings field
- Add i18n keys for all workspace directory UI strings (en/zh/de/ja/pt/ru)"
Shell commands that are semantically equivalent to file/network tool operations are now analyzed and matched against Read/Edit/Write/ WebFetch/ListFiles permission rules, preventing agents from bypassing configured rules via the run_shell_command tool. New file: packages/core/src/permissions/shell-semantics.ts - extractShellOperations(cmd, cwd) => ShellOperation[] - Covers 50+ commands: cat/head/tail/diff/grep/rg/ls/find/tree, touch/mkdir/cp/mv/rm/chmod/chown/sed/awk/dd/curl/wget + redirects - Handles transparent prefixes: sudo (-u/-g flag values), env, timeout (skips DURATION), nohup, nice, time, etc. - Tokenizer respects single/double quotes and backslash escapes - Redirect extraction: >, >>, <, 2>, &> Changes: packages/core/src/permissions/permission-manager.ts - DECISION_PRIORITY constant for combining decisions - evaluateSingle(): after base Bash-rule decision, evaluate virtual ops from shell semantics and return the most restrictive result - evaluateShellVirtualOps(): evaluate ShellOperation list via evaluateSingle - hasRelevantRules(): also check virtual ops so confirmation dialog appears when Read/Edit/etc. rules match equivalent shell commands Changes: packages/core/src/permissions/index.ts - Export extractShellOperations and ShellOperation Tests: packages/core/src/permissions/shell-semantics.test.ts - 52 unit tests: read/list/write/edit/web_fetch ops, redirections, prefix commands (sudo -u, timeout DURATION), quotes, variable filtering
LaZzyMan
requested review from
DennisYu07,
DragonnZhang,
Mingholy,
gwinthis,
pomelo-nwu and
tanzhenxin
as code owners
Contributor
Code Review: Structured Permission System (PR #2283)📋 Review SummaryThis PR introduces a comprehensive, rule-based permission system that replaces the coarse-grained 🔍 General FeedbackPositive aspects:
Architectural decisions:
Recurring patterns:
🎯 Specific Feedback🔴 Critical
🟡 High
🟢 Medium
🔵 Low
✅ Highlights
|
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
This was referenced Mar 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Introduces a structured, rule-based permission system for Qwen Code. Instead of a simple allow/deny list of tool names, users can now write fine-grained rules like
Bash(git *),Read(./secrets/**), orWebFetch(domain:example.com)to control exactly what the agent is allowed to do. Rules persist tosettings.jsonand can be managed interactively via a new/permissionsdialog. This PR also closes a critical shell bypass gap where a shell command likecat /etc/passwdcould silently bypass aRead(/etc/passwd)deny rule.Key reviewable areas:
packages/core/src/permissions/— the entire permission enginepackages/core/src/utils/shellAstParser.ts+shell-semantics.ts— shell bypass preventionpackages/core/src/core/coreToolScheduler.ts— L3→L4→L5 permission flow integrationpackages/cli/src/ui/components/PermissionsDialog.tsx— the new/permissionsUIDive Deeper
Motivation
The previous permission model had two problems:
allowedTools/excludeToolscould only toggle entire tools on or off. There was no way to say "allowgitbut notrm -rf", or "allow reads inside the project but not/etc".Read,Edit, orWebFetchcould be trivially circumvented by running the equivalent shell command (cat,cp,curl) because the shell tool had no awareness of those rules.Architecture: The Permission Engine (
packages/core/src/permissions/)The new system lives in four files:
types.ts— Core VocabularyA
PermissionRuleis a parsed representation of a rule string:rule-parser.ts— Rule Parsing & MatchingBash→run_shell_command,Edit(meta-category covers botheditandwrite_file),Read(coversread_file,grep_search,glob,list_directory), etc. Full Claude Code compatibility.git add . && git commitis split into["git add .", "git commit"]and each sub-command is evaluated independently. The most restrictive result wins.command: glob matching via picomatch against the shell command stringpath: gitignore-style matching (picomatch) with project root resolutiondomain: hostname equality / subdomain matchingliteral: simple string equalitypermission-manager.ts— Evaluation EngineEvaluation priority (highest wins):
Two rule stores:
persistentRules(fromsettings.json) andsessionRules(in-memory, cleared on restart).Key methods:
evaluate(ctx)— returnsPermissionDecisionfor an invocation contexthasRelevantRules(ctx)— fast path: skips the PM entirely if no rule matches, preserving the tool's owngetDefaultPermission()resultisToolEnabled(toolName)— registry-level: returns false for whole-tool deny rules (e.g."Bash"with no specifier removes the tool from the registry entirely)addSessionAllowRule()/addPersistentRule()— rule managementlistRules()— returns all active rules with type and scope for the UIshell-semantics.ts— Shell Bypass PreventionThe most novel piece. When a shell command is being evaluated,
extractShellOperations()parses it and extracts virtual tool operations:These virtual operations are then evaluated against the existing
Read/Edit/Write/WebFetchrules. Adeny: ["Read(/etc/*)"]rule therefore also blockscat /etc/passwdvia the shell.Known limitations (documented in source): variable expansion
$FILE, command substitution$(...), interpreter scriptspython x.py, pipe targetsfind . | xargs cat.The L3→L4→L5 Permission Flow (
coreToolScheduler.ts)Before this PR,
coreToolSchedulercalledshouldConfirmExecute()on the invocation and made a binary "confirm vs. skip" decision. That API is replaced with a three-layer stack:'allow'; mutating tools return'ask'. The shell tool additionally uses the AST parser to determine if the given command is read-only.pm.hasRelevantRules(ctx)returns true (avoids overhead for unconfigured tools). When it fires it can upgrade or downgrade L3's result.'deny'→ immediate error (no execution, no prompt)'allow'→ auto-approve'ask'→ show confirmation dialog'ask'for all tools exceptask_user_questionexit_plan_modeandask_user_question"Always Allow" Persistence
When the user clicks Always Allow (Project) or Always Allow (User), the confirmation payload carries
permissionRules: string[]— a minimal set of rules pre-computed by the tool (e.g.["Bash(git status)"]). The scheduler reads these and writes them to the appropriatesettings.jsonscope viaPermissionManager.addPersistentRule(). "Always allow" choices are remembered across sessions without manual configuration.Tool API Refactor (
tools.ts)shouldConfirmExecute()(old, combined "should ask?" + "what to show?") is replaced by two explicit methods onBaseToolInvocation:The default
BaseToolInvocationimplementation returns'allow'forgetDefaultPermission()and a generic'info'dialog forgetConfirmationDetails(). Tools with specific confirmation UIs (Shell, Edit, MCP, Plan, AskUserQuestion) override both.Settings Schema
New
permissionstop-level key insettings.json:{ "permissions": { "allow": ["Bash(git *)", "Read(./src/**)"], "ask": ["WebFetch"], "deny": ["Bash(rm -rf *)"] } }Legacy
tools.allowed/tools.exclude/tools.coresettings are automatically migrated topermissions.*on first load viamigrateLegacyPermissions()./permissionsDialog (PermissionsDialog.tsx)A full-featured TUI dialog accessible via the
/permissionscommand. Features:Shell AST Parser (
shellAstParser.ts)Uses web-tree-sitter + tree-sitter-bash (WASM, bundled ~1.5 MB) to parse shell commands into an AST. Used to:
isShellCommandReadOnlyAST()) — enables the shell tool to return'allow'forgit log,cat README.md, etc. without prompting.extractCommandRules()) — when the user clicks "Always Allow", generates tight rules like"Bash(git status)"rather than a blanket"Bash".Reviewer Test Plan
Pull the branch, run
npm run buildthennpm start.1. Rule-based auto-approval
Add to your project's
.qwen/settings.json:{ "permissions": { "allow": ["Bash(git log *)", "Read(./README.md)"] } }Ask the agent to run
git log --oneline -5andcat README.md. Both should execute without prompting.2. Rule-based deny
{ "permissions": { "deny": ["Bash(rm *)"] } }Ask the agent to delete any file. It should get an immediate error, no confirmation dialog.
3. Shell bypass prevention
{ "permissions": { "deny": ["Read(/etc/*)"] } }Ask the agent to run
cat /etc/hostsvia the shell tool. It should be blocked (previously this would slip through).4. Always Allow persistence
In DEFAULT mode, approve a tool call and click "Always Allow (Project)". Restart the session and run the same tool — it should auto-approve. Check that a new rule appears in
.qwen/settings.json.5. /permissions dialog
Type
/permissionsin the chat input. The dialog should open showing current rules. Add a new rule, confirm it appears. Remove it, confirm it disappears.6. ask_user_question in YOLO mode
Set
approvalMode: "yolo"and ask the agent to ask you a question (e.g., "Ask me what color I prefer using ask_user_question"). The confirmation dialog should still appear — YOLO must not skipask_user_question.Testing Matrix
Linked issues / bugs
Related to workspace trust and tool permission discussions.