Skip to content

fix(cli): enforce permissions on shell commands the parser fails to scan - #12585

Merged
marius-kilocode merged 2 commits into
mainfrom
fix-pwsh-permission-scan
Jul 28, 2026
Merged

fix(cli): enforce permissions on shell commands the parser fails to scan#12585
marius-kilocode merged 2 commits into
mainfrom
fix-pwsh-permission-scan

Conversation

@marius-kilocode

Copy link
Copy Markdown
Collaborator

Fixes #12326.

On Windows, where pwsh/powershell is the default shell, bash permission rules were silently skipped for any command containing a bare -- argument, for example git checkout -- <file>. The tree-sitter-powershell grammar parses such commands into an ERROR node instead of a command node, so the permission scanner collected zero patterns and the shell tool executed the command without evaluating a single rule. This bypassed every bash rule, including "git *": "deny" and even a blanket "*": "deny", which is what the reporter hit when an agent reverted a file with git despite a configured deny rule. The same grammar path is inherited from upstream opencode's PowerShell support, so upstream is likely affected as well.

The scanner now fails closed instead of skipping the check:

  • Command text that the grammar drops into ERROR nodes is recovered and evaluated as a permission pattern, so git checkout -- file matches "git *": "deny" again, including when it appears chained before a ; separator.
  • When a non-empty command produces no command nodes at all, the raw command text is checked as the pattern and falls back to ask when no rule matches, so nothing executes unevaluated.
  • Clean parses short-circuit before the fallback, so behavior and prompts are unchanged for every command that parses today, on any shell or platform.

The recovery lives in a Kilo mirror module with a single marked hook in the shared shell scanner, keeping the upstream diff minimal. The interactive terminal shares this scanner and is covered by the same gate. Regression coverage runs on every CI OS since the scanner tests drive the PowerShell grammar without needing a pwsh binary, plus a Windows-runner-only test that executes the full shell tool through real PowerShell.

Comment thread packages/opencode/src/kilocode/tool/shell-unparsed.ts Outdated
@kilo-code-bot

kilo-code-bot Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Code Review Summary

Status: No Issues Found | Recommendation: Merge

The previously flagged WARNING (fail-closed fallback skipped when a command already parsed but an ERROR node lacked a command_name descendant) is fixed in the latest commit: packages/opencode/src/kilocode/tool/shell-unparsed.ts now falls through to the raw-text fallback whenever nothing is recovered from ERROR nodes, regardless of whether other commands parsed cleanly. New tests (ERROR node without command_name falls back to the raw check, partially parsed pipelines still fail closed) directly cover the previously-reported gap.

Files Reviewed (5 files)
  • .changeset/pwsh-permission-fail-closed.md
  • packages/opencode/src/kilocode/tool/shell-unparsed.ts
  • packages/opencode/src/tool/shell.ts
  • packages/opencode/test/kilocode/tool/shell-unparsed.test.ts
  • packages/opencode/test/tool/shell.test.ts
Previous Review Summary (commit 342cba4)

Current summary above is authoritative. Previous snapshots are kept for context only.

Previous review (commit 342cba4)

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
CRITICAL 0
WARNING 1
SUGGESTION 0
Issue Details (click to expand)

WARNING

File Line Issue
packages/opencode/src/kilocode/tool/shell-unparsed.ts 17 Fail-closed fallback is skipped when a valid command already parsed but an ERROR node lacks a command_name descendant, which could reopen a narrower version of the fail-open gap this PR fixes

Otherwise the change is well-scoped: the Kilo-specific recovery logic lives entirely in src/kilocode/tool/shell-unparsed.ts, the shared src/tool/shell.ts hook is a minimal, clearly-marked kilocode_change block, the changeset is written from a user-facing perspective, and the new tests directly exercise the reported bypass (bare -- under pwsh/cmd, chained commands, and unchanged behavior for clean parses).

Files Reviewed (5 files)
  • .changeset/pwsh-permission-fail-closed.md
  • packages/opencode/src/kilocode/tool/shell-unparsed.ts - 1 issue
  • packages/opencode/src/tool/shell.ts
  • packages/opencode/test/kilocode/tool/shell-unparsed.test.ts
  • packages/opencode/test/tool/shell.test.ts

Fix these issues in Kilo Cloud


Reviewed by claude-sonnet-5 · Input: 26 · Output: 7.1K · Cached: 642.1K

Review guidance: REVIEW.md from base branch main

@marius-kilocode
marius-kilocode enabled auto-merge (squash) July 28, 2026 07:30
@marius-kilocode
marius-kilocode merged commit a0a760e into main Jul 28, 2026
30 checks passed
@marius-kilocode
marius-kilocode deleted the fix-pwsh-permission-scan branch July 28, 2026 07:50
t7tran pushed a commit to t7tran/kilocode that referenced this pull request Aug 14, 2026
…can (Kilo-Org#12585)

* fix(cli): enforce permissions on shell commands the parser fails to scan

* fix(cli): fail closed on error chunks without command names, move pwsh execution test to kilo file
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.

Security Settings are being ignored.

2 participants