diff --git a/packages/terminal-security/src/evaluateTerminalCommandSecurity.ts b/packages/terminal-security/src/evaluateTerminalCommandSecurity.ts index 693eef25f9c..e094443f8d1 100644 --- a/packages/terminal-security/src/evaluateTerminalCommandSecurity.ts +++ b/packages/terminal-security/src/evaluateTerminalCommandSecurity.ts @@ -372,7 +372,7 @@ function evaluateSingleCommand( const args = commandTokens.slice(1); // Check for critical commands that should always be disabled - if (isCriticalCommand(baseCommand, args)) { + if (isCriticalCommand(baseCommand, args, originalCommand)) { return "disabled"; } @@ -396,10 +396,73 @@ function evaluateSingleCommand( return "allowedWithPermission"; } +/** + * True when a token points at a filesystem root / home / high-impact system path. + * Also matches shell variables that expand to those locations ($HOME, ${HOME}, ~user + * shell-quote leaves $HOME as a literal token rather than expanding it). + */ +function isDangerousFilesystemTarget(arg: string): boolean { + if (!arg) return false; + + // Shell variable forms that expand to the user's home or root at execution. + if ( + arg === "$HOME" || + arg === "${HOME}" || + arg === "$HOME/" || + arg === "${HOME}/" || + arg.startsWith("$HOME/") || + arg.startsWith("${HOME}/") || + arg === "~" || + arg === "~/" || + arg === "~/*" || + arg.startsWith("~/") + ) { + return true; + } + + const exact = new Set([ + "/", + "/*", + "/usr", + "/etc", + "/bin", + "/sbin", + "/home", + "/root", + "/var", + "/opt", + "/srv", + "/boot", + "/lib", + "/lib64", + ]); + if (exact.has(arg)) return true; + + const prefixes = [ + "/usr/", + "/etc/", + "/bin/", + "/sbin/", + "/home/", + "/root/", + "/var/", + "/opt/", + "/srv/", + "/boot/", + "/lib/", + "/lib64/", + ]; + return prefixes.some((p) => arg === p.slice(0, -1) || arg.startsWith(p)); +} + /** * Checks if a command is critical and should always be disabled */ -function isCriticalCommand(baseCommand: string, args: string[]): boolean { +function isCriticalCommand( + baseCommand: string, + args: string[], + originalCommand: string = "", +): boolean { // System destruction commands - check if base command starts with mkfs if (baseCommand.startsWith("mkfs")) { return true; @@ -418,20 +481,8 @@ function isCriticalCommand(baseCommand: string, args: string[]): boolean { (allTokens.includes("-r") && allTokens.includes("-f")) || (allTokens.includes("--recursive") && allTokens.includes("--force")); - const hasDangerousPath = allTokens.some( - (arg) => - arg === "/" || - arg === "/*" || - arg === "~" || - arg === "~/*" || - arg === "/usr" || - arg === "/etc" || - arg === "/bin" || - arg === "/sbin" || - arg.startsWith("/usr/") || - arg.startsWith("/etc/") || - arg.startsWith("/bin/") || - arg.startsWith("/sbin/"), + const hasDangerousPath = allTokens.some((arg) => + isDangerousFilesystemTarget(arg), ); // If we have rm flags with dangerous paths, it's critical regardless of command @@ -439,6 +490,17 @@ function isCriticalCommand(baseCommand: string, args: string[]): boolean { return true; } + // shell-quote expands/drops unquoted $HOME to "" (or strips ${HOME} prefix), so the + // token list loses the home target. Fall back to the original command string. + // See issue #13001 — "rm -rf $HOME" must stay disabled. + if ( + hasRf && + originalCommand && + /(?:\$\{?HOME\}?|~(?=\/|$|[\s;|&]))/.test(originalCommand) + ) { + return true; + } + // Check for explicit rm command with additional critical paths if (baseCommand === "rm") { // Check for deletion of critical system files (even without -rf) @@ -463,6 +525,25 @@ function isCriticalCommand(baseCommand: string, args: string[]): boolean { } } + // Always-critical destructive utilities (even without -rf path heuristics) + // find -delete can wipe trees; shred/wipefs/truncate destroy data; pkexec escalates. + if ( + baseCommand === "shred" || + baseCommand === "wipefs" || + baseCommand === "pkexec" || + (baseCommand === "truncate" && + args.some( + (arg) => + arg === "-s" || + arg.startsWith("-s") || + arg === "--size" || + arg.startsWith("--size="), + )) || + (baseCommand === "find" && args.includes("-delete")) + ) { + return true; + } + // Windows destructive commands if (baseCommand === "del") { const hasRecursive = args.includes("/s") && args.includes("/q"); diff --git a/packages/terminal-security/test/terminalCommandSecurity.test.ts b/packages/terminal-security/test/terminalCommandSecurity.test.ts index 7d2e484765e..3b4dad29982 100644 --- a/packages/terminal-security/test/terminalCommandSecurity.test.ts +++ b/packages/terminal-security/test/terminalCommandSecurity.test.ts @@ -75,6 +75,109 @@ describe("evaluateTerminalCommandSecurity", () => { ); expect(result).toBe("disabled"); }); + + it("should disable rm -rf on /home", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf /home", + ); + expect(result).toBe("disabled"); + }); + + it("should disable rm -rf on /home/user", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf /home/user", + ); + expect(result).toBe("disabled"); + }); + + it("should disable rm -rf on /root", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf /root", + ); + expect(result).toBe("disabled"); + }); + + it("should disable rm -rf on /var", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf /var", + ); + expect(result).toBe("disabled"); + }); + + it("should disable rm -rf on /opt and /srv", () => { + expect( + evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf /opt", + ), + ).toBe("disabled"); + expect( + evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf /srv/data", + ), + ).toBe("disabled"); + }); + + it("should disable rm -rf $HOME (literal token left by shell-quote)", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf $HOME", + ); + expect(result).toBe("disabled"); + }); + + it("should disable rm -rf ${HOME}/Documents", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "rm -rf ${HOME}/Documents", + ); + expect(result).toBe("disabled"); + }); + + it("should disable find -delete", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "find /tmp -name '*.log' -delete", + ); + expect(result).toBe("disabled"); + }); + + it("should disable shred", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "shred -vfz /var/log/secure", + ); + expect(result).toBe("disabled"); + }); + + it("should disable wipefs", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "wipefs -a /dev/sda", + ); + expect(result).toBe("disabled"); + }); + + it("should disable truncate -s 0 on paths", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "truncate -s 0 important.db", + ); + expect(result).toBe("disabled"); + }); + + it("should disable pkexec", () => { + const result = evaluateTerminalCommandSecurity( + "allowedWithoutPermission", + "pkexec rm -rf /", + ); + expect(result).toBe("disabled"); + }); }); describe("Privilege Escalation", () => { @@ -793,12 +896,12 @@ describe("evaluateTerminalCommandSecurity", () => { expect(result).toBe("allowedWithPermission"); }); - it("should require permission for find with -delete", () => { + it("should disable find with -delete as critical destruction", () => { const result = evaluateTerminalCommandSecurity( - "allowedWithPermission", + "allowedWithoutPermission", "find /tmp -name '*.tmp' -delete", ); - expect(result).toBe("allowedWithPermission"); + expect(result).toBe("disabled"); }); }); @@ -1571,12 +1674,12 @@ describe("evaluateTerminalCommandSecurity", () => { expect(result).toBe("allowedWithPermission"); }); - it("should require permission for shred command", () => { + it("should disable shred command as critical destruction", () => { const result = evaluateTerminalCommandSecurity( "allowedWithoutPermission", "shred -vfz /var/log/secure", ); - expect(result).toBe("allowedWithPermission"); + expect(result).toBe("disabled"); }); });