feat: integrate destructive command guard with auto-approval - #1050
Draft
navedmerchant wants to merge 9 commits into
Draft
feat: integrate destructive command guard with auto-approval#1050navedmerchant wants to merge 9 commits into
navedmerchant wants to merge 9 commits into
Conversation
navedmerchant
requested review from
JamesRobert20,
edelauna,
hannesrudolph and
taltas
as code owners
July 29, 2026 06:17
Contributor
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughAdds Destructive Command Guard support for command auto-approval, including persisted settings, secure binary installation, guarded execution, approval-state recording, denied-command rendering, tests, and localized UI text. ChangesDestructive Command Guard integration
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant SettingsView
participant webviewMessageHandler
participant DCGManager
participant ExecuteCommandTool
participant Task
participant ChatRow
SettingsView->>webviewMessageHandler: updateSettings(destructiveCommandGuardEnabled)
webviewMessageHandler->>DCGManager: ensureDcgInstalled(storageDir)
ExecuteCommandTool->>DCGManager: runDcg(binaryPath, command, cwd)
DCGManager-->>ExecuteCommandTool: allow or deny decision
ExecuteCommandTool->>Task: request approval with protection state
Task-->>ChatRow: autoApprovalDecision
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (8)
src/core/webview/ClineProvider.ts (1)
2443-2443: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReuse a shared DCG default constant.
Both state builders hard-code
false. Define one shared default alongside the setting schema and use it ingetState()andgetStateToPostToWebview()to keep default semantics consistent.Also applies to: 2676-2676
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/webview/ClineProvider.ts` at line 2443, Define a shared default constant for destructive command guard behavior alongside the setting schema, then replace the hard-coded false fallbacks in both getState() and getStateToPostToWebview() with that constant. Preserve the existing nullish-coalescing behavior.Source: Coding guidelines
packages/types/src/message.ts (1)
275-275: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd focused schema tests for
autoApprovalDecision.Cover both accepted values (
"approve"/"deny") and rejection of invalid values inpackages/types/src/__tests__/message.test.ts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/types/src/message.ts` at line 275, Add focused tests in the message schema test suite for autoApprovalDecision, asserting that "approve" and "deny" are accepted while invalid values are rejected. Reuse the existing message schema test patterns and reference the autoApprovalDecision field directly.Source: Coding guidelines
webview-ui/src/components/chat/CommandExecution.tsx (1)
39-45: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueNew
isDeniedprop shadows the localisDeniedinhandleDenyPatternChange(Line 112).Harmless today, but the inner
const isDenied = deniedCommands.includes(pattern)now hides the prop of the same name. Renaming the local (e.g.isPatternDenied) removes the ambiguity.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/chat/CommandExecution.tsx` around lines 39 - 45, Rename the local isDenied variable inside handleDenyPatternChange to a distinct name such as isPatternDenied, and update its references there while preserving the isDenied prop used by CommandExecution.src/core/auto-approval/__tests__/dcg.spec.ts (1)
40-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering
alwaysAllowExecute: falsewith DCG enabled.The suite never asserts that DCG alone cannot auto-approve when the execute toggle is off — the most important negative boundary of this new branch.
♻️ Suggested additional case
it("does not auto-approve via DCG when execute auto-approval is off", async () => { const state = { ...baseState, alwaysAllowExecute: false } expect(await checkAutoApproval({ state, ask: "command", text: "echo safe" })).toEqual({ decision: "ask" }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/auto-approval/__tests__/dcg.spec.ts` around lines 40 - 62, Add a test alongside the existing checkAutoApproval cases that keeps destructiveCommandGuardEnabled enabled while setting baseState.alwaysAllowExecute to false, then verify a safe command returns decision "ask" rather than being auto-approved via DCG.src/core/tools/__tests__/executeCommandTool.spec.ts (1)
210-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd the allow-path and unavailable-binary cases.
Currently only the deny branch is exercised. Two cheap additions with real value: DCG enabled +
decision: "allow"should callaskApproval("command", "echo test")with no protection flag, andgetDcgBinaryPathreturningundefinedshould surface the enablement error viahandleErrorrather than silently executing.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/tools/__tests__/executeCommandTool.spec.ts` around lines 210 - 248, Extend the executeCommandTool tests around the existing DCG deny-case test to cover an enabled guard returning decision "allow", asserting askApproval is called with "command", "echo test", and no protection flag. Add an unavailable-binary case by making getDcgBinaryPath return undefined, and assert handleError receives the DCG enablement error without executing the command.src/services/destructive-command-guard/constants.ts (1)
9-40: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deep-freezing archive metadata.
Readonly<Record<string, DcgArchiveInfo>>only prevents reassigning top-level keys; nested fields likesha256on eachDcgArchiveInforemain mutable at runtime. Since these values pin the trusted checksum used for supply-chain verification, marking the literalas const(and typing accordingly) would harden against accidental mutation elsewhere in the codebase.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/destructive-command-guard/constants.ts` around lines 9 - 40, Update DCG_ARCHIVES to use a deeply immutable literal by applying `as const` and adjusting its type so each archive entry’s `archive`, `binary`, and `sha256` fields cannot be reassigned. Preserve the existing platform entries and checksum values while ensuring nested metadata is read-only at compile time.src/services/destructive-command-guard/manager.ts (1)
51-160: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd unit test coverage for the checksum/download/install security path.
manager.spec.tsonly exercises the pure platform-mapping helpers (getDcgArchiveInfo,isDcgSupportedPlatform,getDcgBinaryPath).downloadFile's trusted-domain/redirect/size enforcement,verifyChecksum,extractSingleBinary's layout validation, andinstallDcg's atomic swap are the actual security guarantees of this module and remain untested. These are mockable viahttps/fs/child_processmodule mocks.Based on learnings, "Prefer the narrowest test layer that proves behavior: focused tests first, integration tests for cross-module contracts, and end-to-end tests only for full workflow confidence."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/destructive-command-guard/manager.ts` around lines 51 - 160, Add focused unit tests in manager.spec.ts for downloadFile’s trusted-host checks, redirect limits, and download-size enforcement; verifyChecksum’s success and mismatch behavior; extractSingleBinary’s single-entry layout validation; and installDcg’s atomic swap behavior. Mock https, fs streams, and child_process.runProcess dependencies to exercise these paths without real network, archive, or process operations, while preserving the existing platform-helper tests.Source: Learnings
src/services/destructive-command-guard/runner.ts (1)
16-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftNo test coverage for
runDcg's branch logic.This function gates command auto-approval and has several distinct failure branches (timeout, output-size overflow, invalid JSON, unsupported schema version, decision/exit-code mismatch) that aren't covered by any test in this cohort (
manager.spec.tsonly tests unrelated helpers). Given this is on the safety-critical approval path, focused unit tests mockingchild_process.spawnwould materially increase confidence.Based on learnings, "For regressions, add the test at the lowest layer that would have failed; add an e2e test only when lower-level tests cannot represent the failure mode."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/services/destructive-command-guard/runner.ts` around lines 16 - 84, Add focused unit tests for runDcg by mocking child_process.spawn and simulating its stdout, stderr, close, and error events. Cover successful allow/deny decisions plus timeout, output-size overflow, invalid JSON, unsupported schema versions, decision/exit-code mismatches, and nonzero termination failures, asserting the resulting resolution or rejection messages.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/tools/ExecuteCommandTool.ts`:
- Around line 128-144: Replace the hardcoded missing-binary error in the
destructive-command guard block of ExecuteCommandTool with the localized
common.errors.destructiveCommandGuard.unavailable message. Add the unavailable
key to that error template in common.json for every locale, preserving the
existing destructive_command_guard_enable_failed template and using the
established localization lookup pattern.
In `@src/services/destructive-command-guard/manager.ts`:
- Around line 162-212: Prevent cross-process installation races in installDcg:
before the destructive fs.rm(finalDir) and fs.rename(stagingDir, finalDir)
replacement, re-check whether the verified binaryPath already exists and return
it while cleaning up staging, or implement an equivalent lightweight lock under
installRoot. Preserve checksum, extraction, and version verification, and ensure
concurrent windows never remove an already valid installation.
- Around line 145-160: Update extractSingleBinary to use a Windows-safe native,
PowerShell, or Node-based ZIP extraction path for .zip archives instead of
invoking runProcess("tar", ...). Preserve the existing tar listing and
extraction behavior for non-ZIP archives, and continue validating that the
archive contains only info.binary before extraction.
In `@webview-ui/src/components/chat/ChatRow.tsx`:
- Around line 294-301: Add stable React keys to both JSX elements returned by
the autoApprovalDecision "deny" branch in the surrounding message-rendering
function, resolving the react/jsx-key lint errors without changing the
denied-state content or styling.
In `@webview-ui/src/i18n/locales/de/settings.json`:
- Around line 331-334: Update the description in the destructiveCommandGuard
localization entry to replace “Zoos Befehlslisten” with the clearer German
wording “Die Befehlslisten von Zoo sind währenddessen deaktiviert.”
In `@webview-ui/src/i18n/locales/fr/settings.json`:
- Around line 332-335: Update the description in the destructiveCommandGuard
translation to replace the informal French pronouns “tu” and “ton” with the
formal “vous” and “votre”, preserving the existing meaning and wording
otherwise.
---
Nitpick comments:
In `@packages/types/src/message.ts`:
- Line 275: Add focused tests in the message schema test suite for
autoApprovalDecision, asserting that "approve" and "deny" are accepted while
invalid values are rejected. Reuse the existing message schema test patterns and
reference the autoApprovalDecision field directly.
In `@src/core/auto-approval/__tests__/dcg.spec.ts`:
- Around line 40-62: Add a test alongside the existing checkAutoApproval cases
that keeps destructiveCommandGuardEnabled enabled while setting
baseState.alwaysAllowExecute to false, then verify a safe command returns
decision "ask" rather than being auto-approved via DCG.
In `@src/core/tools/__tests__/executeCommandTool.spec.ts`:
- Around line 210-248: Extend the executeCommandTool tests around the existing
DCG deny-case test to cover an enabled guard returning decision "allow",
asserting askApproval is called with "command", "echo test", and no protection
flag. Add an unavailable-binary case by making getDcgBinaryPath return
undefined, and assert handleError receives the DCG enablement error without
executing the command.
In `@src/core/webview/ClineProvider.ts`:
- Line 2443: Define a shared default constant for destructive command guard
behavior alongside the setting schema, then replace the hard-coded false
fallbacks in both getState() and getStateToPostToWebview() with that constant.
Preserve the existing nullish-coalescing behavior.
In `@src/services/destructive-command-guard/constants.ts`:
- Around line 9-40: Update DCG_ARCHIVES to use a deeply immutable literal by
applying `as const` and adjusting its type so each archive entry’s `archive`,
`binary`, and `sha256` fields cannot be reassigned. Preserve the existing
platform entries and checksum values while ensuring nested metadata is read-only
at compile time.
In `@src/services/destructive-command-guard/manager.ts`:
- Around line 51-160: Add focused unit tests in manager.spec.ts for
downloadFile’s trusted-host checks, redirect limits, and download-size
enforcement; verifyChecksum’s success and mismatch behavior;
extractSingleBinary’s single-entry layout validation; and installDcg’s atomic
swap behavior. Mock https, fs streams, and child_process.runProcess dependencies
to exercise these paths without real network, archive, or process operations,
while preserving the existing platform-helper tests.
In `@src/services/destructive-command-guard/runner.ts`:
- Around line 16-84: Add focused unit tests for runDcg by mocking
child_process.spawn and simulating its stdout, stderr, close, and error events.
Cover successful allow/deny decisions plus timeout, output-size overflow,
invalid JSON, unsupported schema versions, decision/exit-code mismatches, and
nonzero termination failures, asserting the resulting resolution or rejection
messages.
In `@webview-ui/src/components/chat/CommandExecution.tsx`:
- Around line 39-45: Rename the local isDenied variable inside
handleDenyPatternChange to a distinct name such as isPatternDenied, and update
its references there while preserving the isDenied prop used by
CommandExecution.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7668a3bc-553d-4f41-8d64-ff82c2563941
📒 Files selected for processing (96)
AGENTS.mdpackages/types/src/global-settings.tspackages/types/src/message.tspackages/types/src/vscode-extension-host.tssrc/core/auto-approval/__tests__/dcg.spec.tssrc/core/auto-approval/index.tssrc/core/task/Task.tssrc/core/tools/ExecuteCommandTool.tssrc/core/tools/__tests__/executeCommandTool.spec.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/webviewMessageHandler.tssrc/i18n/locales/ca/common.jsonsrc/i18n/locales/ca/tools.jsonsrc/i18n/locales/de/common.jsonsrc/i18n/locales/de/tools.jsonsrc/i18n/locales/en/common.jsonsrc/i18n/locales/en/tools.jsonsrc/i18n/locales/es/common.jsonsrc/i18n/locales/es/tools.jsonsrc/i18n/locales/fr/common.jsonsrc/i18n/locales/fr/tools.jsonsrc/i18n/locales/hi/common.jsonsrc/i18n/locales/hi/tools.jsonsrc/i18n/locales/id/common.jsonsrc/i18n/locales/id/tools.jsonsrc/i18n/locales/it/common.jsonsrc/i18n/locales/it/tools.jsonsrc/i18n/locales/ja/common.jsonsrc/i18n/locales/ja/tools.jsonsrc/i18n/locales/ko/common.jsonsrc/i18n/locales/ko/tools.jsonsrc/i18n/locales/nl/common.jsonsrc/i18n/locales/nl/tools.jsonsrc/i18n/locales/pl/common.jsonsrc/i18n/locales/pl/tools.jsonsrc/i18n/locales/pt-BR/common.jsonsrc/i18n/locales/pt-BR/tools.jsonsrc/i18n/locales/ru/common.jsonsrc/i18n/locales/ru/tools.jsonsrc/i18n/locales/tr/common.jsonsrc/i18n/locales/tr/tools.jsonsrc/i18n/locales/vi/common.jsonsrc/i18n/locales/vi/tools.jsonsrc/i18n/locales/zh-CN/common.jsonsrc/i18n/locales/zh-CN/tools.jsonsrc/i18n/locales/zh-TW/common.jsonsrc/i18n/locales/zh-TW/tools.jsonsrc/services/destructive-command-guard/__tests__/manager.spec.tssrc/services/destructive-command-guard/constants.tssrc/services/destructive-command-guard/index.tssrc/services/destructive-command-guard/manager.tssrc/services/destructive-command-guard/runner.tswebview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/chat/CommandExecution.tsxwebview-ui/src/components/chat/__tests__/ChatRow.command-denied.spec.tsxwebview-ui/src/components/chat/__tests__/CommandExecution.spec.tsxwebview-ui/src/components/settings/AutoApproveSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
Comment on lines
+128
to
+144
| if (providerState?.destructiveCommandGuardEnabled === true) { | ||
| const { getDcgBinaryPath, runDcg } = await import("../../services/destructive-command-guard") | ||
| const binaryPath = provider ? getDcgBinaryPath(provider.context.globalStorageUri.fsPath) : undefined | ||
| if (!binaryPath) { | ||
| throw new Error("Destructive Command Guard is enabled but is not available for this platform") | ||
| } | ||
| const workingDirectory = customCwd | ||
| ? path.isAbsolute(customCwd) | ||
| ? customCwd | ||
| : path.resolve(task.cwd, customCwd) | ||
| : task.cwd | ||
| const dcgResult = await runDcg(binaryPath, canonicalCommand, workingDirectory) | ||
| dcgBlocked = dcgResult.decision === "deny" | ||
| if (dcgResult.decision === "deny") { | ||
| await task.say("error", formatDcgBlockedMessage(dcgResult.reason, dcgResult.ruleId)) | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'destructiveCommandGuard' src/i18n/locales/en/common.json src/i18n/locales/en/tools.jsonRepository: Zoo-Code-Org/Zoo-Code
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== i18n keys =="
rg -n '"destructiveCommandGuard"|"errors":|unavailable|not available|Destructive Command Guard|Dcg|dcg' src/i18n/locales -g '*.json'
echo
echo "== ExecuteCommandTool outline =="
ast-grep outline src/core/tools/ExecuteCommandTool.ts --view expanded || true
echo
echo "== ExecuteCommandTool relevant lines =="
cat -n src/core/tools/ExecuteCommandTool.ts | sed -n '1,220p'
echo
echo "== t()/i18n usage in ExecuteCommandTool =="
rg -n '\b(t|tFunc|translate)\b|common:errors|tools:' src/core/tools/ExecuteCommandTool.tsRepository: Zoo-Code-Org/Zoo-Code
Length of output: 36373
Use an existing localized DCG setup error instead of a hardcoded tool error.
Line 132 throws a raw English message when the DCG binary path cannot be determined, so DCG enablement failures skip localization and common.json’s destructive_command_guard_enable_failed template. Add an unavailable key under common:errors.destructiveCommandGuard (all locales), then use that here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/tools/ExecuteCommandTool.ts` around lines 128 - 144, Replace the
hardcoded missing-binary error in the destructive-command guard block of
ExecuteCommandTool with the localized
common.errors.destructiveCommandGuard.unavailable message. Add the unavailable
key to that error template in common.json for every locale, preserving the
existing destructive_command_guard_enable_failed template and using the
established localization lookup pattern.
Comment on lines
+145
to
+160
| async function extractSingleBinary(archivePath: string, stagingDir: string, info: DcgArchiveInfo): Promise<void> { | ||
| const listingArgs = info.archive.endsWith(".zip") ? ["-tf", archivePath] : ["-tJf", archivePath] | ||
| const listing = await runProcess("tar", listingArgs) | ||
| const entries = listing.stdout | ||
| .split(/\r?\n/) | ||
| .map((entry) => entry.trim().replace(/^\.\//, "")) | ||
| .filter(Boolean) | ||
| if (entries.length !== 1 || entries[0] !== info.binary) { | ||
| throw new Error("DCG archive has an unexpected layout") | ||
| } | ||
|
|
||
| const extractArgs = info.archive.endsWith(".zip") | ||
| ? ["-xf", archivePath, "-C", stagingDir, info.binary] | ||
| : ["-xJf", archivePath, "-C", stagingDir, info.binary] | ||
| await runProcess("tar", extractArgs) | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does Windows 10/11 bundled tar.exe support extracting .zip archives, and since which build?
💡 Result:
Yes, the tar.exe utility bundled with Windows 10 and Windows 11 supports extracting and creating.zip archives [1]. Because the Windows implementation of tar.exe is based on libarchive's bsdtar, it supports various archive formats beyond traditional tarballs, including.zip and.7z [1]. This utility was first introduced in Windows 10, build 17063 (part of the version 1803 update) [2][3]. Since its inclusion, it has provided a native command-line method for handling these formats without requiring third-party software [1]. To extract a.zip file, you can use the standard tar extraction command: tar -xf archive.zip To create a.zip file, you can use: tar -caf archive.zip foldername The -a flag is particularly useful as it automatically selects the compression format based on the file extension provided [1].
Citations:
- 1: https://learn.microsoft.com/en-us/windows/tar/
- 2: https://ss64.com/nt/tar.html
- 3: https://renenyffenegger.ch/notes/Windows/dirs/Windows/System32/tar_exe
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate manager.ts =="
fd -a 'manager\.ts$' src/services/destructive-command-guard 2>/dev/null || true || true
echo "== file stats =="
wc -l src/services/destructive-command-guard/manager.ts
echo "== relevant section =="
sed -n '1,230p' src/services/destructive-command-guard/manager.ts
echo "== tar/windows extraction usage/search =="
rg -n 'extractSingleBinary|DCG|win32|\.zip|runProcess\("tar"|tar' src/services/destructive-command-guard src || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 50378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== manager existence and exact file =="
git ls-files 'src/services/destructive-command-guard/manager.ts' || true
echo "== targeted source excerpt =="
if [ -f src/services/destructive-command-guard/manager.ts ]; then
sed -n '1,260p' src/services/destructive-command-guard/manager.ts
fi
echo "== targeted DCG usages =="
rg -n 'destructive-command-guard|DCG|extractSingleBinary|runProcess\("tar"|\.zip|win32' src/services/destructive-command-guard src || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 49441
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== semple downloader Windows zip extraction context =="
sed -n '180,280p' src/services/code-index/semble-downloader.ts
sed -n '340,380p' src/services/code-index/semble-downloader.ts
echo "== Node process spawn behavior for missing executable =="
node - <<'JS'
const { spawn } = require("node:child_process");
const p = spawn(process.platform === "win32" ? "nonexistent-this-should-not-exist-xyz" : "/nonexistent-this-should-not-exist-xyz", [], { shell: false });
p.on("error", (err) => {
console.log(JSON.stringify({
code: err.code,
message: err.message,
sysCall: err.syscall,
path: err.path
}));
process.exit(0);
});
setTimeout(() => {
console.log('waited-for-no-error');
process.exit(1);
}, 200);
JS
echo "== semble Windows zip extractor implementation =="
sed -n '1,320p' src/services/code-index/semble-downloader.ts | rg -n 'extractZip|extractZipTo|process\.arch|WinZip|zip' -C 5 || trueRepository: Zoo-Code-Org/Zoo-Code
Length of output: 302
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== find semple downloader candidates =="
fd -a 'semble-downloader\.ts$' . 2>/dev/null || true
echo "== Node process spawn behavior for missing executable =="
node - <<'JS'
const { spawn } = require("node:child_process");
const p = spawn(process.platform === "win32" ? "nonexistent-this-should-not-exist-xyz" : "/nonexistent-this-should-not-exist-xyz", [], { shell: false });
p.on("error", (err) => {
console.log(JSON.stringify({
code: err.code,
message: err.message,
sysCall: err.syscall,
path: err.path
}));
process.exit(0);
});
setTimeout(() => {
console.log('waited-for-no-error');
process.exit(1);
}, 200);
JS
echo "== semple exact file with fd =="
if [ -n "$(fd 'semble-downloader\.ts$' . 2>/dev/null | head -n 1)" ]; then
file="$(fd 'semble-downloader\.ts$' . | head -n 1)"
echo "$file"
sed -n '180,280p' "$file"
sed -n '340,380p' "$file"
fiRepository: Zoo-Code-Org/Zoo-Code
Length of output: 5384
Use a Windows-safe zip extractor before relying on tar.
The .zip archives are only used for Windows (win32-x64, win32-arm64), so spawn("tar", ...) only runs on platforms that support tar zip extraction from the start. Avoid relying on tar being on PATH there; consider a native/PowerShell/Node zip extraction path instead.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/destructive-command-guard/manager.ts` around lines 145 - 160,
Update extractSingleBinary to use a Windows-safe native, PowerShell, or
Node-based ZIP extraction path for .zip archives instead of invoking
runProcess("tar", ...). Preserve the existing tar listing and extraction
behavior for non-ZIP archives, and continue validating that the archive contains
only info.binary before extraction.
Comment on lines
+162
to
+212
| async function installDcg(storageDir: string): Promise<string> { | ||
| const info = getDcgArchiveInfo() | ||
| if (!info) { | ||
| throw new Error(`Destructive Command Guard is not available for ${process.platform}-${process.arch}`) | ||
| } | ||
|
|
||
| const installRoot = path.join(storageDir, "destructive-command-guard") | ||
| const finalDir = path.join(installRoot, DCG_VERSION) | ||
| const binaryPath = path.join(finalDir, info.binary) | ||
| try { | ||
| await fs.access(binaryPath) | ||
| return binaryPath | ||
| } catch { | ||
| // First install, or the managed executable was removed. | ||
| } | ||
|
|
||
| await fs.mkdir(installRoot, { recursive: true }) | ||
| const stagingDir = path.join(installRoot, `${DCG_VERSION}.staging-${process.pid}-${Date.now()}`) | ||
| const archivePath = path.join(installRoot, `${info.archive}.${process.pid}.${Date.now()}.download`) | ||
| await fs.mkdir(stagingDir, { recursive: true }) | ||
|
|
||
| try { | ||
| await downloadFile(`${DCG_DOWNLOAD_BASE_URL}/${info.archive}`, archivePath) | ||
| await verifyChecksum(archivePath, info.sha256) | ||
| await extractSingleBinary(archivePath, stagingDir, info) | ||
| const stagedBinary = path.join(stagingDir, info.binary) | ||
| if (process.platform !== "win32") { | ||
| await fs.chmod(stagedBinary, 0o755) | ||
| } | ||
| const version = await runProcess(stagedBinary, ["--version"], 10_000) | ||
| if (!`${version.stdout}\n${version.stderr}`.includes(DCG_VERSION.replace(/^v/, ""))) { | ||
| throw new Error("Downloaded DCG executable reported an unexpected version") | ||
| } | ||
| await fs.rm(finalDir, { recursive: true, force: true }) | ||
| await fs.rename(stagingDir, finalDir) | ||
| return binaryPath | ||
| } finally { | ||
| await fs.rm(archivePath, { force: true }).catch(() => {}) | ||
| await fs.rm(stagingDir, { recursive: true, force: true }).catch(() => {}) | ||
| } | ||
| } | ||
|
|
||
| export function ensureDcgInstalled(storageDir: string): Promise<string> { | ||
| const existing = installationPromises.get(storageDir) | ||
| if (existing) { | ||
| return existing | ||
| } | ||
| const promise = installDcg(storageDir).finally(() => installationPromises.delete(storageDir)) | ||
| installationPromises.set(storageDir, promise) | ||
| return promise | ||
| } |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
In-process dedup doesn't protect against multiple VS Code windows racing the same install.
installationPromises only dedupes concurrent calls within a single process. If two windows on the same profile call ensureDcgInstalled with the same globalStorageUri.fsPath around the same time, both can pass the fs.access(binaryPath) check and later race on fs.rm(finalDir, ...) + fs.rename(stagingDir, finalDir) in installDcg. Since both fetch the same pinned version, the final content should converge, but a consumer that already resolved its own promise could hit a transient ENOENT executing the guard exactly while another window's rm/rename cycle is mid-flight — an availability hiccup for a safety-critical check.
Consider skipping the rm+rename swap when finalDir already contains the verified binary (re-check right before replacing), or use a lightweight cross-process lock file under installRoot.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/destructive-command-guard/manager.ts` around lines 162 - 212,
Prevent cross-process installation races in installDcg: before the destructive
fs.rm(finalDir) and fs.rename(stagingDir, finalDir) replacement, re-check
whether the verified binaryPath already exists and return it while cleaning up
staging, or implement an equivalent lightweight lock under installRoot. Preserve
checksum, extraction, and version verification, and ensure concurrent windows
never remove an already valid installation.
Comment on lines
+294
to
+301
| if (message.autoApprovalDecision === "deny") { | ||
| return [ | ||
| <OctagonX className="size-4 text-vscode-errorForeground" aria-label="Denied command icon" />, | ||
| <span className="font-bold text-vscode-errorForeground"> | ||
| {t("chat:commandExecution.denied")} | ||
| </span>, | ||
| ] | ||
| } |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ESLint react/jsx-key errors on the new denied branch.
🐛 Proposed fix
if (message.autoApprovalDecision === "deny") {
return [
- <OctagonX className="size-4 text-vscode-errorForeground" aria-label="Denied command icon" />,
- <span className="font-bold text-vscode-errorForeground">
+ <OctagonX
+ key="denied-icon"
+ className="size-4 text-vscode-errorForeground"
+ aria-label="Denied command icon"
+ />,
+ <span key="denied-title" className="font-bold text-vscode-errorForeground">
{t("chat:commandExecution.denied")}
</span>,
]
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (message.autoApprovalDecision === "deny") { | |
| return [ | |
| <OctagonX className="size-4 text-vscode-errorForeground" aria-label="Denied command icon" />, | |
| <span className="font-bold text-vscode-errorForeground"> | |
| {t("chat:commandExecution.denied")} | |
| </span>, | |
| ] | |
| } | |
| if (message.autoApprovalDecision === "deny") { | |
| return [ | |
| <OctagonX | |
| key="denied-icon" | |
| className="size-4 text-vscode-errorForeground" | |
| aria-label="Denied command icon" | |
| />, | |
| <span key="denied-title" className="font-bold text-vscode-errorForeground"> | |
| {t("chat:commandExecution.denied")} | |
| </span>, | |
| ] | |
| } |
🧰 Tools
🪛 ESLint
[error] 296-296: Missing "key" prop for element in array
(react/jsx-key)
[error] 297-299: Missing "key" prop for element in array
(react/jsx-key)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/components/chat/ChatRow.tsx` around lines 294 - 301, Add
stable React keys to both JSX elements returned by the autoApprovalDecision
"deny" branch in the surrounding message-rendering function, resolving the
react/jsx-key lint errors without changing the denied-state content or styling.
Source: Linters/SAST tools
Comment on lines
+331
to
+334
| "destructiveCommandGuard": { | ||
| "label": "Schutz vor destruktiven Befehlen aktivieren", | ||
| "description": "Lädt Destructive Command Guard (DCG) für diese Plattform herunter und verwendet es. Von DCG erlaubte Befehle werden automatisch ausgeführt. Von DCG blockierte Befehle benötigen deine Zustimmung. Zoos Befehlslisten sind währenddessen deaktiviert. Die heruntergeladene ausführbare Datei bleibt erhalten, wenn du die Option ausschaltest." | ||
| }, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Improve the German wording for Zoo’s command lists.
Line [333] uses Zoos Befehlslisten, which is awkward and unclear. Prefer Die Befehlslisten von Zoo sind währenddessen deaktiviert.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/i18n/locales/de/settings.json` around lines 331 - 334, Update
the description in the destructiveCommandGuard localization entry to replace
“Zoos Befehlslisten” with the clearer German wording “Die Befehlslisten von Zoo
sind währenddessen deaktiviert.”
Comment on lines
+332
to
+335
| "destructiveCommandGuard": { | ||
| "label": "Activer la protection contre les commandes destructrices", | ||
| "description": "Télécharge et utilise Destructive Command Guard (DCG) pour cette plateforme. Les commandes autorisées par DCG s'exécutent automatiquement. Les commandes bloquées par DCG nécessitent ton approbation. Les listes de commandes de Zoo sont désactivées tant que cette option est active. L'exécutable téléchargé est conservé si tu la désactives." | ||
| }, |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Keep the French register consistent.
The surrounding settings use formal vous/votre, but this description switches to informal tu/ton. Use the same formal register throughout the settings screen.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@webview-ui/src/i18n/locales/fr/settings.json` around lines 332 - 335, Update
the description in the destructiveCommandGuard translation to replace the
informal French pronouns “tu” and “ton” with the formal “vous” and “votre”,
preserving the existing meaning and wording otherwise.
navedmerchant
force-pushed
the
feat/autoapprovecommands
branch
from
July 29, 2026 06:34
d7b7b5b to
bce10d6
Compare
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/core/webview/webviewMessageHandler.ts (1)
577-577: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winWrap
casebodies in blocks to fixno-case-declarations.
const mcpHub = ...(577) andconst vsCodeLmModels = ...(1383) arelet/constdeclarations directly insidecaseclauses without a block, which both ESLint and Biome flag — the declaration is otherwise visible to siblingcaseclauses in the same switch.🐛 Proposed fix
- case "webviewDidLaunch": + case "webviewDidLaunch": { // Load custom modes first ... - provider.isViewLaunched = true - break + provider.isViewLaunched = true + break + }- case "requestVsCodeLmModels": - const vsCodeLmModels = await getVsCodeLmModels() - // TODO: Cache like we do for OpenRouter, etc? - await provider.postMessageToWebview({ type: "vsCodeLmModels", vsCodeLmModels }) - break + case "requestVsCodeLmModels": { + const vsCodeLmModels = await getVsCodeLmModels() + // TODO: Cache like we do for OpenRouter, etc? + await provider.postMessageToWebview({ type: "vsCodeLmModels", vsCodeLmModels }) + break + }As per coding guidelines, "Fix lint violations in new TypeScript code instead of adding suppressions."
Also applies to: 1382-1386
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/webview/webviewMessageHandler.ts` at line 577, Wrap the affected switch case bodies in explicit blocks to satisfy no-case-declarations. Update the case containing provider.getMcpHub() and the case containing the vsCodeLmModels declaration, keeping each case’s existing logic and control flow unchanged while scoping its const declarations to that case.Sources: Coding guidelines, Linters/SAST tools
src/core/webview/__tests__/ClineProvider.spec.ts (1)
847-847: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winReplace
as anywith the typed pattern already used above.Line 826 in this same file types the identical expression as
ReturnType<typeof vi.fn>; line 847 usesas anyinstead, which ESLint flags (no-explicit-any).🐛 Proposed fix
- const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as any).mock.calls[0][0] + const messageHandler = (mockWebviewView.webview.onDidReceiveMessage as ReturnType<typeof vi.fn>).mock + .calls[0][0]As per coding guidelines, "Avoid
as any; use typed APIs, bracket notation for private members, or precise test doubles and type guards."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/webview/__tests__/ClineProvider.spec.ts` at line 847, Replace the `as any` cast in the `messageHandler` assignment with the existing `ReturnType<typeof vi.fn>` typed pattern used for the identical `onDidReceiveMessage` expression above, preserving the current test behavior and satisfying the no-explicit-any rule.Sources: Coding guidelines, Linters/SAST tools
♻️ Duplicate comments (1)
webview-ui/src/components/chat/ChatRow.tsx (1)
294-301: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMissing
keyprops on the denied-command JSX array — previously flagged, still unresolved.ESLint still reports
react/jsx-keyon both elements returned from this branch.🐛 Proposed fix
if (message.autoApprovalDecision === "deny") { return [ - <OctagonX className="size-4 text-vscode-errorForeground" aria-label="Denied command icon" />, - <span className="font-bold text-vscode-errorForeground"> + <OctagonX + key="denied-icon" + className="size-4 text-vscode-errorForeground" + aria-label="Denied command icon" + />, + <span key="denied-title" className="font-bold text-vscode-errorForeground"> {t("chat:commandExecution.denied")} </span>, ] }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@webview-ui/src/components/chat/ChatRow.tsx` around lines 294 - 301, Add stable key props to both JSX elements returned by the denied branch of the message rendering logic, specifically the OctagonX icon and the denied-command span. Keep their existing content and styling unchanged while ensuring each array element has a unique key.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/services/destructive-command-guard/manager.ts`:
- Around line 84-95: Update the oversized-download handling in the response data
listener to destroy the piped output write stream when received exceeds
DCG_MAX_ARCHIVE_BYTES, alongside destroying the request. Preserve the existing
size-limit error and ensure the output stream cannot remain open after aborting.
---
Outside diff comments:
In `@src/core/webview/__tests__/ClineProvider.spec.ts`:
- Line 847: Replace the `as any` cast in the `messageHandler` assignment with
the existing `ReturnType<typeof vi.fn>` typed pattern used for the identical
`onDidReceiveMessage` expression above, preserving the current test behavior and
satisfying the no-explicit-any rule.
In `@src/core/webview/webviewMessageHandler.ts`:
- Line 577: Wrap the affected switch case bodies in explicit blocks to satisfy
no-case-declarations. Update the case containing provider.getMcpHub() and the
case containing the vsCodeLmModels declaration, keeping each case’s existing
logic and control flow unchanged while scoping its const declarations to that
case.
---
Duplicate comments:
In `@webview-ui/src/components/chat/ChatRow.tsx`:
- Around line 294-301: Add stable key props to both JSX elements returned by the
denied branch of the message rendering logic, specifically the OctagonX icon and
the denied-command span. Keep their existing content and styling unchanged while
ensuring each array element has a unique key.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ae067f4b-724a-4c0d-b9b7-6cd367315dfd
📒 Files selected for processing (96)
AGENTS.mdpackages/types/src/global-settings.tspackages/types/src/message.tspackages/types/src/vscode-extension-host.tssrc/core/auto-approval/__tests__/dcg.spec.tssrc/core/auto-approval/index.tssrc/core/task/Task.tssrc/core/tools/ExecuteCommandTool.tssrc/core/tools/__tests__/executeCommandTool.spec.tssrc/core/webview/ClineProvider.tssrc/core/webview/__tests__/ClineProvider.spec.tssrc/core/webview/webviewMessageHandler.tssrc/i18n/locales/ca/common.jsonsrc/i18n/locales/ca/tools.jsonsrc/i18n/locales/de/common.jsonsrc/i18n/locales/de/tools.jsonsrc/i18n/locales/en/common.jsonsrc/i18n/locales/en/tools.jsonsrc/i18n/locales/es/common.jsonsrc/i18n/locales/es/tools.jsonsrc/i18n/locales/fr/common.jsonsrc/i18n/locales/fr/tools.jsonsrc/i18n/locales/hi/common.jsonsrc/i18n/locales/hi/tools.jsonsrc/i18n/locales/id/common.jsonsrc/i18n/locales/id/tools.jsonsrc/i18n/locales/it/common.jsonsrc/i18n/locales/it/tools.jsonsrc/i18n/locales/ja/common.jsonsrc/i18n/locales/ja/tools.jsonsrc/i18n/locales/ko/common.jsonsrc/i18n/locales/ko/tools.jsonsrc/i18n/locales/nl/common.jsonsrc/i18n/locales/nl/tools.jsonsrc/i18n/locales/pl/common.jsonsrc/i18n/locales/pl/tools.jsonsrc/i18n/locales/pt-BR/common.jsonsrc/i18n/locales/pt-BR/tools.jsonsrc/i18n/locales/ru/common.jsonsrc/i18n/locales/ru/tools.jsonsrc/i18n/locales/tr/common.jsonsrc/i18n/locales/tr/tools.jsonsrc/i18n/locales/vi/common.jsonsrc/i18n/locales/vi/tools.jsonsrc/i18n/locales/zh-CN/common.jsonsrc/i18n/locales/zh-CN/tools.jsonsrc/i18n/locales/zh-TW/common.jsonsrc/i18n/locales/zh-TW/tools.jsonsrc/services/destructive-command-guard/__tests__/manager.spec.tssrc/services/destructive-command-guard/constants.tssrc/services/destructive-command-guard/index.tssrc/services/destructive-command-guard/manager.tssrc/services/destructive-command-guard/runner.tswebview-ui/src/components/chat/ChatRow.tsxwebview-ui/src/components/chat/CommandExecution.tsxwebview-ui/src/components/chat/__tests__/ChatRow.command-denied.spec.tsxwebview-ui/src/components/chat/__tests__/CommandExecution.spec.tsxwebview-ui/src/components/settings/AutoApproveSettings.tsxwebview-ui/src/components/settings/SettingsView.tsxwebview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsxwebview-ui/src/i18n/locales/ca/chat.jsonwebview-ui/src/i18n/locales/ca/settings.jsonwebview-ui/src/i18n/locales/de/chat.jsonwebview-ui/src/i18n/locales/de/settings.jsonwebview-ui/src/i18n/locales/en/chat.jsonwebview-ui/src/i18n/locales/en/settings.jsonwebview-ui/src/i18n/locales/es/chat.jsonwebview-ui/src/i18n/locales/es/settings.jsonwebview-ui/src/i18n/locales/fr/chat.jsonwebview-ui/src/i18n/locales/fr/settings.jsonwebview-ui/src/i18n/locales/hi/chat.jsonwebview-ui/src/i18n/locales/hi/settings.jsonwebview-ui/src/i18n/locales/id/chat.jsonwebview-ui/src/i18n/locales/id/settings.jsonwebview-ui/src/i18n/locales/it/chat.jsonwebview-ui/src/i18n/locales/it/settings.jsonwebview-ui/src/i18n/locales/ja/chat.jsonwebview-ui/src/i18n/locales/ja/settings.jsonwebview-ui/src/i18n/locales/ko/chat.jsonwebview-ui/src/i18n/locales/ko/settings.jsonwebview-ui/src/i18n/locales/nl/chat.jsonwebview-ui/src/i18n/locales/nl/settings.jsonwebview-ui/src/i18n/locales/pl/chat.jsonwebview-ui/src/i18n/locales/pl/settings.jsonwebview-ui/src/i18n/locales/pt-BR/chat.jsonwebview-ui/src/i18n/locales/pt-BR/settings.jsonwebview-ui/src/i18n/locales/ru/chat.jsonwebview-ui/src/i18n/locales/ru/settings.jsonwebview-ui/src/i18n/locales/tr/chat.jsonwebview-ui/src/i18n/locales/tr/settings.jsonwebview-ui/src/i18n/locales/vi/chat.jsonwebview-ui/src/i18n/locales/vi/settings.jsonwebview-ui/src/i18n/locales/zh-CN/chat.jsonwebview-ui/src/i18n/locales/zh-CN/settings.jsonwebview-ui/src/i18n/locales/zh-TW/chat.jsonwebview-ui/src/i18n/locales/zh-TW/settings.json
🚧 Files skipped from review as they are similar to previous changes (83)
- src/i18n/locales/hi/common.json
- src/i18n/locales/ru/common.json
- webview-ui/src/i18n/locales/tr/chat.json
- src/i18n/locales/es/common.json
- src/i18n/locales/en/tools.json
- packages/types/src/global-settings.ts
- webview-ui/src/i18n/locales/ja/chat.json
- src/i18n/locales/pt-BR/tools.json
- src/i18n/locales/ja/tools.json
- webview-ui/src/i18n/locales/fr/chat.json
- src/i18n/locales/pl/tools.json
- src/i18n/locales/vi/tools.json
- webview-ui/src/i18n/locales/ru/settings.json
- webview-ui/src/components/chat/tests/ChatRow.command-denied.spec.tsx
- src/i18n/locales/ca/tools.json
- webview-ui/src/i18n/locales/vi/chat.json
- src/i18n/locales/id/tools.json
- src/i18n/locales/ko/tools.json
- webview-ui/src/i18n/locales/id/chat.json
- webview-ui/src/i18n/locales/fr/settings.json
- src/services/destructive-command-guard/tests/manager.spec.ts
- webview-ui/src/i18n/locales/hi/chat.json
- src/i18n/locales/es/tools.json
- src/i18n/locales/ru/tools.json
- src/i18n/locales/nl/tools.json
- webview-ui/src/components/settings/tests/AutoApproveSettings.spec.tsx
- src/i18n/locales/zh-TW/tools.json
- src/i18n/locales/it/common.json
- src/services/destructive-command-guard/index.ts
- packages/types/src/message.ts
- src/i18n/locales/de/tools.json
- src/i18n/locales/pl/common.json
- packages/types/src/vscode-extension-host.ts
- src/i18n/locales/tr/tools.json
- webview-ui/src/i18n/locales/pt-BR/chat.json
- src/i18n/locales/zh-CN/tools.json
- src/i18n/locales/it/tools.json
- src/i18n/locales/ca/common.json
- webview-ui/src/i18n/locales/en/chat.json
- webview-ui/src/i18n/locales/zh-CN/chat.json
- webview-ui/src/i18n/locales/ja/settings.json
- webview-ui/src/i18n/locales/ru/chat.json
- webview-ui/src/i18n/locales/ko/chat.json
- webview-ui/src/i18n/locales/it/chat.json
- webview-ui/src/i18n/locales/es/chat.json
- src/i18n/locales/nl/common.json
- webview-ui/src/i18n/locales/de/chat.json
- webview-ui/src/i18n/locales/nl/chat.json
- webview-ui/src/i18n/locales/ca/chat.json
- webview-ui/src/i18n/locales/ko/settings.json
- webview-ui/src/i18n/locales/pl/settings.json
- src/i18n/locales/pt-BR/common.json
- webview-ui/src/i18n/locales/es/settings.json
- src/i18n/locales/zh-TW/common.json
- webview-ui/src/i18n/locales/tr/settings.json
- src/i18n/locales/de/common.json
- src/core/auto-approval/tests/dcg.spec.ts
- webview-ui/src/i18n/locales/it/settings.json
- webview-ui/src/i18n/locales/vi/settings.json
- webview-ui/src/i18n/locales/nl/settings.json
- src/i18n/locales/zh-CN/common.json
- webview-ui/src/i18n/locales/pl/chat.json
- src/core/task/Task.ts
- webview-ui/src/components/settings/SettingsView.tsx
- webview-ui/src/components/chat/tests/CommandExecution.spec.tsx
- webview-ui/src/i18n/locales/zh-TW/settings.json
- webview-ui/src/i18n/locales/id/settings.json
- src/i18n/locales/ko/common.json
- webview-ui/src/components/chat/CommandExecution.tsx
- webview-ui/src/i18n/locales/pt-BR/settings.json
- src/i18n/locales/hi/tools.json
- webview-ui/src/i18n/locales/hi/settings.json
- src/i18n/locales/tr/common.json
- src/core/webview/ClineProvider.ts
- src/core/tools/ExecuteCommandTool.ts
- webview-ui/src/i18n/locales/de/settings.json
- src/i18n/locales/fr/tools.json
- webview-ui/src/components/settings/AutoApproveSettings.tsx
- src/services/destructive-command-guard/constants.ts
- src/i18n/locales/ja/common.json
- src/core/tools/tests/executeCommandTool.spec.ts
- src/core/auto-approval/index.ts
- webview-ui/src/i18n/locales/ca/settings.json
Comment on lines
+84
to
+95
| let received = 0 | ||
| const output = createWriteStream(destination, { flags: "wx", mode: 0o600 }) | ||
| response.on("data", (chunk: Buffer) => { | ||
| received += chunk.length | ||
| if (received > DCG_MAX_ARCHIVE_BYTES) { | ||
| request.destroy(new Error("DCG archive exceeds the download size limit")) | ||
| } | ||
| }) | ||
| response.pipe(output) | ||
| output.on("finish", () => output.close(() => resolve())) | ||
| output.on("error", reject) | ||
| }) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Destroy the write stream when aborting an oversized download.
When the streaming size check trips (received > DCG_MAX_ARCHIVE_BYTES), only request.destroy(...) is called — the piped output write stream is left open, leaking a file descriptor until GC finalizes it.
🐛 Proposed fix
response.on("data", (chunk: Buffer) => {
received += chunk.length
if (received > DCG_MAX_ARCHIVE_BYTES) {
request.destroy(new Error("DCG archive exceeds the download size limit"))
+ output.destroy()
}
})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let received = 0 | |
| const output = createWriteStream(destination, { flags: "wx", mode: 0o600 }) | |
| response.on("data", (chunk: Buffer) => { | |
| received += chunk.length | |
| if (received > DCG_MAX_ARCHIVE_BYTES) { | |
| request.destroy(new Error("DCG archive exceeds the download size limit")) | |
| } | |
| }) | |
| response.pipe(output) | |
| output.on("finish", () => output.close(() => resolve())) | |
| output.on("error", reject) | |
| }) | |
| let received = 0 | |
| const output = createWriteStream(destination, { flags: "wx", mode: 0o600 }) | |
| response.on("data", (chunk: Buffer) => { | |
| received += chunk.length | |
| if (received > DCG_MAX_ARCHIVE_BYTES) { | |
| request.destroy(new Error("DCG archive exceeds the download size limit")) | |
| output.destroy() | |
| } | |
| }) | |
| response.pipe(output) | |
| output.on("finish", () => output.close(() => resolve())) | |
| output.on("error", reject) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/services/destructive-command-guard/manager.ts` around lines 84 - 95,
Update the oversized-download handling in the response data listener to destroy
the piped output write stream when received exceeds DCG_MAX_ARCHIVE_BYTES,
alongside destroying the request. Preserve the existing size-limit error and
ensure the output stream cannot remain open after aborting.
github-actions
Bot
added
the
awaiting-review
navedmerchant
marked this pull request as draft
July 29, 2026 06:58
github-actions
Bot
removed
the
awaiting-review
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
1 participant
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.
Related GitHub Issue
Closes: #1049
Description
Integrates Destructive Command Guard (DCG) into command execution so modern models can run complex commands and generated scripts without unnecessary pauses while preserving an explicit approval boundary for dangerous operations.
Key implementation details:
destructiveCommandGuardEnabledsetting across shared types, extension state, settings submission, and provider state.Reviewers should pay particular attention to the safety boundary between DCG classification and
checkAutoApproval, plus binary lifecycle handling in the DCG manager.Test Procedure
Automated tests run from the package directories that declare Vitest:
cd src npx vitest run core/auto-approval/__tests__/dcg.spec.ts core/tools/__tests__/executeCommandTool.spec.ts core/webview/__tests__/ClineProvider.spec.ts services/destructive-command-guard/__tests__/manager.spec.tsResult: 4 test files passed, 141 tests passed.
cd webview-ui npx vitest run src/components/chat/__tests__/ChatRow.command-denied.spec.tsx src/components/chat/__tests__/CommandExecution.spec.tsx src/components/settings/__tests__/AutoApproveSettings.spec.tsxResult: 3 test files passed, 47 tests passed.
Manual verification:
Pre-Submission Checklist
*.visual.tsxsnapshot inwebview-ui/. Seewebview-ui/AGENTS.md→ "When a UI change needs a snapshot".Visual Snapshots
Videos (interaction / animation only)
Not applicable; this change does not introduce animation or a motion-dependent flow.
Documentation Updates
Additional Notes
The DCG integration is opt-in. Existing command allowlist and denylist behavior remains available when DCG is disabled.
Get in Touch
GitHub: @navedmerchant
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Bug Fixes
Tests