diff --git a/internal/agent/command_prefix.go b/internal/agent/command_prefix.go index 2d0f5a606..b2f719457 100644 --- a/internal/agent/command_prefix.go +++ b/internal/agent/command_prefix.go @@ -373,73 +373,69 @@ func validSedPrintArg(arg string) bool { return true } +// gitApprovableReadOnlySubcommands are the only git subcommands this prefix +// matcher will ever auto-approve. Every other resolvable subcommand (push, +// commit, ...) — and the case where no subcommand resolves at all — is +// rejected by the zero value of the map lookup below, so this list is the +// single place that grants approval; nothing else needs to enumerate it. +var gitApprovableReadOnlySubcommands = map[string]bool{ + "status": true, "log": true, "diff": true, "show": true, "branch": true, +} + +// safeGitCommand approves only exact lowercase spellings of +// status/log/diff/show/branch with no global option this matcher has not +// specifically vetted as safe. +// +// Subcommand resolution goes through sandbox.GitSubcommand so option parsing +// stays shared with the classifier, but authorization compares Original, never +// Normalized. Git aliases are case-sensitive: STATUS can be an arbitrary alias +// even though the classifier conservatively recognizes its normalized spelling +// as status. func safeGitCommand(command []string) bool { - subIndex, subcommand, ok := gitSubcommand(command) - if !ok { + if len(command) < 2 { return false } + selection, ok := sandbox.GitSubcommand(command[1:]) + if !ok || !gitApprovableReadOnlySubcommands[selection.Original] { + return false + } + // GitSubcommand indexes into command[1:] (it does not expect the "git" + // argv[0] itself); shift its answer back into command's own indexing so the + // slices below retain their command-relative meaning. + subIndex := selection.Index + 1 if gitHasUnsafeGlobalOption(command[1:subIndex]) { return false } args := command[subIndex+1:] - switch subcommand { - case "status", "log", "diff", "show": - return gitArgsReadOnly(args) - case "branch": + if selection.Original == "branch" { return gitArgsReadOnly(args) && gitBranchReadOnly(args) - default: - return false - } -} - -func gitSubcommand(command []string) (int, string, bool) { - for index := 1; index < len(command); index++ { - arg := command[index] - if gitOptionConsumesValue(arg) { - index++ - continue - } - if gitOptionHasInlineValue(arg) || arg == "--" || strings.HasPrefix(arg, "-") { - continue - } - switch arg { - case "status", "log", "diff", "show", "branch": - return index, arg, true - default: - return 0, "", false - } } - return 0, "", false -} - -func gitOptionConsumesValue(arg string) bool { - switch arg { - case "-C", "-c", "--config-env", "--exec-path", "--git-dir", "--namespace", "--super-prefix", "--work-tree": - return true - default: - return false - } -} - -func gitOptionHasInlineValue(arg string) bool { - return strings.HasPrefix(arg, "--config-env=") || - strings.HasPrefix(arg, "--exec-path=") || - strings.HasPrefix(arg, "--git-dir=") || - strings.HasPrefix(arg, "--namespace=") || - strings.HasPrefix(arg, "--super-prefix=") || - strings.HasPrefix(arg, "--work-tree=") || - ((strings.HasPrefix(arg, "-C") || strings.HasPrefix(arg, "-c")) && len(arg) > 2) + return gitArgsReadOnly(args) } +// gitHasUnsafeGlobalOption rejects global options this matcher has not +// specifically vetted as compatible with the read-only subcommands above. +// -C changes which repository status/log/diff/show/branch inspects — a prefix +// approved against the workspace repo must not silently extend to `git -C +// /elsewhere status`, which reports on a DIFFERENT repository the approval +// was never about. --upload-pack lets a fetch/clone run an arbitrary program +// on the remote side; it is listed here defensively even though none of the +// five approvable subcommands accept it, so a future addition to that list +// does not silently inherit the gap. +// +// This still reads git's shared skip-list (GitGlobalOptionConsumesValue) to +// step over an option's separate-token value correctly; only the "-C is +// unsafe regardless" judgment is specific to this matcher, so it stays local +// rather than joining the classifier's shared grammar. func gitHasUnsafeGlobalOption(args []string) bool { for index := 0; index < len(args); index++ { arg := args[index] switch { case strings.HasPrefix(arg, "--upload-pack"): return true - case arg == "-C" || strings.HasPrefix(arg, "-C"): + case strings.HasPrefix(arg, "-C"): return true - case gitOptionConsumesValue(arg): + case sandbox.GitGlobalOptionConsumesValue(arg): index++ } } diff --git a/internal/agent/command_prefix_test.go b/internal/agent/command_prefix_test.go index a6e2adfac..744b14c59 100644 --- a/internal/agent/command_prefix_test.go +++ b/internal/agent/command_prefix_test.go @@ -35,6 +35,36 @@ func TestProposedCommandPrefixHonorsValidatedRequestedPrefix(t *testing.T) { } } +func TestSafeGitCommandConsumesAttrSourceOptionValue(t *testing.T) { + for _, command := range [][]string{ + {"git", "--attr-source", "HEAD", "status"}, + {"git", "--attr-source=HEAD", "status"}, + } { + if !safeGitCommand(command) { + t.Errorf("safeGitCommand(%q) = false; want true", command) + } + } +} + +func TestSafeGitCommandRejectsCaseDistinctAliasSubcommand(t *testing.T) { + for _, command := range [][]string{ + {"git", "-c", "alias.STATUS=!curl", "STATUS", "https://example.invalid"}, + {"git", "-c", "alias.Status=!curl", "Status", "https://example.invalid"}, + } { + if safeGitCommand(command) { + t.Errorf("safeGitCommand(%q) = true; case-distinct aliases must not receive reusable prefixes", command) + } + } + if prefix := proposedCommandPrefix("bash", map[string]any{ + "command": `make test && git -c alias.STATUS=!curl STATUS https://example.invalid`, + }); prefix != nil { + t.Fatalf("case-distinct alias was accepted as a safe tail and exposed prefix %#v", prefix) + } + if !safeGitCommand([]string{"git", "--attr-source", "HEAD", "status"}) { + t.Fatal("ordinary lowercase status with a vetted global option must remain approvable") + } +} + func TestProposedCommandPrefixSupportsSegmentedCommands(t *testing.T) { got := proposedCommandPrefix("bash", map[string]any{"command": "ps aux | head -5"}) if runtime.GOOS == "windows" { @@ -231,3 +261,82 @@ func TestProposedCommandPrefixRejectsRequestedUnsafeLauncherPrefix(t *testing.T) t.Fatalf("unsafe requested launcher prefix should be rejected, got %#v", got) } } + +// A terminal global makes git print and exit, so nothing after it is a +// subcommand. The sandbox classifier already stopped there; this parser walked +// past it and resolved `git --help status` to the read-only prefix +// `git status`, auto-approving a command the user never ran. The two scans read +// one option grammar (sandbox.GitTerminalGlobalOption) so they cannot drift. +func TestSafeGitCommandStopsAtTerminalGlobalOptions(t *testing.T) { + for _, command := range [][]string{ + {"git", "--help", "status"}, + {"git", "-h", "status"}, + {"git", "--version", "log"}, + {"git", "--exec-path", "status"}, + {"git", "-C", "repo", "--help", "diff"}, + } { + if safeGitCommand(command) { + t.Errorf("safeGitCommand(%q) = true; a terminal global ends the subcommand scan", command) + } + } + // An inline --exec-path= is a value-carrying global, not terminal, so + // the subcommand after it is still real. + for _, command := range [][]string{ + {"git", "--exec-path=/usr/libexec/git-core", "status"}, + {"git", "--namespace", "ns", "status"}, + } { + if !safeGitCommand(command) { + t.Errorf("safeGitCommand(%q) = false; want true", command) + } + } +} + +// TestSafeGitCommandRejectsDashCEvenWithAnApprovableSubcommand isolates the +// -C rejection from the terminal-global short circuit above (`-C repo --help +// diff` never reaches gitHasUnsafeGlobalOption's -C case at all, because +// --help stops the scan first). -C changes which repository the read-only +// subcommand inspects, so an approval for the workspace repo must not extend +// to a different one named this way — even for status/log/diff/show/branch, +// which are otherwise auto-approved. +func TestSafeGitCommandRejectsDashCEvenWithAnApprovableSubcommand(t *testing.T) { + for _, command := range [][]string{ + {"git", "-C", "repo", "status"}, + {"git", "-Crepo", "status"}, + {"git", "-C", "repo", "branch"}, + } { + if safeGitCommand(command) { + t.Errorf("safeGitCommand(%q) = true; -C must never be auto-approved", command) + } + } +} + +// TestSafeGitCommandRejectsSubcommandsOutsideTheApprovedList proves the +// refactor onto sandbox.GitSubcommand (a shared reader that resolves ANY +// subcommand, not just the five this matcher approves) did not widen what +// gets auto-approved: a resolvable-but-unapproved subcommand, and no +// subcommand at all, both stay rejected. +func TestSafeGitCommandRejectsSubcommandsOutsideTheApprovedList(t *testing.T) { + for _, command := range [][]string{ + {"git", "push", "origin", "main"}, + {"git", "commit", "-m", "msg"}, + {"git", "fetch", "origin"}, + {"git"}, + {"git", "-C", "repo"}, + } { + if safeGitCommand(command) { + t.Errorf("safeGitCommand(%q) = true; want rejected", command) + } + } + // The approved subcommands still resolve correctly through the same shared + // reader, with a global option in front — end-to-end proof the subIndex + // arithmetic across the sandbox.GitSubcommand boundary is still right. + for _, command := range [][]string{ + {"git", "status"}, + {"git", "--git-dir", "/repo/.git", "log"}, + {"git", "branch"}, + } { + if !safeGitCommand(command) { + t.Errorf("safeGitCommand(%q) = false; want approved", command) + } + } +} diff --git a/internal/agent/loop_test.go b/internal/agent/loop_test.go index f17e9be46..a004e48cf 100644 --- a/internal/agent/loop_test.go +++ b/internal/agent/loop_test.go @@ -2441,27 +2441,38 @@ func TestRunPersistentCommandPrefixStillPromptsForNetwork(t *testing.T) { } } -func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) { +func TestRunApprovedGitPushPromptAppliesTurnNetworkGrant(t *testing.T) { root := t.TempDir() - command := "PATH=.:$PATH curl https://example.com" + command := "PATH=.:$PATH git push gitlawb://example.com/repo.git main" if runtime.GOOS == "windows" { if windowsTestUsesPowerShell() { - command = `$env:PATH = '.;' + $env:PATH; curl.cmd https://example.com` + command = `$env:PATH = '.;' + $env:PATH; git.cmd push gitlawb://example.com/repo.git main` } else { - command = "set PATH=.;%PATH% && curl https://example.com" + command = "set PATH=.;%PATH% && git push gitlawb://example.com/repo.git main" } - fakeCurl := filepath.Join(root, "curl.cmd") - if err := os.WriteFile(fakeCurl, []byte("@echo fake curl %*\r\n"), 0o755); err != nil { + fakeGit := filepath.Join(root, "git.cmd") + if err := os.WriteFile(fakeGit, []byte("@echo fake git %*\r\n"), 0o755); err != nil { t.Fatal(err) } } else { - fakeCurl := filepath.Join(root, "curl") - if err := os.WriteFile(fakeCurl, []byte("#!/bin/sh\necho fake curl \"$@\"\n"), 0o755); err != nil { + fakeGit := filepath.Join(root, "git") + if err := os.WriteFile(fakeGit, []byte("#!/bin/sh\necho fake git \"$@\"\n"), 0o755); err != nil { t.Fatal(err) } } + engine := sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.DefaultPolicy(), + Backend: sandbox.Backend{Name: sandbox.BackendUnavailable, Message: "native sandbox unavailable"}, + }) + var overlay []sandbox.NetworkMode registry := tools.NewRegistry() - registry.Register(tools.NewScopedBashTool(root, nil)) + registry.Register(&networkOverlayProbeTool{ + delegate: tools.NewScopedBashTool(root, nil), + engine: engine, + observed: &overlay, + t: t, + }) provider := &mockProvider{ turns: [][]zeroruntime.StreamEvent{ { @@ -2478,15 +2489,11 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) { } var requests []PermissionRequest var events []PermissionEvent - result, err := Run(context.Background(), "curl", provider, Options{ + result, err := Run(context.Background(), "push changes", provider, Options{ Registry: registry, PermissionMode: PermissionModeAsk, Autonomy: "medium", - Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ - WorkspaceRoot: root, - Policy: sandbox.DefaultPolicy(), - Backend: sandbox.Backend{Name: sandbox.BackendUnavailable, Message: "native sandbox unavailable"}, - }), + Sandbox: engine, OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { requests = append(requests, request) return PermissionDecision{Action: PermissionDecisionAllow, Reason: "approve network once"}, nil @@ -2520,9 +2527,62 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) { t.Fatalf("expected tool result to be sent back to provider, got %d requests", len(provider.requests)) } lastMessage := provider.requests[1].Messages[len(provider.requests[1].Messages)-1] - if !strings.Contains(lastMessage.Content, "fake curl https://example.com") { + if !strings.Contains(lastMessage.Content, "fake git push gitlawb://example.com/repo.git main") { t.Fatalf("expected approved network command output after degraded execution, got %q", lastMessage.Content) } + // The command running is not proof the GRANT was applied: under + // BackendUnavailable the degraded path executes regardless. Assert the turn + // overlay itself — the approval must have widened the plan's policy to + // NetworkAllow while the tool ran, and must not outlive the turn. + if len(overlay) != 1 { + t.Fatalf("expected one recorded overlay observation, got %#v", overlay) + } + if overlay[0] != sandbox.NetworkAllow { + t.Fatalf("plan policy network = %q during the approved call, want %q", overlay[0], sandbox.NetworkAllow) + } + if after := planNetworkMode(t, engine); after == sandbox.NetworkAllow { + t.Fatalf("turn network grant outlived the turn: policy network = %q", after) + } +} + +// planNetworkMode reads the network mode the engine would actually build a +// command plan with, which is where a turn grant's overlay becomes observable. +func planNetworkMode(t *testing.T, engine *sandbox.Engine) sandbox.NetworkMode { + t.Helper() + + plan, err := engine.BuildCommandPlan(sandbox.CommandSpec{Name: "git", Args: []string{"push"}}) + if err != nil { + t.Fatalf("BuildCommandPlan returned error: %v", err) + } + return sandbox.NormalizeNetworkMode(plan.Policy.Network) +} + +// networkOverlayProbeTool is a bash-named shell tool that records the network +// mode in effect at the moment the approved call runs, then delegates to the +// real scoped bash tool so the command still executes. +type networkOverlayProbeTool struct { + delegate tools.Tool + engine *sandbox.Engine + observed *[]sandbox.NetworkMode + t *testing.T +} + +func (tool *networkOverlayProbeTool) Name() string { return tool.delegate.Name() } +func (tool *networkOverlayProbeTool) Description() string { return tool.delegate.Description() } +func (tool *networkOverlayProbeTool) Parameters() tools.Schema { + return tool.delegate.Parameters() +} + +func (tool *networkOverlayProbeTool) Safety() tools.Safety { + if safe, ok := tool.delegate.(interface{ Safety() tools.Safety }); ok { + return safe.Safety() + } + return tools.Safety{SideEffect: tools.SideEffectShell, Permission: tools.PermissionPrompt} +} + +func (tool *networkOverlayProbeTool) Run(ctx context.Context, args map[string]any) tools.Result { + *tool.observed = append(*tool.observed, planNetworkMode(tool.t, tool.engine)) + return tool.delegate.Run(ctx, args) } func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) { @@ -2568,6 +2628,62 @@ func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) { } } +func TestRunDoesNotOfferPrefixApprovalForCaseDistinctGitAlias(t *testing.T) { + root := t.TempDir() + command := `make test && git -c alias.STATUS=!curl STATUS https://example.invalid` + retryTool := &sandboxDeniedRetryTool{} + registry := tools.NewRegistry() + registry.Register(retryTool) + provider := &mockProvider{ + turns: [][]zeroruntime.StreamEvent{ + { + {Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "bash"}, + {Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"command":` + quoteJSONString(command) + `}`}, + {Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"}, + {Type: zeroruntime.StreamEventDone}, + }, + { + {Type: zeroruntime.StreamEventText, Content: "done"}, + {Type: zeroruntime.StreamEventDone}, + }, + }, + } + requestCount := 0 + + _, err := Run(context.Background(), "inspect status", provider, Options{ + Registry: registry, + PermissionMode: PermissionModeAsk, + Autonomy: "medium", + Sandbox: sandbox.NewEngine(sandbox.EngineOptions{ + WorkspaceRoot: root, + Policy: sandbox.DefaultPolicy(), + Backend: sandbox.Backend{Name: sandbox.BackendUnavailable, Message: "native sandbox unavailable"}, + }), + OnPermissionRequest: func(_ context.Context, request PermissionRequest) (PermissionDecision, error) { + requestCount++ + if len(request.CommandPrefix) != 0 { + t.Fatalf("case-distinct alias must not have a command prefix: %#v", request.CommandPrefix) + } + if containsPermissionDecision(request.AvailableDecisions, PermissionDecisionAllowPrefix) || + containsPermissionDecision(request.AvailableDecisions, PermissionDecisionAlwaysAllowPrefix) { + t.Fatalf("case-distinct alias must not offer reusable prefix approval: %#v", request.AvailableDecisions) + } + return PermissionDecision{Action: PermissionDecisionDeny, Reason: "deny alias execution"}, nil + }, + }) + if err != nil { + t.Fatal(err) + } + if requestCount != 1 { + t.Fatalf("permission requests = %d, want one non-prefix request", requestCount) + } + for index, call := range retryTool.calls { + if shellCommandRequiresEscalated(call) { + t.Fatalf("call %d received require_escalated: %#v", index, call) + } + } +} + func TestRunPromptsForDestructiveShellInsteadOfSandboxDeny(t *testing.T) { root := t.TempDir() command := "echo rm -rf /" diff --git a/internal/sandbox/analyzer.go b/internal/sandbox/analyzer.go index 7973fa951..d9ab3cea7 100644 --- a/internal/sandbox/analyzer.go +++ b/internal/sandbox/analyzer.go @@ -39,7 +39,8 @@ var powerShellRemoveItemPrograms = map[string]bool{ // networkPrograms are commands that perform network egress/ingress. var networkPrograms = map[string]bool{ - "curl": true, "wget": true, "ssh": true, "scp": true, "sftp": true, + "curl": true, "wget": true, "fetch": true, "aria2c": true, + "ssh": true, "scp": true, "sftp": true, "rsync": true, "nc": true, "ncat": true, "netcat": true, "telnet": true, "ftp": true, "iwr": true, "irm": true, "invoke-webrequest": true, "invoke-restmethod": true, @@ -159,9 +160,9 @@ func analyzeInto(script string, result *AnalysisResult, seen map[string]bool, de if !ok || len(call.Args) == 0 { return true } - // Resolve the real program behind wrapper prefixes (sudo, env, nice, ...) - // so `sudo rm -rf`, `env curl …`, and `bash -c 'vim x'` are classified on - // the payload, not the launcher — matching DetectInteractiveCommand. + if resolveASTCommandNetwork(call.Args, depth).needsNetworkGate() { + result.Network = true + } prog, rest := effectiveProgram(call.Args) if prog == "" { return true @@ -170,75 +171,180 @@ func analyzeInto(script string, result *AnalysisResult, seen map[string]bool, de seen[prog] = true result.Programs = append(result.Programs, prog) } - // `sh -c ` runs the payload as a fresh command; recurse into it so - // a program hidden behind a shell launcher is still classified. - if depth < maxAnalyzerDepth && shellPrograms[prog] { - if payload := dashCPayload(rest); payload != "" { - analyzeInto(payload, result, seen, depth+1) - } - } if _, interactive := interactivePrograms[prog]; interactive && !replSuppressed(prog, rest) { result.Interactive = true } - if commandUsesNetwork(prog, rest) { - result.Network = true - } if destructivePrograms[prog] || (prog == "rm" && hasRecursiveForce(rest)) || (powerShellRemoveItemPrograms[prog] && hasPowerShellRecursiveForce(rest)) || (prog == "find" && hasFindDelete(rest)) { result.Destructive = true } + if shellPrograms[prog] { + if index, found := shellCommandPayloadIndex(prog, wordTexts(rest)); found && index < len(rest) && + isLiteralWord(rest[index]) && depth < maxAnalyzerDepth { + analyzeInto(wordText(rest[index]), result, seen, depth+1) + } + } return true }) } -func commandUsesNetwork(prog string, args []*syntax.Word) bool { - if networkPrograms[prog] { - return true +// resolveASTCommandNetwork maps shell AST words onto the same tri-state argv +// resolver used by the unparseable fallback. Literal invocations take exactly +// one path. For dynamic words, the literal portions still establish known +// network programs, while executable or interpreter-source expansions remain +// unresolved and therefore keep the network gate. +func resolveASTCommandNetwork(words []*syntax.Word, depth int) commandResolution { + if depth > maxAnalyzerDepth { + return commandUnresolved } - words := literalWordTexts(args) - if localServerPrograms[prog] { - return true + if fields, ok := literalCallFields(words); ok { + program, args := effectiveProgram(words) + switch program { + case "cmd", "call", "start", "%comspec%": + return commandNetworkResolution(cmdLauncherUsesNetwork(program, args, depth)) + default: + return resolveCommandArgv(fields, depth) + } + } + if envSplitSourceDynamic(words) { + return commandUnresolved + } + program, args := effectiveProgram(words) + if program == "" { + return commandUnresolved + } + textArgs := wordTexts(args) + resolution := resolveCommandArgv(append([]string{program}, textArgs...), depth) + switch { + case shellPrograms[program]: + index, found := shellCommandPayloadIndex(program, textArgs) + if !found || index >= len(args) { + return resolution + } + if !isLiteralWord(args[index]) { + return commandUnresolved + } + case program == "powershell" || program == "pwsh": + source := fallbackPowerShellPayload(program, textArgs) + if source.opaque || powerShellSourceDynamic(source, args) { + return commandUnresolved + } + case program == "eval": + if _, ok := literalCallFields(args); !ok { + return commandUnresolved + } + case program == "cmd" || program == "call" || program == "start" || program == "%comspec%": + if cmdLauncherUsesNetwork(program, args, depth) { + return commandKnownNetwork + } + case program == "busybox" && busyboxSourceDynamic(args): + return commandUnresolved + case program == "strace" && straceSourceDynamic(args): + return commandUnresolved + case program == "git" && resolution == commandKnownLocal: + if gitSelectionDynamic(textArgs, func(index int) bool { return !isLiteralWord(args[index]) }) { + return commandUnresolved + } + } + return resolution +} + +// gitSelectionDynamic reports whether an expansion this scan cannot read could +// choose what git actually does: the SUBCOMMAND itself, or `git archive`'s +// --remote, the one option that turns a local tree export into a request to +// another host. +// +// wordText drops an expansion, so the reconstructed argv shows a subcommand git +// may never run: `git $VERB origin main` reads as `git origin main` — an +// unrecognized, therefore local, subcommand — while the shell runs `git push`. +// A dynamic word BEFORE the subcommand counts too, because it can shift which +// token git reads as the subcommand: an empty expansion or one that word-splits +// changes the position, even as the value of a global option that normally +// consumes it. +// +// Past the subcommand nothing else can change the answer, so ordinary forms like +// `git commit -m "$MESSAGE"` stay proven-local and keep their quiet path. +// +// dynamic reports whether the word at an index is unreadable. The AST path +// supplies isLiteralWord and the fallback supplies fallbackTokenLooksDynamic, so +// neither can classify a shape the other fails closed on. +func gitSelectionDynamic(words []string, dynamic func(index int) bool) bool { + invocation := parseGitInvocation(words) + limit := len(words) + if invocation.kind == gitCommandSubcommand { + limit = min(invocation.subcommandIndex+1, len(words)) + } + for index := range limit { + if dynamic(index) { + return true + } + } + if invocation.kind != gitCommandSubcommand || invocation.subcommand != "archive" { + return false + } + for index := invocation.subcommandIndex + 1; index < len(words); index++ { + if dynamic(index) { + return true + } + } + return false +} + +// literalProgramNetworkResolution classifies a program after launcher argv has +// been resolved. Wrapper, delegated-child, and interpreter-source grammars live +// in resolveCommandArgv so AST and fallback paths cannot select different +// children before reaching this program-specific table. +func literalProgramNetworkResolution(prog string, words []string) commandResolution { + originalWords := words + prog = normalizeProgramToken(trimCMDEchoPrefixToken(prog)) + normalized := make([]string, len(words)) + for index := range words { + normalized[index] = strings.ToLower(strings.TrimSpace(words[index])) + } + words = normalized + if networkPrograms[prog] || localServerPrograms[prog] { + return commandKnownNetwork } switch prog { case "python", "python2", "python3", "py": - return pythonModuleUsesNetwork(words) + return commandNetworkResolution(pythonModuleUsesNetwork(words)) case "npm": - return packageManagerUsesNetwork(words, map[string]string{ + return commandNetworkResolution(packageManagerUsesNetwork(words, map[string]string{ "run": "run", "exec": "exec", "x": "exec", - }) + })) case "pnpm": - return packageManagerUsesNetwork(words, map[string]string{ + return commandNetworkResolution(packageManagerUsesNetwork(words, map[string]string{ "run": "run", "exec": "exec", "dlx": "exec", - }) + })) case "yarn": - return packageManagerUsesNetwork(words, map[string]string{ + return commandNetworkResolution(packageManagerUsesNetwork(words, map[string]string{ "run": "run", "exec": "exec", "dlx": "exec", - }) + })) case "bun": - return packageManagerUsesNetwork(words, map[string]string{ + return commandNetworkResolution(packageManagerUsesNetwork(words, map[string]string{ "run": "run", "x": "exec", - }) + })) case "npx": - return npxUsesNetwork(words) + return commandNetworkResolution(npxUsesNetwork(words)) case "pip", "pip2", "pip3": - return firstSubcommand(words, nil) == "install" + return commandNetworkResolution(firstSubcommand(words, nil) == "install") case "go": - return firstSubcommand(words, nil) == "get" + return commandNetworkResolution(firstSubcommand(words, nil) == "get") case "git": - return gitUsesNetwork(words) + return gitNetworkResolution(originalWords) case "gh": - return ghUsesNetwork(words) + return commandNetworkResolution(ghUsesNetwork(words)) default: - return false + return commandKnownLocal } } @@ -277,27 +383,603 @@ func packageManagerOffline(words []string) bool { return false } -func gitUsesNetwork(words []string) bool { - switch firstSubcommand(words, nil) { - case "clone", "fetch", "pull", "push", "ls-remote", "archive": +func gitNetworkResolution(words []string) commandResolution { + invocation := parseGitInvocation(words) + if invocation.kind != gitCommandSubcommand { + // No subcommand at all, or a global option that makes git print locally + // and exit before any subcommand runs. + return commandKnownLocal + } + // Git builtin names and aliases are case-sensitive. A case-distinct spelling + // such as STATUS can therefore select an arbitrary alias even though status + // is a known local builtin. Classification may normalize to recognize known + // builtins, but it must not report an alias-shaped invocation as proven local. + if invocation.subcommandOriginal != invocation.subcommand { + return commandUnresolved + } + switch invocation.subcommand { + // send-pack is push's plumbing counterpart — `git send-pack origin main` + // performs exactly the egress `git push` does, so leaving it out gave the + // same operation two different answers depending on which spelling was + // used. + case "clone", "fetch", "pull", "push", "ls-remote", "send-pack": + return commandKnownNetwork + case "archive": + // `git archive HEAD` streams a tree out of the local object store and needs + // no egress at all; only `--remote=` sends the request to another + // host. Classifying every archive as network cost a proactive network + // prompt on a purely local command. + return commandNetworkResolution(gitTargetsRemoteArchive(words, invocation.subcommandIndex)) + default: + return commandKnownLocal + } +} + +// gitTargetsRemoteArchive reports whether an archive subcommand has an active +// --remote option. Git accepts archive options after positional operands, so +// this must keep scanning until an unconsumed `--`. Value-taking options are +// consumed first: in `archive -o -- --remote=origin HEAD`, `-o` owns the first +// `--` and the later remote is active; in `archive -o --remote HEAD`, --remote +// is only the output filename. +func gitTargetsRemoteArchive(words []string, subcommandIndex int) bool { + for index := subcommandIndex + 1; index < len(words); index++ { + word := strings.ToLower(words[index]) + switch { + case word == "--": + return false + case strings.HasPrefix(word, "--remote="): + return true + case word == "--remote": + return index+1 < len(words) + case gitArchivePreliminaryOptionConsumesValue(word): + index++ + } + } + return false +} + +// gitArchivePreliminaryOptionConsumesValue models the first parse-options pass +// in git archive. That pass consumes only transport/output options and retains +// format/prefix/mtime options for a later parser, so those later options must +// not hide an immediately following --remote. +func gitArchivePreliminaryOptionConsumesValue(option string) bool { + if strings.Contains(option, "=") { + return false + } + switch option { + case "-o", "--output", "--exec": return true default: return false } } +// gitCommandKind distinguishes the three outcomes of reading a git command +// line, which callers must treat differently. +type gitCommandKind int + +const ( + // gitCommandNone: no subcommand was found (`git`, `git -C repo`). + gitCommandNone gitCommandKind = iota + // gitCommandTerminalGlobal: a global option that makes git print locally + // and exit — no subcommand runs, whatever words follow it. + gitCommandTerminalGlobal + // gitCommandSubcommand: a subcommand git will actually execute. + gitCommandSubcommand +) + +type gitInvocation struct { + kind gitCommandKind + // subcommand is set only for gitCommandSubcommand. + subcommand string + // subcommandOriginal preserves the exact token Git will dispatch. Git alias + // lookup is case-sensitive, so authorization callers must not replace it + // with subcommand's normalized classifier spelling. + subcommandOriginal string + // subcommandIndex is the position in the original argument slice. + subcommandIndex int + // terminalOption is set only for gitCommandTerminalGlobal. + terminalOption string +} + +// gitTerminalGlobalOptions are git's global options that print something from +// the local installation and exit. Everything after one of them is help/version +// output text, not a command: `git -C repo --help push` prints git-push's +// manual page without contacting a remote, so a subcommand scan that walked +// past them would classify a purely local command as network. +// Bare `--exec-path` belongs here for the same reason: git documents it as +// `--exec-path[=]`, so without an inline value it prints the compiled-in +// exec path and exits. `git --exec-path /tmp push` neither reads /tmp as the +// option's value nor runs push, so treating it as a value-taking option made a +// local informational command request egress. +var gitTerminalGlobalOptions = map[string]bool{ + "-h": true, "--help": true, + "-v": true, "--version": true, + "--html-path": true, "--man-path": true, "--info-path": true, + "--list-cmds": true, "--exec-path": true, +} + +// parseGitInvocation resolves what a git command line actually does, past git's +// GLOBAL options. It is the single reader both classification paths use — the +// AST path through gitUsesNetwork and the unparseable fallback through +// matchesUnparseableGitNetwork — so the two cannot disagree about an option, as +// they did while each carried its own skip list. +// +// The generic firstSubcommand cannot do this job: git's value-taking globals put +// their value in the next token, so scanning for the first non-dash token returns +// that value instead — `git -C repo push origin main` looked like the subcommand +// "repo" and so classified as no-network, dropping the proactive network prompt +// for the most common form of the command. internal/agent/command_prefix.go +// resolves the same option set for its own prefix matching. +func parseGitInvocation(words []string) gitInvocation { + for index := 0; index < len(words); index++ { + original := words[index] + word := strings.ToLower(original) + if word == "" { + continue + } + if gitTerminalGlobalOptions[word] || strings.HasPrefix(word, "--list-cmds=") { + return gitInvocation{kind: gitCommandTerminalGlobal, terminalOption: word} + } + if strings.HasPrefix(word, "-") { + // A joined value (--git-dir=/x, -C/x) is one token and needs no skip; + // a separated one puts its value in the next token. + if GitGlobalOptionConsumesValue(word) { + index++ + } + continue + } + if isNumericToken(word) { + continue + } + return gitInvocation{ + kind: gitCommandSubcommand, + subcommand: word, + subcommandOriginal: original, + subcommandIndex: index, + } + } + return gitInvocation{kind: gitCommandNone} +} + +// GitGlobalOptionConsumesValue lists git's global options whose value is a +// separate token. It is shared with internal/agent's command-prefix parser so +// the two security-sensitive scans cannot drift — internal/agent reaches this +// through GitSubcommand, which resolves the whole grammar (terminal globals +// included) in one place, rather than importing the terminal-global table +// separately. +// +// `--exec-path` is deliberately absent: its value is inline-only +// (`--exec-path=`), and the bare spelling is terminal — see +// gitTerminalGlobalOptions. +func GitGlobalOptionConsumesValue(option string) bool { + // The comparison is on the NORMALIZED name, so every case below must be + // written lowercase — an uppercase case here would be unreachable rather + // than protective. -c (config override, `-c name=value`) and -C (run as if + // started in ) are different git options that both take a + // separate-token value, so folding them onto one case is correct today and + // `git -C repo push` consumes repo either way. + // + // If their value-taking behavior ever diverges, this cannot be fixed by + // re-adding a -C case: the callers normalize before calling, so the + // distinction has to be restored at the call site by passing the original + // token. TestGitGlobalOptionConsumesValueFoldsShortCaseDeliberately pins + // the fold so that change is a deliberate one. + switch strings.ToLower(option) { + case "-c", "--attr-source", "--config-env", "--git-dir", "--namespace", "--super-prefix", "--work-tree": + return true + default: + return false + } +} + +// GitSubcommandInfo is the shared result of resolving Git's global-option +// grammar. Original is the exact token Git dispatches; Normalized is for +// conservative classification only. Authorization must compare Original +// against an explicit allowlist because Git alias lookup is case-sensitive. +type GitSubcommandInfo struct { + Index int + Original string + Normalized string +} + +// GitSubcommand resolves the git subcommand a command line will actually run, +// or reports that none runs — no subcommand is present (e.g. bare `git`, or +// `git -C repo` with nothing after it), or a terminal global like --help/ +// --version made everything after it non-executed help/version text instead. +func GitSubcommand(words []string) (GitSubcommandInfo, bool) { + invocation := parseGitInvocation(words) + if invocation.kind != gitCommandSubcommand { + return GitSubcommandInfo{}, false + } + return GitSubcommandInfo{ + Index: invocation.subcommandIndex, + Original: invocation.subcommandOriginal, + Normalized: invocation.subcommand, + }, true +} + func npxUsesNetwork(_ []string) bool { return true } -func literalWordTexts(args []*syntax.Word) []string { +func wordTexts(args []*syntax.Word) []string { words := make([]string, 0, len(args)) for _, arg := range args { - words = append(words, strings.ToLower(strings.TrimSpace(wordText(arg)))) + words = append(words, strings.TrimSpace(wordText(arg))) } return words } +// powerShellSourceDynamic reports whether the words carrying a PowerShell +// host's command source contain an expansion. wordText silently drops those, +// so `powershell -Command "$PAYLOAD"` otherwise extracts an empty payload and +// classifies as clean while the host runs whatever the shell expanded. +func powerShellSourceDynamic(source powerShellPayload, args []*syntax.Word) bool { + if source.sourceIndex < 0 { + return false + } + for index := source.sourceIndex; index < len(args); index++ { + if !isLiteralWord(args[index]) { + return true + } + } + return false +} + +// envSplitSourceDynamic reports whether an env invocation takes its +// -S/--split-string argv from a word this scan cannot resolve statically. +// +// The literal reconstruction used by envSplitCommandFields is an optimization, +// not a proof of safety: `PAYLOAD='curl https://…'; env -S "$PAYLOAD"` runs the +// expanded argv, so an unreadable operand must classify as network-sensitive +// rather than fall through to wrapper handling that reports no executable. +func envSplitSourceDynamic(args []*syntax.Word) bool { + texts := make([]string, len(args)) + for index, arg := range args { + texts[index] = wordText(arg) + } + start, ok := envArgumentStart(texts) + if !ok { + return false + } + seenSplit := false + for index := start; index < len(texts); index++ { + text := texts[index] + if text == "--" { + return false + } + if _, _, _, split := envSplitOption([]string{text}, 0); split { + // A joined operand (-S"$PAYLOAD") reconstructs to the option alone, so + // the option token itself carries the dynamic source. + if !isLiteralWord(args[index]) { + return true + } + seenSplit = true + continue + } + if !isLiteralWord(args[index]) { + if seenSplit { + return true + } + continue + } + if seenSplit { + // The -S operand is literal, but that is only proof about the SPLIT + // STRING, not about the invocation: GNU env appends the remaining + // argv to the argv the split string produced, so + // `env -S 'sh -c' "$PAYLOAD"` runs an argument this scan cannot + // read. Keep scanning; a dynamic token anywhere after the split + // string extends the command unknowably and fails closed. Trailing + // LITERAL tokens are fine — the ordinary literal path reads the + // whole reconstructed argv, this one included. + for rest := index + 1; rest < len(args); rest++ { + if !isLiteralWord(args[rest]) { + return true + } + } + return false + } + if strings.Contains(text, "=") && !strings.HasPrefix(text, "=") && !strings.HasPrefix(text, "-") { + continue + } + if strings.HasPrefix(text, "-") { + if wrapperConsumesValue("env", text) && index+1 < len(texts) { + index++ + } + continue + } + // A command position was reached without a split string; ordinary wrapper + // resolution classifies this invocation. + return false + } + return false +} + +// busyboxSourceDynamic reports whether a BusyBox invocation's applet-name +// operand — the token busyboxCommandArgs treats as the delegated child +// executable — comes from a word this scan cannot resolve statically. +// +// busyboxCommandArgs runs on wordTexts, which silently drops expansions: +// `APPLET=curl; busybox "$APPLET" https://…` would otherwise resolve the +// applet position to an empty string, which is neither a recognized BusyBox +// flag nor the executable this scan can name, and the invocation reads as an +// ordinary unrecognized command rather than as "unknown, assume network." +func busyboxSourceDynamic(args []*syntax.Word) bool { + if len(args) == 0 { + return false + } + return !isLiteralWord(args[0]) +} + +// straceSourceDynamic reports whether the delegated argv loses information in +// the literal reconstruction passed to resolveCommandArgv. This includes both a +// dynamic child executable and dynamic env -S source below a literal env child. +// straceChildIndex is shared with straceCommandArgs so this check walks strace's +// option grammar exactly once rather than duplicating it. +func straceSourceDynamic(args []*syntax.Word) bool { + index, ok := straceChildIndex(wordTexts(args)) + if !ok || index >= len(args) { + return false + } + if !isLiteralWord(args[index]) { + return true + } + return envSplitSourceDynamic(args[index:]) +} + +// envArgumentStart returns the index just past an `env` program token, allowing +// the wrapper prefixes (sudo, nice, ...) that may precede it. +func envArgumentStart(texts []string) (int, bool) { + wrapper := "" + for index := 0; index < len(texts); index++ { + text := texts[index] + if text == "" { + if wrapper == "" { + return 0, false + } + continue + } + if strings.Contains(text, "=") && !strings.HasPrefix(text, "=") && !strings.HasPrefix(text, "-") { + continue + } + if strings.HasPrefix(text, "-") { + if wrapperConsumesValue(wrapper, text) && index+1 < len(texts) { + index++ + } + continue + } + if isNumericToken(text) { + continue + } + token := normalizeProgramToken(text) + if token == "env" { + return index + 1, true + } + if wrapperPrograms[token] { + wrapper = token + continue + } + return 0, false + } + return 0, false +} + +func literalCallFields(args []*syntax.Word) ([]string, bool) { + fields := make([]string, 0, len(args)) + for _, arg := range args { + if !isLiteralWord(arg) { + return nil, false + } + fields = append(fields, wordText(arg)) + } + return fields, true +} + +func cmdLauncherUsesNetwork(program string, args []*syntax.Word, depth int) bool { + switch program { + case "cmd", "call", "start", "%comspec%": + default: + return false + } + tokens := make([]fallbackCommandToken, 0, len(args)+1) + tokens = append(tokens, fallbackCommandToken{value: program}) + for _, arg := range args { + if !isLiteralWord(arg) { + return true + } + tokens = append(tokens, fallbackCommandToken{ + value: wordText(arg), + quoted: literalWordIsQuoted(arg), + }) + } + for _, body := range cmdCommandBodyTokenInfoCandidates(tokens) { + if cmdBodyUsesNetwork(body, depth) { + return true + } + } + return false +} + +// cmdBodyUsesNetwork classifies one CMD payload candidate. +// +// A single QUOTED token is genuinely ambiguous: `cmd /c "git push origin main"` +// is a command line, while `cmd /c "C:\Program Files\curl.exe"` is one program +// path that happens to contain a space. CMD itself resolves that by trying the +// path first and falling back to parsing it as a command line, so this +// classifies it BOTH ways and fails closed if either reading reaches the +// network. Treating it only as a program name — taking its basename — is what +// made `git push origin main` look like an unrecognized executable. +func cmdBodyUsesNetwork(body []fallbackCommandToken, depth int) bool { + if len(body) == 0 { + return false + } + if fallbackBodyUsesNetwork(fallbackTokenValues(body), depth) { + return true + } + if len(body) == 1 && body[0].quoted && strings.ContainsAny(body[0].value, " \t") { + return classifyInterpreterSource(body[0].value, interpreterSourceCMD, depth+1).needsNetworkGate() + } + return false +} + +func literalWordIsQuoted(word *syntax.Word) bool { + if word == nil || len(word.Parts) == 0 { + return false + } + switch word.Parts[0].(type) { + case *syntax.SglQuoted, *syntax.DblQuoted: + return true + default: + return false + } +} + +// classifyCommandText is the single entry point for interpreter SOURCE — a +// string another program will run as a command line — as opposed to argv +// tokens naming a program to execute. It is the difference between +// `cmd /c "git push origin main"` meaning "run the command line git push +// origin main" and meaning "run the program named `git push origin main`"; +// reading the second as the first is what let quoted one-liners through. +// +// Every launcher that carries command text routes through here: CMD /c and +// /k, CALL, start, eval, shell -c/--command, PowerShell -Command, and the +// fallback tokenizer's equivalents. It re-tokenizes the payload rather than +// taking its basename, runs BOTH the AST scan and the unparseable matcher (a +// CMD one-liner is legitimately not POSIX, so the parser failing on it is +// expected, not proof of safety), and fails closed when the text cannot be +// read at all. +type interpreterSourceLanguage uint8 + +const ( + interpreterSourcePOSIX interpreterSourceLanguage = iota + interpreterSourceCMD + interpreterSourcePowerShell +) + +// classifyInterpreterSource is the single bounded classifier for text a +// launcher will execute. Language-specific syntax that the shared parser cannot +// faithfully interpret is unresolved rather than silently treated as local. +func classifyInterpreterSource(payload string, language interpreterSourceLanguage, depth int) commandResolution { + payload = strings.TrimSpace(payload) + if payload == "" { + return commandKnownLocal + } + if depth > maxAnalyzerDepth { + return commandUnresolved + } + if language == interpreterSourcePowerShell { + if unreadable := powerShellSourceUnreadable(payload); unreadable != "" { + return commandUnresolved + } + if resolution, found := classifyPowerShellExpressionEvaluation(payload, depth); found { + return resolution + } + } + result := AnalysisResult{} + analyzeInto(payload, &result, map[string]bool{}, depth) + if result.Network || matchesUnparseableNetworkAt(payload, depth) { + return commandKnownNetwork + } + return commandKnownLocal +} + +// powerShellBlockPunctuation opens or closes a PowerShell script block or +// statement block. Every block-structured form the language has — try/catch/ +// finally, if/else, foreach, for, while, do, switch, function, trap, param, and +// a script block handed to ForEach-Object or the call operator — is written +// with braces, so their presence is the single signal that the source has +// structure the readers below do not model. +const powerShellBlockPunctuation = "{}" + +// powerShellSourceUnreadable reports why PowerShell source cannot be proven +// local by the readers classifyInterpreterSource runs, or "" when it can. +// +// Those readers are the POSIX AST scan and the unparseable-command matcher. +// Neither models PowerShell grammar. They happen to read a simple +// `Verb-Noun -Arg value` line correctly because it tokenizes like a POSIX +// command, and that precision is worth keeping — but "the POSIX reader found no +// network program in this PowerShell source" is not evidence the source is +// local. `try { Invoke-WebRequest https://host } catch {}` is valid PowerShell +// that performs the request, and neither reader models try/catch, so it drew no +// network category at all. +// +// A conservative unresolved result is the contract here: only source a reader +// can actually parse and prove local may skip the gate. Unresolved costs a +// prompt on a shell request, not a denial, so the failure mode of being wrong +// in this direction is a question rather than a broken command. +func powerShellSourceUnreadable(payload string) string { + if strings.ContainsRune(payload, '`') { + // Backtick escaping and line continuation change PowerShell token + // boundaries but have no equivalent in the POSIX/CMD readers. + return "backtick escaping" + } + if strings.ContainsAny(payload, powerShellBlockPunctuation) { + return "script or statement block" + } + return "" +} + +// classifyPowerShellExpressionEvaluation handles the PowerShell edge that +// executes a string as a second PowerShell program. The shared POSIX reader can +// locate a simple Invoke-Expression/iex call, but it cannot infer source from an +// expression that constructs the argument. Only one literal single-quoted +// operand is therefore recursively classified; every other evaluator shape is +// unresolved and retains the network gate. +func classifyPowerShellExpressionEvaluation(payload string, depth int) (commandResolution, bool) { + file, err := syntax.NewParser().Parse(strings.NewReader(payload), "") + if err != nil { + return commandUnresolved, powerShellExpressionEvaluatorToken(payload) + } + + found := false + resolution := commandKnownLocal + syntax.Walk(file, func(node syntax.Node) bool { + call, ok := node.(*syntax.CallExpr) + if !ok || len(call.Args) == 0 || !isPowerShellExpressionEvaluator(wordText(call.Args[0])) { + return true + } + found = true + if len(call.Args) != 2 { + resolution = commandUnresolved + return true + } + parts := call.Args[1].Parts + if len(parts) != 1 { + resolution = commandUnresolved + return true + } + quoted, ok := parts[0].(*syntax.SglQuoted) + if !ok { + resolution = commandUnresolved + return true + } + nested := classifyInterpreterSource(quoted.Value, interpreterSourcePowerShell, depth+1) + if nested == commandKnownNetwork || resolution == commandKnownNetwork { + resolution = commandKnownNetwork + } else if nested == commandUnresolved { + resolution = commandUnresolved + } + return true + }) + return resolution, found +} + +func powerShellExpressionEvaluatorToken(payload string) bool { + for _, command := range fallbackCommandTokenInfo(payload) { + for _, token := range command { + if !token.quoted && isPowerShellExpressionEvaluator(token.value) { + return true + } + } + } + return false +} + +func isPowerShellExpressionEvaluator(token string) bool { + return strings.EqualFold(token, "Invoke-Expression") || strings.EqualFold(token, "iex") +} + func pythonModuleUsesNetwork(words []string) bool { for index := 0; index < len(words); index++ { if words[index] != "-m" || index+1 >= len(words) { @@ -357,6 +1039,45 @@ func firstSubcommand(words []string, aliases map[string]string) string { // wordText returns the literal text of a shell word, concatenating its plain and // quoted literal parts (so "vim", 'vim', and vim all yield "vim"). Parts that are // expansions ($x, $(...)) contribute nothing — the program name is taken as-is. +// unescapeDoubleQuoted applies POSIX double-quote escape removal to the literal +// text the parser preserves verbatim inside a double-quoted word. +// +// The parser keeps `\"` as two characters because escape removal is the +// shell's job at expansion time, not the parser's. For an argv token that is +// harmless; for the `-c` operand of a shell launcher it is not, because the +// text IS the next command. Without this, one level of +// `sh -c "sh -c \"curl …\""` handed the recursion the fragment `\"sh` — the +// rest of the payload silently dropped — and the nested curl disappeared from +// classification entirely at nesting level 3. +// +// Inside double quotes a backslash is special ONLY before $, `, ", \, and a +// newline; everywhere else it is a literal backslash and must be preserved, or +// a Windows path inside quotes would lose its separators. +func unescapeDoubleQuoted(value string) string { + if !strings.Contains(value, `\`) { + return value + } + var builder strings.Builder + builder.Grow(len(value)) + for index := 0; index < len(value); index++ { + if value[index] != '\\' || index+1 >= len(value) { + builder.WriteByte(value[index]) + continue + } + switch next := value[index+1]; next { + case '$', '`', '"', '\\': + builder.WriteByte(next) + index++ + case '\n': + // A quoted line continuation removes both characters. + index++ + default: + builder.WriteByte('\\') + } + } + return builder.String() +} + func wordText(word *syntax.Word) string { if word == nil { return "" @@ -371,7 +1092,7 @@ func wordText(word *syntax.Word) string { case *syntax.DblQuoted: for _, inner := range typed.Parts { if lit, ok := inner.(*syntax.Lit); ok { - builder.WriteString(lit.Value) + builder.WriteString(unescapeDoubleQuoted(lit.Value)) } } } @@ -423,17 +1144,6 @@ func effectiveProgram(args []*syntax.Word) (string, []*syntax.Word) { return "", nil } -// dashCPayload returns the literal text of the word following `-c` in an AST arg -// list (the command a shell launcher will run), or "" when there is none. -func dashCPayload(args []*syntax.Word) string { - for index := 0; index < len(args); index++ { - if wordText(args[index]) == "-c" && index+1 < len(args) { - return wordText(args[index+1]) - } - } - return "" -} - // replSuppressed reports whether a REPL program (python/node/...) was invoked // non-interactively — with an inline-eval flag or a script argument — mirroring // nonInteractiveREPLFlags used by the regex detector. Non-REPL interactive diff --git a/internal/sandbox/analyzer_test.go b/internal/sandbox/analyzer_test.go index 9f44eca25..335c1eea5 100644 --- a/internal/sandbox/analyzer_test.go +++ b/internal/sandbox/analyzer_test.go @@ -1,8 +1,12 @@ package sandbox -import "testing" +import ( + "strings" + "testing" +) func TestAnalyzeCommand(t *testing.T) { + t.Setenv("ZERO_TEST_ENV_COMMAND", "curl") cases := []struct { name string script string @@ -85,6 +89,140 @@ func TestAnalyzeCommand(t *testing.T) { {name: "git clone", script: "git clone https://example.com/repo.git", network: true}, {name: "git fetch", script: "git fetch origin", network: true}, {name: "git status is offline", script: "git status", network: false}, + {name: "git pull", script: "git pull origin main", network: true}, + {name: "git push custom transport", script: "git push gitlawb://example.com/repo.git main", network: true}, + {name: "git ls-remote", script: "git ls-remote origin", network: true}, + {name: "git remote archive", script: "git archive --remote=origin HEAD", network: true}, + {name: "git remote archive separated value", script: "git archive --remote origin HEAD", network: true}, + // Only --remote leaves the machine; a local archive streams the object + // store and must not cost a network prompt (issue #703 review). + {name: "git local archive", script: "git archive HEAD", network: false}, + {name: "git -C local archive", script: "git -C repo archive HEAD -o out.tar", network: false}, + // After `--` every token is a pathspec, so this archives a tree entry + // named "--remote" from the local object store (issue #703 review). + {name: "git archive pathspec named --remote", script: "git archive HEAD -- --remote", network: false}, + {name: "git -C archive pathspec named --remote", script: "git -C repo archive HEAD -- --remote=origin", network: false}, + {name: "git archive missing remote operand", script: "git archive HEAD --remote", network: false}, + {name: "git archive post-tree --remote value", script: "git archive HEAD --remote=origin", network: true}, + // A real --remote before the separator still is one. + {name: "git remote archive with pathspec", script: "git archive --remote=origin HEAD -- src", network: true}, + {name: "git remote archive after output option", script: "git archive -o out.tar --remote origin HEAD", network: true}, + {name: "git remote archive after mtime option", script: "git archive --mtime 2024-01-01 --remote origin HEAD", network: true}, + {name: "git remote archive after format without value", script: "git archive --format --remote=origin HEAD", network: true}, + {name: "git remote archive after mtime without value", script: "git archive --mtime --remote=origin HEAD", network: true}, + {name: "git output consumes --remote", script: "git archive -o --remote HEAD", network: false}, + {name: "git joined output named --remote", script: "git archive --output=--remote HEAD", network: false}, + {name: "git global -C consumes --remote", script: "git -C --remote archive HEAD", network: false}, + {name: "git output consumes separator before remote", script: "git archive -o -- --remote=origin HEAD", network: true}, + {name: "bash clustered login command", script: `bash -lc 'curl https://x.test'`, network: true}, + {name: "bash clustered command erexit", script: `bash -ce 'git push origin main'`, network: true}, + {name: "dash clustered command erexit", script: `dash -ce 'curl https://x.test'`, network: true}, + {name: "bash long command flag is invalid", script: `bash --command 'curl https://x.test'`, network: false}, + {name: "bash args after command payload are positional", script: `bash -c 'echo ok' curl https://x.test`, network: false}, + {name: "bash script makes later command flag positional", script: `bash /dev/null -c 'curl https://x.test'`, network: false}, + {name: "bash option terminator makes command flag positional", script: `bash -- -c 'curl https://x.test'`, network: false}, + {name: "bash invalid command cluster", script: `bash -Zc 'curl https://x.test'`, network: false}, + {name: "bash noexec command cluster", script: `bash -nc 'curl https://x.test'`, network: false}, + {name: "bash plus option before command", script: `bash +n -c 'curl https://x.test'`, network: true}, + {name: "bash plus command flag", script: `bash +c 'curl https://x.test'`, network: true}, + {name: "bash dump strings does not execute", script: `bash --dump-strings -c 'curl https://x.test'`, network: false}, + {name: "shell dynamic command payload fails closed", script: `sh -c "$ZERO_TEST_SHELL_COMMAND"`, network: true}, + {name: "nested shell dynamic command payload fails closed", script: `sh -c 'sh -c "$1"' sh 'curl https://x.test'`, network: true}, + {name: "shell command terminator before dynamic payload fails closed", script: `PAYLOAD='curl https://x.test'; sh -c -- "$PAYLOAD"`, network: true}, + {name: "env split shell terminator selects payload", script: `env -S 'sh -c' -- 'curl https://x.test'`, network: true}, + {name: "PowerShell slash command", script: `powershell /Command Invoke-WebRequest https://x.test`, network: true}, + {name: "PowerShell abbreviated command", script: `pwsh -co iwr https://x.test`, network: true}, + {name: "PowerShell local command", script: `pwsh /Command Get-ChildItem`, network: false}, + {name: "PowerShell expression evaluation", script: `powershell -Command "Invoke-Expression 'curl https://x.test'"`, network: true}, + {name: "pwsh expression evaluation alias", script: `pwsh -Command "iex 'git push origin main'"`, network: true}, + {name: "PowerShell dynamic expression evaluation", script: `powershell -Command 'iex $PAYLOAD'`, network: true}, + {name: "PowerShell local expression evaluation", script: `powershell -Command "iex 'Write-Output hello'"`, network: false}, + {name: "PowerShell backtick command name fails closed", script: "powershell -Command 'Invoke`-WebRequest https://x.test'", network: true}, + {name: "PowerShell backtick continuation fails closed", script: "powershell -Command 'Invoke`\n-WebRequest https://x.test'", network: true}, + {name: "PowerShell abbreviated execution policy", script: `powershell -ep RemoteSigned curl https://x.test`, network: true}, + {name: "pwsh abbreviated execution policy before command", script: `pwsh -ep Bypass -Command curl https://x.test`, network: true}, + {name: "Windows PowerShell bare command", script: `powershell Invoke-WebRequest https://x.test`, network: true}, + {name: "pwsh bare token is file mode", script: `pwsh Invoke-WebRequest https://x.test`, network: false}, + {name: "PowerShell joined command flag is invalid", script: `powershell -Command:Invoke-WebRequest https://x.test`, network: false}, + {name: "pwsh command with args", script: `pwsh -CommandWithArgs Invoke-WebRequest https://x.test`, network: true}, + {name: "pwsh abbreviated command with args", script: `pwsh -cwa Invoke-RestMethod https://x.test`, network: true}, + {name: "PowerShell invalid startup option", script: `powershell -DefinitelyInvalid Invoke-WebRequest https://x.test`, network: false}, + {name: "env split curl argv", script: `env -S 'curl https://x.test'`, network: true}, + {name: "env joined split git argv", script: `env --split-string='git push origin main'`, network: true}, + {name: "env clustered split option", script: `env -iS 'curl https://x.test'`, network: true}, + {name: "env split introduces split option", script: `env -S '-S "curl https://x.test"'`, network: true}, + {name: "env split introduces nested env", script: `env -S 'env -S "curl https://x.test"'`, network: true}, + {name: "env split underscore separator", script: `env -S 'curl\_https://x.test'`, network: true}, + {name: "env split environment executable", script: `env -S '${ZERO_TEST_ENV_COMMAND} https://x.test'`, network: true}, + {name: "env split metacharacters stay arguments", script: `env -S 'printf x; curl https://x.test'`, network: false}, + {name: "env split trailing args stay arguments", script: `env -S 'printf ok' curl https://x.test`, network: false}, + {name: "env split environment argument", script: `env -S 'printf ${ZERO_TEST_ENV_COMMAND}'`, network: false}, + {name: "env split shell command", script: `env -S 'sh -c "curl https://x.test"'`, network: true}, + {name: "env split argv0 before curl", script: `env -S '--argv0 harmless curl https://x.test'`, network: true}, + {name: "env split argv0 named curl", script: `env -S '--argv0 curl printf ok'`, network: false}, + // GNU env accepts unambiguous long-option abbreviations: "--split" and + // "--split-strin" name --split-string exactly like the full spelling does, + // so the launcher's argv reconstruction must not depend on that one spelling. + {name: "env abbreviated split option", script: `env --split 'curl https://x.test'`, network: true}, + {name: "env abbreviated split option with value", script: `env --split='curl https://x.test'`, network: true}, + {name: "env minimal abbreviated split option", script: `env --s 'curl https://x.test'`, network: true}, + {name: "env near-full abbreviated split option", script: `env --split-strin 'curl https://x.test'`, network: true}, + {name: "env abbreviated split metacharacters stay arguments", script: `env --split 'printf x; curl https://x.test'`, network: false}, + {name: "env non-split long option is not split-string", script: `env --unset FOO curl https://x.test`, network: true}, + {name: "env non-split long option leaves literal payload alone", script: `env --chdir=/tmp printf ok`, network: false}, + {name: "busybox wget applet", script: `busybox wget https://x.test`, network: true}, + {name: "busybox shell command", script: `busybox sh -c 'curl https://x.test'`, network: true}, + {name: "busybox echo network text", script: `busybox echo wget https://x.test`, network: false}, + {name: "busybox option terminator is unresolved", script: `busybox -- curl https://x.test`, network: true}, + {name: "busybox unknown option is unresolved", script: `busybox -x curl https://x.test`, network: true}, + // The applet name comes from a shell expansion this scan cannot read + // statically; wordText silently drops it, so the AST resolver must fail + // closed here rather than reading the blanked token as a clean unknown. + {name: "busybox dynamic applet fails closed", script: `APPLET=curl; busybox "$APPLET" https://x.test`, network: true}, + {name: "busybox literal applet stays classified on content", script: `busybox echo "not a program" https://x.test`, network: false}, + {name: "busybox ordinary env wrapper", script: `busybox env curl https://x.test`, network: true}, + {name: "busybox env split wrapper", script: `busybox env -S 'git push origin main'`, network: true}, + {name: "busybox ordinary env local control", script: `busybox env true`, network: false}, + {name: "strace curl command", script: `strace -f -o trace.log curl https://x.test`, network: true}, + {name: "strace shell command", script: `strace sh -c 'git push origin main'`, network: true}, + {name: "strace trace path before curl", script: `strace -P /tmp curl https://x.test`, network: true}, + {name: "strace trace path named curl", script: `strace -P curl true`, network: false}, + {name: "strace attach and curl", script: `strace -p 123 curl https://x.test`, network: true}, + {name: "strace long trace option", script: `strace --trace network curl https://x.test`, network: true}, + {name: "strace tips traces curl", script: `strace --tips curl https://x.test`, network: true}, + {name: "strace joined tips traces git", script: `strace --tips=full git push origin main`, network: true}, + {name: "strace abbreviated tips traces curl", script: `strace --tip curl https://x.test`, network: true}, + {name: "strace ambiguous long option is unresolved", script: `strace --ver curl https://x.test`, network: true}, + {name: "strace clustered options", script: `strace -fqo trace.log curl https://x.test`, network: true}, + {name: "strace output named curl", script: `strace -o curl true`, network: false}, + {name: "strace invalid option is unresolved", script: `strace --definitely-invalid curl https://x.test`, network: true}, + // The traced command comes from a shell expansion this scan cannot read + // statically; straceSourceDynamic must fail closed the same way + // busyboxSourceDynamic does above, rather than reading the blanked token + // as a clean unknown command. + {name: "strace dynamic child fails closed", script: `APPLET=curl; strace "$APPLET" https://x.test`, network: true}, + {name: "strace literal child stays classified on content", script: `strace true "not a program" https://x.test`, network: false}, + {name: "strace ordinary env wrapper", script: `strace env curl https://x.test`, network: true}, + {name: "strace env split wrapper", script: `strace env -S 'git push origin main'`, network: true}, + {name: "strace dynamic env split source", script: `PAYLOAD='curl https://x.test'; strace env -S "$PAYLOAD"`, network: true}, + {name: "strace literal env split source", script: `strace env -S 'printf ok'`, network: false}, + {name: "strace ordinary env local control", script: `strace env true`, network: false}, + {name: "git local commit", script: `git commit -m "local change"`, network: false}, + // git's value-taking global options put their value in the NEXT token, so a + // generic "first non-dash token" scan reads the value as the subcommand and + // misses the network verb entirely (issue #703 review). + {name: "git -C push", script: "git -C repo push origin main", network: true}, + {name: "git -c config push", script: "git -c http.sslVerify=false push origin main", network: true}, + {name: "git --git-dir fetch", script: "git --git-dir /repo/.git fetch origin", network: true}, + {name: "git --work-tree pull", script: "git --work-tree /repo pull origin main", network: true}, + {name: "git --attr-source push", script: "git --attr-source HEAD push origin main", network: true}, + {name: "git -C local commit", script: `git -C repo commit -m "local change"`, network: false}, + {name: "git help topic is offline", script: "git --help push", network: false}, + {name: "git -C help topic is offline", script: "git -C repo --help push", network: false}, + {name: "git -c version topic is offline", script: "git -c user.name=test --version push", network: false}, + {name: "git.exe push", script: "git.exe push origin main", network: true}, + {name: "git.cmd push", script: "git.cmd push origin main", network: true}, + {name: "git.exe local commit", script: `git.exe commit -m "local change"`, network: false}, {name: "gh release download", script: "gh release download v1.0.0", network: true}, {name: "no network", script: "ls -la && echo done", network: false}, {name: "process pattern is not network", script: `pkill -f "python3 -m http.server 8000"`, network: false}, @@ -104,8 +242,210 @@ func TestAnalyzeCommand(t *testing.T) { } } +func TestAnalyzeCommandFailsClosedAtShellLauncherDepthLimit(t *testing.T) { + command := "curl https://x.test" + for range maxAnalyzerDepth + 1 { + command = "sh -c '" + strings.ReplaceAll(command, "'", `'"'"'`) + "'" + } + if got := AnalyzeCommand(command); !got.Network { + t.Fatalf("AnalyzeCommand(%q) = %#v; want network", command, got) + } +} + func TestAnalyzeCommandEmptyIsClean(t *testing.T) { if got := AnalyzeCommand(" "); got.Interactive || got.Destructive || got.Network || got.TooComplex { t.Fatalf("empty script should be clean, got %#v", got) } } + +// unescapeDoubleQuoted implements POSIX double-quote escape removal, which the +// parser deliberately leaves to expansion time. For an argv token that is +// harmless; for a shell launcher's -c operand the text IS the next command, so +// keeping `\"` verbatim handed the recursion a fragment and lost everything +// after it. +func TestUnescapeDoubleQuoted(t *testing.T) { + cases := []struct{ in, want string }{ + {`plain`, `plain`}, + {`sh -c \"curl x\"`, `sh -c "curl x"`}, + {`a\\b`, `a\b`}, + {`\$HOME`, `$HOME`}, + {"\\`cmd\\`", "`cmd`"}, + // A backslash before anything else is literal: a quoted Windows path + // must survive intact. + {`C:\Users\me\file.txt`, `C:\Users\me\file.txt`}, + {`C:\temp\n`, `C:\temp\n`}, + {"line\\" + "\n" + "cont", "linecont"}, + } + for _, testCase := range cases { + if got := unescapeDoubleQuoted(testCase.in); got != testCase.want { + t.Errorf("unescapeDoubleQuoted(%q) = %q, want %q", testCase.in, got, testCase.want) + } + } +} + +// A quoted CMD payload is command SOURCE, but a quoted path containing a space +// is a program name. CMD resolves that ambiguity by trying both, so the +// classifier does too and fails closed if either reading reaches the network. +func TestAnalyzeCommandReadsQuotedCMDPayloadBothWays(t *testing.T) { + network := []string{ + `cmd /c "git push origin main"`, + `cmd /c "curl https://evil.test"`, + `cmd /k "git fetch origin"`, + `call "git push origin main"`, + `start "" "curl https://evil.test"`, + // Program-path reading: the quoted token IS the executable. + `cmd /c "C:\Program Files\curl\curl.exe"`, + } + for _, command := range network { + if analysis := AnalyzeCommand(command); !analysis.Network { + t.Errorf("AnalyzeCommand(%q).Network = false, want true", command) + } + } + local := []string{ + `cmd /c "git status"`, + `cmd /c "echo hello world"`, + `cmd /c "C:\Program Files\git\bin\git.exe status"`, + `call "git status"`, + } + for _, command := range local { + if analysis := AnalyzeCommand(command); analysis.Network { + t.Errorf("AnalyzeCommand(%q).Network = true, want false", command) + } + } +} + +// GNU env appends the remaining argv to the argv the split string produced, so +// proving the -S operand is literal proves nothing about the whole invocation: +// `env -S 'sh -c' "$PAYLOAD"` runs text this scan cannot read. +func TestAnalyzeCommandFailsClosedOnDynamicArgvAfterEnvSplit(t *testing.T) { + dynamic := []string{ + `env -S 'git push origin main' "$EXTRA"`, + `env -S 'printf ok' $PAYLOAD`, + `env -S 'printf ok' "${PAYLOAD}" https://evil.test`, + `env -S 'sh -c' "$PAYLOAD"`, + `env --split-string='printf ok' $PAYLOAD`, + `sudo env -S 'printf ok' $PAYLOAD`, + } + for _, command := range dynamic { + if analysis := AnalyzeCommand(command); !analysis.Network { + t.Errorf("AnalyzeCommand(%q).Network = false, want true", command) + } + } + // Trailing LITERAL argv is fully readable and must not cost a prompt. + literal := []string{ + `env -S 'printf ok' literal args`, + `env -S 'git status' --`, + `env -S 'echo hi'`, + } + for _, command := range literal { + if analysis := AnalyzeCommand(command); analysis.Network { + t.Errorf("AnalyzeCommand(%q).Network = true, want false", command) + } + } +} + +// eval and CMD's echo-suppression prefix were handled on the fallback path but +// not on the AST path, so the identical text was network only when something +// else in the command happened to defeat the parser. +func TestAnalyzeCommandClassifiesEvalAndEchoPrefixOnParseablePath(t *testing.T) { + network := []string{ + `eval git push origin main`, + `eval "curl https://evil.test"`, + `eval $PAYLOAD`, + `@curl https://evil.test`, + `@git push origin main`, + } + for _, command := range network { + analysis := AnalyzeCommand(command) + if analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) is TooComplex; this test must cover the parseable path", command) + } + if !analysis.Network { + t.Errorf("AnalyzeCommand(%q).Network = false, want true", command) + } + } + for _, command := range []string{`eval echo hi`, `eval git status`, `@echo hello`} { + if analysis := AnalyzeCommand(command); analysis.Network { + t.Errorf("AnalyzeCommand(%q).Network = true, want false", command) + } + } +} + +// send-pack is push's plumbing counterpart and performs the same egress; both +// classification paths read one subcommand list, so covering gitUsesNetwork +// covers the fallback too. +func TestGitSendPackIsNetwork(t *testing.T) { + for _, command := range []string{ + `git send-pack origin main`, + `git -C repo send-pack origin main`, + `git send-pack origin main & rem '`, + } { + if analysis := AnalyzeCommand(command); !analysis.Network && !matchesUnparseableNetwork(command) { + t.Errorf("%q classified as local; send-pack pushes to a remote", command) + } + } +} + +func TestGitSubcommandPreservesOriginalAndNormalizedSpelling(t *testing.T) { + selection, ok := GitSubcommand([]string{"-c", "alias.STATUS=!curl", "STATUS"}) + if !ok { + t.Fatal("GitSubcommand did not resolve the alias-shaped subcommand") + } + if selection.Index != 2 || selection.Original != "STATUS" || selection.Normalized != "status" { + t.Fatalf("GitSubcommand = %#v, want index 2, original STATUS, normalized status", selection) + } + + lowercase, ok := GitSubcommand([]string{"--attr-source", "HEAD", "status"}) + if !ok || lowercase.Index != 2 || lowercase.Original != "status" || lowercase.Normalized != "status" { + t.Fatalf("lowercase GitSubcommand = %#v ok=%v", lowercase, ok) + } +} + +// TestGitGlobalOptionConsumesValueFoldsShortCaseDeliberately pins the case-fold +// in GitGlobalOptionConsumesValue. -c and -C are DIFFERENT git options that +// today both take a separate-token value, and the callers normalize the option +// name before asking, so the two necessarily share one answer here. That is +// safe only while their value-taking behavior agrees; if git ever changes one +// of them, this test fails and the distinction has to be restored at the call +// site by passing the original token, not by adding an unreachable uppercase +// case to a switch that compares lowercased input. +func TestGitGlobalOptionConsumesValueFoldsShortCaseDeliberately(t *testing.T) { + for _, option := range []string{"-c", "-C"} { + if !GitGlobalOptionConsumesValue(option) { + t.Fatalf("GitGlobalOptionConsumesValue(%q) = false, want true", option) + } + } + // Long options are all-lowercase in git; the fold must not invent new ones. + for _, option := range []string{"-d", "-x", "--exec-path", "--not-an-option"} { + if GitGlobalOptionConsumesValue(option) { + t.Fatalf("GitGlobalOptionConsumesValue(%q) = true, want false", option) + } + } +} + +// TestGitSubcommandConsumesDashCOperand proves the -C operand is consumed in +// both the separated and joined spellings, so the subcommand git dispatches is +// the one found. Missing this would make `git -C repo push` resolve `repo` as +// the subcommand, and an unknown subcommand is classified proven-local — the +// network gate would be dropped on a real push. +func TestGitSubcommandConsumesDashCOperand(t *testing.T) { + for _, tc := range []struct { + words []string + want string + index int + }{ + {words: []string{"-C", "repo", "push", "origin", "main"}, want: "push", index: 2}, + {words: []string{"-Crepo", "push", "origin", "main"}, want: "push", index: 1}, + {words: []string{"-C", "repo", "status"}, want: "status", index: 2}, + {words: []string{"-c", "a=b", "push"}, want: "push", index: 2}, + } { + info, ok := GitSubcommand(tc.words) + if !ok || info.Normalized != tc.want || info.Index != tc.index { + t.Fatalf("GitSubcommand(%q) = %+v ok=%v, want %q at %d", tc.words, info, ok, tc.want, tc.index) + } + } + // `git -C repo` runs nothing: the operand is consumed and no subcommand is left. + if info, ok := GitSubcommand([]string{"-C", "repo"}); ok { + t.Fatalf("GitSubcommand(-C repo) = %+v, want no subcommand", info) + } +} diff --git a/internal/sandbox/combined_shell_options_test.go b/internal/sandbox/combined_shell_options_test.go new file mode 100644 index 000000000..f209647bb --- /dev/null +++ b/internal/sandbox/combined_shell_options_test.go @@ -0,0 +1,50 @@ +package sandbox + +import ( + "context" + "testing" +) + +// Shells accept command mode inside a short-option cluster. In particular, +// `sh -lc` is a common login-shell spelling; treating its `-l` as invalid made +// the parser stop before the command payload and let network use bypass the +// network-deny prompt. +func TestEvaluateCombinedShellOptionsPreservesNetworkGate(t *testing.T) { + engine := NewEngine(EngineOptions{Policy: Policy{Mode: ModeEnforce, Network: NetworkDeny}}) + networkCases := []string{ + `bash -lc "curl https://evil.test"`, + `bash -ic "curl https://evil.test"`, + `bash -xc "curl https://evil.test"`, + `bash -ec "curl https://evil.test"`, + `sh -lc "curl https://evil.test"`, + `zsh -lc "curl https://evil.test"`, + `bash -lc "git push origin main"`, + } + for _, command := range networkCases { + t.Run(command, func(t *testing.T) { + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": command}, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked { + t.Fatalf("Evaluate(%q) = action %q reason %q risk %v, want network prompt", command, decision.Action, decision.Reason, decision.Risk.Categories) + } + }) + } + + for _, command := range []string{ + `bash -lc "printf ok"`, + `sh -lc "printf ok"`, + `zsh -lc "printf ok"`, + } { + t.Run("local/"+command, func(t *testing.T) { + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": command}, + }) + if decision.Reason == ReasonNetworkBlocked || HasRiskCategory(decision.Risk, "network") { + t.Fatalf("Evaluate(%q) = %#v, want proven-local network classification", command, decision) + } + }) + } +} diff --git a/internal/sandbox/engine_test.go b/internal/sandbox/engine_test.go index 5f9633a40..292df1b4f 100644 --- a/internal/sandbox/engine_test.go +++ b/internal/sandbox/engine_test.go @@ -45,6 +45,194 @@ func TestEvaluatePromptsForNetworkToolsButExemptsThemFromShellNetworkPolicy(t *t } } +// TestEvaluatePromptsForUnparseableNetworkBehindWrapper is the engine-level +// regression for jatmn's #726 P2 finding. Classification is what decides egress +// here: when the fallback stopped recognizing a network program hidden behind a +// wrapper, an already-permission-granted shell command went from ActionPrompt / +// ReasonNetworkBlocked to a plain ActionAllow — silently granting network to a +// command too obfuscated to parse. +func TestEvaluatePromptsForUnparseableNetworkBehindWrapper(t *testing.T) { + engine := NewEngine(EngineOptions{Policy: Policy{Mode: ModeEnforce, Network: NetworkDeny}}) + for _, command := range []string{ + `sudo curl https://evil.test && "unterminated`, + `env git push origin main && "unterminated`, + `curl.exe https://evil.test && "unterminated`, + "true\ncurl https://evil.test && \"unterminated", + `echo $(curl https://evil.test) && "unterminated`, + `if true; then git push; fi && "unterminated`, + `eval "curl https://evil.test" && "unterminated`, + `f(){ curl https://evil.test; }; f && "unterminated`, + `function f() (git push origin main); f && "unterminated`, + `cmd.exe /c curl https://evil.test & rem '`, + `cmd.exe /d /c git push origin main & rem '`, + `cmd.exe /k git push origin main & rem '`, + `powershell.exe -Command curl https://evil.test & rem '`, + `powershell.exe -c curl https://evil.test & rem '`, + `pwsh.exe -c git push origin main & rem '`, + `if 1==1 (curl https://evil.test) & rem '`, + `if 1==1 ((git push origin main)) & rem '`, + `for %i in (x) do (curl https://evil.test) & rem '`, + `for /f %i in ('curl https://evil.test') do echo %i & rem '`, + "for /f \"usebackq\" %i in (`git push origin main`) do echo %i & rem '", + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); !analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) = %#v; test must exercise the fallback", command, analysis) + } + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": command}, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked { + t.Fatalf("Evaluate(%q) = action %q reason %q, want a network prompt", command, decision.Action, decision.Reason) + } + }) + } +} + +// TestEvaluateLauncherResolutionContract pins the tri-state resolver at the +// enforcement boundary: known network and unresolved executable source retain +// the network gate, while explicit local controls do not prompt for egress. +func TestEvaluateLauncherResolutionContract(t *testing.T) { + engine := NewEngine(EngineOptions{Policy: Policy{Mode: ModeEnforce, Network: NetworkDeny}}) + networkCases := []string{ + `PAYLOAD='curl https://evil.test'; sh -c -- "$PAYLOAD"`, + `PAYLOAD='curl https://evil.test'; env -S 'sh -c' -- "$PAYLOAD" && "unterminated`, + "powershell -Command 'Invoke`-WebRequest https://evil.test'", + "powershell -Command 'Invoke`\n-WebRequest https://evil.test'", + `powershell -Command Invoke-WebRequest https://evil.test`, + `strace --tip curl https://evil.test`, + `strace --ver curl https://evil.test`, + `strace --tips curl https://evil.test`, + `strace env curl https://evil.test`, + `strace env -S 'git push origin main'`, + `PAYLOAD='curl https://evil.test'; strace env -S "$PAYLOAD"`, + `PAYLOAD='curl https://evil.test'; strace env -S "$PAYLOAD" && "unterminated`, + `busybox env curl https://evil.test`, + `busybox env -S 'git push origin main'`, + `busybox -- curl https://evil.test`, + `busybox -x curl https://evil.test`, + `strace --definitely-invalid curl https://evil.test`, + `APPLET=curl; strace "$APPLET" https://evil.test`, + `APPLET=curl; busybox "$APPLET" https://evil.test`, + `strace env curl https://evil.test && "unterminated`, + `busybox env curl https://evil.test && "unterminated`, + } + for _, command := range networkCases { + t.Run("network/"+command, func(t *testing.T) { + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": command}, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked { + t.Fatalf("Evaluate(%q) = action %q reason %q, want network prompt", command, decision.Action, decision.Reason) + } + }) + } + + localCases := []string{ + `sh -c -- 'printf ok'`, + `env -S 'sh -c' -- 'printf ok' && "unterminated`, + `powershell -Command Get-ChildItem`, + `strace env -S 'printf ok'`, + `strace env -S 'printf ok' && "unterminated`, + `strace --tips true`, + `strace env true`, + `busybox env true`, + } + for _, command := range localCases { + t.Run("local/"+command, func(t *testing.T) { + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": command}, + }) + if decision.Reason == ReasonNetworkBlocked || HasRiskCategory(decision.Risk, "network") { + t.Fatalf("Evaluate(%q) = %#v, want proven-local network classification", command, decision) + } + }) + } +} + +// Three shapes where a reading the classifier could not perform was being +// reported as "no network" rather than as unresolved. Each ran through the +// engine, because the property that matters is the prompt the operator sees. +func TestEngineFailsClosedOnUnreadableCommandShapes(t *testing.T) { + engine := NewEngine(EngineOptions{Policy: Policy{Mode: ModeEnforce, Network: NetworkDeny}}) + networkCases := []struct { + name string + command string + }{ + // The expansion chooses git's subcommand. The reconstructed argv shows + // `git origin main`, an unknown local subcommand, while the shell runs + // whatever VERB holds — `push` included. + {name: "git subcommand from an expansion", command: `VERB=push; git $VERB origin main`}, + {name: "git subcommand from a quoted expansion", command: `VERB=push; git "$VERB" origin main`}, + // Same reading through the unparseable fallback rather than the AST. + {name: "git subcommand from an expansion, fallback", command: `git $VERB origin main && "unterminated`}, + { + name: "case-distinct git alias supplied through config", + command: `git -c alias.STATUS=!curl STATUS https://example.invalid`, + }, + { + name: "case-distinct git alias through cmd fallback", + command: `git -c alias.STATUS=!curl STATUS https://example.invalid & rem '`, + }, + // archive is local until --remote makes it a fetch from another host, so + // an unreadable option word past the subcommand counts for archive alone. + {name: "git archive options from an expansion", command: `OPTS=--remote=origin; git archive $OPTS HEAD`}, + // `do` inside the IN set is a set element, not the loop keyword. Selecting + // the first one resolved the loop body from `x` and never reached the curl + // that runs. The spaced spelling is the one that was actually wrong: with + // `(do` joined, the token is not `do` and the old scan landed correctly by + // accident, which is why the joined form is kept alongside it rather than + // instead of it. + {name: "cmd FOR with do inside the set", command: `for %i in (do x) do curl https://evil.test`}, + {name: "cmd FOR with a spaced set", command: `for %i in ( do x ) do curl https://evil.test`}, + // CMD has no single-quote quoting: the quote is literal text, `&` is a + // separator, and curl runs. POSIX tokenization hid it inside echo's + // argument. + {name: "cmd separator hidden by a POSIX quote", command: `echo ' & curl https://evil.test`}, + {name: "cmd separator hidden by a POSIX quote, piped", command: `echo ' | curl https://evil.test`}, + } + for _, testCase := range networkCases { + t.Run("network/"+testCase.name, func(t *testing.T) { + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": testCase.command}, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked { + t.Fatalf("Evaluate(%q) = action %q reason %q, want network prompt", testCase.command, decision.Action, decision.Reason) + } + }) + } + + // The fail-closed rule must not swallow the ordinary readable forms: a + // literal local subcommand stays quiet even with expansions in its operands, + // a local archive stays local, and a FOR loop over a network-looking set + // still resolves from its body. + localCases := []struct { + name string + command string + }{ + {name: "literal local subcommand with a dynamic operand", command: `git commit -m "$MESSAGE"`}, + {name: "local archive", command: `git archive -o out.tar HEAD`}, + {name: "archive with a dynamic operand past --", command: `git archive -o out.tar -- HEAD`}, + {name: "for loop body is echo", command: `for %i in (curl) do echo %i`}, + {name: "posix quote hiding no separator", command: `echo 'plain text'`}, + } + for _, testCase := range localCases { + t.Run("local/"+testCase.name, func(t *testing.T) { + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": testCase.command}, + }) + if decision.Reason == ReasonNetworkBlocked || HasRiskCategory(decision.Risk, "network") { + t.Fatalf("Evaluate(%q) = %#v, want proven-local network classification", testCase.command, decision) + } + }) + } +} + func TestEngineBashAllowGrantDoesNotBypassNetworkPrompt(t *testing.T) { store, err := NewGrantStore(StoreOptions{ FilePath: filepath.Join(t.TempDir(), "sandbox-grants.json"), @@ -93,6 +281,256 @@ func TestEngineClassifiesPowerShellCurlCommandAsNetwork(t *testing.T) { } } +// TestEngineClassifiesCMDInvocationFormsAsNetwork covers jatmn's #726 finding +// at the decision layer it actually costs: Evaluate only reaches the network +// prompt when the risk carries the "network" category, so a CMD form the +// fallback resolved to a keyword instead of its program reached ActionAllow (or +// ran under the deny profile) with no approval flow. On the Windows unelevated +// backend that prompt is the network boundary. +func TestEngineClassifiesCMDInvocationFormsAsNetwork(t *testing.T) { + for _, command := range []string{ + `@curl https://evil.test & rem '`, + `cmd.exe /c call curl https://evil.test & rem '`, + `if not 1==2 curl https://evil.test & rem '`, + `start "" curl https://evil.test & rem '`, + `start "" c^u^r^l https://evil.test & rem '`, + `call start "" curl https://evil.test & rem '`, + `if 1==1 start "" curl https://evil.test & rem '`, + `cmd /c /d curl https://evil.test & rem '`, + `powershell /Command curl https://evil.test & rem '`, + `powershell -co curl https://evil.test & rem '`, + `if 1==1 (curl https://evil.test) & rem '`, + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); !analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) parsed; this case must exercise the fallback", command) + } + engine := NewEngine(EngineOptions{WorkspaceRoot: t.TempDir(), Policy: DefaultPolicy()}) + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", + SideEffect: SideEffectShell, + Permission: PermissionPrompt, + PermissionMode: PermissionModeAsk, + Args: map[string]any{"command": command}, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked || + !HasRiskCategory(decision.Risk, "network") { + t.Fatalf("Evaluate(%q) = %#v, want a network prompt", command, decision) + } + }) + } +} + +func TestEngineClassifiesParseableCMDLaunchersAsNetwork(t *testing.T) { + for _, command := range []string{ + `cmd /c git push origin main`, + `cmd /c curl https://evil.test`, + `call git push origin main`, + `start curl https://evil.test`, + `start "" /b curl https://evil.test`, + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) reported TooComplex; this case must exercise the AST path", command) + } + engine := NewEngine(EngineOptions{WorkspaceRoot: t.TempDir(), Policy: DefaultPolicy()}) + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", + SideEffect: SideEffectShell, + Permission: PermissionPrompt, + PermissionMode: PermissionModeAsk, + Args: map[string]any{"command": command}, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked || + !HasRiskCategory(decision.Risk, "network") { + t.Fatalf("Evaluate(%q) = %#v, want a network prompt", command, decision) + } + }) + } +} + +func TestEnginePromptsForReviewedUnparseableNetworkForms(t *testing.T) { + t.Setenv("ZERO_TEST_ENV_COMMAND", "curl") + for _, command := range []string{ + `cu^rl https://evil.test & rem '`, + `cmd /c cu^rl https://evil.test & rem '`, + `%ComSpec% /c curl https://evil.test & rem '`, + `start "x" curl https://evil.test & rem '`, + `cmd /c start "x" curl https://evil.test & rem '`, + `start /b "x" curl https://evil.test & rem '`, + `if cmdextversion 1 curl https://evil.test & rem '`, + `if not cmdextversion 1 git push origin main & rem '`, + `git pus^h origin main & rem '`, + `git archive --rem^ote=origin HEAD & rem '`, + `git archive HEAD --remote=origin`, + `git archive --format --remote=origin HEAD`, + `bash -lc 'curl https://evil.test' && "unterminated`, + `dash -ce 'git push origin main' && "unterminated`, + `bash +n -c 'curl https://evil.test' && "unterminated`, + `sh -c "$ZERO_TEST_SHELL_COMMAND"`, + `env -S 'curl https://evil.test' && "unterminated`, + `env -S 'curl\_https://evil.test' && "unterminated`, + `env -S '${ZERO_TEST_ENV_COMMAND} https://evil.test' && "unterminated`, + `env -iS 'curl https://evil.test' && "unterminated`, + `env -S '-S "curl https://evil.test"' && "unterminated`, + `env -S 'env -S "curl https://evil.test"' && "unterminated`, + `env -S '--argv0 harmless curl https://evil.test' && "unterminated`, + `env --split-string 'git push origin main' && "unterminated`, + `set "N=curl" & call %N% https://evil.test & rem '`, + `set "N=curl" & call %N:x=y% https://evil.test & rem '`, + `set "N=curl" & cmd /c %N% https://evil.test & rem '`, + `set "N=curl" & start %N% https://evil.test & rem '`, + `set "N=git" & call !N! push origin main & rem '`, + `sh -c "$PAYLOAD" & rem '`, + `sh -c "${PAYLOAD}" & rem '`, + `CMD=curl env -S '${CMD} https://evil.test'`, + `PAYLOAD='curl https://evil.test' env -S 'sh -c "${PAYLOAD}"'`, + `PAYLOAD='curl https://evil.test' env -S 'sh -c "${PAYLOAD}"' & rem '`, + `exec -a harmless curl https://evil.test && "unterminated`, + `powershell -NoProfile curl https://evil.test`, + `pwsh -cwa Invoke-WebRequest https://evil.test & rem '`, + `busybox sh -c 'curl https://evil.test' && "unterminated`, + `strace -P /tmp sh -c 'git push origin main' && "unterminated`, + `strace --trace network curl https://evil.test && "unterminated`, + `strace --tips curl https://evil.test && "unterminated`, + `strace -fqo trace.log curl https://evil.test && "unterminated`, + `powershell -ep RemoteSigned curl https://evil.test & rem '`, + // A CMD FOR metavariable supplies the executable. CMD substitutes it + // before launching the body, so the spelling proves nothing about what + // runs and the network gate must survive. %%i is the batch spelling of + // the same reference, and %~dpi the modified form. + `for /f %i in (list.txt) do %i https://evil.test`, + // PowerShell source with block structure. The POSIX AST scan and the + // unparseable matcher do not model try/catch, if, foreach, while or a + // script block piped into ForEach-Object, so "no network program found" + // is not evidence the source is local — it must stay gated. + `powershell -Command 'try { Invoke-WebRequest https://evil.test } catch {}'`, + `powershell -Command "try { Invoke-WebRequest https://evil.test } catch {}"`, + `pwsh -Command 'if ($true) { Invoke-WebRequest https://evil.test }'`, + `powershell -Command 'foreach ($i in 1..3) { curl https://evil.test }'`, + `powershell -Command 'while ($true) { iwr https://evil.test }'`, + `powershell -Command 'Get-Content urls.txt | ForEach-Object { Invoke-WebRequest $_ }'`, + // A block whose body names nothing recognisable is still unreadable, so it + // is gated on the grammar rather than on spotting a network verb. + `powershell -Command 'try { & $tool $target } catch {}'`, + `powershell -Command "Invoke-Expression 'curl https://evil.test'"`, + `powershell -Command "iex 'git push origin main'"`, + `pwsh -Command "Invoke-Expression 'git push origin main'"`, + `pwsh -Command "iex 'curl https://evil.test'"`, + `powershell -Command 'iex $PAYLOAD'`, + `pwsh -Command "Invoke-Expression ('curl ' + 'https://evil.test')"`, + `for /f %%i in (list.txt) do %%i https://evil.test`, + `for /f %i in (list.txt) do %~dpi https://evil.test`, + `for /f %i in (list.txt) do call %i https://evil.test`, + // A batch parameter in the same position is equally unreadable. + `%1 https://evil.test & rem '`, + // git -C takes a separate-token value; the subcommand is what follows it, + // so push must still be found in both the separated and joined spellings. + `git -C repo push origin main`, + `git -Crepo push origin main`, + `git -C repo push origin main & rem '`, + `git -Crepo push origin main & rem '`, + // PowerShell source that exists but cannot be read statically: an + // undecodable encoded payload, valid encoded network source, and a + // Command operand supplied by an expansion. + `powershell -EncodedCommand curl & rem '`, + `powershell -EncodedCommand YwB1AHIAbAAgAGgAdAB0AHAAcwA6AC8ALwBlAHYAaQBsAC4AdABlAHMAdAA=`, + `pwsh -ec YwB1AHIAbAAgAGgAdAB0AHAAcwA6AC8ALwBlAHYAaQBsAC4AdABlAHMAdAA= & rem '`, + `PAYLOAD='curl https://evil.test'; powershell -Command "$PAYLOAD"`, + `PAYLOAD='curl https://evil.test'; powershell "$PAYLOAD"`, + // GNU env -S receives the shell-expanded value and executes its argv, so + // an operand this scan cannot resolve must not read as network-free. + `PAYLOAD='curl https://evil.test'; env -S "$PAYLOAD"`, + `PAYLOAD='curl https://evil.test'; env -S"$PAYLOAD"`, + `PAYLOAD='curl https://evil.test'; env --split-string="$PAYLOAD"`, + `env -S "$PAYLOAD" && "unterminated`, + // CMD's START continues taking switches after its optional window title. + `start "" /b curl https://evil.test & rem '`, + `start "" /wait curl https://evil.test & rem '`, + `start "" /d C:\ curl https://evil.test & rem '`, + `start "" /b git push origin main & rem '`, + } { + t.Run(command, func(t *testing.T) { + decision := NewEngine(EngineOptions{Policy: Policy{Mode: ModeEnforce, Network: NetworkDeny}}).Evaluate(context.Background(), Request{ + ToolName: "bash", + SideEffect: SideEffectShell, + Args: map[string]any{"command": command}, + Permission: PermissionAllow, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked { + t.Fatalf("Evaluate(%q) = action %q reason %q, want prompt/network blocked", command, decision.Action, decision.Reason) + } + }) + } +} + +// TestEngineDoesNotPromptForNonNetworkCommandForms is the negative half: text +// that resembles a network invocation but is actually a local argument, +// encoded payload, script name, or malformed remote option must not cost a +// network grant. +func TestEngineDoesNotPromptForNonNetworkCommandForms(t *testing.T) { + for _, command := range []string{ + "git -C repo --help push", + "git -h push", + `git -C repo -h push & rem '`, + "git archive HEAD -- --remote=origin", + `git archive -o --remote HEAD & rem '`, + `git archive HEAD --remote & rem '`, + // A decodable encoded payload is read, not guessed at: this one is the + // UTF-16LE source `evil`, a local program name. + `powershell -e ZQB2AGkAbAA= & rem '`, + `pwsh -File curl & rem '`, + // Bare --exec-path prints the local exec path and exits; git never reaches + // push, and /tmp is an operand rather than the option's value. + `git --exec-path`, + `git --exec-path /tmp push`, + `git --exec-path /tmp push & rem '`, + `pwsh Invoke-WebRequest https://evil.test`, + `powershell -Command:Invoke-WebRequest https://evil.test`, + `powershell -DefinitelyInvalid Invoke-WebRequest https://evil.test`, + `env -S 'printf curl; git push' && "unterminated`, + `env -S 'printf ok' curl https://evil.test && "unterminated`, + `env -S '--argv0 curl printf ok' && "unterminated`, + `start MyTitle curl https://evil.test & rem '`, + `bash /dev/null -c 'curl https://evil.test' && "unterminated`, + `bash -- -c 'curl https://evil.test' && "unterminated`, + `bash -Zc 'curl https://evil.test' && "unterminated`, + `bash -nc 'curl https://evil.test' && "unterminated`, + // FOR bodies whose executable IS readable keep their precision: the + // metavariable is ordinary data in an argument position, so recognizing + // it in executable position must not cost these a network prompt. + `for %i in (*.txt) do echo %i`, + // Simple PowerShell source the readers CAN tokenise and prove local keeps + // its quiet path: the fail-closed rule above is about block grammar, not + // about treating every PowerShell command as unknown. + `powershell -Command 'Write-Output hello'`, + `powershell -Command 'Get-Process'`, + `powershell -NoProfile -Command 'Get-ChildItem -Path .'`, + `powershell -Command "Invoke-Expression 'Write-Output hello'"`, + `pwsh -Command "iex 'Get-ChildItem -Path .'"`, + `for /f %i in (list.txt) do type %i`, + `for %i in (a b c) do copy %i backup\%i`, + `for /f %i in (list.txt) do echo %i >> out.txt`, + // -C's operand is consumed, so a local subcommand after it stays local. + `git -C repo status`, + `git -Crepo status & rem '`, + } { + t.Run(command, func(t *testing.T) { + engine := NewEngine(EngineOptions{WorkspaceRoot: t.TempDir(), Policy: DefaultPolicy()}) + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", + SideEffect: SideEffectShell, + Permission: PermissionPrompt, + PermissionMode: PermissionModeAsk, + Args: map[string]any{"command": command}, + }) + if decision.Reason == ReasonNetworkBlocked || HasRiskCategory(decision.Risk, "network") { + t.Fatalf("Evaluate(%q) = %#v, want no network classification", command, decision) + } + }) + } +} + func TestUnsandboxedExecutionAllowedPreservesDeniedReads(t *testing.T) { root := t.TempDir() policy := DefaultPolicy() @@ -876,3 +1314,105 @@ func TestEvaluateAllowsWritesInsideDefaultTempRoot(t *testing.T) { t.Fatalf("temp-root write risk=%v, must not be out_of_workspace", decision.Risk) } } + +// TestEvaluatePromptsForParseableNetworkLaunchers is the parity matrix for the +// PARSEABLE path. +// +// Every launcher covered by TestEvaluatePromptsForUnparseableNetworkBehindWrapper +// is exercised there behind an unparseable suffix (`& rem '`, `&& "unterminated`), +// which forces classification down the fallback. That proved the fallback and +// hid the real exposure: production input is usually parseable, and a launcher +// the AST path did not recurse into stayed clean no matter what the fallback +// would have said. `cmd /c "git push origin main"` and `eval git push origin main` +// were both caught by the fallback and both allowed here. +// +// So each case below must ALSO be parseable — the guard asserts it — and must +// still reach ActionPrompt/ReasonNetworkBlocked with the shell permission +// already granted. A new launcher belongs in both tables. +func TestEvaluatePromptsForParseableNetworkLaunchers(t *testing.T) { + engine := NewEngine(EngineOptions{Policy: Policy{Mode: ModeEnforce, Network: NetworkDeny}}) + for _, command := range []string{ + // Quoted CMD payloads are command SOURCE, not a program name. + `cmd /c "git push origin main"`, + `cmd /c "curl https://evil.test"`, + `cmd /d /c "git push origin main"`, + `cmd /k "curl https://evil.test"`, + `call "git push origin main"`, + `call "curl https://evil.test"`, + // Unquoted forms, which already worked, stay working. + `cmd /c git push origin main`, + `cmd.exe /d /c curl https://evil.test`, + // eval runs its arguments as shell source on this path too. + `eval git push origin main`, + `eval "curl https://evil.test"`, + `eval curl https://evil.test`, + // GNU env appends trailing argv to the split string's argv, so a dynamic + // token after a literal -S operand extends the command unreadably. + `env -S 'git push origin main' "$EXTRA"`, + `env -S 'printf ok' $PAYLOAD`, + `env -S 'printf ok' "${PAYLOAD}" https://evil.test`, + `env -S "$PAYLOAD"`, + // CMD's echo-suppression prefix is not part of the program name. + `@curl https://evil.test`, + `@git push origin main`, + // Nested shell launchers past the level where double-quote escape + // removal used to hand the recursion a fragment. + `sh -c "sh -c \"sh -c \\\"curl https://evil.test\\\"\""`, + `sh -c "sh -c \"git push origin main\""`, + // push's plumbing counterpart performs the same egress. + `git send-pack origin main`, + `git -C repo send-pack origin main`, + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) is TooComplex; this table must exercise the PARSEABLE path", command) + } + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": command}, + }) + if decision.Action != ActionPrompt || decision.Reason != ReasonNetworkBlocked { + t.Fatalf("Evaluate(%q) = action %q reason %q, want a network prompt", command, decision.Action, decision.Reason) + } + }) + } +} + +// The counterpart to the matrix above: failing closed must not mean flagging +// every launcher. These run locally and must NOT cost a network prompt, or the +// classifier trains users to approve egress reflexively. +func TestEvaluateAllowsParseableLocalLaunchers(t *testing.T) { + engine := NewEngine(EngineOptions{Policy: Policy{Mode: ModeEnforce, Network: NetworkDeny}}) + for _, command := range []string{ + `cmd /c "git status"`, + `cmd /c "echo hello world"`, + // A quoted path containing a space is a program name, not a command line. + `cmd /c "C:\Program Files\git\bin\git.exe status"`, + `call "git status"`, + `eval echo hi`, + `eval git status`, + // Trailing LITERAL argv after a split string is fully readable. + `env -S 'printf ok' literal args`, + `env -S 'git status' --`, + `@echo hello`, + `@git status`, + `sh -c "sh -c \"git status\""`, + // Escape removal must not damage an ordinary quoted Windows path. + `echo "C:\Users\me\file.txt"`, + `git status`, + `git --help push`, + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) is TooComplex; this table must exercise the PARSEABLE path", command) + } + decision := engine.Evaluate(context.Background(), Request{ + ToolName: "bash", SideEffect: SideEffectShell, PermissionGranted: true, + Args: map[string]any{"command": command}, + }) + if decision.Reason == ReasonNetworkBlocked { + t.Fatalf("Evaluate(%q) requested a network prompt for a local command", command) + } + }) + } +} diff --git a/internal/sandbox/risk.go b/internal/sandbox/risk.go index faa4e09bc..252c450dd 100644 --- a/internal/sandbox/risk.go +++ b/internal/sandbox/risk.go @@ -1,12 +1,15 @@ package sandbox import ( + "encoding/base64" "fmt" "path/filepath" "regexp" "sort" "strconv" "strings" + "unicode/utf16" + "unicode/utf8" ) var ( @@ -30,10 +33,6 @@ var ( // A purely local pipe into a shell (e.g. `printf … | sh`, `cat ./s | bash`) // is NOT a piped installer and must not be flagged. pipedInstallerPattern = regexp.MustCompile(`(?i)\b(curl|wget|fetch|aria2c)\b[^|]*\|\s*(ba|z|k|da)?sh\b`) - // unparseableNetworkPattern is used only after the shell parser fails. At - // that point the command is already marked too complex, so this intentionally - // favors catching obvious network programs over proving exact shell syntax. - unparseableNetworkPattern = regexp.MustCompile(`(?i)\b(curl|wget|fetch|aria2c|ssh|scp|sftp|rsync|nc|ncat|netcat|telnet|ftp|npx|http-server|vite|next|nuxt|astro)\b|\b(npm|pnpm|yarn|bun|pip|pip2|pip3)\s+(install|add|publish|login|start|serve|dev|preview|run\s+(start|serve|dev|preview)|exec|x|dlx)\b|\bgo\s+get\b|\bgit\s+clone\b|\bpython(2|3)?\s+-m\s+(http\.server|pip\s+install)\b|\bgh\s+(api|repo\s+clone|release\s+download)\b`) // destructiveExtraPatterns hold high-severity patterns that the legacy // destructiveCommandPattern does not already cover. Folded in from the // blueprint safe_bash.go without duplicating existing matches. @@ -52,6 +51,8 @@ var ( // mkfs. form (e.g. mkfs.ext4) not caught by the bare \bmkfs\b above when followed by a dot. regexp.MustCompile(`(?i)\bmkfs\.[a-z0-9]+\b`), } + cmdForFSingleQuotedCommandPattern = regexp.MustCompile(`(?is)(?:^|[&|;\r\n]\s*)@?(?:call\s+)?for\s+/f\b(.*?)\bin\s*\(\s*'([^']*)'\s*\)\s+do\b`) + cmdForFBacktickCommandPattern = regexp.MustCompile("(?is)(?:^|[&|;\\r\\n]\\s*)@?(?:call\\s+)?for\\s+/f\\b(.*?)\\bin\\s*\\(\\s*`([^`]*)`\\s*\\)\\s+do\\b") ) func matchesDestructive(command string) bool { @@ -66,8 +67,1278 @@ func matchesDestructive(command string) bool { return false } +// maxUnparseableShellDepth bounds `sh -c ` recursion in the fallback. +// It IS maxAnalyzerDepth rather than a copy of its value: a fallback that gave +// up a level earlier than the parseable path would drop the network category on +// exactly the deeply-nested launcher chains this path exists to fail closed on. +const maxUnparseableShellDepth = maxAnalyzerDepth + +// commandResolution is the shared security result for executable argv and +// interpreter source. Unresolved input is distinct from a proven-local command +// and retains the network gate at every caller. +type commandResolution uint8 + +const ( + commandKnownLocal commandResolution = iota + commandKnownNetwork + commandUnresolved +) + +func (resolution commandResolution) needsNetworkGate() bool { + return resolution != commandKnownLocal +} + +func commandNetworkResolution(network bool) commandResolution { + if network { + return commandKnownNetwork + } + return commandKnownLocal +} + func matchesUnparseableNetwork(command string) bool { - return unparseableNetworkPattern.MatchString(command) + return matchesUnparseableNetworkAt(command, 0) +} + +// matchesUnparseableNetworkAt scans each fallback segment for a network program. +// It resolves the segment's real program the same way the parseable path does — +// past environment assignments and wrapper prefixes (sudo, env, timeout, nice, +// xargs, ...) and the option values those wrappers consume — because a fallback +// that only looked at the first token would let `sudo curl …`, `env git fetch …`, +// or `PATH=.:$PATH git push …` through. The point of this path is to fail closed +// on a command too obfuscated to parse; a wrapper prefix is the cheapest possible +// obfuscation. +// +// Resolving the program (rather than matching the network name anywhere in the +// string, as an earlier revision did) is what keeps `git status push` and +// `echo https://example.com/repo.git push` out: a network verb only counts when +// it belongs to a program actually being invoked. +func matchesUnparseableNetworkAt(command string, depth int) bool { + if depth < maxUnparseableShellDepth { + for _, payload := range fallbackCMDForFCommands(command) { + if classifyInterpreterSource(payload, interpreterSourceCMD, depth+1).needsNetworkGate() { + return true + } + } + } + command = maskFallbackCMDForFLiteralBackticks(command) + for _, tokenInfo := range fallbackCommandTokenInfo(command) { + tokens := fallbackTokenValues(tokenInfo) + if depth < maxUnparseableShellDepth { + if split := envSplitCommandFields(tokens); split.recognized { + if split.executableEnvironmentDependent || split.commandSourceEnvironmentDependent || + fallbackBodyUsesNetwork(split.command, depth+1) { + return true + } + continue + } + } + for _, body := range fallbackCommandBodies(tokens) { + if fallbackBodyUsesNetwork(body, depth) { + return true + } + } + for _, body := range cmdCommandBodyTokenInfoCandidates(tokenInfo) { + // Shared with the AST path's cmdLauncherUsesNetwork so a quoted + // payload cannot be command text on one path and a program name on + // the other. + if cmdBodyUsesNetwork(body, depth) { + return true + } + } + } + // The segments above were cut under POSIX quoting. CMD honors only `"`, so + // re-cut the same text under its rules before handing it to the CMD readers; + // otherwise a quote form CMD ignores hides a separator it acts on. + if posixSingleQuoteHidesCMDSeparator(command) { + for _, tokenInfo := range fallbackCMDCommandTokenInfo(command) { + for _, body := range cmdCommandBodyTokenInfoCandidates(tokenInfo) { + if cmdBodyUsesNetwork(body, depth) { + return true + } + } + } + } + return false +} + +// fallbackCommandBodies resolves the executable position of one segment under +// every command language the text may belong to. The fallback runs on input the +// POSIX parser rejected, and on Windows that input is frequently valid CMD +// source rather than broken sh — `@curl …`, `call curl …`, `if not 1==2 curl …` +// and `start "" curl …` all execute curl under cmd.exe while resolving to a +// non-program token ("@curl", "call", "not", "start") under POSIX rules. Each +// runtime's resolver therefore gets its own shot at the segment; a resolution +// can only ADD the network category, which is the direction this fail-closed +// path is allowed to err in. +func fallbackCommandBodies(tokens []string) [][]string { + if body := fallbackCommandBodyFields(tokens); len(body) > 0 { + return [][]string{body} + } + return nil +} + +// fallbackBodyUsesNetwork classifies one resolved command body, recursing into +// the payloads of launchers that run command text of their own. +func fallbackBodyUsesNetwork(body []string, depth int) bool { + return resolveCommandArgv(body, depth).needsNetworkGate() +} + +// resolveCommandArgv is the bounded, wrapper-aware resolver shared by the AST +// and fallback paths. It distinguishes a proven-local command from network +// access and from argv/source that the supported launcher grammars cannot +// resolve. Only a proven-local result may omit the network gate. +func resolveCommandArgv(body []string, depth int) commandResolution { + if len(body) == 0 { + return commandKnownLocal + } + if depth > maxAnalyzerDepth { + return commandUnresolved + } + if split := envSplitCommandFields(body); split.recognized { + if split.executableEnvironmentDependent || split.commandSourceEnvironmentDependent { + return commandUnresolved + } + if len(split.command) == 0 { + return commandKnownLocal + } + return resolveCommandArgv(split.command, depth+1) + } + if resolved := commandBodyFields(body); len(resolved) > 0 { + body = resolved + } else { + return commandKnownLocal + } + program, args := executableTokenBase(body[0]), body[1:] + if program == "%comspec%" { + program = "cmd" + } + if fallbackTokenLooksDynamic(body[0]) { + return commandUnresolved + } + switch program { + case "busybox": + command, status := busyboxDelegatedCommand(args) + if status != commandKnownLocal { + return status + } + if len(command) == 0 { + return commandKnownLocal + } + return resolveCommandArgv(command, depth+1) + case "strace": + command, status := straceDelegatedCommand(args) + if status != commandKnownLocal { + return status + } + if len(command) == 0 { + return commandKnownLocal + } + return resolveCommandArgv(command, depth+1) + case "eval": + if len(args) == 0 { + return commandKnownLocal + } + return classifyInterpreterSource(strings.Join(args, " "), interpreterSourcePOSIX, depth+1) + case "call": + if len(args) == 0 { + return commandKnownLocal + } + if fallbackTokenLooksDynamic(args[0]) { + return commandUnresolved + } + return resolveCommandArgv(args, depth+1) + } + if shellPrograms[program] { + index, found := shellCommandPayloadIndex(program, args) + if !found || index >= len(args) { + return commandKnownLocal + } + payload := args[index] + if fallbackTokenLooksDynamic(payload) { + return commandUnresolved + } + return classifyInterpreterSource(payload, interpreterSourcePOSIX, depth+1) + } + if program == "powershell" || program == "pwsh" { + source := fallbackPowerShellPayload(program, args) + switch { + case source.opaque: + return commandUnresolved + case source.payload == "": + return commandKnownLocal + default: + return classifyInterpreterSource(source.payload, interpreterSourcePowerShell, depth+1) + } + } + if payload := fallbackCommandInterpreterPayload(program, args); payload != "" { + resolution := classifyInterpreterSource(payload, interpreterSourceCMD, depth+1) + if resolution.needsNetworkGate() { + return resolution + } + if fallbackPayloadUsesNetwork(strings.Join(fallbackCommandInterpreterArgs(program, args), " "), depth+1) { + return commandKnownNetwork + } + return commandKnownLocal + } + resolution := literalProgramNetworkResolution(program, args) + if program == "git" && resolution == commandKnownLocal && + gitSelectionDynamic(args, func(index int) bool { return fallbackTokenLooksDynamic(args[index]) }) { + // The fallback never expands these values, so a subcommand it cannot read + // is not a subcommand it has proven local. See gitSelectionDynamic. + return commandUnresolved + } + return resolution +} + +// fallbackTokenLooksDynamic reports whether a token selected as executable +// source still contains an unresolved POSIX or CMD expansion. The fallback +// never expands these values, so treating the spelling as an ordinary unknown +// program would turn "cannot resolve" into "does not use the network." +func fallbackTokenLooksDynamic(token string) bool { + if strings.ContainsAny(token, "$`") { + return true + } + if containsCMDVariableExpansion(token, '%') || containsCMDVariableExpansion(token, '!') { + return true + } + return containsCMDSingleDelimiterExpansion(token) +} + +func containsCMDVariableExpansion(token string, delimiter byte) bool { + for start := 0; start < len(token); start++ { + if token[start] != delimiter { + continue + } + endOffset := strings.IndexByte(token[start+1:], delimiter) + if endOffset < 0 { + return false + } + content := token[start+1 : start+1+endOffset] + if delimiter == '%' { + if colon := strings.IndexByte(content, ':'); colon >= 0 { + content = content[:colon] + } + } + if validCMDVariableName(content) { + return true + } + start += endOffset + 1 + } + return false +} + +func validCMDVariableName(name string) bool { + if name == "" { + return false + } + for index, r := range name { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || r == '_' || + (index > 0 && r >= '0' && r <= '9') { + continue + } + return false + } + return true +} + +// containsCMDSingleDelimiterExpansion reports whether a token IS a CMD +// expansion written with one leading percent rather than a matched pair: a FOR +// metavariable (%i, the batch %%i spelling, and the ~-modified %~dpi forms) or +// a batch parameter (%1). containsCMDVariableExpansion cannot see these, +// because it looks for %NAME% and these have no closing delimiter. +// +// CMD substitutes them before launching the command, so in an EXECUTABLE +// position the spelling says nothing about what runs: `for /f %i in (list.txt) +// do %i https://host` executes whatever list.txt names. Reading %i as an +// ordinary unknown program turned "cannot resolve" into "proven local" and +// dropped the network gate. +// +// The reference must BE the token, not merely appear inside it, and the name +// is a single character. A metavariable name is one char by CMD's grammar, so +// `%local` is `%l` followed by literal text rather than a reference, and +// `100%local` does not start with the sigil at all — both are ordinary program +// spellings that keep their proven-local answer. Only executable and +// interpreter-source positions consult this (see the callers of +// fallbackTokenLooksDynamic), so literal FOR set data and an echo-style body's +// arguments keep their precision too: `for %i in (*.txt) do echo %i` still +// resolves through the literal program `echo`. +func containsCMDSingleDelimiterExpansion(token string) bool { + if !strings.HasPrefix(token, "%") { + return false + } + rest := token[1:] + // A batch file doubles the sigil (%%i); both spellings reach the same + // substitution, so step over the second one and read the name. + rest = strings.TrimPrefix(rest, "%") + if rest == "" { + return false + } + // %~dpi and friends: the modifier run only ever introduces a metavariable + // reference, so the '~' alone is conclusive. + if rest[0] == '~' { + return true + } + if !isCMDMetavariableName(rest[0]) { + return false + } + // One character names the variable; anything else that could continue a + // name means this token is literal text, not a reference. + return len(rest) == 1 || !isCMDMetavariableName(rest[1]) +} + +// isCMDMetavariableName reports whether c can name a FOR metavariable or a +// batch parameter. CMD accepts any single alphanumeric character here, and the +// name is case-sensitive (%i and %I are different variables). +func isCMDMetavariableName(c byte) bool { + return (c >= 'a' && c <= 'z') || (c >= 'A' && c <= 'Z') || (c >= '0' && c <= '9') +} + +// fallbackPayloadUsesNetwork classifies CMD payload text, so it reads the text +// under CMD's quoting as well as the shared POSIX tokenization — a `'` CMD +// treats as ordinary text must not hide the separator that follows it. +func fallbackPayloadUsesNetwork(payload string, depth int) bool { + tokenizations := [][][]fallbackCommandToken{fallbackCommandTokenInfo(payload)} + if posixSingleQuoteHidesCMDSeparator(payload) { + tokenizations = append(tokenizations, fallbackCMDCommandTokenInfo(payload)) + } + for _, tokenInfos := range tokenizations { + for _, tokenInfo := range tokenInfos { + if len(tokenInfo) > 0 && strings.EqualFold(tokenInfo[0].value, "start") { + if body := cmdStartPayloadTokenInfo(tokenInfo[1:]); len(body) > 0 && fallbackBodyUsesNetwork(fallbackTokenValues(body), depth) { + return true + } + } + for _, body := range cmdCommandBodyTokenInfoCandidates(tokenInfo) { + if fallbackBodyUsesNetwork(fallbackTokenValues(body), depth) { + return true + } + } + } + } + return false +} + +var fallbackLeadingShellKeywords = map[string]bool{ + "!": true, "if": true, "while": true, "until": true, "then": true, + "do": true, "else": true, "elif": true, "in": true, "coproc": true, + "{": true, "}": true, +} + +// fallbackCommandBodyFields resolves a command at a shell-control boundary. +// In addition to ordinary wrappers, an unparseable compound command can put +// control-flow keywords and redirections before the executable (`then curl`, +// `do wget`, `>out git push`). Those prefixes do not change the program being +// invoked and must not make the fail-closed network check lose sight of it. +func fallbackCommandBodyFields(fields []string) []string { + // Shell keywords are syntax only at the original command boundary. Do not + // reinterpret a wrapper's payload as syntax: `command if curl` attempts to + // execute a program named "if" and does not invoke curl. + for len(fields) > 0 && fallbackLeadingShellKeywords[strings.ToLower(fields[0])] { + fields = fields[1:] + } + body := commandBodyFields(fields) + for len(body) > 0 { + word := strings.ToLower(body[0]) + if consumesRedirectTarget(word) { + if len(body) == 1 { + return nil + } + body = commandBodyFields(body[2:]) + continue + } + if isRedirectToken(word) { + body = commandBodyFields(body[1:]) + continue + } + return body + } + return nil +} + +// cmdComparisonOperators are CMD's IF comparison operators, which sit between +// the two values being compared (`if %x% equ 1 curl …`). +var cmdComparisonOperators = map[string]bool{ + "equ": true, "neq": true, "lss": true, "leq": true, "gtr": true, "geq": true, +} + +type fallbackCommandToken struct { + value string + quoted bool +} + +func fallbackTokenValues(tokens []fallbackCommandToken) []string { + values := make([]string, len(tokens)) + for index := range tokens { + values[index] = tokens[index].value + } + return values +} + +func cmdCommandBodyTokenInfoCandidates(fields []fallbackCommandToken) [][]fallbackCommandToken { + fields = normalizeCMDCommandTokens(fields) + for len(fields) > 0 { + fields = trimCMDEchoPrefix(fields) + if len(fields) == 0 { + return nil + } + switch strings.ToLower(fields[0].value) { + case "call", "else", "then", "do": + fields = fields[1:] + case "cmd", "cmd.exe", "%comspec%": + payload := cmdInterpreterPayloadTokenInfo(fields[1:]) + if len(payload) == 0 { + return nil + } + fields = payload + case "start": + body := cmdStartPayloadTokenInfo(fields[1:]) + if len(body) == 0 { + return nil + } + body = trimCMDEchoPrefix(body) + if len(body) == 0 { + return nil + } + return [][]fallbackCommandToken{body} + case "if": + values := cmdConditionPayload(fallbackTokenValues(fields[1:])) + if len(values) == 0 { + return nil + } + fields = fields[len(fields)-len(values):] + case "for": + values := cmdForPayload(fallbackTokenValues(fields[1:])) + if len(values) == 0 { + return nil + } + fields = fields[len(fields)-len(values):] + default: + return [][]fallbackCommandToken{fields} + } + } + return nil +} + +func cmdInterpreterPayloadTokenInfo(fields []fallbackCommandToken) []fallbackCommandToken { + for index, field := range fields { + flag := strings.ToLower(field.value) + if !strings.HasPrefix(flag, "/c") && !strings.HasPrefix(flag, "/k") { + continue + } + if len(field.value) > 2 { + joined := field + joined.value = field.value[2:] + return append([]fallbackCommandToken{joined}, fields[index+1:]...) + } + index++ + for index < len(fields) && isCMDInterpreterSwitch(fields[index].value) { + index++ + } + return fields[index:] + } + return nil +} + +func normalizeCMDCommandTokens(fields []fallbackCommandToken) []fallbackCommandToken { + normalized := make([]fallbackCommandToken, 0, len(fields)) + for _, field := range fields { + if field.quoted { + normalized = append(normalized, field) + continue + } + // The shared tokenizer reads `\` as a POSIX escape, so a Windows path that + // ends in a separator swallows the following word: `start "" /d C:\ curl …` + // arrives as the single token `C:\ curl`, which lets a value-taking switch + // consume the executable. CMD has no such escape, so split those apart + // before resolving CMD's own grammar. + for _, part := range splitCMDEscapedWhitespace(field.value) { + normalized = append(normalized, fallbackCommandToken{value: normalizeCMDToken(part)}) + } + } + return normalized +} + +func splitCMDEscapedWhitespace(value string) []string { + var parts []string + start := 0 + for index := 0; index+1 < len(value); index++ { + if value[index] != '\\' || (value[index+1] != ' ' && value[index+1] != '\t') { + continue + } + if part := value[start : index+1]; part != "" { + parts = append(parts, part) + } + start = index + 2 + index++ + } + if part := value[start:]; part != "" || len(parts) == 0 { + parts = append(parts, part) + } + return parts +} + +// trimCMDEchoPrefixToken strips CMD's echo-suppression prefix from a single +// program token. It is the one-token form of trimCMDEchoPrefix, shared so the +// AST and fallback paths cannot disagree about whether "@curl" names curl. +func trimCMDEchoPrefixToken(token string) string { + return strings.TrimLeft(strings.TrimSpace(token), "@") +} + +func trimCMDEchoPrefix(fields []fallbackCommandToken) []fallbackCommandToken { + for len(fields) > 0 { + fields[0].value = strings.TrimLeft(fields[0].value, "@") + if fields[0].value != "" { + break + } + fields = fields[1:] + } + return fields +} + +// cmdStartPayloadTokenInfo resolves the command START will launch. START's +// grammar is switches, an optional quoted window title, then more switches: +// `start "" /b curl …` is a real invocation of curl. A single prefix strip left +// `/b` in the executable position, so neither the CMD nor the POSIX resolver +// contributed the network category for that form. +func cmdStartPayloadTokenInfo(fields []fallbackCommandToken) []fallbackCommandToken { + fields = cmdStartOperandsTokenInfo(fields) + if len(fields) > 0 && fields[0].quoted { + fields = cmdStartOperandsTokenInfo(fields[1:]) + } + return fields +} + +func cmdStartOperandsTokenInfo(fields []fallbackCommandToken) []fallbackCommandToken { + for len(fields) > 0 && strings.HasPrefix(fields[0].value, "/") { + switch strings.ToLower(fields[0].value) { + case "/d", "/node", "/affinity": + if len(fields) < 2 { + return nil + } + fields = fields[2:] + default: + fields = fields[1:] + } + } + return fields +} + +// cmdConditionPayload skips an IF condition and returns the guarded command. +func cmdConditionPayload(fields []string) []string { + for len(fields) > 0 { + word := strings.ToLower(strings.Trim(fields[0], `"`)) + switch { + case strings.HasPrefix(word, "/"): // /I, /D — case-insensitivity switches + fields = fields[1:] + case word == "not": + fields = fields[1:] + case word == "errorlevel" || word == "exist" || word == "defined" || word == "cmdextversion": + // Keyword plus its single operand. + if len(fields) < 2 { + return nil + } + return fields[2:] + case len(fields) >= 2 && (fields[1] == "==" || cmdComparisonOperators[strings.ToLower(fields[1])]): + // A spaced comparison: . + if len(fields) < 3 { + return nil + } + return fields[3:] + case strings.Contains(word, "=="): + // The whole comparison arrived as one token (`1==1`). + return fields[1:] + default: + // Unknown condition grammar cannot establish a command boundary. + return nil + } + } + return nil +} + +// cmdForPayload returns the command after DO, which is the only part of a FOR +// loop that executes. `for %i in (curl) do echo %i` must not resolve to curl. +// +// DO counts only OUTSIDE the IN (...) set. `do` is an ordinary word inside it, +// and CMD accepts it there: in `for %i in (do x) do curl https://evil.test` the +// first `do` is a set ELEMENT, so selecting it resolved the classifier from `x` +// and never reached the curl that actually runs. Tracking parenthesis depth +// picks the loop keyword instead, and an unbalanced set — which CMD itself +// rejects — yields no body rather than a guessed one. +func cmdForPayload(fields []string) []string { + depth := 0 + quoted := false + for index, field := range fields { + if depth == 0 && strings.EqualFold(strings.Trim(field, `"`), "do") { + return fields[index+1:] + } + depth, quoted = cmdParenDepth(field, depth, quoted) + } + return nil +} + +// cmdParenDepth advances the parenthesis depth across one token, ignoring +// parentheses inside double quotes. Quote state is carried between tokens +// because a quoted run with a space in it can span them. +func cmdParenDepth(field string, depth int, quoted bool) (int, bool) { + for _, r := range field { + switch { + case r == '"': + quoted = !quoted + case quoted: + case r == '(': + depth++ + case r == ')': + if depth > 0 { + depth-- + } + } + } + return depth, quoted +} + +// fallbackCMDForFCommands extracts the command source that CMD FOR /F executes +// from its IN clause. Without usebackq, single quotes denote command text; with +// usebackq, backticks do. The other quote form is literal input and must not be +// classified as an invocation. +func fallbackCMDForFCommands(command string) []string { + var payloads []string + commandBoundaries := fallbackCMDCommandBoundaries(command) + for _, indexes := range cmdForFSingleQuotedCommandPattern.FindAllStringSubmatchIndex(command, -1) { + if len(indexes) < 6 || !commandBoundaries[indexes[0]] { + continue + } + match := []string{command[indexes[0]:indexes[1]], command[indexes[2]:indexes[3]], command[indexes[4]:indexes[5]]} + if len(match) == 3 && !cmdForFUsesBackq(match[1]) { + payloads = append(payloads, match[2]) + } + } + for _, indexes := range cmdForFBacktickCommandPattern.FindAllStringSubmatchIndex(command, -1) { + if len(indexes) < 6 || !commandBoundaries[indexes[0]] { + continue + } + match := []string{command[indexes[0]:indexes[1]], command[indexes[2]:indexes[3]], command[indexes[4]:indexes[5]]} + if len(match) == 3 && cmdForFUsesBackq(match[1]) { + payloads = append(payloads, match[2]) + } + } + return payloads +} + +func cmdForFUsesBackq(options string) bool { + for _, option := range strings.Fields(strings.Trim(options, ` "`)) { + if strings.EqualFold(strings.Trim(option, `"`), "usebackq") { + return true + } + } + return false +} + +func maskFallbackCMDForFLiteralBackticks(command string) string { + masked := []byte(command) + commandBoundaries := fallbackCMDCommandBoundaries(command) + for _, indexes := range cmdForFBacktickCommandPattern.FindAllStringSubmatchIndex(command, -1) { + if len(indexes) < 6 || !commandBoundaries[indexes[0]] || + cmdForFUsesBackq(command[indexes[2]:indexes[3]]) { + continue + } + for index := indexes[4]; index < indexes[5]; index++ { + masked[index] = ' ' + } + } + return string(masked) +} + +// fallbackCMDCommandBoundaries records whether each byte offset is outside a +// quoted or caret-escaped CMD region. FOR /F regex matches can then check their +// start in O(1) instead of rescanning every preceding byte for every match. +func fallbackCMDCommandBoundaries(command string) []bool { + boundaries := make([]bool, len(command)+1) + quoted := false + escaped := false + for index := 0; index < len(command); index++ { + boundaries[index] = !quoted && !escaped + switch { + case escaped: + escaped = false + case command[index] == '^': + escaped = true + case command[index] == '"': + quoted = !quoted + } + } + boundaries[len(command)] = !quoted && !escaped + return boundaries +} + +func consumesRedirectTarget(word string) bool { + word = strings.TrimLeft(word, "0123456789") + return word == ">" || word == ">>" || word == "<" || word == "<<" || word == "<<-" || + word == "<<<" || word == "<>" || word == ">|" +} + +func isRedirectToken(word string) bool { + word = strings.TrimLeft(word, "0123456789") + return strings.HasPrefix(word, ">") || strings.HasPrefix(word, "<") +} + +// shellCommandPayloadIndex returns the one argv element a shell launcher will +// execute for -c. It parses only the leading option region: a script operand or +// `--` makes later `-c` text positional, and an invalid option cluster is not +// treated as executable source. +func shellCommandPayloadIndex(program string, args []string) (int, bool) { + noExecute, dumpStrings := false, false + for index := 0; index < len(args); index++ { + option := args[index] + if option == "--" || option == "-" || option == "+" || + (!strings.HasPrefix(option, "-") && !strings.HasPrefix(option, "+")) { + return 0, false + } + if strings.HasPrefix(option, "--") { + if program != "bash" { + return 0, false + } + name := option + if equals := strings.IndexByte(option, '='); equals >= 0 { + name = option[:equals] + } + switch name { + case "--dump-po-strings", "--dump-strings": + return 0, false + case "--debug", "--debugger", "--login", "--noediting", "--noprofile", "--norc", "--posix", + "--pretty-print", "--restricted", "--verbose": + if name != option { + return 0, false + } + case "--init-file", "--rcfile": + if name == option { + index++ + if index >= len(args) { + return 0, false + } + } + case "--help", "--version": + return 0, false + default: + return 0, false + } + continue + } + + validOptions := "abefhkmnptuvxBCEHPTilrsDc" + valueOptions := "o" + switch program { + case "bash": + valueOptions += "O" + case "dash", "sh": + validOptions = "abCefnuvxIimslpc" + default: + // ksh/zsh share the common invocation flags used here, including -l. + validOptions = "abefhkmnptuvxBCilrsc" + } + command, values := false, 0 + enable := option[0] == '-' + for _, flag := range option[1:] { + switch { + case flag == 'c': + command = true + case flag == 'n': + noExecute = enable + case program == "bash" && flag == 'D': + dumpStrings = true + case strings.ContainsRune(valueOptions, flag): + values++ + case strings.ContainsRune(validOptions, flag): + default: + return 0, false + } + } + index += values + if index >= len(args) { + return 0, false + } + if command { + if noExecute || dumpStrings { + return 0, false + } + sourceIndex := index + 1 + if sourceIndex < len(args) && args[sourceIndex] == "--" { + sourceIndex++ + } + if sourceIndex >= len(args) { + return 0, false + } + return sourceIndex, true + } + } + return 0, false +} + +func fallbackCommandInterpreterPayload(program string, args []string) string { + for index, arg := range args { + flag := strings.ToLower(arg) + if program == "cmd" && (strings.HasPrefix(flag, "/c") || strings.HasPrefix(flag, "/k")) { + if len(arg) > 2 { + return strings.TrimSpace(strings.Join(append([]string{arg[2:]}, args[index+1:]...), " ")) + } + index++ + for index < len(args) && isCMDInterpreterSwitch(args[index]) { + index++ + } + return strings.Join(args[index:], " ") + } + } + if program == "powershell" || program == "pwsh" { + return fallbackPowerShellPayload(program, args).payload + } + return "" +} + +func fallbackCommandInterpreterArgs(program string, args []string) []string { + for index, arg := range args { + flag := strings.ToLower(arg) + if program == "cmd" && (strings.HasPrefix(flag, "/c") || strings.HasPrefix(flag, "/k")) { + if len(arg) > 2 { + return append([]string{arg[2:]}, args[index+1:]...) + } + index++ + for index < len(args) && isCMDInterpreterSwitch(args[index]) { + index++ + } + return args[index:] + } + } + return nil +} + +func isCMDInterpreterSwitch(arg string) bool { + flag := strings.ToLower(arg) + switch flag { + case "/d", "/s", "/q", "/a", "/u": + return true + default: + return strings.HasPrefix(flag, "/e:") || strings.HasPrefix(flag, "/f:") || strings.HasPrefix(flag, "/v:") + } +} + +// powerShellPayload describes what a PowerShell host invocation will execute. +// +// The three outcomes are distinct and must not be collapsed: readable source +// (payload), source that exists but cannot be read statically (opaque), and no +// inline source at all (a script File, a version query, an invalid switch). +// Treating the middle case as the last one is how `-EncodedCommand ` parsed cleanly and was allowed without a +// network grant. +type powerShellPayload struct { + payload string + opaque bool + // sourceIndex is where the command source begins in args, or -1 when the + // invocation carries none. Callers on the AST path use it to check whether + // the source words are static literals. + sourceIndex int +} + +// maxPowerShellEncodedCommandBytes bounds the attacker-controlled base64 this +// path will decode. +const maxPowerShellEncodedCommandBytes = 64 << 10 + +func fallbackPowerShellPayload(program string, args []string) powerShellPayload { + none := powerShellPayload{sourceIndex: -1} + for index := 0; index < len(args); index++ { + arg := args[index] + if !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "/") { + // Windows PowerShell's first positional parameter is Command. PowerShell + // 6+ changed pwsh's default to File, a script path rather than source. + if program == "pwsh" { + return none + } + return powerShellPayload{payload: strings.Join(args[index:], " "), sourceIndex: index} + } + flag := strings.TrimLeft(strings.ToLower(arg), "-/") + if strings.Contains(flag, ":") { + // The native host does not accept colon-joined option values. + return none + } + switch { + case program == "pwsh" && (flag == "commandwithargs" || flag == "cwa"): + if index+1 < len(args) { + return powerShellPayload{payload: args[index+1], sourceIndex: index + 1} + } + return none + case powerShellFlagAbbreviates(flag, "command"): + if index+1 < len(args) { + return powerShellPayload{payload: strings.Join(args[index+1:], " "), sourceIndex: index + 1} + } + return none + case flag == "ec" || powerShellFlagAbbreviates(flag, "encodedcommand"): + if index+1 >= len(args) { + return none + } + // Encoded source is still source. Decode the bounded, valid forms so a + // benign payload stays quiet, and fail closed on anything that does not + // decode rather than reading it as network-free. + if decoded, ok := decodePowerShellEncodedCommand(args[index+1]); ok { + return powerShellPayload{payload: decoded, sourceIndex: index + 1} + } + return powerShellPayload{opaque: true, sourceIndex: index + 1} + case powerShellFlagAbbreviates(flag, "file"): + // A script path, not inline source: nothing here to classify. + return none + case program == "pwsh" && (flag == "v" || flag == "version"): + return none + case powerShellOptionConsumesValue(program, flag): + if index+1 >= len(args) { + return none + } + index++ + case powerShellValuelessOption(program, flag): + case flag == "h" || flag == "help" || flag == "?": + return none + default: + // Unknown host switches terminate option parsing with an error; do not + // reinterpret a later network-looking argument as PowerShell source. + return none + } + } + return none +} + +// decodePowerShellEncodedCommand decodes the base64 UTF-16LE source carried by +// -EncodedCommand. A value that is not valid base64, not an even number of +// bytes, or larger than the bound is not decodable here and is reported as +// such so the caller can fail closed. +func decodePowerShellEncodedCommand(value string) (string, bool) { + value = strings.Trim(value, `"'`) + if value == "" || len(value) > maxPowerShellEncodedCommandBytes { + return "", false + } + raw, err := base64.StdEncoding.DecodeString(value) + if err != nil { + return "", false + } + if len(raw) == 0 || len(raw)%2 != 0 { + return "", false + } + units := make([]uint16, 0, len(raw)/2) + for index := 0; index+1 < len(raw); index += 2 { + units = append(units, uint16(raw[index])|uint16(raw[index+1])<<8) + } + decoded := string(utf16.Decode(units)) + if strings.ContainsRune(decoded, 0) || !utf8.ValidString(decoded) { + return "", false + } + return decoded, true +} + +func powerShellFlagAbbreviates(flag, fullName string) bool { + return flag != "" && strings.HasPrefix(fullName, flag) +} + +func powerShellOptionConsumesValue(program, flag string) bool { + switch flag { + case "configurationfile", "configurationname", "config", "custompipename", + "encodedarguments", "ea", + "executionpolicy", "ex", "ep", "inputformat", "inp", "if", + "outputformat", "o", "of", "psconsolefile", "settingsfile", "settings", + "windowstyle", "w", "workingdirectory", "wd", "wo": + return true + case "version", "v": + return program == "powershell" + default: + return false + } +} + +func powerShellValuelessOption(program, flag string) bool { + switch flag { + case "mta", "sta", "noexit", "noe", "nologo", "nol", "noninteractive", "noni", + "noprofile", "nop": + return true + case "interactive", "login", "noprofileloadtime", "sshservermode": + return program == "pwsh" + default: + return false + } +} + +func normalizeCMDToken(token string) string { + var out strings.Builder + for index := 0; index < len(token); index++ { + if token[index] == '^' && index+1 < len(token) { + index++ + } + out.WriteByte(token[index]) + } + return out.String() +} + +// executableTokenBase reduces a raw fallback token to a comparable program name. +// It strips quoting, any directory prefix, and a Windows executable suffix, so +// this path recognizes curl.exe and git.cmd exactly as normalizeProgramToken does +// on the parseable path — a token that normalized differently here used to be how +// `curl.exe https://… && "unterminated` lost its network classification. +// +// Drive-relative spellings go through windowsExecutablePathBasename for the same +// reason: `C:git.exe` has no separator to cut on, so a plain basename scan leaves +// `c:git` and the deny never matches a program the parseable path classifies. +func executableTokenBase(token string) string { + token = strings.Trim(token, `\"'`) + if basename, ok := windowsExecutablePathBasename(token); ok { + token = basename + } else if slash := strings.LastIndexAny(token, `/\`); slash >= 0 { + token = token[slash+1:] + } + return trimExecutableSuffix(strings.ToLower(token)) +} + +type fallbackDelimiterFrame struct { + opener rune + quote rune +} + +// fallbackCommandTokens performs deliberately small shell/cmd tokenization. +// It preserves quoted spaces even when the command's trailing quote is +// unmatched (the condition that sends classification down this fallback). +func fallbackCommandTokens(command string) [][]string { + infos := fallbackCommandTokenInfo(command) + commands := make([][]string, len(infos)) + for index := range infos { + commands[index] = fallbackTokenValues(infos[index]) + } + return commands +} + +func fallbackCommandTokenInfo(command string) [][]fallbackCommandToken { + return fallbackCommandTokenInfoIn(command, false) +} + +// fallbackCMDCommandTokenInfo tokenizes the same text under CMD's quoting rules, +// where `"` is the ONLY quote character. +// +// The POSIX reading of `echo ' & curl https://evil.test` puts `& curl …` inside +// the echo argument, so the segment resolves to echo and classifies local. CMD +// has no single-quote quoting at all: the `'` is literal text, `&` is a command +// separator, and curl runs. The same holds for backticks, which execute nothing +// under CMD. Any text handed to a CMD reader is tokenized both ways so a quote +// form one runtime ignores cannot hide a separator from the other. +func fallbackCMDCommandTokenInfo(command string) [][]fallbackCommandToken { + return fallbackCommandTokenInfoIn(command, true) +} + +// posixSingleQuoteHidesCMDSeparator reports the one disagreement that matters: +// a POSIX single-quoted region — quoted to this tokenizer, ordinary text to CMD +// — containing a CMD command separator. In `echo ' & curl https://evil.test` +// the quote swallows `& curl …` into echo's argument, while cmd.exe runs curl. +// +// Merely containing a `'` is not enough to justify re-reading the text: POSIX +// constructs whose quoted regions hide nothing (`env -S 'printf ok' …`) would +// re-segment into shapes that mean nothing under CMD, and a fail-closed pass +// that fires on them is just noise. `;` is deliberately not a separator here — +// CMD treats it as an argument delimiter, not a command boundary. Double quotes +// quote under both languages, so they hide nothing from either. +func posixSingleQuoteHidesCMDSeparator(command string) bool { + escaped := false + var quote rune + for _, r := range command { + switch { + case escaped: + escaped = false + case r == '\\': + escaped = true + case quote == 0 && (r == '\'' || r == '"'): + quote = r + case quote == r: + quote = 0 + case quote == '\'' && (r == '&' || r == '|'): + return true + } + } + return false +} + +func fallbackCommandTokenInfoIn(command string, cmdQuoting bool) [][]fallbackCommandToken { + // Command strings commonly preserve cmd.exe's escaped quote spelling. + command = strings.ReplaceAll(command, `\"`, `"`) + var commands [][]fallbackCommandToken + var tokens []fallbackCommandToken + var word strings.Builder + var quote rune + wordQuoted := false + var delimiters []fallbackDelimiterFrame + backtick := false + escaped := false + flush := func() { + if word.Len() > 0 || wordQuoted { + tokens = append(tokens, fallbackCommandToken{value: word.String(), quoted: wordQuoted}) + word.Reset() + wordQuoted = false + } + } + flushCommand := func() { + flush() + if len(tokens) > 0 { + commands = append(commands, tokens) + tokens = nil + } + } + runes := []rune(command) + for index, r := range runes { + if escaped { + word.WriteRune(r) + escaped = false + continue + } + if r == '\\' { + word.WriteRune(r) + escaped = true + continue + } + // Backticks execute inside unquoted and double-quoted text, but are + // literal inside single quotes. Preserve the surrounding quote while the + // substitution body is scanned as its own command. CMD has no command + // substitution, so under its rules a backtick is ordinary text. + if r == '`' && quote != '\'' && !cmdQuoting { + flushCommand() + if backtick { + if len(delimiters) > 0 && delimiters[len(delimiters)-1].opener == '`' { + quote = delimiters[len(delimiters)-1].quote + delimiters = delimiters[:len(delimiters)-1] + } + } else { + delimiters = append(delimiters, fallbackDelimiterFrame{opener: '`', quote: quote}) + quote = 0 + } + backtick = !backtick + continue + } + if r == '\'' && cmdQuoting { + // Literal text to CMD, and deliberately NOT a word boundary: the + // point of this pass is that the quote hides nothing. + word.WriteRune(r) + continue + } + if r == '\'' || r == '"' { + switch quote { + case 0: + quote = r + wordQuoted = true + case r: + quote = 0 + default: + word.WriteRune(r) + } + continue + } + // A verified function declaration makes its brace or subshell body a new + // executable region. Without this boundary, `f(){ curl ...; }; f` resolves + // the whole segment to the syntax token `f()` and hides the body. + if r == '{' && quote == 0 && fallbackFunctionDeclaration(tokens, word.String()) { + flushCommand() + continue + } + // Command and process substitutions execute even inside double quotes. + // Ordinary/arithmetic/array parentheses do not: splitting all parens made + // `${curl}`, `$((curl))`, and `arr=(curl)` look like curl invocations. + if r == '(' && quote != '\'' { + current := word.String() + if quote == 0 && current == "" && fallbackFunctionDeclaration(tokens, "") { + flushCommand() + delimiters = append(delimiters, fallbackDelimiterFrame{opener: '(', quote: 0}) + continue + } + nextIsParen := index+1 < len(runes) && runes[index+1] == '(' + substitution := (strings.HasSuffix(current, "$") && !nextIsParen) || + strings.HasSuffix(current, "<") || strings.HasSuffix(current, ">") + // CMD conditionals put the command group after condition tokens, e.g. + // `if 1==1 (curl ...)`; the opening parenthesis is still a command + // boundary even though it is not the segment's first token. + grouping := quote == 0 && word.Len() == 0 && + (len(tokens) == 0 || startsCMDCommandGroup(fallbackTokenValues(tokens))) + if substitution || grouping { + flushCommand() + delimiters = append(delimiters, fallbackDelimiterFrame{opener: '(', quote: quote}) + quote = 0 + continue + } + } + if r == ')' && quote == 0 && len(delimiters) > 0 && delimiters[len(delimiters)-1].opener == '(' { + flushCommand() + quote = delimiters[len(delimiters)-1].quote + delimiters = delimiters[:len(delimiters)-1] + continue + } + // A case pattern's closing parenthesis starts the command body. It is not + // paired with an opening command-group parenthesis. + if r == ')' && quote == 0 && len(tokens) > 0 && tokens[0].value == "case" { + flushCommand() + continue + } + // A newline separates commands exactly as ;/&/| do. Treating it as mere + // whitespace kept a multi-line script as one segment, so anything after the + // first line was scanned as arguments of the first line's program and the + // program on line two was never resolved. + if quote == 0 && (r == ';' || r == '&' || r == '|' || r == '\n' || r == '\r') { + flushCommand() + continue + } + if quote == 0 && (r == ' ' || r == '\t') { + flush() + continue + } + word.WriteRune(r) + } + flushCommand() + return commands +} + +func fallbackFunctionDeclaration(tokens []fallbackCommandToken, current string) bool { + fields := fallbackTokenValues(tokens) + if current != "" { + fields = append(fields, current) + } + switch len(fields) { + case 1: + return fallbackFunctionNameToken(fields[0]) + case 2: + return (isShellFunctionName(fields[0]) && fields[1] == "()") || + (strings.EqualFold(fields[0], "function") && + (fallbackFunctionNameToken(fields[1]) || isShellFunctionName(fields[1]))) + case 3: + return strings.EqualFold(fields[0], "function") && isShellFunctionName(fields[1]) && fields[2] == "()" + default: + return false + } +} + +func fallbackFunctionNameToken(token string) bool { + return strings.HasSuffix(token, "()") && isShellFunctionName(strings.TrimSuffix(token, "()")) +} + +func isShellFunctionName(name string) bool { + if name == "" { + return false + } + for index, r := range name { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || r == '_' || (index > 0 && r >= '0' && r <= '9') { + continue + } + return false + } + return true +} + +func startsCMDCommandGroup(tokens []string) bool { + if len(tokens) == 0 { + return false + } + switch strings.TrimPrefix(strings.ToLower(tokens[0]), "@") { + case "if", "else": + return true + case "for": + // Parentheses after IN contain data; only the group after DO executes. + return strings.EqualFold(tokens[len(tokens)-1], "do") + default: + return false + } } func Classify(request Request) Risk { diff --git a/internal/sandbox/risk_hardening_test.go b/internal/sandbox/risk_hardening_test.go index ce2f89994..20348097a 100644 --- a/internal/sandbox/risk_hardening_test.go +++ b/internal/sandbox/risk_hardening_test.go @@ -1,6 +1,9 @@ package sandbox -import "testing" +import ( + "strings" + "testing" +) func classifyCommand(command string) Risk { return Classify(Request{ @@ -273,6 +276,9 @@ func TestClassifyASTCatchesNetworkProgramsRegexMisses(t *testing.T) { "ftp ftp.example.com", "sftp user@host", "sudo telnet example.com 23", + "git fetch origin", + "git pull origin main", + "git push gitlawb://example.com/repo.git main", } { risk := classifyCommand(command) if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { @@ -291,12 +297,660 @@ func TestClassifyFlagsUnparseableCommand(t *testing.T) { } func TestClassifyUnparseableNetworkCommandFailsClosed(t *testing.T) { - risk := classifyCommand(`curl https://example.com && "unterminated`) - if !HasRiskCategory(risk, "unparseable_command") { - t.Fatalf("Classify of unparseable network command = categories %v; want unparseable_command", risk.Categories) + for _, command := range []string{ + `curl https://example.com && "unterminated`, + `git fetch origin && "unterminated`, + `git pull origin main && "unterminated`, + `git push gitlawb://example.com/repo.git main && "unterminated`, + `git ls-remote gitlawb://example.com/repo.git & rem '`, + `git archive --remote=gitlawb://example.com/repo.git HEAD & rem '`, + `git -C repo push gitlawb://example.com/repo.git main && "unterminated`, + // git.exe runs under cmd.exe, which has no notion of a trailing single + // quote — this parses fine there but fails the POSIX shell parser used + // by AnalyzeCommand, so it must still be caught by the regex fallback. + `git.exe push origin main & rem '`, + `git.cmd push origin main & rem '`, + // cmd.exe accepts quoted executable paths, option values, and verbs. + // Preserve those token boundaries when the trailing REM quote forces the + // fallback path, including joined short and long option-value forms. + `"C:\Program Files\Git\cmd\git.exe" "push" origin main & rem '`, + `git.exe -C "C:\Program Files\repo" push origin main & rem '`, + `git.exe -C "C:\Program Files\repo" "push" origin main & rem '`, + `git.exe -C"C:\Program Files\repo" "push" origin main & rem '`, + `git.exe --git-dir="C:\Program Files\repo\.git" push origin main & rem '`, + `git.exe "--git-dir=C:\Program Files\repo\.git" push origin main & rem '`, + `git -C repo push origin main & rem '`, + `git -c user.name=test fetch origin & rem '`, + `git -C "C:\Program Files\repo" push origin main & rem '`, + // More value-taking global options than the fallback regex used to cap + // its generic-token scan at (formerly {0,8}) — every option here still + // precedes the actual subcommand. + `git -c a=1 -c b=2 -c c=3 -c d=4 -c e=5 push gitlawb://example.com/repo.git main && "unterminated`, + } { + t.Run(command, func(t *testing.T) { + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { + t.Errorf("Classify(%q) = level %s, categories %v; want critical network", command, risk.Level, risk.Categories) + } + }) + } +} + +// TestClassifyUnparseableNetworkBehindWrapperFailsClosed is the regression test +// for jatmn's #726 P2/P3 findings: resolving the fallback's program from +// tokens[0] alone dropped the network category whenever the real program sat +// behind a wrapper (sudo/env/timeout/xargs), an environment assignment, a shell +// launcher's -c payload, a Windows executable suffix, or a newline — each of +// which base main still caught with its whole-string match. An unparseable +// command is already too obfuscated to analyze, so a wrapper prefix must not be +// enough to buy egress without a prompt. +func TestClassifyUnparseableNetworkBehindWrapperFailsClosed(t *testing.T) { + for _, command := range []string{ + // Wrapper programs, including ones whose options consume a value. + `sudo curl https://example.com && "unterminated`, + `sudo -u root curl https://example.com && "unterminated`, + `env curl https://example.com && "unterminated`, + `env git fetch origin && "unterminated`, + `sudo git push origin main && "unterminated`, + `sudo npm install && "unterminated`, + `timeout 5 curl https://example.com && "unterminated`, + `xargs curl https://example.com && "unterminated`, + // Environment-assignment prefixes. + `PATH=.:$PATH git push origin main && "unterminated`, + `GIT_SSH_COMMAND=ssh git push origin main && "unterminated`, + // A shell launcher's payload is a single token to the fallback tokenizer, + // so the program inside it is only visible by recursing into it. + `sh -c 'curl https://example.com' && "unterminated`, + `bash -c "git push origin main" && "unterminated`, + // Windows executable suffixes normalize on the parseable path already. + `curl.exe https://example.com && "unterminated`, + `wget.exe https://example.com && "unterminated`, + `sudo curl.exe https://example.com && "unterminated`, + // A newline separates commands; the network program is on its own line. + "true\ncurl https://example.com && \"unterminated", + "echo start\r\ngit push origin main && \"unterminated", + // Shell short-option clusters still carry one command payload. + `bash -lc 'curl https://example.com' && "unterminated`, + `bash -ce "git push origin main" && "unterminated`, + `dash -ce 'curl https://example.com' && "unterminated`, + `bash +n -c 'curl https://example.com' && "unterminated`, + // A drive-relative Windows spelling has no separator to cut on, so the + // basename scan alone left "c:git" and never matched (same review). + `C:git.exe push origin main & rem '`, + `C:curl.exe https://example.com & rem '`, + // Recursion goes through more than one launcher layer. + `sh -c "sh -c 'curl https://example.com'" && "unterminated`, + } { + t.Run(command, func(t *testing.T) { + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { + t.Errorf("Classify(%q) = level %s, categories %v; want critical network", command, risk.Level, risk.Categories) + } + }) + } +} + +// TestClassifyUnparseableNetworkInShellConstructFailsClosed covers compound +// shell forms where the invoked program is not the first token after a basic +// ;/&/| separator. The fallback must still find the real invocation without +// returning to an anywhere-in-the-string regex that would misclassify +// `git status push` and URL/path text containing `.git push`. +func TestClassifyUnparseableNetworkInShellConstructFailsClosed(t *testing.T) { + for _, command := range []string{ + `echo $(curl https://evil.test) && "unterminated`, + "echo `curl https://evil.test` && \"unterminated", + `x=$(curl https://evil.test) && "unterminated`, + `echo "$(curl https://evil.test)" && "unterminated`, + "echo \"`git push`\" && \"unterminated", + `x="$(curl https://evil.test)" && "unterminated`, + `(curl https://evil.test) && "unterminated`, + `( curl https://evil.test ) && "unterminated`, + `{ curl https://evil.test ; } && "unterminated`, + `cat <(curl https://evil.test) && "unterminated`, + `if true; then curl https://evil.test; fi && "unterminated`, + `for i in 1 2; do curl https://evil.test; done && "unterminated`, + `while :; do wget https://evil.test; done && "unterminated`, + `case x in x) curl https://evil.test;; esac && "unterminated`, + `>out curl https://evil.test && "unterminated`, + `2>err curl https://evil.test && "unterminated`, + `<<< payload curl https://evil.test && "unterminated`, + `<<- EOF curl https://evil.test && "unterminated`, + `coproc curl https://evil.test; wait && "unterminated`, + `eval "curl https://evil.test" && "unterminated`, + `! curl https://evil.test && "unterminated`, + `if true; then git push; fi && "unterminated`, + `(git -C repo push) && "unterminated`, + `echo $(git push) && "unterminated`, + `eval "git push" && "unterminated`, + `f() { curl https://evil.test; }; f && "unterminated`, + `f () { curl https://evil.test; }; f && "unterminated`, + `f(){ curl https://evil.test; }; f && "unterminated`, + `function f { curl https://evil.test; }; f && "unterminated`, + `function f() { curl https://evil.test; }; f && "unterminated`, + `f() ( curl https://evil.test ); f && "unterminated`, + `f() { git push origin main; }; f && "unterminated`, + // CMD command groups follow condition tokens rather than beginning a + // segment, and may themselves contain nested groups. + `if 1==1 (curl https://evil.test) & rem '`, + `if 1==1 ((git push origin main)) & rem '`, + `for %i in (x) do (curl https://evil.test) & rem '`, + } { + t.Run(command, func(t *testing.T) { + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { + t.Errorf("Classify(%q) = level %s, categories %v; want critical network", command, risk.Level, risk.Categories) + } + }) } - if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { - t.Fatalf("Classify of unparseable network command = level %s, categories %v; want critical network", risk.Level, risk.Categories) +} + +// TestClassifyUnparseableCMDInvocationFormsFailClosed covers jatmn's #726 +// finding that the fallback used POSIX rules to find the executable position in +// what is often valid CMD source. Each command below runs a network program +// under cmd.exe and is rejected by the POSIX parser only because CMD's `rem` +// comment swallows the trailing apostrophe; under POSIX resolution each one +// stops at a token that is not the program ("@curl", "call", "not", "start"), +// which dropped the network category and with it the engine's network prompt. +func TestClassifyUnparseableCMDInvocationFormsFailClosed(t *testing.T) { + for _, command := range []string{ + `@curl https://evil.test & rem '`, + `@@curl https://evil.test & rem '`, + `call curl https://evil.test & rem '`, + `call @curl https://evil.test & rem '`, + `cmd.exe /c call curl https://evil.test & rem '`, + `@cmd.exe /c curl https://evil.test & rem '`, + `start "" curl https://evil.test & rem '`, + `start "download window" curl https://evil.test & rem '`, + `start "" c^u^r^l https://evil.test & rem '`, + `start /b /wait curl https://evil.test & rem '`, + `start /d C:\tmp curl https://evil.test & rem '`, + `if not 1==2 curl https://evil.test & rem '`, + `if /i "%mode%" == "fetch" git push origin main & rem '`, + `if exist repo git push origin main & rem '`, + `if errorlevel 1 curl https://evil.test & rem '`, + `if defined PROXY curl https://evil.test & rem '`, + `if %retries% gtr 0 curl https://evil.test & rem '`, + `for %i in (x) do call curl https://evil.test & rem '`, + `for /f %i in ('curl https://evil.test') do echo %i & rem '`, + `for /f %i in ('cu^rl https://evil.test') do echo %i & rem '`, + "for /f \"usebackq\" %i in (`git ls-remote origin`) do echo %i & rem '", + `cu^rl https://evil.test & rem '`, + `cmd /c cu^rl https://evil.test & rem '`, + `cmd /c"curl https://evil.test" & rem '`, + `cmd /c /d curl https://evil.test & rem '`, + `%ComSpec% /c curl https://evil.test & rem '`, + `git pus^h origin main & rem '`, + `git archive --rem^ote=origin HEAD & rem '`, + `powershell /Command curl https://evil.test & rem '`, + `powershell -Command Invoke-WebRequest https://evil.test & rem '`, + `pwsh -co iwr https://evil.test & rem '`, + `powershell -co curl https://evil.test & rem '`, + `powershell -ep RemoteSigned curl https://evil.test & rem '`, + `pwsh -ep Bypass -Command curl https://evil.test & rem '`, + `start "x" curl https://evil.test & rem '`, + `cmd /c start "x" curl https://evil.test & rem '`, + `start /b /wait "x" curl https://evil.test & rem '`, + `if cmdextversion 1 curl https://evil.test & rem '`, + `if not cmdextversion 1 git push origin main & rem '`, + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); !analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) parsed; this case must exercise the fallback", command) + } + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { + t.Errorf("Classify(%q) = level %s, categories %v; want critical network", command, risk.Level, risk.Categories) + } + }) + } +} + +// TestGitGlobalOptionsResolveIdenticallyOnBothPaths pins the whole result table +// of git's global-option scan, on the AST path and the fallback path at once. +// Terminal globals print from the local installation and exit, so the words +// after them are output text rather than a subcommand: `git -h push` renders +// git-push's usage without contacting a remote. Both paths read the same +// parser, so a future option cannot be taught to one and not the other. +func TestGitGlobalOptionsResolveIdenticallyOnBothPaths(t *testing.T) { + for _, testCase := range []struct { + args string + network bool + }{ + {"push origin main", true}, + {"-C repo push origin main", true}, + // -C is a joined-value spelling too: `-Crepo` is one token, and its embedded + // value must not be mistaken for the subcommand. + {"-Crepo push origin main", true}, + {"-C repo -C repo2 push origin main", true}, + {"--git-dir /repo/.git fetch origin", true}, + // Every separate-value global against every remote verb this scanner + // treats as network, not just push+fetch above: the option-consumption + // rule is shared, so one representative pairing per option is what + // actually exercises drift, but each remote VERB gets its own row too — + // a verb this scanner has never seen paired with an option is exactly + // where a hand-maintained verb list would be missed. + {"--work-tree /repo pull origin main", true}, + {"--attr-source HEAD clone https://example.com/repo.git", true}, + {"--config-env foo=bar ls-remote origin", true}, + {"--namespace ns send-pack origin main", true}, + {"--super-prefix sub/ fetch origin", true}, + {"--no-pager push origin main", true}, + {"PUSH origin main", true}, + {"--Git-Dir repo PUSH origin main", true}, + // Local commit forms must stay quiet with a global in front, not just + // bare — a global that is handled correctly for push but breaks commit + // would be a regression this scanner would otherwise never see, since + // every existing local-form row here is bare. + {"-C repo commit -m msg", false}, + {"--git-dir /repo/.git commit -m msg", false}, + {"--work-tree /repo status", false}, + // Value-layer negative: the global's VALUE is a token that is itself a + // remote verb's spelling. A scanner that fails to skip the value (instead + // of reading it as the next word to classify) would misread this as + // local — "push" here is --git-dir's argument, never git's own argv. + {"--git-dir push status", false}, + {"-C push status", false}, + {"archive --mtime 2024-01-01 --remote origin HEAD", true}, + {"archive --format --remote=origin HEAD", true}, + {"archive --mtime --remote=origin HEAD", true}, + {"archive HEAD --remote", false}, + {"archive HEAD --remote=origin", true}, + {"archive -o -- --remote=origin HEAD", true}, + {"archive -o --remote HEAD", false}, + {"archive HEAD -- --remote=origin", false}, + {"--help push", false}, + {"-h push", false}, + {"--version push", false}, + {"-v push", false}, + {"--html-path push", false}, + {"--man-path push", false}, + {"--info-path push", false}, + {"-C repo --help push", false}, + {"-C repo -h push", false}, + {"--list-cmds=main push", false}, + // `--exec-path[=]`: bare, it prints the local exec path and exits, so + // it neither takes /tmp as a value nor reaches push. With an inline value + // it is an ordinary nonterminal global. + {"--exec-path", false}, + {"--exec-path /tmp push", false}, + {"--exec-path=/tmp push", true}, + {"-C repo --exec-path push", false}, + {"-C repo --list-cmds=main push", false}, + {"-C repo --version push", false}, + {"status", false}, + {"", false}, + } { + t.Run(testCase.args, func(t *testing.T) { + parseable := "git " + testCase.args + if analysis := AnalyzeCommand(parseable); analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) reported TooComplex; this case must exercise the AST path", parseable) + } + if got := HasRiskCategory(classifyCommand(parseable), "network"); got != testCase.network { + t.Errorf("AST Classify(%q) network = %v, want %v", parseable, got, testCase.network) + } + + // The same words through the fallback, made unparseable by a CMD + // comment the POSIX parser cannot close. + unparseable := parseable + " & rem '" + if analysis := AnalyzeCommand(unparseable); !analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) parsed; this case must exercise the fallback", unparseable) + } + if got := HasRiskCategory(classifyCommand(unparseable), "network"); got != testCase.network { + t.Errorf("fallback Classify(%q) network = %v, want %v", unparseable, got, testCase.network) + } + }) + } +} + +func TestClassifyUnparseableLocalPowerShellCommandStaysNonNetwork(t *testing.T) { + command := `powershell -Command Get-ChildItem -File . & rem '` + risk := classifyCommand(command) + if HasRiskCategory(risk, "network") { + t.Fatalf("Classify(%q) = categories %v; want no network category", command, risk.Categories) + } +} + +func TestClassifyUnparseableCommandBearingWrapperValuesFailClosed(t *testing.T) { + t.Setenv("ZERO_TEST_ENV_COMMAND", "curl") + for _, command := range []string{ + `env -S 'curl https://evil.test' && "unterminated`, + `env --split-string 'git push origin main' && "unterminated`, + `env -S 'curl\_https://evil.test' && "unterminated`, + `env -S '${ZERO_TEST_ENV_COMMAND} https://evil.test' && "unterminated`, + `env -iS 'curl https://evil.test' && "unterminated`, + `env -S '-S "curl https://evil.test"' && "unterminated`, + `env -S 'env -S "curl https://evil.test"' && "unterminated`, + `env -S '--argv0 harmless curl https://evil.test' && "unterminated`, + `busybox sh -c 'curl https://evil.test' && "unterminated`, + `strace -P /tmp sh -c 'git push origin main' && "unterminated`, + `strace -p 123 curl https://evil.test && "unterminated`, + `strace --trace network curl https://evil.test && "unterminated`, + `strace --tips curl https://evil.test && "unterminated`, + `strace -fqo trace.log curl https://evil.test && "unterminated`, + `strace --tip curl https://evil.test && "unterminated`, + `strace --ver curl https://evil.test && "unterminated`, + `strace --definitely-invalid curl https://evil.test && "unterminated`, + `strace env curl https://evil.test && "unterminated`, + `strace env -S 'git push origin main' && "unterminated`, + `busybox env curl https://evil.test && "unterminated`, + `busybox env -S 'git push origin main' && "unterminated`, + `busybox -- curl https://evil.test && "unterminated`, + `busybox -x curl https://evil.test && "unterminated`, + `env -S 'sh -c' -- 'curl https://evil.test' && "unterminated`, + "powershell -Command 'Invoke`-WebRequest https://evil.test' & rem '", + `pwsh -cwa Invoke-WebRequest https://evil.test & rem '`, + `exec -a harmless curl https://evil.test && "unterminated`, + `exec --argv0 harmless git push origin main && "unterminated`, + // Source that exists but cannot be read statically: an encoded PowerShell + // payload that does not decode, and an env split string the shell expands. + `powershell -EncodedCommand curl & rem '`, + `env -S "$PAYLOAD" && "unterminated`, + `env --split-string="$PAYLOAD" && "unterminated`, + // GNU env accepts an unambiguous abbreviation of --split-string; the + // fallback tokenizer must recognize it exactly like the full spelling, + // not fall through to ordinary wrapper handling that reports no executable. + `env --split 'curl https://evil.test' && "unterminated`, + `env --split='curl https://evil.test' && "unterminated`, + // The delegated child program is a shell expansion this fallback tokenizer + // preserves verbatim (it never expands anything); matching the literal + // "$APPLET" token against known program names must not read as clean. + `APPLET=curl; busybox "$APPLET" https://evil.test && "unterminated`, + `APPLET=curl; strace "$APPLET" https://evil.test && "unterminated`, + // START keeps taking switches after its optional window title. + `start "" /b curl https://evil.test & rem '`, + `start "" /d C:\ curl https://evil.test & rem '`, + `start "" /wait git push origin main & rem '`, + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); !analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) parsed; this case must exercise the fallback", command) + } + risk := classifyCommand(command) + if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { + t.Fatalf("Classify(%q) = level %s, categories %v; want critical network", command, risk.Level, risk.Categories) + } + }) + } +} + +func TestClassifyUnparseableDynamicCommandSourcesFailClosed(t *testing.T) { + for _, command := range []string{ + `set "N=curl" & call %N% https://evil.test & rem '`, + `set "N=curl" & cmd /c %N% https://evil.test & rem '`, + `set "N=curl" & start %N% https://evil.test & rem '`, + `set "N=curl" & call %N:x=y% https://evil.test & rem '`, + `set "N=git" & call !N! push origin main & rem '`, + `sh -c "$PAYLOAD" & rem '`, + `sh -c "${PAYLOAD}" & rem '`, + "sh -c \"$(printf curl)\" & rem '", + } { + t.Run(command, func(t *testing.T) { + if analysis := AnalyzeCommand(command); !analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) parsed; this case must exercise the fallback", command) + } + risk := classifyCommand(command) + if risk.Level != RiskCritical || !HasRiskCategory(risk, "network") { + t.Fatalf("Classify(%q) = level %s, categories %v; want critical network", command, risk.Level, risk.Categories) + } + }) + } +} + +func TestClassifyUnparseableLiteralPercentBangAndShellSourceStayLocal(t *testing.T) { + for _, command := range []string{ + `call 100%local printf ok & rem '`, + `call wow!local printf ok & rem '`, + `sh -c 'printf ok' & rem '`, + } { + t.Run(command, func(t *testing.T) { + if risk := classifyCommand(command); HasRiskCategory(risk, "network") { + t.Fatalf("Classify(%q) = categories %v; want no network category", command, risk.Categories) + } + }) + } +} + +func TestEnvSplitAssignmentDependenciesMatchASTAndFallback(t *testing.T) { + for _, testCase := range []struct { + command string + network bool + }{ + {`CMD=curl env -S '${CMD} https://evil.test'`, true}, + {`PAYLOAD='curl https://evil.test' env -S 'sh -c "${PAYLOAD}"'`, true}, + {`VALUE=x env -S 'printf ${VALUE}'`, false}, + {`env -S 'sh -c "printf ok"'`, false}, + } { + t.Run(testCase.command, func(t *testing.T) { + if analysis := AnalyzeCommand(testCase.command); analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) reported TooComplex; this case must exercise the AST path", testCase.command) + } + if got := HasRiskCategory(classifyCommand(testCase.command), "network"); got != testCase.network { + t.Errorf("AST Classify(%q) network = %v, want %v", testCase.command, got, testCase.network) + } + + unparseable := testCase.command + " & rem '" + if analysis := AnalyzeCommand(unparseable); !analysis.TooComplex { + t.Fatalf("AnalyzeCommand(%q) parsed; this case must exercise the fallback", unparseable) + } + if got := HasRiskCategory(classifyCommand(unparseable), "network"); got != testCase.network { + t.Errorf("fallback Classify(%q) network = %v, want %v", unparseable, got, testCase.network) + } + }) + } +} + +// These malformed forms contain network-looking text in non-executing variable, +// arithmetic, array, escaped-backtick, or ordinary argument contexts. They must +// stay non-network so fallback tokenization does not over-flag inert text. +func TestClassifyUnparseableShellSyntaxTextStaysNonNetwork(t *testing.T) { + for _, command := range []string{ + `echo ${curl} && "unterminated`, + `echo $((curl)) && "unterminated`, + `arr=(curl) && "unterminated`, + "echo \\`curl\\` && \"unterminated", + `command if curl https://evil.test && "unterminated`, + `env then git push && "unterminated`, + `for %i in (curl) do echo %i & rem '`, + `env -S 'printf curl; git push' && "unterminated`, + `env -S 'printf ok' curl https://evil.test && "unterminated`, + // A decodable encoded payload is read rather than guessed at: these decode + // to the UTF-16LE source `evil`, a local program name. + `powershell -e ZQB2AGkAbAA= & rem '`, + `pwsh -ec ZQB2AGkAbAA= & rem '`, + `pwsh -File curl & rem '`, + `powershell -DefinitelyInvalid Invoke-WebRequest https://evil.test & rem '`, + // CMD's START runs nothing when its only quoted argument is the window + // title, and neither ECHO nor REM executes the text that follows it. + `start "curl https://evil.test" & rem '`, + // An unquoted first START operand is the executable, not a window title; + // the later curl token is only an argument to MyTitle. + `start MyTitle curl https://evil.test & rem '`, + `call start MyTitle curl https://evil.test & rem '`, + `echo for /f %i in ('curl https://evil.test') do echo %i & rem '`, + `rem for /f %i in ('curl https://evil.test') do echo %i & rem '`, + `echo "x & for /f %i in ('curl https://evil.test') do echo %i" & rem '`, + `echo x ^& for /f %i in ('curl https://evil.test') do echo %i & rem '`, + "for /f \"delims=usebackq\" %i in (`curl https://evil.test`) do echo %i & rem '", + `env -S '--argv0 curl printf ok' && "unterminated`, + // An abbreviated long option that isn't --split-string must not be + // misread as one; env's own resolution of these flags is unaffected. + `env --unset curl printf ok && "unterminated`, + `env --chdir=/tmp printf ok && "unterminated`, + // The delegated child program resolves to ordinary literal text, not an + // unread expansion; it must still be classified on its own content. + `busybox echo curl https://evil.test && "unterminated`, + `strace true curl https://evil.test && "unterminated`, + `bash /dev/null -c 'curl https://evil.test' && "unterminated`, + `bash -- -c 'curl https://evil.test' && "unterminated`, + `bash -Zc 'curl https://evil.test' && "unterminated`, + `bash -nc 'curl https://evil.test' && "unterminated`, + `cmd /c start & rem '`, + `echo call curl https://evil.test & rem '`, + `rem start curl https://evil.test & rem '`, + } { + t.Run(command, func(t *testing.T) { + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + if HasRiskCategory(risk, "network") { + t.Errorf("Classify(%q) = level %s, categories %v; want no network category", command, risk.Level, risk.Categories) + } + }) + } +} + +func TestClassifyUnparseableMismatchedDelimitersDoesNotPanic(t *testing.T) { + for _, command := range []string{ + "echo `curl)` && \"unterminated", + "echo `curl(` && \"unterminated", + "echo )`curl` && \"unterminated", + } { + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + } +} + +func FuzzFallbackCommandTokensDoesNotPanic(f *testing.F) { + for _, command := range []string{"", "`", ")", "(`)", "echo `curl)`"} { + f.Add(command) + } + f.Fuzz(func(t *testing.T, command string) { + fallbackCommandTokens(command) + }) +} + +// FuzzFallbackNetworkResolutionDoesNotPanic covers the resolvers layered on top +// of the tokenizer. They index into token slices around keywords, conditions and +// option values, and this path exists to be total over input the shell parser +// already rejected: a panic here is a request-triggerable crash. +func FuzzFallbackNetworkResolutionDoesNotPanic(f *testing.F) { + for _, command := range []string{ + "", "if", "if not", "start", "start /d", "start @", "start @@", "call", "@", "@curl", + "for %i in (x) do", "if errorlevel", "if %x% equ", "git -C", + "echo `curl)` && \"unterminated", `cmd /c`, `cmd /c start & rem '`, `if 1==1 (curl https://evil.test`, + } { + f.Add(command) + } + f.Fuzz(func(t *testing.T, command string) { + matchesUnparseableNetwork(command) + }) +} + +func TestClassifyDeepCMDLaunchersDoesNotRecurseUnboundedly(t *testing.T) { + command := strings.Repeat("cmd /c ", 512) + `curl https://evil.test & rem '` + risk := classifyCommand(command) + if !HasRiskCategory(risk, "network") { + t.Fatalf("Classify(deep cmd chain) = categories %v; want network", risk.Categories) + } +} + +func TestFallbackBodyDeepExecChainDoesNotRecurseUnboundedly(t *testing.T) { + body := append(make([]string, 512), "curl", "https://evil.test") + for index := 0; index < 512; index++ { + body[index] = "exec" + } + if !fallbackBodyUsesNetwork(body, 0) { + t.Fatal("fallbackBodyUsesNetwork(deep exec chain) = false; want network") + } +} + +// TestUnparseableShellDepthMatchesAnalyzerDepth pins the two launcher-recursion +// caps together, which is the property jatmn's #703 review asked for: a fallback +// that gave up a level earlier than the AST path would drop the network category +// on exactly the deeply-nested chains it exists to fail closed on. +// +// Asserted as constants rather than by driving a four-deep command: the fallback +// tokenizer is deliberately small and does not model nested escaped quotes, so +// a literal four-layer `sh -c` string would be testing the tokenizer's escaping +// rather than the depth limit. The behavior that recursion happens at all, and +// through more than one layer, is covered above. +func TestUnparseableShellDepthMatchesAnalyzerDepth(t *testing.T) { + if maxUnparseableShellDepth != maxAnalyzerDepth { + t.Fatalf("maxUnparseableShellDepth = %d, maxAnalyzerDepth = %d; the fallback must not give up before the parseable path", + maxUnparseableShellDepth, maxAnalyzerDepth) + } +} + +// TestClassifyUnparseableLocalGitArchiveStaysNonNetwork pins the other half of +// the archive gate: the fallback must agree with the AST path that only a +// --remote archive talks to another host. +func TestClassifyUnparseableLocalGitArchiveStaysNonNetwork(t *testing.T) { + for _, command := range []string{ + `git archive HEAD & rem '`, + `git archive -o out.tar HEAD & rem '`, + `git -C repo archive HEAD & rem '`, + `git.exe archive HEAD & rem '`, + // A pathspec named --remote after the end-of-options separator is a + // local tree entry, not a remote (issue #703 review). + `git archive HEAD -- --remote & rem '`, + `git archive HEAD -- --remote=origin & rem '`, + } { + t.Run(command, func(t *testing.T) { + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + if HasRiskCategory(risk, "network") { + t.Errorf("Classify(%q) = categories %v; want no network category for a local archive", command, risk.Categories) + } + }) + } +} + +// TestClassifyUnparseableNonGitOptionTokenStaysNonNetwork guards against the +// fallback regex treating an arbitrary bare token before a network verb as if +// it were a git global option. `status` in `git status push` is a pathspec +// argument to `git status`, not a value-taking global option, so `push` here +// is not the git subcommand and must not be classified as network — even +// though the trailing unmatched quote (a cmd.exe REM comment, invalid under +// the POSIX parser AnalyzeCommand uses) still forces the unparseable-command +// fallback path. +func TestClassifyUnparseableNonGitOptionTokenStaysNonNetwork(t *testing.T) { + for _, command := range []string{ + `git status push & rem '`, + `git "status" push & rem '`, + `git 'status' push & rem "`, + `git.exe -C "C:\Program Files\push" status & rem '`, + `git.exe --git-dir="C:\Program Files\push\.git" status & rem '`, + `git.exe "--git-dir=C:\Program Files\push\.git" status & rem '`, + `git -C push status & rem '`, + `git -c push status & rem '`, + `git -C "push" status & rem '`, + `git -c "push" status & rem '`, + `git --help push & rem '`, + `git --version push & rem '`, + `echo https://example.com/repo.git push & rem '`, + `echo ssh://git@example.com/repo.git push & rem '`, + `echo C:\repos\repo.git push & rem '`, + `echo git.example.com push & rem '`, + } { + t.Run(command, func(t *testing.T) { + risk := classifyCommand(command) + if !HasRiskCategory(risk, "unparseable_command") { + t.Errorf("Classify(%q) = categories %v; want unparseable_command", command, risk.Categories) + } + if HasRiskCategory(risk, "network") { + t.Errorf("Classify(%q) = level %s, categories %v; want no network category", command, risk.Level, risk.Categories) + } + if risk.Level != RiskHigh { + t.Errorf("Classify(%q) = level %s; want high (unparseable_command only)", command, risk.Level) + } + }) } } @@ -327,3 +981,100 @@ func TestClassifyBenignCommandStaysClean(t *testing.T) { } } } + +// Unit-level counterparts to TestEngineFailsClosedOnUnreadableCommandShapes, +// pinning the two readers directly so a regression names the layer that broke. +func TestGitSelectionDynamicFailsClosedOnUnreadableSelection(t *testing.T) { + for _, testCase := range []struct { + name string + words []string + // dynamicIndexes are positions the caller could not read. + dynamicIndexes []int + want bool + }{ + {name: "subcommand token", words: []string{"", "origin", "main"}, dynamicIndexes: []int{0}, want: true}, + {name: "global option value shifts the subcommand", words: []string{"-C", "", "status"}, dynamicIndexes: []int{1}, want: true}, + {name: "archive option past the subcommand", words: []string{"archive", "", "HEAD"}, dynamicIndexes: []int{1}, want: true}, + {name: "no subcommand at all", words: []string{""}, dynamicIndexes: []int{0}, want: true}, + {name: "literal local subcommand", words: []string{"commit", "-m", "message"}, want: false}, + // Past a non-archive subcommand nothing can change git's answer, so the + // common `git commit -m "$MESSAGE"` keeps its quiet path. + {name: "operand past a local subcommand", words: []string{"commit", "-m", ""}, dynamicIndexes: []int{2}, want: false}, + {name: "operand past a network subcommand", words: []string{"push", ""}, dynamicIndexes: []int{1}, want: false}, + {name: "literal archive", words: []string{"archive", "-o", "out.tar", "HEAD"}, want: false}, + } { + t.Run(testCase.name, func(t *testing.T) { + dynamic := make(map[int]bool, len(testCase.dynamicIndexes)) + for _, index := range testCase.dynamicIndexes { + dynamic[index] = true + } + if got := gitSelectionDynamic(testCase.words, func(index int) bool { return dynamic[index] }); got != testCase.want { + t.Fatalf("gitSelectionDynamic(%q, dynamic=%v) = %t, want %t", + testCase.words, testCase.dynamicIndexes, got, testCase.want) + } + }) + } +} + +func TestCMDForPayloadSelectsDOOutsideTheSet(t *testing.T) { + for _, testCase := range []struct { + name string + fields []string + want []string + }{ + { + name: "do is a set element", + fields: []string{"%i", "in", "(", "do", "x", ")", "do", "curl", "https://evil.test"}, + want: []string{"curl", "https://evil.test"}, + }, + { + name: "set element joined to the paren", + fields: []string{"%i", "in", "(do", "x)", "do", "curl", "https://evil.test"}, + want: []string{"curl", "https://evil.test"}, + }, + { + name: "quoted paren is not a set boundary", + fields: []string{"%i", "in", `("a)b"`, "x)", "do", "curl"}, + want: []string{"curl"}, + }, + { + name: "ordinary loop", + fields: []string{"%i", "in", "(curl)", "do", "echo", "%i"}, + want: []string{"echo", "%i"}, + }, + // CMD rejects an unbalanced set, so there is no loop body to classify. + {name: "unterminated set", fields: []string{"%i", "in", "(", "do", "curl"}, want: nil}, + } { + t.Run(testCase.name, func(t *testing.T) { + got := cmdForPayload(testCase.fields) + if strings.Join(got, " ") != strings.Join(testCase.want, " ") { + t.Fatalf("cmdForPayload(%q) = %q, want %q", testCase.fields, got, testCase.want) + } + }) + } +} + +func TestPOSIXSingleQuoteHidesCMDSeparator(t *testing.T) { + for _, testCase := range []struct { + command string + want bool + }{ + {command: `echo ' & curl https://evil.test`, want: true}, + {command: `echo ' | curl https://evil.test`, want: true}, + {command: `echo 'a & b' ok`, want: true}, + // Double quotes quote under both languages, so they hide nothing. + {command: `echo " & curl https://evil.test`, want: false}, + // The separator is outside the quotes: both readers already see it. + {command: `env -S 'printf ${VALUE}' & rem '`, want: false}, + {command: `env -S 'printf ok' curl https://evil.test && "unterminated`, want: false}, + // `;` delimits arguments in CMD; it does not start a command. + {command: `env -S 'printf curl; git push' && "unterminated`, want: false}, + {command: `echo 'plain text'`, want: false}, + } { + t.Run(testCase.command, func(t *testing.T) { + if got := posixSingleQuoteHidesCMDSeparator(testCase.command); got != testCase.want { + t.Fatalf("posixSingleQuoteHidesCMDSeparator(%q) = %t, want %t", testCase.command, got, testCase.want) + } + }) + } +} diff --git a/internal/sandbox/safe_command.go b/internal/sandbox/safe_command.go index 807004710..010b9910e 100644 --- a/internal/sandbox/safe_command.go +++ b/internal/sandbox/safe_command.go @@ -147,6 +147,18 @@ var interactiveSegments = []struct { // that would block a non-interactive agent. goos selects platform-specific // rules (pass "" to use the host runtime.GOOS). func DetectInteractiveCommand(command string, goos string) InteractiveCommandResult { + return detectInteractiveCommandAt(command, goos, 0) +} + +func detectInteractiveCommandAt(command string, goos string, depth int) InteractiveCommandResult { + if depth > maxAnalyzerDepth { + return InteractiveCommandResult{ + Interactive: true, + Command: "nested shell launcher", + Reason: "shell launcher nesting exceeds the safe inspection limit, so its payload cannot be proven non-interactive", + Suggestion: "Simplify the nested shell command or run its non-interactive payload directly.", + } + } command = strings.TrimSpace(command) if command == "" { return InteractiveCommandResult{} @@ -192,10 +204,12 @@ func DetectInteractiveCommand(command string, goos string) InteractiveCommandRes // command; recurse into it so an interactive program inside the payload // is detected (e.g. `sh -c 'vim x'`). if payload := shellDashCPayload(first, fields); payload != "" { - if inner := DetectInteractiveCommand(payload, goos); inner.Interactive { + if inner := detectInteractiveCommandAt(payload, goos, depth+1); inner.Interactive { return inner } - continue + // strings.Fields cannot preserve a quoted multi-word payload. If its + // best-effort read is clean, keep going so the AST pass below can + // inspect the actual payload rather than treating truncation as safe. } program, ok := interactivePrograms[first] if !ok { @@ -229,7 +243,7 @@ func DetectInteractiveCommand(command string, goos string) InteractiveCommandRes // the parser cannot handle (Windows cmd.exe, obfuscation) yields no commands // and falls through unchanged — the guard never hard-blocks on a parse error. for _, fields := range astCommandFields(command) { - if result, ok := inspectCommandFields(fields, goos); ok { + if result, ok := inspectCommandFields(fields, goos, depth); ok { return result } } @@ -243,7 +257,7 @@ func DetectInteractiveCommand(command string, goos string) InteractiveCommandRes // non-interactive suppressions (hasNonInteractiveFlag covers REPL flags and // trailing-command clients like ssh). It mirrors the hand-written passes above // so an AST-extracted command is classified identically to a plainly-split one. -func inspectCommandFields(fields []string, goos string) (InteractiveCommandResult, bool) { +func inspectCommandFields(fields []string, goos string, depth int) (InteractiveCommandResult, bool) { body := strings.ToLower(commandBody(fields)) for _, seg := range interactiveSegments { if body == seg.match || strings.HasPrefix(body, seg.match+" ") { @@ -258,8 +272,14 @@ func inspectCommandFields(fields []string, goos string) (InteractiveCommandResul if first == "" { return InteractiveCommandResult{}, false } - if payload := shellDashCPayload(first, fields); payload != "" { - if inner := DetectInteractiveCommand(payload, goos); inner.Interactive { + start := programIndex(first, fields) + args := fields[start+1:] + payloadIndex, hasPayload := shellCommandPayloadIndex(first, args) + if hasPayload && payloadIndex < len(args) { + // astCommandFields already removed the launcher's outer quoting. Do not + // trim again: trailing quotes can belong to a nested launcher's payload. + payload := args[payloadIndex] + if inner := detectInteractiveCommandAt(payload, goos, depth+1); inner.Interactive { return inner, true } return InteractiveCommandResult{}, false @@ -297,12 +317,13 @@ var wrapperPrograms = map[string]bool{ var wrapperValueOptionsByProg = map[string]map[string]bool{ "sudo": {"-u": true, "--user": true, "-g": true, "--group": true, "-p": true, "--prompt": true, "-C": true, "--close-from": true, "-r": true, "--role": true, "-T": true, "--command-timeout": true, "-U": true, "--other-user": true, "-h": true, "--host": true, "-D": true, "--chdir": true, "-R": true, "--chroot": true}, "doas": {"-u": true, "-C": true}, - "env": {"-u": true, "--unset": true, "-S": true, "--split-string": true, "-C": true, "--chdir": true}, + "env": {"-u": true, "--unset": true, "-S": true, "--split-string": true, "-C": true, "--chdir": true, "-a": true, "--argv0": true}, "timeout": {"-s": true, "--signal": true, "-k": true, "--kill-after": true}, "nice": {"-n": true, "--adjustment": true}, "ionice": {"-c": true, "--class": true, "-n": true, "--classdata": true, "-p": true, "--pid": true}, "stdbuf": {"-i": true, "--input": true, "-o": true, "--output": true, "-e": true, "--error": true}, "xargs": {"-a": true, "--arg-file": true, "-d": true, "--delimiter": true, "-E": true, "-I": true, "--replace": true, "-L": true, "--max-lines": true, "-n": true, "--max-args": true, "-P": true, "--max-procs": true, "-s": true, "--max-chars": true}, + "exec": {"-a": true, "--argv0": true}, } // wrapperConsumesValue reports whether option is a value-consuming flag of the @@ -361,10 +382,20 @@ func firstProgram(fields []string) string { // command boundary (e.g. `sudo git rebase -i` -> "git rebase -i") instead of // matching the segment text anywhere as a raw substring. func commandBody(fields []string) string { + return strings.Join(commandBodyFields(fields), " ") +} + +// commandBodyFields is commandBody's scan, returning the fields from the first +// real command token onward (nil when the segment is nothing but assignments and +// wrappers). Callers that need to reason about the program and its arguments +// separately — the unparseable-command fallback in risk.go — use this rather +// than re-splitting commandBody's joined string, which would lose the token +// boundaries the fallback tokenizer worked to preserve. +func commandBodyFields(fields []string) []string { wrapper := "" for index := 0; index < len(fields); index++ { field := fields[index] - if strings.Contains(field, "=") && !strings.HasPrefix(field, "=") { + if strings.Contains(field, "=") && !strings.HasPrefix(field, "=") && !strings.HasPrefix(field, "-") { continue } if strings.HasPrefix(field, "-") { @@ -381,9 +412,543 @@ func commandBody(fields []string) string { continue } // First real command token: the body starts here. - return strings.Join(fields[index:], " ") + return fields[index:] + } + return nil +} + +// envSplitCommandFields resolves GNU env's -S/--split-string launcher when env +// appears at an actual command position (possibly behind another wrapper). The +// split string is argv syntax, not shell source, so metacharacters remain inert +// unless the resulting executable is itself a shell with -c. +type envSplitCommandResult struct { + command []string + recognized bool + executableEnvironmentDependent bool + commandSourceEnvironmentDependent bool +} + +func envSplitCommandFields(fields []string) envSplitCommandResult { + wrapper := "" + for index := 0; index < len(fields); index++ { + field := fields[index] + if strings.Contains(field, "=") && !strings.HasPrefix(field, "=") && !strings.HasPrefix(field, "-") { + continue + } + if strings.HasPrefix(field, "-") { + if wrapperConsumesValue(wrapper, field) && index+1 < len(fields) { + index++ + } + continue + } + if isNumericToken(field) { + continue + } + token := normalizeProgramToken(field) + if token == "env" { + return envSplitCommand(fields[index+1:]) + } + if wrapperPrograms[token] { + wrapper = token + continue + } + return envSplitCommandResult{} + } + return envSplitCommandResult{} +} + +func envSplitCommand(args []string) envSplitCommandResult { + current := append([]string(nil), args...) + dependent := make([]bool, len(current)) + recognized := false + for rewrite := 0; rewrite < 16; rewrite++ { + commandIndex := len(current) + rewritten := false + for index := 0; index < len(current); index++ { + arg := current[index] + if arg == "--" { + commandIndex = index + 1 + break + } + if strings.Contains(arg, "=") && !strings.HasPrefix(arg, "=") && !strings.HasPrefix(arg, "-") { + continue + } + value, consumed, retainedOption, split := envSplitOption(current, index) + if split { + recognized = true + if consumed == 0 { + return envSplitCommandResult{recognized: true} + } + fields, fieldDependencies, ok := splitEnvString(value) + if !ok { + // The split string is executable argv this scan cannot read. + // "Cannot inspect" is not "does not use the network": env still + // runs whatever the shell expanded into it, so keep the gate. + return envSplitCommandResult{recognized: true, executableEnvironmentDependent: true} + } + replacement := fields + replacementDependencies := fieldDependencies + if retainedOption != "" { + replacement = append([]string{retainedOption}, replacement...) + replacementDependencies = append([]bool{false}, replacementDependencies...) + } + current = append(append(append([]string(nil), current[:index]...), replacement...), current[index+consumed:]...) + dependent = append(append(append([]bool(nil), dependent[:index]...), replacementDependencies...), dependent[index+consumed:]...) + rewritten = true + break + } + if strings.HasPrefix(arg, "-") { + if wrapperConsumesValue("env", arg) { + index++ + } + continue + } + commandIndex = index + break + } + if rewritten { + continue + } + if !recognized { + return envSplitCommandResult{} + } + if commandIndex >= len(current) { + return envSplitCommandResult{recognized: true} + } + command := current[commandIndex:] + // Preserve a nested env invocation so fallbackBodyUsesNetwork can process + // its own -S grammar. Other ordinary wrappers can be resolved directly. + if normalizeProgramToken(command[0]) != "env" { + command = commandBodyFields(command) + commandIndex = len(current) - len(command) + } + commandDependencies := dependent[commandIndex:] + return envSplitCommandResult{ + command: command, + recognized: true, + executableEnvironmentDependent: len(commandDependencies) > 0 && commandDependencies[0], + commandSourceEnvironmentDependent: envSplitCommandSourceDependent(command, commandDependencies), + } + } + // Excessive rewrites are attacker-controlled ambiguity. Keep the network + // gate rather than recursing without a bound. + return envSplitCommandResult{recognized: true, executableEnvironmentDependent: true} +} + +func envSplitCommandSourceDependent(command []string, dependent []bool) bool { + if len(command) == 0 || len(command) != len(dependent) { + return false + } + program := normalizeProgramToken(command[0]) + if dependent[0] { + return true + } + args, argDependent := command[1:], dependent[1:] + if shellPrograms[program] { + if index, ok := shellCommandPayloadIndex(program, args); ok && index < len(argDependent) { + return argDependent[index] + } + return false + } + if program == "powershell" || program == "pwsh" || program == "cmd" || + program == "call" || program == "start" || program == "env" { + for _, value := range argDependent { + if value { + return true + } + } + } + return false +} + +// envSplitLongOptionName is GNU env's only long option beginning with "s", so +// any unambiguous prefix of it ("--s", "--split", "--split-strin", ...) names +// the same option under GNU getopt_long's abbreviation rule. +const envSplitLongOptionName = "split-string" + +// matchesEnvSplitLongOption reports whether name (the text of a "--name" or +// "--name=value" option, with "--" and any "=value" suffix already stripped) +// is an unambiguous abbreviation of --split-string. Modelling the runtime +// option grammar here, rather than the single exact spelling, matters: `env +// --split 'curl https://…'` is accepted by GNU env exactly like +// `--split-string`, and treating only the full spelling as recognized let a +// shortened form fall through to ordinary wrapper handling with no network +// classification at all. +func matchesEnvSplitLongOption(name string) bool { + if name == "" { + return false + } + return strings.HasPrefix(envSplitLongOptionName, name) +} + +func envSplitOption(args []string, index int) (value string, consumed int, retainedOption string, ok bool) { + arg := args[index] + switch { + case arg == "-S": + if index+1 >= len(args) { + return "", 0, "", true + } + return args[index+1], 2, "", true + case strings.HasPrefix(arg, "--"): + name := arg[2:] + hasValue := false + optionValue := "" + if equals := strings.IndexByte(name, '='); equals >= 0 { + hasValue = true + optionValue = name[equals+1:] + name = name[:equals] + } + if !matchesEnvSplitLongOption(name) { + return "", 0, "", false + } + if hasValue { + return optionValue, 1, "", true + } + if index+1 >= len(args) { + return "", 0, "", true + } + return args[index+1], 2, "", true + case strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--"): + position := strings.IndexByte(arg[1:], 'S') + if position < 0 { + return "", 0, "", false + } + position++ + for _, option := range arg[1:position] { + if option != 'i' && option != '0' && option != 'v' { + return "", 0, "", false + } + } + retained := "" + if position > 1 { + retained = "-" + arg[1:position] + } + if position+1 < len(arg) { + return arg[position+1:], 1, retained, true + } + if index+1 >= len(args) { + return "", 0, "", true + } + return args[index+1], 2, retained, true + default: + return "", 0, "", false + } +} + +func splitEnvString(value string) ([]string, []bool, bool) { + var fields []string + var dependent []bool + var word strings.Builder + var quote rune + started := false + wordDependent := false + flush := func() { + if started { + fields = append(fields, word.String()) + dependent = append(dependent, wordDependent) + word.Reset() + started = false + wordDependent = false + } + } + runes := []rune(value) + for index := 0; index < len(runes); index++ { + r := runes[index] + if r == '\\' { + if index+1 >= len(runes) { + return nil, nil, false + } + index++ + escaped := runes[index] + if quote == '\'' && escaped != '\\' && escaped != '\'' { + word.WriteRune('\\') + word.WriteRune(escaped) + started = true + continue + } + switch escaped { + case 'c': + if quote != 0 { + return nil, nil, false + } + flush() + return fields, dependent, true + case '_': + if quote == '"' { + word.WriteRune(' ') + started = true + } else { + flush() + } + case 'f': + word.WriteRune('\f') + started = true + case 'n': + word.WriteRune('\n') + started = true + case 'r': + word.WriteRune('\r') + started = true + case 't': + word.WriteRune('\t') + started = true + case 'v': + word.WriteRune('\v') + started = true + case '#', '$', '"', '\'', '\\': + word.WriteRune(escaped) + started = true + default: + return nil, nil, false + } + continue + } + if r == '\'' || r == '"' { + if quote == 0 { + quote = r + started = true + continue + } + if quote == r { + quote = 0 + continue + } + } + if r == '$' && quote != '\'' { + if index+2 >= len(runes) || runes[index+1] != '{' { + return nil, nil, false + } + end := index + 2 + for end < len(runes) && runes[end] != '}' { + end++ + } + if end >= len(runes) || !validEnvSplitVariableName(string(runes[index+2:end])) { + return nil, nil, false + } + word.WriteString(string(runes[index : end+1])) + started = true + wordDependent = true + index = end + continue + } + if r == '#' && quote == 0 && !started { + break + } + if quote == 0 && (r == ' ' || r == '\t' || r == '\n' || r == '\r' || r == '\v' || r == '\f') { + flush() + continue + } + word.WriteRune(r) + started = true + } + if quote != 0 { + return nil, nil, false + } + flush() + return fields, dependent, true +} + +func validEnvSplitVariableName(name string) bool { + if name == "" { + return false + } + for index, r := range name { + if (r >= 'a' && r <= 'z') || (r >= 'A' && r <= 'Z') || r == '_' || + (index > 0 && r >= '0' && r <= '9') { + continue + } + return false + } + return true +} + +func busyboxDelegatedCommand(args []string) ([]string, commandResolution) { + if len(args) == 0 { + return nil, commandKnownLocal + } + switch args[0] { + case "--help", "--list", "--list-full", "--install", "--show": + return nil, commandKnownLocal + } + if strings.HasPrefix(args[0], "-") { + return nil, commandUnresolved + } + if fallbackTokenLooksDynamic(args[0]) { + return nil, commandUnresolved + } + return args, commandKnownLocal +} + +var ( + straceRequiredLongOptions = map[string]bool{ + "--columns": true, "--detach-on": true, "--env": true, "--interruptible": true, + "--stack-trace-frame-limit": true, "--syscall-limit": true, "--output": true, + "--summary-syscall-overhead": true, "--attach": true, "--trace-path": true, + "--string-limit": true, "--summary-sort-by": true, "--user": true, + "--summary-columns": true, "--const-print-style": true, "--argv0": true, + "--color": true, "--trace": true, "--trace-fds": true, "--abbrev": true, + "--verbose": true, "--raw": true, "--signals": true, "--status": true, + "--read": true, "--write": true, "--fault": true, "--inject": true, + "--kvm": true, "--decode-pids": true, + } + straceOptionalLongOptions = map[string]bool{ + "--daemonize": true, "--daemonised": true, "--daemonized": true, + "--stack-trace": true, "--stack-traces": true, "--relative-timestamps": true, + "--absolute-timestamps": true, "--timestamps": true, "--syscall-times": true, + "--strings-in-hex": true, "--tips": true, "--namespace": true, + "--quiet": true, "--silent": true, "--silence": true, "--decode-fds": true, + "--secontext": true, + } + straceValuelessLongOptions = map[string]bool{ + "--output-append-mode": true, "--summary-only": true, "--summary": true, + "--debug": true, "--follow-forks": true, "--output-separately": true, + "--instruction-pointer": true, "--kill-on-exit": true, "--syscall-number": true, + "--arg-names": true, "--no-abbrev": true, "--summary-wall-clock": true, + "--pidns-translation": true, "--successful-only": true, "--failed-only": true, + "--failing-only": true, "--seccomp-bpf": true, "--always-show-pid": true, + } +) + +type straceLongOptionKind uint8 + +const ( + straceLongOptionInvalid straceLongOptionKind = iota + straceLongOptionRequired + straceLongOptionOptional + straceLongOptionValueless + straceLongOptionTerminal +) + +func straceDelegatedCommand(args []string) ([]string, commandResolution) { + index, status := straceChildResolution(args) + if status != commandKnownLocal || index < 0 || index >= len(args) { + return nil, status + } + if fallbackTokenLooksDynamic(args[index]) { + return nil, commandUnresolved + } + return args[index:], commandKnownLocal +} + +// straceChildIndex retains the index surface used by AST literalness checks. +// Unresolved option grammar is deliberately not exposed as "no child"; the +// shared argv resolver consumes straceChildResolution directly and keeps the +// network gate in that state. +func straceChildIndex(args []string) (int, bool) { + index, status := straceChildResolution(args) + return index, status == commandKnownLocal && index >= 0 +} + +func straceChildResolution(args []string) (int, commandResolution) { + for index := 0; index < len(args); index++ { + arg := args[index] + switch arg { + case "--": + if index+1 >= len(args) { + return -1, commandKnownLocal + } + return index + 1, commandKnownLocal + case "-": + return index, commandKnownLocal + } + if strings.HasPrefix(arg, "--") { + name, hasValue := arg, false + if equals := strings.IndexByte(arg, '='); equals >= 0 { + name, hasValue = arg[:equals], true + } + kind, ok := resolveStraceLongOption(name) + if !ok { + return -1, commandUnresolved + } + switch kind { + case straceLongOptionTerminal: + return -1, commandKnownLocal + case straceLongOptionRequired: + if !hasValue { + index++ + if index >= len(args) { + return -1, commandUnresolved + } + } + case straceLongOptionOptional: + // GNU getopt_long accepts optional values only as --option=value. + case straceLongOptionValueless: + if hasValue { + return -1, commandUnresolved + } + default: + return -1, commandUnresolved + } + continue + } + if strings.HasPrefix(arg, "-") { + cluster := []rune(arg[1:]) + for clusterIndex, option := range cluster { + switch { + case option == 'h' || option == 'V': + return -1, commandKnownLocal + case strings.ContainsRune("abeEIoOpPsSuUX", option): + if clusterIndex+1 == len(cluster) { + index++ + if index >= len(args) { + return -1, commandUnresolved + } + } + clusterIndex = len(cluster) + case strings.ContainsRune("AcCdDfFiknNqrtTvwxyYzZ", option): + default: + return -1, commandUnresolved + } + if clusterIndex == len(cluster) { + break + } + } + continue + } + return index, commandKnownLocal + } + return -1, commandKnownLocal +} + +func resolveStraceLongOption(name string) (straceLongOptionKind, bool) { + if kind := exactStraceLongOption(name); kind != straceLongOptionInvalid { + return kind, true + } + matchedKind := straceLongOptionInvalid + matches := 0 + visit := func(options map[string]bool, kind straceLongOptionKind) { + for option := range options { + if strings.HasPrefix(option, name) { + matchedKind = kind + matches++ + } + } + } + visit(straceRequiredLongOptions, straceLongOptionRequired) + visit(straceOptionalLongOptions, straceLongOptionOptional) + visit(straceValuelessLongOptions, straceLongOptionValueless) + for _, option := range []string{"--help", "--version"} { + if strings.HasPrefix(option, name) { + matchedKind = straceLongOptionTerminal + matches++ + } + } + return matchedKind, matches == 1 +} + +func exactStraceLongOption(name string) straceLongOptionKind { + switch { + case straceRequiredLongOptions[name]: + return straceLongOptionRequired + case straceOptionalLongOptions[name]: + return straceLongOptionOptional + case straceValuelessLongOptions[name]: + return straceLongOptionValueless + case name == "--help" || name == "--version": + return straceLongOptionTerminal + default: + return straceLongOptionInvalid } - return "" } // isNumericToken reports whether a token is purely digits (e.g. the duration @@ -415,13 +980,8 @@ func shellDashCPayload(program string, fields []string) string { return "" } args := fields[start+1:] - for i, arg := range args { - if arg == "-c" || arg == "--command" { - if i+1 < len(args) { - return strings.Join(args[i+1:], " ") - } - return "" - } + if payloadIndex, found := shellCommandPayloadIndex(program, args); found && payloadIndex < len(args) { + return strings.Trim(args[payloadIndex], `"'`) } return "" } @@ -454,8 +1014,20 @@ func normalizeProgramToken(field string) string { token = token[i+1:] } } - token = strings.ToLower(token) - for _, suffix := range []string{".exe", ".cmd", ".bat", ".com"} { + return trimExecutableSuffix(strings.ToLower(token)) +} + +// executableSuffixes are the Windows executable extensions stripped from a +// program token so curl.exe, git.cmd, and curl all normalize to one name. Both +// the parseable path (normalizeProgramToken) and the unparseable fallback +// (executableTokenBase) strip the same set; a token that normalizes differently +// on the two paths is exactly how a command slips past the fallback. +var executableSuffixes = []string{".exe", ".cmd", ".bat", ".com"} + +// trimExecutableSuffix removes one trailing Windows executable extension from an +// already-lowercased token. +func trimExecutableSuffix(token string) string { + for _, suffix := range executableSuffixes { if strings.HasSuffix(token, suffix) { return strings.TrimSuffix(token, suffix) } @@ -487,9 +1059,9 @@ func windowsExecutablePathBasename(token string) (string, bool) { } func hasWindowsExecutableSuffix(token string) bool { - token = strings.ToLower(token) - for _, suffix := range []string{".exe", ".cmd", ".bat", ".com"} { - if strings.HasSuffix(token, suffix) { + lowered := strings.ToLower(token) + for _, suffix := range executableSuffixes { + if strings.HasSuffix(lowered, suffix) { return true } } diff --git a/internal/sandbox/safe_command_test.go b/internal/sandbox/safe_command_test.go index 15a2c5d6f..c5239498e 100644 --- a/internal/sandbox/safe_command_test.go +++ b/internal/sandbox/safe_command_test.go @@ -407,6 +407,20 @@ func TestDetectInteractiveMongoEvalAndFullPaths(t *testing.T) { } } +func TestDetectInteractiveCommandBoundsNestedShellLaunchers(t *testing.T) { + command := "vim file" + for range maxAnalyzerDepth + 2 { + command = "bash -c '" + strings.ReplaceAll(command, "'", `'"'"'`) + "'" + } + got := DetectInteractiveCommand(command, "linux") + if !got.Interactive || got.Command != "nested shell launcher" { + t.Fatalf("DetectInteractiveCommand(deep shell chain) = %+v; want conservative depth-limit result", got) + } + if !strings.Contains(got.Reason, "cannot be proven non-interactive") { + t.Fatalf("DetectInteractiveCommand(deep shell chain).Reason = %q; want unresolved inspection reason", got.Reason) + } +} + // The hand-written segment splitter misses interactive programs hidden by // constructs only a real shell parser resolves (a newline separator collapsed // to a space; a brace group that shifts the real command position). The AST diff --git a/internal/tools/bash_tool_test.go b/internal/tools/bash_tool_test.go index 297f9816a..462cd10bd 100644 --- a/internal/tools/bash_tool_test.go +++ b/internal/tools/bash_tool_test.go @@ -789,6 +789,29 @@ func TestBashToolBlocksInteractiveCommandBeforeExecution(t *testing.T) { } } +func TestBashToolBlocksOverDepthShellLauncherBeforeExecution(t *testing.T) { + command := "vim main.go" + // Six launchers exceed sandbox.maxAnalyzerDepth (4) while leaving the + // interactive payload beyond the detector's inspection budget. + for range 6 { + command = "bash -c '" + strings.ReplaceAll(command, "'", `'"'"'`) + "'" + } + + result := NewScopedBashTool(t.TempDir(), nil).Run(context.Background(), map[string]any{ + "command": command, + }) + + if result.Status != StatusError || result.Meta["safety_block"] != "interactive_command" { + t.Fatalf("over-depth shell launcher result = %#v; want pre-execution safety block", result) + } + if result.Meta["exit_code"] != "-1" || result.Meta["safety_cmd"] != "nested shell launcher" { + t.Fatalf("over-depth shell launcher metadata = %#v; want unresolved launcher blocked before execution", result.Meta) + } + if !strings.Contains(result.Output, "cannot be proven non-interactive") { + t.Fatalf("over-depth shell launcher output = %q; want conservative inspection-limit reason", result.Output) + } +} + func TestBashToolBlocksInteractiveCommandThroughSandbox(t *testing.T) { root := t.TempDir() engine := sandbox.NewEngine(sandbox.EngineOptions{