Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1,124 changes: 1,124 additions & 0 deletions bun.lock

Large diffs are not rendered by default.

2 changes: 2 additions & 0 deletions src/shared/experiments.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { AssertEqual, Equals, Keys, Values, ExperimentId, Experiments } from "@roo-code/types"

export const EXPERIMENT_IDS = {
COMMAND_FILTERING_MODE: "commandFilteringMode",
MULTI_FILE_APPLY_DIFF: "multiFileApplyDiff",
POWER_STEERING: "powerSteering",
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
Expand All @@ -16,6 +17,7 @@ interface ExperimentConfig {
}

export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
COMMAND_FILTERING_MODE: { enabled: false },
MULTI_FILE_APPLY_DIFF: { enabled: false },
POWER_STEERING: { enabled: false },
PREVENT_FOCUS_DISRUPTION: { enabled: false },
Expand Down
10 changes: 8 additions & 2 deletions webview-ui/src/components/chat/ChatView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
soundEnabled,
soundVolume,
cloudIsAuthenticated,
experiments,
} = useExtensionState()

const messagesRef = useRef(messages)
Expand Down Expand Up @@ -1056,9 +1057,14 @@ const ChatViewComponent: React.ForwardRefRenderFunction<ChatViewRef, ChatViewPro
const getCommandDecisionForMessage = useCallback(
(message: ClineMessage | undefined): CommandDecision => {
if (message?.type !== "ask") return "ask_user"
return getCommandDecision(message.text || "", allowedCommands || [], deniedCommands || [])
return getCommandDecision(
message.text || "",
allowedCommands || [],
deniedCommands || [],
experiments,
)
},
[allowedCommands, deniedCommands],
[allowedCommands, deniedCommands, experiments],
)

// Check if a command message should be auto-approved.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { getCommandDecisionAlternate } from "../command-validation"

describe("getCommandDecisionAlternate", () => {
const allowedCommands = ["npm install", "echo"]
const deniedCommands = ["rm", "sudo"]

it("should deny a command on the denylist", () => {
const decision = getCommandDecisionAlternate("rm -rf /", allowedCommands, deniedCommands)
expect(decision).toBe("auto_deny")
})

it("should deny a command on the allowlist", () => {
const decision = getCommandDecisionAlternate("npm install", allowedCommands, deniedCommands)
expect(decision).toBe("auto_deny")
})

it("should approve a command not on any list", () => {
const decision = getCommandDecisionAlternate("git status", allowedCommands, deniedCommands)
expect(decision).toBe("auto_approve")
})

it("should deny a command chain if one sub-command is on the denylist", () => {
const decision = getCommandDecisionAlternate("git status && rm -rf /", allowedCommands, deniedCommands)
expect(decision).toBe("auto_deny")
})

it("should deny a command chain if one sub-command is on the allowlist", () => {
const decision = getCommandDecisionAlternate("git status && npm install", allowedCommands, deniedCommands)
expect(decision).toBe("auto_deny")
})

it("should approve a command chain if no sub-commands are on any list", () => {
const decision = getCommandDecisionAlternate("git status && git commit", allowedCommands, deniedCommands)
expect(decision).toBe("auto_approve")
})

it("should approve an empty command", () => {
const decision = getCommandDecisionAlternate("", allowedCommands, deniedCommands)
expect(decision).toBe("auto_approve")
})

it("should approve a command if both lists are empty", () => {
const decision = getCommandDecisionAlternate("any command", [], [])
expect(decision).toBe("auto_approve")
})

it("should handle undefined denylist", () => {
const decision = getCommandDecisionAlternate("npm install", allowedCommands, undefined)
expect(decision).toBe("auto_deny")
})

it("should handle undefined allowlist", () => {
const decision = getCommandDecisionAlternate("rm -rf /", [], deniedCommands)
expect(decision).toBe("auto_deny")
})
})
122 changes: 80 additions & 42 deletions webview-ui/src/utils/__tests__/command-validation.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
createCommandValidator,
containsSubshell,
} from "../command-validation"
import { experimentDefault } from "@roo/experiments"

