fix(command-mode): close three confirm-gate bypasses: absolute path, find -delete/-exec rm, diskutil - #862
Conversation
…find -delete/-exec rm, diskutil isDestructiveCommand only matched bare command names, so an absolute-path invocation (/bin/mv, /usr/bin/sudo, /bin/chmod, etc.) skipped the confirm gate a bare-name equivalent would trigger. find -delete / find -exec rm and diskutil's erase/reformat subcommands were never covered at all. None of these three classes need anything adversarial-looking from the model -- an absolute path, find, or diskutil are all ordinary tool choices, so a plausible model output can land in any of them silently. Resolves the leading command to its bare name the same way a shell would (last path component of the first token) so any path prefix is recognized uniformly, plus dedicated checks for find and diskutil. Scoped narrowly: diskutil list/info (read-only) stay unflagged. Verified the gap survives PR altic-dev#434's independent hardening pass on the same function (fetched its branch, reran the same adversarial inputs against its actual patched code) before writing this, so this is a distinct gap, not overlapping work -- see altic-dev#861. Closes altic-dev#861 Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Greptile SummaryThis PR broadens Command Mode’s destructive-command confirmation classifier and adds regression coverage.
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| let leadingToken = cmd | ||
| .drop(while: { $0 == " " || $0 == "\t" }) | ||
| .prefix(while: { $0 != " " && $0 != "\t" }) | ||
| let commandName = (leadingToken as NSString).lastPathComponent |
There was a problem hiding this comment.
When a destructive executable is quoted, such as "/bin/rm" -rf ~/Documents, this parser retains the closing quote and derives rm" instead of rm. The classifier therefore returns false, while /bin/zsh -c resolves and executes /bin/rm without confirmation.
Knowledge Base Used: AI Enhancement Pipeline
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/CommandModeService.swift
Line: 610-613
Comment:
**Quoted executables bypass confirmation**
When a destructive executable is quoted, such as `"/bin/rm" -rf ~/Documents`, this parser retains the closing quote and derives `rm"` instead of `rm`. The classifier therefore returns false, while `/bin/zsh -c` resolves and executes `/bin/rm` without confirmation.
**Knowledge Base Used:** [AI Enhancement Pipeline](https://app.greptile.com/altic/-/custom-context/knowledge-base/altic-dev/fluidvoice/-/docs/ai-enhancement.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
| "erasedisk", "erasevolume", "secureerase", | ||
| "reformat", "partitiondisk", "zerodisk", "unmountdisk", | ||
| ] | ||
| if destructiveDiskutilSubcommands.contains(where: { cmd.contains($0) }) { |
There was a problem hiding this comment.
Searching the entire command for each subcommand name also matches ordinary arguments, so a benign command such as diskutil info /Volumes/EraseDisk is suspended behind manual confirmation. Parse and compare the actual diskutil subcommand token instead.
Knowledge Base Used: AI Enhancement Pipeline
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Fluid/Services/CommandModeService.swift
Line: 639
Comment:
**Diskutil arguments trigger false positives**
Searching the entire command for each subcommand name also matches ordinary arguments, so a benign command such as `diskutil info /Volumes/EraseDisk` is suspended behind manual confirmation. Parse and compare the actual diskutil subcommand token instead.
**Knowledge Base Used:** [AI Enhancement Pipeline](https://app.greptile.com/altic/-/custom-context/knowledge-base/altic-dev/fluidvoice/-/docs/ai-enhancement.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
|
please fix the issues and I can check if I Can merge it. THanks! |
2 participants
Description
isDestructiveCommand(Sources/Fluid/Services/CommandModeService.swift) only matched bare command names, so a model reaching for an absolute path (/bin/mv,/usr/bin/sudo,/bin/chmod,/usr/bin/killall, etc.) skipped the confirm gate its bare-name equivalent triggers.find -delete/find -exec rmanddiskutil's erase/reformat subcommands were never covered at all. None of these need anything adversarial-looking from the model — an absolute path,find, ordiskutilare all ordinary tool choices.Resolves the leading command to its bare name the same way a shell would (last path component of the first token) so any path prefix is recognized uniformly, plus dedicated checks for
findanddiskutil. Scoped narrowly:diskutil list/diskutil info(read-only) stay unflagged — verified by a dedicated false-positive test.Relationship to #434: that PR does extensive, independent hardening on this same function (pipe-to-shell, redirects, whitespace-prefix). I checked rather than assumed it doesn't cover this — fetched #434's actual branch, mechanically extracted its patched code, and reran these same adversarial inputs against it before writing this fix; all three classes here survive #434's hardening unchanged. Filed as issue #861 first with the same evidence, since I didn't want to push a competing PR onto the same lines without flagging the overlap. Happy to rebase onto #434 if it merges first, or however you'd prefer to sequence the two.
Type of Change
Related Issue or Discussion
Closes #861
Testing
swiftlint lint --strict --config .swiftlint.yml Sources/Fluid/Services/CommandModeService.swift— 0 violationsswiftformat --config .swiftformat --lint— clean on the changed region (the file carries 37 pre-existing formatting violations elsewhere, confirmed against the unmodified original before touching anything; none of them are in this diff)New test file
CommandModeDestructiveCommandGapTests.swift, 5 tests: the three fixed bypass classes, a regression check that original bare-command detection still works, and a false-positive check on benign commands (diskutil list,find . -name '*.txt',/usr/bin/python3 --version).Full-suite regression check: ran the complete test suite with and without this change (
git stashto get a clean baseline, samexcodebuild testinvocation both times) and diffed the exact failure sets — identical 27 pre-existing failures both times (audio/microphone-hardware tests that need real CoreAudio device state this environment doesn't have; unrelated to Command Mode). Zero regressions from this change.xcodebuild -project Fluid.xcodeproj -scheme Fluid -configuration Debug buildon macOS 26.1 / Xcode 26.3 (Apple Silicon): BUILD SUCCEEDED, no new warnings.Screenshots / Video
Notes
Not a live bug today —
find/diskutilaren't reachable through anything else in the app, and the confirm gate is defense-in-depth for the model's own mistakes rather than the only line of defense (Command Mode already grants real shell access by design). The value is closing the gap before a plausible model output lands in it, not fixing something currently broken.