[li-xiu-qi]

Resolves #239

Problem

To prevent third-party MCP servers and dynamic plugins from bypassing dangerous-command inspection by wrapping shell commands in custom tool names, the Step permission policy generically inspects tool inputs for fields named command, cmd, or script and runs command-policy analysis on them.

Because the permission decision entry point (decideStepToolCall) currently receives only a bare toolName: string and lacks origin metadata distinguishing trusted core built-in tools from untrusted third-party extensions, this defensive heuristic applies indiscriminately across all tools.

That heuristic is wrong for Step's built-in workflow tool: its script field holds JavaScript executed inside a QuickJS memory sandbox with no access to an operating-system shell, child processes, or network. Feeding multi-line JavaScript into the Bash AST parser causes the parser to report the code as unanalyzable (unresolved), which pins the workflow behind a forced confirmation under Approve for Me. Furthermore, a script that merely constructs a string mentioning a hazardous pattern (such as `rm -rf ./tmp/cache` inside a template literal) gets flagged as a dangerous command, even though no shell exists in the sandbox. These false positives interrupt unattended runs and degrade routine workflow automation.

Changes

• Introduce NON_SHELL_SCRIPT_TOOLS, initially containing workflow, to identify tools whose script field carries program source rather than a shell command.
• Skip command-policy analysis specifically for that field. The exemption is field-scoped rather than tool-scoped: command and cmd remain fully analyzable so that any future shell-invoking argument on the tool will still be inspected.
• Avoid security regression: rather than restricting command inspection to a hardcoded whitelist (like bash or powershell), which would leave custom MCP tools unmonitored, the global heuristic remains active for all other tools and arguments.
• Match normalized tool names exactly to prevent accidental substring bypasses.
• Preserve explicit per-tool allow/deny overrides and then_run chained-command handling without modification.

Behavior and compatibility

decideStepToolCall is exported from the package entry point. Its signature and return shape are unchanged; only the decision for one specific input shape changes.

Scenario	Before	After
Inline workflow under Ask	Confirm, sometimes with incomplete shell analysis	Confirm through the ordinary tool policy
Inline workflow under Approve for Me	Could confirm as unanalyzable or hazardous	Allow without shell analysis
Inline workflow under Full Access	Allow	Unchanged
Saved-name / scriptPath workflow	Existing behavior	Unchanged
Explicit workflow: "deny"	Deny	Unchanged
Real shell tools and other script fields	Existing analysis	Unchanged

No migration or escape path is needed: no persisted data, prompt, default, or accepted input changes. Consumers that call decideStepToolCall directly and relied on workflow.script being read as a shell command will see a different decision, but that reading was the defect. Adding another tool whose script is non-shell source requires listing it in NON_SHELL_SCRIPT_TOOLS explicitly; this PR deliberately does not infer language or safety from the field name alone.

Agents started by a workflow still run their own tool-call permission checks. This change only stops treating the outer JavaScript source as a shell command.

Validation

Platform: Windows 11, Node.js 24.21.0, pnpm@9.15.9. All commands run from packages/coding-agent.

Permission and command-policy suites, all 318 passed:

pnpm exec vitest --run test/step-permissions.test.ts test/step-command-policy.test.ts test/step-command-policy-case.test.ts test/step-command-policy-uncertainty.test.ts test/step-command-policy-wrappers.test.ts

Workflow suites, all 69 passed:

pnpm exec vitest --run test/workflow-extension.test.ts test/workflow-registration.test.ts test/workflow-script-lint.test.ts test/workflow-ultraloop-opt-in.test.ts

Static checks, all passed:

pnpm exec tsgo --noEmit
pnpm exec biome check --error-on-warnings packages/coding-agent/src/step/permissions.ts packages/coding-agent/test/step-permissions.test.ts
node scripts/check-public-boundary.mjs
node scripts/check-coding-agent-entry-freeze.mjs

Upstream CI on head 41c29b33: build-check-test and Check commit attribution both completed with conclusion: success.

Defect probes, run against test/step-permissions.test.ts to confirm the new tests actually bite:

• Removing the script exemption: 7 failed, 53 passed (60).
• Restoring the older whole-tool exemption: 2 failed, 58 passed (60), both boundary tests, keeps workflow.command analyzable and keeps workflow.cmd analyzable.
• Restoring the final implementation: all 60 passed.

Intentional skip: the main describe block in test/step-command-policy-shell-semantics.test.ts is skipIf(process.platform === "win32"), so 171 cases are skipped on Windows and were not executed locally.

Limitations

• NON_SHELL_SCRIPT_TOOLS is a targeted point fix for workflow.script. Because decideStepToolCall currently operates only on tool names without origin metadata (isInternal / isBuiltin vs third-party MCP), it cannot separate trust domains natively. The systemic long-term solution is to introduce tool-origin metadata or capability manifests separating built-in tools from untrusted extensions, which requires cross-package protocol updates and will be tracked in a follow-up proposal issue.
• Verification is local on Windows 11. The shell-semantics suite is skipped on this platform, so those 171 cases still need coverage from CI or another OS.
• test/workflow-runtime.test.ts has one pre-existing Windows failure, a transient progress write failure does not poison later writes at line 532: the assertion expects EISDIR or ENOTDIR, while Windows rename raises EPERM. The same failure reproduces on this PR's parent commit, so it is unrelated to this change and will be tracked in a separate issue.

[li-xiu-qi]

Closing this PR in favor of the systemic architectural proposal in #327.

While the NON_SHELL_SCRIPT_TOOLS whitelist introduced here successfully stops false positives for workflow.script, our retrospective shows that it remains an inherently static point fix. Hardcoding specific extension names into the core permission controller re-couples what the microkernel architecture intended to decouple.

To maintain the long-term cleanliness and extensibility of the codebase, we believe this should be resolved at the provenance level—by distinguishing tool origins (builtin vs mcp) within ToolDefinition and decideStepToolCall as proposed in #327, rather than accumulating name-based exemptions.