describe("Command Validation", () => {
describe("parseCommand", () => {
Expand Down Expand Up @@ -765,87 +766,114 @@ describe("Unified Command Decision Functions", () => {
describe("getCommandDecision", () => {
const allowedCommands = ["npm", "echo"]
const deniedCommands = ["npm test"]
const experiments = experimentDefault

it("returns auto_approve for commands with all sub-commands auto-approved", () => {
expect(getCommandDecision("npm install", allowedCommands, deniedCommands)).toBe("auto_approve")
expect(getCommandDecision("npm install && echo done", allowedCommands, deniedCommands)).toBe("auto_approve")
expect(getCommandDecision("npm install", allowedCommands, deniedCommands, experiments)).toBe("auto_approve")
expect(getCommandDecision("npm install && echo done", allowedCommands, deniedCommands, experiments)).toBe(
"auto_approve",
)
})

it("returns auto_deny for commands with any sub-command auto-denied", () => {
expect(getCommandDecision("npm test", allowedCommands, deniedCommands)).toBe("auto_deny")
expect(getCommandDecision("npm install && npm test", allowedCommands, deniedCommands)).toBe("auto_deny")
expect(getCommandDecision("npm test", allowedCommands, deniedCommands, experiments)).toBe("auto_deny")
expect(getCommandDecision("npm install && npm test", allowedCommands, deniedCommands, experiments)).toBe(
"auto_deny",
)
})

it("returns ask_user for commands with mixed or unknown sub-commands", () => {
expect(getCommandDecision("dangerous", allowedCommands, deniedCommands)).toBe("ask_user")
expect(getCommandDecision("npm install && dangerous", allowedCommands, deniedCommands)).toBe("ask_user")
expect(getCommandDecision("dangerous", allowedCommands, deniedCommands, experiments)).toBe("ask_user")
expect(getCommandDecision("npm install && dangerous", allowedCommands, deniedCommands, experiments)).toBe(
"ask_user",
)
})

it("properly validates subshell commands by checking all parsed commands", () => {
// Subshells without denied prefixes should be auto-approved if all commands are allowed
expect(getCommandDecision("npm install $(echo test)", allowedCommands, deniedCommands)).toBe("auto_approve")
expect(getCommandDecision("npm install `echo test`", allowedCommands, deniedCommands)).toBe("auto_approve")
expect(getCommandDecision("npm install $(echo test)", allowedCommands, deniedCommands, experiments)).toBe(
"auto_approve",
)
expect(getCommandDecision("npm install `echo test`", allowedCommands, deniedCommands, experiments)).toBe(
"auto_approve",
)

// Subshells with denied prefixes should be auto-denied
expect(getCommandDecision("npm install $(npm test)", allowedCommands, deniedCommands)).toBe("auto_deny")
expect(getCommandDecision("npm install `npm test --coverage`", allowedCommands, deniedCommands)).toBe(
expect(getCommandDecision("npm install $(npm test)", allowedCommands, deniedCommands, experiments)).toBe(
"auto_deny",
)
expect(
getCommandDecision("npm install `npm test --coverage`", allowedCommands, deniedCommands, experiments),
).toBe("auto_deny")

// Main command with denied prefix should also be auto-denied
expect(getCommandDecision("npm test $(echo hello)", allowedCommands, deniedCommands)).toBe("auto_deny")
expect(getCommandDecision("npm test $(echo hello)", allowedCommands, deniedCommands, experiments)).toBe(
"auto_deny",
)
})

it("properly validates subshell commands when no denylist is present", () => {
expect(getCommandDecision("npm install $(echo test)", allowedCommands)).toBe("auto_approve")
expect(getCommandDecision("npm install `echo test`", allowedCommands)).toBe("auto_approve")
expect(getCommandDecision("npm install $(echo test)", allowedCommands, undefined, experiments)).toBe(
"auto_approve",
)
expect(getCommandDecision("npm install `echo test`", allowedCommands, undefined, experiments)).toBe(
"auto_approve",
)
})

it("handles empty command", () => {
expect(getCommandDecision("", allowedCommands, deniedCommands)).toBe("auto_approve")
expect(getCommandDecision("", allowedCommands, deniedCommands, experiments)).toBe("auto_approve")
})

it("handles complex chained commands", () => {
// All sub-commands auto-approved
expect(getCommandDecision("npm install && echo success && npm run build", ["npm", "echo"], [])).toBe(
"auto_approve",
)
expect(
getCommandDecision("npm install && echo success && npm run build", ["npm", "echo"], [], experiments),
).toBe("auto_approve")

// One sub-command auto-denied
expect(getCommandDecision("npm install && npm test && echo done", allowedCommands, deniedCommands)).toBe(
"auto_deny",
)
expect(
getCommandDecision("npm install && npm test && echo done", allowedCommands, deniedCommands, experiments),
).toBe("auto_deny")

// Mixed decisions (some ask_user)
expect(getCommandDecision("npm install && dangerous && echo done", allowedCommands, deniedCommands)).toBe(
"ask_user",
)
expect(
getCommandDecision(
"npm install && dangerous && echo done",
allowedCommands,
deniedCommands,
experiments,
),
).toBe("ask_user")
})

it("demonstrates the three-tier system comprehensively", () => {
const allowed = ["npm"]
const denied = ["npm test"]

// Auto-approved: all sub-commands match allowlist, none match denylist
expect(getCommandDecision("npm install", allowed, denied)).toBe("auto_approve")
expect(getCommandDecision("npm install && npm run build", allowed, denied)).toBe("auto_approve")
expect(getCommandDecision("npm install", allowed, denied, experiments)).toBe("auto_approve")
expect(getCommandDecision("npm install && npm run build", allowed, denied, experiments)).toBe(
"auto_approve",
)

// Auto-denied: any sub-command matches denylist
expect(getCommandDecision("npm test", allowed, denied)).toBe("auto_deny")
expect(getCommandDecision("npm install && npm test", allowed, denied)).toBe("auto_deny")
expect(getCommandDecision("npm test", allowed, denied, experiments)).toBe("auto_deny")
expect(getCommandDecision("npm install && npm test", allowed, denied, experiments)).toBe("auto_deny")

// Ask user: commands that match neither list
expect(getCommandDecision("dangerous", allowed, denied)).toBe("ask_user")
expect(getCommandDecision("npm install && dangerous", allowed, denied)).toBe("ask_user")
expect(getCommandDecision("dangerous", allowed, denied, experiments)).toBe("ask_user")
expect(getCommandDecision("npm install && dangerous", allowed, denied, experiments)).toBe("ask_user")
})
})

describe("CommandValidator Integration Tests", () => {
const experiments = experimentDefault
describe("CommandValidator Class", () => {
let validator: CommandValidator

beforeEach(() => {
validator = new CommandValidator(["npm", "echo", "git"], ["npm test", "git push"])
validator = new CommandValidator(["npm", "echo", "git"], ["npm test", "git push"], experiments)
})

describe("Basic validation methods", () => {
Expand All @@ -872,7 +900,7 @@ describe("Unified Command Decision Functions", () => {

describe("Configuration management", () => {
it("updates command lists", () => {
validator.updateCommandLists(["echo"], ["echo hello"])
validator.updateCommandLists(["echo"], ["echo hello"], experiments)

expect(validator.validateCommand("npm install")).toBe("ask_user")
expect(validator.validateCommand("echo world")).toBe("auto_approve")
Expand All @@ -886,7 +914,7 @@ describe("Unified Command Decision Functions", () => {
})

it("handles undefined denied commands", () => {
const validatorNoDeny = new CommandValidator(["npm"])
const validatorNoDeny = new CommandValidator(["npm"], undefined, experiments)
const lists = validatorNoDeny.getCommandLists()
expect(lists.allowedCommands).toEqual(["npm"])
expect(lists.deniedCommands).toBeUndefined()
Expand Down Expand Up @@ -953,13 +981,13 @@ describe("Unified Command Decision Functions", () => {
it("detects if rules are configured", () => {
expect(validator.hasRules()).toBe(true)

const emptyValidator = new CommandValidator([], [])
const emptyValidator = new CommandValidator([], [], experiments)
expect(emptyValidator.hasRules()).toBe(false)

const allowOnlyValidator = new CommandValidator(["npm"], [])
const allowOnlyValidator = new CommandValidator(["npm"], [], experiments)
expect(allowOnlyValidator.hasRules()).toBe(true)

const denyOnlyValidator = new CommandValidator([], ["rm"])
const denyOnlyValidator = new CommandValidator([], ["rm"], experiments)
expect(denyOnlyValidator.hasRules()).toBe(true)
})

Expand All @@ -972,7 +1000,7 @@ describe("Unified Command Decision Functions", () => {
})

it("detects wildcard configuration", () => {
const wildcardValidator = new CommandValidator(["*", "npm"], ["rm"])
const wildcardValidator = new CommandValidator(["*", "npm"], ["rm"], experiments)
const stats = wildcardValidator.getStats()
expect(stats.hasWildcard).toBe(true)
})
Expand All @@ -999,23 +1027,25 @@ describe("Unified Command Decision Functions", () => {
})

describe("Factory function", () => {
const experiments = experimentDefault
it("creates validator instances correctly", () => {
const validator = createCommandValidator(["npm"], ["rm"])
const validator = createCommandValidator(["npm"], ["rm"], experiments)
expect(validator).toBeInstanceOf(CommandValidator)
expect(validator.validateCommand("npm test")).toBe("auto_approve")
expect(validator.validateCommand("rm file")).toBe("auto_deny")
})

it("handles optional denied commands", () => {
const validator = createCommandValidator(["npm"])
const validator = createCommandValidator(["npm"], undefined, experiments)
expect(validator.validateCommand("npm test")).toBe("auto_approve")
expect(validator.validateCommand("dangerous")).toBe("ask_user")
})
})

describe("Subshell edge cases", () => {
const experiments = experimentDefault
it("handles multiple subshells correctly", () => {
const validator = createCommandValidator(["echo", "npm"], ["rm", "sudo"])
const validator = createCommandValidator(["echo", "npm"], ["rm", "sudo"], experiments)

// Multiple subshells, none with denied prefixes but subshell commands not in allowlist
// parseCommand extracts subshells as separate commands, so date and pwd are not allowed
Expand All @@ -1030,7 +1060,11 @@ describe("Unified Command Decision Functions", () => {
})

it("handles complex commands with subshells", () => {
const validator = createCommandValidator(["npm", "git", "echo"], ["git push", "npm publish"])
const validator = createCommandValidator(
["npm", "git", "echo"],
["git push", "npm publish"],
experiments,
)

// Subshell with allowed command - git status is extracted as separate command
// Since "git status" starts with "git" which is allowed, it's approved
Expand All @@ -1049,13 +1083,15 @@ describe("Unified Command Decision Functions", () => {
})

describe("Real-world integration scenarios", () => {
const experiments = experimentDefault
describe("Development workflow validation", () => {
let devValidator: CommandValidator

beforeEach(() => {
devValidator = createCommandValidator(
["npm", "git", "echo", "ls", "cat"],
["git push", "rm", "sudo", "npm publish"],
experiments,
)
})

Expand Down Expand Up @@ -1106,6 +1142,7 @@ describe("Unified Command Decision Functions", () => {
prodValidator = createCommandValidator(
["ls", "cat", "grep", "tail"],
["*"], // Deny everything by default
experiments,
)
})

Expand All @@ -1132,6 +1169,7 @@ describe("Unified Command Decision Functions", () => {
complexValidator = createCommandValidator(
["git", "git push", "git push --dry-run", "npm", "npm test"],
["git push", "npm test --coverage"],
experiments,
)
})

Expand Down Expand Up @@ -1166,7 +1204,7 @@ describe("Unified Command Decision Functions", () => {
const largeAllowList = Array.from({ length: 1000 }, (_, i) => `command${i}`)
const largeDenyList = Array.from({ length: 500 }, (_, i) => `dangerous${i}`)

const largeValidator = createCommandValidator(largeAllowList, largeDenyList)
const largeValidator = createCommandValidator(largeAllowList, largeDenyList, experiments)

// Should still work efficiently
expect(largeValidator.isAutoApproved("command500 --flag")).toBe(true)
Expand All @@ -1175,7 +1213,7 @@ describe("Unified Command Decision Functions", () => {
})

it("handles batch validation efficiently", () => {
const batchValidator = createCommandValidator(["npm"], ["rm"])
const batchValidator = createCommandValidator(["npm"], ["rm"], experiments)
const commands = Array.from({ length: 100 }, (_, i) => `npm test${i}`)
const results = batchValidator.validateCommands(commands)

Expand Down
Loading
Loading