Skip to content

fix: detect shell builtins and show warning instead of executing#1

Merged
slhmy merged 5 commits into
mainfrom
feat/shell-builtin-warning
Apr 20, 2026
Merged

fix: detect shell builtins and show warning instead of executing#1
slhmy merged 5 commits into
mainfrom
feat/shell-builtin-warning

Conversation

@slhmy

@slhmy slhmy commented Apr 20, 2026

Copy link
Copy Markdown
Owner

Summary

  • Detect shell builtins (cd, export, source, etc.) before execution
  • Display helpful warning explaining why these commands cannot work via subprocess
  • Suggest user run the command directly in their shell

Problem

Shell builtins modify the parent shell's state and cannot work when executed via subprocess. Previously, running v0k cd /path would silently fail with no directory change.

Solution

Added detection for common shell builtins and a friendly error message that explains the limitation and guides the user to run the command directly.

Test plan

  • Added unit test for is_shell_builtin() function
  • All existing tests pass (18 tests)
  • Manually tested: v0k cd /tmp shows warning and does not execute

Note

Low Risk
Low risk: adds a simple pre-execution guard for known shell builtins and a unit test; main behavior change is early failure with messaging for those commands.

Overview
Prevents v0k from attempting to execute shell builtins (e.g., cd, export, source) during execute_with_healing by detecting them up front and returning a clear warning that the user must run the command directly in their shell.

Adds a centralized SHELL_BUILTINS list with an is_shell_builtin() helper and covers it with a new unit test.

Reviewed by Cursor Bugbot for commit 372460a. Bugbot is set up for automated code reviews on this repo. Configure here.

slhmy added 2 commits April 20, 2026 12:02
Shell builtins (cd, export, source, etc.) modify the parent shell's
state and cannot work when executed via subprocess. Previously, running
`v0k cd /path` would silently fail with no directory change.

Now v0k detects these commands before execution and displays a helpful
warning explaining why it cannot work and suggesting the user run the
command directly in their shell.
Shorten the hint to two lines:
- "`<cmd>` is a shell builtin — run it directly:"
- "  <resolved command>"

Also fix formatting to pass CI (cargo fmt).
Comment thread src/main.rs Outdated
Move the shell builtin check inside the loop so that AI-suggested
healing fixes are also validated. Previously, if the AI suggested
a builtin like 'cd' as a fix, the protection would be bypassed.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 372460a. Configure here.

Comment thread src/main.rs
);
eprintln!("{}", format!(" {}", current_cmd.display).blue());
return Err(format!("`{}` is a shell builtin", current_cmd.program));
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builtin check happens after unnecessary AI API call

Medium Severity

The is_shell_builtin check is only inside execute_with_healing, but handle_external_command calls brain::review_command_for_shell (an AI API call) before ever reaching execute_with_healing. When AI is configured and the user runs e.g. v0k cd /tmp, an unnecessary API call is made. Worse, the AI could rewrite the builtin (e.g. cd /tmpbash -c "cd /tmp"), producing a non-builtin program that bypasses the detection entirely while still failing to change the parent shell's directory — the exact problem this PR aims to solve.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 372460a. Configure here.

@cursor

cursor Bot commented Apr 20, 2026

Copy link
Copy Markdown

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Move builtin check earlier in the flow to:
1. Avoid unnecessary AI API calls for builtins
2. Prevent AI from rewriting builtins into non-builtin forms
   (e.g. cd /tmp → bash -c "cd /tmp") that bypass detection

Add checks in:
- handle_external_command: before brain::review_command_for_shell
- execute_brain_response: before showing confirmation prompt
@cursor

cursor Bot commented Apr 20, 2026

Copy link
Copy Markdown

You have used all of your free Bugbot PR reviews.

To receive reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

@slhmy slhmy merged commit 5b6bc52 into main Apr 20, 2026
2 checks passed
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.

1 participant