diff --git a/src/cmd/ai_claude_environment.go b/src/cmd/ai_claude_environment.go index 9c1c8ecb..1fb456b5 100644 --- a/src/cmd/ai_claude_environment.go +++ b/src/cmd/ai_claude_environment.go @@ -5,9 +5,13 @@ import ( "context" "errors" "fmt" + "net" + "net/url" "os" "path/filepath" + "runtime" "sort" + "strconv" "strings" "time" @@ -35,6 +39,9 @@ const ( claudeHookKickBudget = 1500 * time.Millisecond ) +// claudeEnvironmentGOOS picks the liveness guard; tests override it. +var claudeEnvironmentGOOS = runtime.GOOS + type claudeEnvironmentStatus struct { Schema int `json:"schema"` PID int `json:"pid"` @@ -70,7 +77,7 @@ type claudeRuntimeHealth struct { // renderClaudeEnvironment produces the POSIX shell a harness sources before each // command. Proxy routing, Phase runtime markers, and trust overrides live inside -// a liveness guard on the issuing proxy PID, so a crashed proxy can never strand +// a liveness guard on the issuing proxy, so a crashed proxy can never strand // a shell behind a dead port. Decoys, adapter endpoints, and explicit unsets // stay outside the guard: while Phase AI is enabled, provider tools either reach // the Phase proxy or fail on the decoy. They never fall through to an ambient @@ -79,6 +86,13 @@ func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, pr if proxyLive && pid <= 0 { return "", errors.New("Phase AI proxy PID is required for a live environment") } + var guard string + if proxyLive { + var err error + if guard, err = claudeProxyLivenessGuard(pid, plan); err != nil { + return "", err + } + } keys := make([]string, 0, len(plan.Set)) for key, value := range plan.Set { if !phaseproxy.ValidShellEnvironmentKey(key) { @@ -115,7 +129,7 @@ func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, pr } if proxyLive { out.WriteString(claudeProxyBlockBegin + "\n") - fmt.Fprintf(&out, "if kill -0 %d 2>/dev/null; then\n", pid) + out.WriteString("if " + guard + "; then\n") for _, key := range keys { if !phaseproxy.RuntimeControlEnvironmentKey(key) { continue @@ -133,6 +147,35 @@ func renderClaudeEnvironment(pid int, plan phaseproxy.ProcessEnvironmentPlan, pr return out.String(), nil } +// claudeProxyLivenessGuard keeps routing on only while the proxy is alive. Git +// Bash cannot signal native Windows PIDs, so on Windows it probes the port. +func claudeProxyLivenessGuard(pid int, plan phaseproxy.ProcessEnvironmentPlan) (string, error) { + if claudeEnvironmentGOOS != "windows" { + return fmt.Sprintf("kill -0 %d 2>/dev/null", pid), nil + } + raw := "" + for _, key := range []string{"HTTPS_PROXY", "HTTP_PROXY", "https_proxy", "http_proxy"} { + if raw = plan.Set[key]; raw != "" { + break + } + } + if raw == "" { + return "false", nil + } + proxyURL, err := url.Parse(raw) + if err != nil { + return "", errors.New("Phase AI proxy URL is invalid") + } + host, port, err := net.SplitHostPort(proxyURL.Host) + number, portErr := strconv.Atoi(port) + if err != nil || portErr != nil || net.ParseIP(host).To4() == nil || number < 1 || number > 65535 { + return "", fmt.Errorf("Phase AI proxy address %q is not an IPv4 address and port", proxyURL.Host) + } + // true is a regular builtin, so a refused connection fails the test without + // ending a non-interactive shell. + return fmt.Sprintf("true 2>/dev/null /dev/null /dev/null; then", os.Getpid())) || + guard, err := claudeProxyLivenessGuard(os.Getpid(), phaseproxy.ProcessEnvironmentPlan{Set: map[string]string{"HTTP_PROXY": managed.states[index].ProxyURL}}) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(environment, "if "+guard+"; then") || !strings.Contains(environment, "export HTTP_PROXY='"+managed.states[index].ProxyURL+"'") || !strings.Contains(environment, "export AWS_ACCESS_KEY_ID='AKIADECOY'") { t.Fatalf("cycle %d did not publish a live sourced environment:\n%s", index+1, environment) @@ -289,7 +293,7 @@ func TestClaudeDaemonRecyclesSameProcessAndGenerationOnOneEngine(t *testing.T) { if err != nil { t.Fatal(err) } - if strings.Contains(string(final), "kill -0") || strings.Contains(string(final), "HTTP_PROXY") || + if strings.Contains(string(final), claudeProxyBlockBegin) || strings.Contains(string(final), "HTTP_PROXY") || !strings.Contains(string(final), "export AWS_ACCESS_KEY_ID='AKIADECOY'") { t.Fatalf("daemon exit left proxy routing in the sourced environment:\n%s", final) } @@ -929,7 +933,7 @@ func TestClaudeHeartbeatReportsStatusAndRetiresStaleRouting(t *testing.T) { if err != nil { t.Fatal(err) } - if strings.Contains(string(environment), "kill -0") || strings.Contains(string(environment), "HTTPS_PROXY") || + if strings.Contains(string(environment), claudeProxyBlockBegin) || strings.Contains(string(environment), "HTTPS_PROXY") || !strings.Contains(string(environment), "export AWS_ACCESS_KEY_ID='AKIADECOY'") { t.Fatalf("heartbeat left routing for a dead proxy:\n%s", environment) } diff --git a/src/cmd/ai_enable.go b/src/cmd/ai_enable.go index 8101a1d6..ced21daa 100644 --- a/src/cmd/ai_enable.go +++ b/src/cmd/ai_enable.go @@ -245,7 +245,7 @@ func runAIEnable(cmd *cobra.Command, args []string) error { fmt.Printf(" Earlier Phase skill migrated to: %s\n", migratedPath) } - installations, err := installConfiguredAIHarnessAdapters(cmd.Context(), cfg.ConfiguredHarnessIDs(), executable) + installations, skipped, err := installConfiguredAIHarnessAdapters(cmd.Context(), cfg.ConfiguredHarnessIDs(), selectedTarget.ID, executable) if err != nil { return err } @@ -263,7 +263,7 @@ func runAIEnable(cmd *cobra.Command, args []string) error { if user, err := config.GetDefaultUser(); err == nil { principal = user } - fmt.Print(formatAIEnableSummary(selectedTarget, cfg, principal, installations)) + fmt.Print(formatAIEnableSummary(selectedTarget, cfg, principal, installations, skipped)) return nil } @@ -280,7 +280,7 @@ func trustAttemptPossible(flagChanged, flagValue, interactive bool) bool { // line, then the steps the human must take before the harness can use Phase. // It is separated from the install log by blank lines so the next steps are // the last thing on screen. -func formatAIEnableSummary(target ai.SkillTarget, cfg *config.AIConfig, principal *config.UserConfig, installations map[string]string) string { +func formatAIEnableSummary(target ai.SkillTarget, cfg *config.AIConfig, principal *config.UserConfig, installations, skipped map[string]string) string { var out strings.Builder out.WriteString("\n") if target.Name != "" { @@ -324,6 +324,11 @@ func formatAIEnableSummary(target ai.SkillTarget, cfg *config.AIConfig, principa out.WriteString(util.BoldYellow("⚠️ Codex: its Phase hook was refreshed too.") + "\n") out.WriteString(fmt.Sprintf(" Open the Codex CLI, run /hooks, and trust every hook from %s@%s again.\n", ai.CodexBootstrapPluginName, ai.CodexBootstrapMarketplaceName)) } + if reason := skipped[ai.HarnessCodex]; reason != "" { + out.WriteString("\n") + out.WriteString(util.BoldYellow("⚠️ Codex: its Phase plugin is out of date and was not refreshed, because "+reason+".") + "\n") + out.WriteString(" Rerun `phase ai enable --harness codex` with the Codex CLI on PATH to refresh it.\n") + } if target.ID != "" { out.WriteString("\n Add another harness with `phase ai enable --harness `; existing integrations stay installed.\n") } @@ -393,9 +398,12 @@ func claudeHookExecutablePath() (string, error) { // installConfiguredAIHarnessAdapters refreshes one major-versioned helper from // the current CLI, then points every thin harness adapter at it. Provider and -// discovery changes do not require reinstalling those adapters. -func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, executable string) (map[string]string, error) { +// discovery changes do not require reinstalling those adapters. When another +// harness is selected, a current Codex adapter is left alone, and an outdated +// one is returned as skipped when the Codex CLI is not on PATH. +func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, selected, executable string) (map[string]string, map[string]string, error) { installed := make(map[string]string, len(harnesses)) + skipped := make(map[string]string) hookExecutable := executable for _, harness := range harnesses { if harness == ai.HarnessCopilot { @@ -403,7 +411,7 @@ func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, } shared, err := ai.InstallSharedHookExecutable(executable) if err != nil { - return nil, fmt.Errorf("install shared Phase Agent hook: %w", err) + return nil, nil, fmt.Errorf("install shared Phase Agent hook: %w", err) } hookExecutable = shared.ExecutablePath break @@ -413,69 +421,76 @@ func installConfiguredAIHarnessAdapters(ctx context.Context, harnesses []string, case ai.HarnessClaudeCode: path, err := ai.DefaultClaudeBootstrapPluginPath() if err != nil { - return nil, err + return nil, nil, err } if err := ai.InstallClaudeBootstrapPlugin(path, hookExecutable); err != nil { - return nil, fmt.Errorf("install Claude Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Claude Phase discovery adapter: %w", err) } installed[harness] = "Claude Phase discovery adapter installed to: " + path case ai.HarnessCursor: path, err := ai.DefaultCursorBootstrapPluginPath() if err != nil { - return nil, err + return nil, nil, err } if err := ai.InstallCursorBootstrapPlugin(path, hookExecutable); err != nil { - return nil, fmt.Errorf("install Cursor Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Cursor Phase discovery adapter: %w", err) } installed[harness] = "Cursor Phase discovery adapter installed to: " + path case ai.HarnessCodex: path, err := ai.DefaultCodexBootstrapMarketplacePath() if err != nil { - return nil, err + return nil, nil, err + } + if harness != selected && ai.CodexBootstrapPluginCurrent(path, hookExecutable) { + continue } codexExecutable, err := exec.LookPath("codex") + if err != nil && harness != selected { + skipped[harness] = "the Codex CLI is not on PATH" + continue + } if err != nil { - return nil, errors.New("Codex CLI is required to refresh the configured Phase plugin; install Codex, then rerun `phase ai enable`") + return nil, nil, errors.New("Codex CLI is required to install the Phase plugin; install Codex or add its CLI to PATH, then rerun `phase ai enable --harness codex`") } installation, err := ai.InstallCodexBootstrapPlugin(path, hookExecutable) if err != nil { - return nil, fmt.Errorf("install Codex Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Codex Phase discovery adapter: %w", err) } if err := ai.ActivateCodexBootstrapPlugin(ctx, codexExecutable, path); err != nil { - return nil, fmt.Errorf("activate Codex Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("activate Codex Phase discovery adapter: %w", err) } installed[harness] = "Codex Phase discovery adapter installed: " + installation.Selector case ai.HarnessOpenCode: path, err := ai.DefaultOpenCodePluginPath() if err != nil { - return nil, err + return nil, nil, err } runtimeDirectory, err := claudeHookRuntimeDir() if err != nil { - return nil, err + return nil, nil, err } installation, err := ai.InstallOpenCodePlugin(path, hookExecutable, claudeEnvironmentJSONPath(runtimeDirectory)) if err != nil { - return nil, fmt.Errorf("install OpenCode Phase plugin: %w", err) + return nil, nil, fmt.Errorf("install OpenCode Phase plugin: %w", err) } installed[harness] = "OpenCode Phase plugin installed to: " + installation.PluginPath case ai.HarnessDevin: path, err := ai.DefaultDevinConfigPath() if err != nil { - return nil, err + return nil, nil, err } installation, err := ai.InstallDevinHooks(path, hookExecutable) if err != nil { - return nil, fmt.Errorf("install Devin Phase discovery adapter: %w", err) + return nil, nil, fmt.Errorf("install Devin Phase discovery adapter: %w", err) } installed[harness] = "Devin Phase discovery adapter installed in: " + installation.ConfigPath case ai.HarnessCopilot: // Copilot consumes only the Agent Skill; it has no automatic helper. default: - return nil, fmt.Errorf("unsupported configured Agent harness %q", harness) + return nil, nil, fmt.Errorf("unsupported configured Agent harness %q", harness) } } - return installed, nil + return installed, skipped, nil } func codexHookReviewRequired(installations map[string]string) bool { diff --git a/src/cmd/ai_enable_test.go b/src/cmd/ai_enable_test.go index 67119493..de563d18 100644 --- a/src/cmd/ai_enable_test.go +++ b/src/cmd/ai_enable_test.go @@ -79,9 +79,10 @@ func TestInstallConfiguredAIHarnessAdaptersRefreshesEveryHelper(t *testing.T) { if err != nil { t.Fatal(err) } - installed, err := installConfiguredAIHarnessAdapters( + installed, _, err := installConfiguredAIHarnessAdapters( context.Background(), []string{ai.HarnessClaudeCode, ai.HarnessCursor}, + ai.HarnessCursor, source, ) if err != nil { @@ -141,6 +142,44 @@ func TestCodexHookReviewRequiredWhenExistingAdapterIsRefreshed(t *testing.T) { } } +func TestInstallConfiguredAIHarnessAdaptersDoNotNeedCodexForAnotherHarness(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("PATH", t.TempDir()) + source, err := os.Executable() + if err != nil { + t.Fatal(err) + } + // Codex was enabled earlier, and Cursor is added where codex is not on PATH. + harnesses := []string{ai.HarnessCodex, ai.HarnessCursor} + installed, skipped, err := installConfiguredAIHarnessAdapters(context.Background(), harnesses, ai.HarnessCursor, source) + if err != nil { + t.Fatalf("adding Cursor failed because the Codex CLI is missing: %v", err) + } + if installed[ai.HarnessCursor] == "" || installed[ai.HarnessCodex] != "" || skipped[ai.HarnessCodex] == "" { + t.Fatalf("installed=%v skipped=%v", installed, skipped) + } + if _, _, err := installConfiguredAIHarnessAdapters(context.Background(), harnesses, ai.HarnessCodex, source); err == nil || !strings.Contains(err.Error(), "PATH") { + t.Fatalf("enabling Codex itself must still require its CLI, got %v", err) + } + // Once the Codex adapter is up to date, adding Cursor leaves it alone. + root, err := ai.DefaultCodexBootstrapMarketplacePath() + if err != nil { + t.Fatal(err) + } + helper, err := ai.DefaultSharedHookExecutablePath() + if err != nil { + t.Fatal(err) + } + if _, err := ai.InstallCodexBootstrapPlugin(root, helper); err != nil { + t.Fatal(err) + } + if installed, skipped, err := installConfiguredAIHarnessAdapters(context.Background(), harnesses, ai.HarnessCursor, source); err != nil || len(skipped) != 0 || installed[ai.HarnessCodex] != "" { + t.Fatalf("an up-to-date Codex adapter was touched or reported: installed=%v skipped=%v err=%v", installed, skipped, err) + } +} + func TestAIEnableHarnessRestartMessagesRepeatOriginalPrompt(t *testing.T) { tests := []struct { harness string @@ -174,7 +213,7 @@ func TestFormatAIEnableSummaryEndsWithNextSteps(t *testing.T) { codex, _ := ai.SkillTargetByID(ai.HarnessCodex) cfg := &config.AIConfig{Enabled: true, MaskSecretValues: true} agent := &config.UserConfig{PrincipalType: config.PrincipalAgent} - summary := formatAIEnableSummary(codex, cfg, agent, map[string]string{ai.HarnessCodex: "Codex Phase discovery adapter installed: phase-ai-bootstrap@phase-cli-local"}) + summary := formatAIEnableSummary(codex, cfg, agent, map[string]string{ai.HarnessCodex: "Codex Phase discovery adapter installed: phase-ai-bootstrap@phase-cli-local"}, nil) if !strings.HasPrefix(summary, "\n") { t.Fatalf("summary must be separated from the install log:\n%s", summary) } @@ -197,7 +236,7 @@ func TestFormatAIEnableSummaryEndsWithNextSteps(t *testing.T) { claude, _ := ai.SkillTargetByID(ai.HarnessClaudeCode) user := &config.UserConfig{PrincipalType: "user"} - summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true}, user, map[string]string{ai.HarnessCodex: "refreshed"}) + summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true}, user, map[string]string{ai.HarnessCodex: "refreshed"}, nil) for _, required := range []string{ "AI tools can read secret values", "No default Workflow yet", "👉 Before Claude Code can use Phase:", " " + aiEnableHarnessRestartMessage(ai.HarnessClaudeCode) + "\n", "automatically while the proxy runs", @@ -210,19 +249,30 @@ func TestFormatAIEnableSummaryEndsWithNextSteps(t *testing.T) { if strings.Contains(summary, "1. ") { t.Fatalf("a single step must not be numbered:\n%s", summary) } - summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true, DefaultWorkflowID: "wf-123"}, nil, nil) + summary = formatAIEnableSummary(claude, &config.AIConfig{Enabled: true, DefaultWorkflowID: "wf-123"}, nil, nil, nil) if !strings.Contains(summary, "Default Workflow: wf-123") { t.Fatalf("default Workflow missing:\n%s", summary) } - custom := formatAIEnableSummary(ai.SkillTarget{}, cfg, nil, nil) + custom := formatAIEnableSummary(ai.SkillTarget{}, cfg, nil, nil, nil) if !strings.Contains(custom, "✅ Phase AI enabled\n") || !strings.Contains(custom, "phase ai run -- ") || strings.Contains(custom, "Add another harness") { t.Fatalf("custom-path summary:\n%s", custom) } copilot, _ := ai.SkillTargetByID(ai.HarnessCopilot) - if summary := formatAIEnableSummary(copilot, cfg, nil, nil); !strings.Contains(summary, "phase ai exec -- ") { + if summary := formatAIEnableSummary(copilot, cfg, nil, nil, nil); !strings.Contains(summary, "phase ai exec -- ") { t.Fatalf("Copilot summary must point at phase ai exec:\n%s", summary) } - t.Logf("codex summary:%s", formatAIEnableSummary(codex, cfg, agent, nil)) + t.Logf("codex summary:%s", formatAIEnableSummary(codex, cfg, agent, nil, nil)) +} + +func TestFormatAIEnableSummaryReportsASkippedCodexRefresh(t *testing.T) { + cursor, _ := ai.SkillTargetByID(ai.HarnessCursor) + summary := formatAIEnableSummary(cursor, &config.AIConfig{Enabled: true}, nil, + map[string]string{ai.HarnessCursor: "Cursor Phase discovery adapter installed"}, + map[string]string{ai.HarnessCodex: "the Codex CLI is not on PATH"}) + if !strings.Contains(summary, "⚠️ Codex: its Phase plugin is out of date and was not refreshed, because the Codex CLI is not on PATH.") || + !strings.Contains(summary, "`phase ai enable --harness codex`") || strings.Contains(summary, "refreshed too") { + t.Fatalf("a skipped Codex refresh must be reported, without asking to re-trust hooks that did not change:\n%s", summary) + } } func TestTrustAttemptPossibleOnlyWhenADialogCanAppear(t *testing.T) { diff --git a/src/cmd/ai_harness_hook.go b/src/cmd/ai_harness_hook.go index fac79f26..50d3b994 100644 --- a/src/cmd/ai_harness_hook.go +++ b/src/cmd/ai_harness_hook.go @@ -240,6 +240,9 @@ func runAICodexEvent(cmd *cobra.Command, args []string) error { if err != nil || strings.TrimSpace(original) == "" { return denyCodexShell(cmd) } + if shellCommandUsesPowerShellWeb(original) { + return denyCodexShellWithReason(cmd, harnessPowerShellWebMessage()) + } if !shellCommandRequiresPhase(original) { return nil } @@ -367,6 +370,9 @@ func runAICursorEvent(cmd *cobra.Command, args []string) error { strings.TrimSpace(original) == "" { return denyCursorShell(cmd) } + if shellCommandUsesPowerShellWeb(original) { + return denyCursorShellWithReason(cmd, harnessPowerShellWebMessage()) + } if !shellCommandRequiresPhase(original) { return nil } @@ -490,6 +496,9 @@ func runAIDevinEvent(cmd *cobra.Command, args []string) error { if err != nil || strings.TrimSpace(original) == "" { return denyDevin(cmd, harnessFailureMessage("Devin")) } + if shellCommandUsesPowerShellWeb(original) { + return denyDevin(cmd, harnessPowerShellWebMessage()) + } if !shellCommandRequiresPhase(original) { return nil } @@ -615,6 +624,10 @@ func harnessMixedCommandMessage() string { return "This shell call mixes a proxy-local Phase control request with ordinary work. Split it into separate tool calls so the Phase control request can be handled independently." } +func harnessPowerShellWebMessage() string { + return "PowerShell's Invoke-WebRequest and Invoke-RestMethod do not use the Phase AI proxy, so they cannot reach phase.invalid. Send the same Phase control request with curl in its own tool call instead, for example: curl.exe -sS http://phase.invalid/v1/status" +} + func harnessUnavailableContext(name string) string { return "Phase AI credentials are currently unavailable in this " + name + " session. Ordinary tools, including git and gh, remain available. Commands that need Phase-managed credentials stay blocked until the authenticated proxy recovers." } @@ -692,10 +705,14 @@ func buildHarnessWrapperCommand(harness, leaseID, original string) (string, erro // harnessWrapperCommandLine quotes the wrapper for the shell the harness runs // it in. On Windows, Codex uses PowerShell, which needs the call operator to // run a quoted executable path; Cursor uses its default terminal profile. +// Other harnesses get the plain form every Windows shell runs when there is one. func harnessWrapperCommandLine(goos, harness string, arguments []string) string { if goos == "windows" { if harness == ai.HarnessCodex || (harness == ai.HarnessCursor && !cursorWindowsShellIsBashOrCmd(cursorWindowsSettings())) { - return quotePowerShellCommand(arguments) + return ai.PowerShellCallCommand(arguments) + } + if line, ok := ai.WindowsPlainCommand(arguments); ok { + return line } return quoteWindowsCommand(arguments) } @@ -869,25 +886,6 @@ func stripJSONTrailingCommas(source []byte) []byte { return out } -// quotePowerShellCommand renders `& 'exe' 'arg' ...`. PowerShell treats the -// typographic single quotes as quote characters too, so each is doubled. -func quotePowerShellCommand(arguments []string) string { - quoted := make([]string, len(arguments)) - for index, argument := range arguments { - var builder strings.Builder - builder.WriteByte('\'') - for _, character := range argument { - if character == '\'' || character == '\u2018' || character == '\u2019' || character == '\u201A' || character == '\u201B' { - builder.WriteRune(character) - } - builder.WriteRune(character) - } - builder.WriteByte('\'') - quoted[index] = builder.String() - } - return "& " + strings.Join(quoted, " ") -} - func quoteWindowsCommand(arguments []string) string { quoted := make([]string, len(arguments)) for index, argument := range arguments { @@ -1230,6 +1228,10 @@ func startHarnessCommandLeaseRenewal(done <-chan struct{}, interval time.Duratio return renewal.Wait } +// windowsPowerShellPrelude drops Windows PowerShell's curl and wget aliases of +// Invoke-WebRequest, which ignores HTTP(S)_PROXY. +const windowsPowerShellPrelude = "Remove-Item -Path Alias:curl, Alias:wget -Force -ErrorAction SilentlyContinue; " + func executeHarnessShell(ctx context.Context, command string, environment []string, stdin io.Reader, stdout, stderr io.Writer) error { shell := util.GetDefaultShell() if len(shell) == 0 { @@ -1239,7 +1241,7 @@ func executeHarnessShell(ctx context.Context, command string, environment []stri if runtime.GOOS == "windows" { base := strings.ToLower(shell[0]) if strings.Contains(base, "powershell") || strings.Contains(base, "pwsh") { - arguments = []string{"-NoLogo", "-NoProfile", "-NonInteractive", "-Command", command} + arguments = []string{"-NoLogo", "-NoProfile", "-NonInteractive", "-Command", windowsPowerShellPrelude + command} } else { arguments = []string{"/D", "/S", "/C", command} } @@ -1275,6 +1277,11 @@ func runAIOpenCodeCommand(cmd *cobra.Command, _ []string) error { }) } original := string(commandBytes) + if shellCommandUsesPowerShellWeb(original) { + return emitOpenCodeCommandResponse(cmd, openCodeCommandResponse{ + SchemaVersion: 1, Managed: true, Error: harnessPowerShellWebMessage(), + }) + } if shellCommandMixesPhase(original) { return emitOpenCodeCommandResponse(cmd, openCodeCommandResponse{ SchemaVersion: 1, Managed: true, Error: harnessMixedCommandMessage(), diff --git a/src/cmd/ai_harness_hook_test.go b/src/cmd/ai_harness_hook_test.go index 64f8d5dc..dcf2d949 100644 --- a/src/cmd/ai_harness_hook_test.go +++ b/src/cmd/ai_harness_hook_test.go @@ -644,6 +644,54 @@ func TestHarnessWrapperCommandEnforcesCrossPlatformArgumentLimit(t *testing.T) { } } +func TestHarnessGuardsRejectPowerShellWebCallsToPhase(t *testing.T) { + const call = "Invoke-RestMethod http://phase.invalid/v1/status" + t.Setenv("PHASE_AI_ENABLED", "") + t.Run("codex", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessCodex) + command, output := newHarnessHookCommand(`{"session_id":"codex-session","hook_event_name":"PreToolUse","tool_name":"Bash", + "tool_input":{"command":"` + call + `"},"tool_use_id":"tool-one"}`) + if err := runAICodexEvent(command, []string{"guard"}); err != nil { + t.Fatal(err) + } + var result codexToolOutput + if err := json.Unmarshal(output.Bytes(), &result); err != nil { + t.Fatal(err) + } + if result.HookSpecificOutput.PermissionDecision != "deny" || result.HookSpecificOutput.PermissionDecisionReason != harnessPowerShellWebMessage() { + t.Fatalf("Codex guard output = %#v", result) + } + }) + t.Run("cursor", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessCursor) + command, output := newHarnessHookCommand(`{"hook_event_name":"preToolUse","tool_name":"Shell", + "tool_input":{"command":"` + call + `"},"tool_use_id":"tool-two"}`) + if err := runAICursorEvent(command, []string{"guard"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(output.String(), "do not use the Phase AI proxy") || strings.Contains(output.String(), "__harness-exec") { + t.Fatalf("Cursor guard output = %s", output.String()) + } + }) + t.Run("devin", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessDevin) + command, output := newHarnessHookCommand(`{"session_id":"devin-session","hook_event_name":"PreToolUse","tool_name":"exec", + "tool_input":{"command":"` + call + `"},"tool_use_id":"devin-tool"}`) + if err := runAIDevinEvent(command, []string{"guard"}); err != nil { + t.Fatal(err) + } + if !strings.Contains(output.String(), "do not use the Phase AI proxy") || strings.Contains(output.String(), "__harness-exec") { + t.Fatalf("Devin guard output = %s", output.String()) + } + }) + t.Run("opencode", func(t *testing.T) { + withHarnessHookConfig(t, ai.HarnessOpenCode) + if response := runOpenCodeCommandForTest(t, call); !response.Managed || response.Error != harnessPowerShellWebMessage() { + t.Fatalf("OpenCode command response = %#v", response) + } + }) +} + func TestCursorGuardUsesSessionEnvironmentAndCursorSchema(t *testing.T) { withHarnessHookConfig(t, ai.HarnessCursor) prepareHarnessHookRuntime(t) @@ -1288,11 +1336,17 @@ func TestHarnessWrapperCommandLineMatchesTheHarnessShell(t *testing.T) { t.Fatalf("%s on Windows:\n got %s\nwant %s", harness, got, want) } } - if got := quotePowerShellCommand([]string{"a\u2019b"}); got != "& 'a\u2019\u2019b'" { + if got := ai.PowerShellCallCommand([]string{"a\u2019b"}); got != "& 'a\u2019\u2019b'" { t.Fatalf("typographic quote not doubled: %s", got) } if got := harnessWrapperCommandLine("windows", ai.HarnessDevin, arguments); !strings.HasPrefix(got, `"C:\Users`) { - t.Fatalf("Devin on Windows keeps the cmd form: %s", got) + t.Fatalf("Devin falls back to the cmd form for a path that needs quoting: %s", got) + } + plain := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__harness-exec", "--lease", "Y29kZXg6YQ"} + for _, harness := range []string{ai.HarnessDevin, ai.HarnessOpenCode} { + if got, want := harnessWrapperCommandLine("windows", harness, plain), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __harness-exec --lease Y29kZXg6YQ"; got != want { + t.Fatalf("%s on Windows:\n got %s\nwant %s", harness, got, want) + } } if got := harnessWrapperCommandLine("linux", ai.HarnessCodex, []string{"/home/o'brien/phase", "ai"}); got != `'/home/o'"'"'brien/phase' 'ai'` { t.Fatalf("POSIX form = %s", got) diff --git a/src/cmd/ai_harness_shell_windows_test.go b/src/cmd/ai_harness_shell_windows_test.go new file mode 100644 index 00000000..18631ac0 --- /dev/null +++ b/src/cmd/ai_harness_shell_windows_test.go @@ -0,0 +1,25 @@ +//go:build windows + +package cmd + +import ( + "bytes" + "context" + "os" + "strings" + "testing" + + "github.com/phasehq/cli/pkg/util" +) + +func TestExecuteHarnessShellRunsRealCurlUnderWindowsPowerShell(t *testing.T) { + shell := util.GetDefaultShell() + if len(shell) == 0 || (!strings.Contains(strings.ToLower(shell[0]), "powershell") && !strings.Contains(strings.ToLower(shell[0]), "pwsh")) { + t.Skip("the default shell is not PowerShell") + } + var output bytes.Buffer + err := executeHarnessShell(context.Background(), "(Get-Command curl).CommandType", os.Environ(), nil, &output, &output) + if err != nil || strings.TrimSpace(output.String()) != "Application" { + t.Fatalf("curl in the wrapped PowerShell = %q (err %v), want the real executable, not the Invoke-WebRequest alias", output.String(), err) + } +} diff --git a/src/cmd/ai_shell_scope.go b/src/cmd/ai_shell_scope.go index aebdaa08..1dd6ec04 100644 --- a/src/cmd/ai_shell_scope.go +++ b/src/cmd/ai_shell_scope.go @@ -3,12 +3,22 @@ package cmd import ( "net/url" "path/filepath" + "regexp" "strings" "unicode" ) const phaseShellRedirect = "\x00redirect" +var powerShellWebCmdlet = regexp.MustCompile(`(?i)(^|[\s;|&({="'])(invoke-webrequest|invoke-restmethod|iwr|irm)($|[\s;|)}"'])`) + +// shellCommandUsesPowerShellWeb reports a PowerShell web cmdlet, as a whole +// word, in a command naming phase.invalid. Windows PowerShell's web cmdlets +// ignore HTTP(S)_PROXY, so hooks deny the call and point the Agent at curl. +func shellCommandUsesPowerShellWeb(command string) bool { + return strings.Contains(strings.ToLower(command), "phase.invalid") && powerShellWebCmdlet.MatchString(command) +} + type phaseShellWord struct { value string control bool diff --git a/src/cmd/ai_shell_scope_test.go b/src/cmd/ai_shell_scope_test.go index 1b3987d7..4b88c0d0 100644 --- a/src/cmd/ai_shell_scope_test.go +++ b/src/cmd/ai_shell_scope_test.go @@ -60,6 +60,31 @@ func TestShellInterpreterScriptsArePhaseManaged(t *testing.T) { } } +func TestShellCommandUsesPowerShellWeb(t *testing.T) { + for _, test := range []struct { + command string + want bool + }{ + {command: "Invoke-RestMethod http://phase.invalid/v1/status", want: true}, + {command: `irm "http://phase.invalid/v1/requests" -Method Post -Body $body`, want: true}, + {command: "$r = Invoke-WebRequest -Uri http://phase.invalid/v1/status -UseBasicParsing", want: true}, + {command: "(iwr http://phase.invalid/v1/status).Content", want: true}, + {command: `$u = "http://phase.invalid/v1/status"; IRM $u`, want: true}, + {command: `pwsh -NoProfile -Command "iwr http://phase.invalid/v1/status"`, want: true}, + {command: `powershell -c 'Invoke-RestMethod http://phase.invalid/v1/status'`, want: true}, + {command: `Write-Output "irmx" phase.invalid`, want: false}, + {command: "curl.exe -sS http://phase.invalid/v1/status", want: false}, + {command: "curl --fail http://phase.invalid/v1/status", want: false}, + {command: "Invoke-RestMethod https://api.github.com/user", want: false}, + {command: "Write-Output confirm phase.invalid", want: false}, + {command: "Get-Content irm.txt; Get-Content phase.invalid.md", want: false}, + } { + if got := shellCommandUsesPowerShellWeb(test.command); got != test.want { + t.Errorf("shellCommandUsesPowerShellWeb(%q) = %v, want %v", test.command, got, test.want) + } + } +} + func TestShellCommandMixesPhase(t *testing.T) { for _, test := range []struct { command string diff --git a/src/pkg/ai/codex_plugin.go b/src/pkg/ai/codex_plugin.go index 732ee3e9..75fcffea 100644 --- a/src/pkg/ai/codex_plugin.go +++ b/src/pkg/ai/codex_plugin.go @@ -209,6 +209,20 @@ func InstallCodexBootstrapPlugin(root, phaseExecutable string) (CodexBootstrapIn return installation, nil } +// CodexBootstrapPluginCurrent reports, without the Codex CLI, whether root holds +// this build's plugin version and hooks for phaseExecutable. +func CodexBootstrapPluginCurrent(root, phaseExecutable string) bool { + pluginRoot := filepath.Join(root, "plugins", CodexBootstrapPluginName) + var manifest codexPluginManifest + payload, err := os.ReadFile(filepath.Join(pluginRoot, ".codex-plugin", "plugin.json")) + if err != nil || json.Unmarshal(payload, &manifest) != nil || manifest.Version != HarnessAdapterProtocolVersion { + return false + } + want, err := json.MarshalIndent(codexBootstrapHooks(phaseExecutable), "", " ") + got, readErr := os.ReadFile(filepath.Join(pluginRoot, "hooks", "hooks.json")) + return err == nil && readErr == nil && bytes.Equal(bytes.TrimSpace(got), want) +} + // ActivateCodexBootstrapPlugin registers the Phase-owned marketplace and // installs (or refreshes) the plugin via Codex's supported CLI. Hook trust is // deliberately left to Codex's /hooks review flow. @@ -363,11 +377,7 @@ func codexBootstrapHooks(hookExecutable string) codexHookFile { func codexPluginCommand(executable string, windows bool, arguments ...string) string { if windows { - parts := []string{quoteWindowsCommandArgument(executable)} - for _, argument := range arguments { - parts = append(parts, quoteWindowsCommandArgument(argument)) - } - return strings.Join(parts, " ") + return WindowsShellCommand(append([]string{executable}, arguments...)) } parts := []string{quotePOSIXCommandArgument(executable)} for _, argument := range arguments { @@ -380,10 +390,6 @@ func quotePOSIXCommandArgument(value string) string { return "'" + strings.ReplaceAll(value, "'", `'"'"'`) + "'" } -func quoteWindowsCommandArgument(value string) string { - return `"` + strings.ReplaceAll(value, `"`, `\"`) + `"` -} - func codexPluginSelector() string { return CodexBootstrapPluginName + "@" + CodexBootstrapMarketplaceName } diff --git a/src/pkg/ai/codex_plugin_test.go b/src/pkg/ai/codex_plugin_test.go index 96ac7a90..06bed9da 100644 --- a/src/pkg/ai/codex_plugin_test.go +++ b/src/pkg/ai/codex_plugin_test.go @@ -368,6 +368,19 @@ func TestCodexBootstrapPluginRefusesUnownedPaths(t *testing.T) { } } +func TestCodexBootstrapPluginCurrentMatchesWhatInstallWrites(t *testing.T) { + root, executable := installCodexTestPlugin(t) + if !CodexBootstrapPluginCurrent(root, executable) { + t.Fatal("the plugin InstallCodexBootstrapPlugin just wrote is not current") + } + if CodexBootstrapPluginCurrent(root, executable+"-moved") { + t.Fatal("hooks that run another helper path are current") + } + if CodexBootstrapPluginCurrent(filepath.Join(root, "missing"), executable) { + t.Fatal("a missing plugin is current") + } +} + func installCodexTestPlugin(t *testing.T) (string, string) { t.Helper() parent := t.TempDir() diff --git a/src/pkg/ai/cursor_plugin.go b/src/pkg/ai/cursor_plugin.go index 5c91957e..e1e30e17 100644 --- a/src/pkg/ai/cursor_plugin.go +++ b/src/pkg/ai/cursor_plugin.go @@ -129,16 +129,12 @@ func cursorBootstrapExecutablePath(root string) string { } func cursorHookCommand(executable string, args ...string) string { - command := quotePOSIXCommandArgument(executable) if runtime.GOOS == "windows" { - command = quoteWindowsCommandArgument(executable) + return WindowsShellCommand(append([]string{executable}, args...)) } + command := quotePOSIXCommandArgument(executable) for _, arg := range args { - if runtime.GOOS == "windows" { - command += " " + quoteWindowsCommandArgument(arg) - } else { - command += " " + quotePOSIXCommandArgument(arg) - } + command += " " + quotePOSIXCommandArgument(arg) } return command } diff --git a/src/pkg/ai/devin_hooks.go b/src/pkg/ai/devin_hooks.go index a0bc0fd0..ee80d9d2 100644 --- a/src/pkg/ai/devin_hooks.go +++ b/src/pkg/ai/devin_hooks.go @@ -61,7 +61,7 @@ func InstallDevinHooks(configPath, phaseExecutable string) (DevinHooksInstall, e if err != nil { return installation, err } - removePhaseDevinGroups(hooks, helperPath) + removePhaseDevinGroups(hooks, windowsExecutableForms(helperPath)...) for event, groups := range phaseDevinHooks(helperPath) { for _, group := range groups { payload, err := json.Marshal(group) @@ -99,7 +99,7 @@ func UninstallDevinHooks(configPath string) error { if err != nil { return err } - removePhaseDevinGroups(hooks, helperPath) + removePhaseDevinGroups(hooks, windowsExecutableForms(helperPath)...) hooksPayload, err := json.Marshal(hooks) if err != nil { return err @@ -156,13 +156,12 @@ func phaseDevinHooks(helperPath string) map[string][]devinHookGroup { } func shellCommand(arguments []string) string { + if runtime.GOOS == "windows" { + return WindowsShellCommand(arguments) + } quoted := make([]string, len(arguments)) for index, argument := range arguments { - if runtime.GOOS == "windows" { - quoted[index] = quoteWindowsCommandArgument(argument) - } else { - quoted[index] = quotePOSIXCommandArgument(argument) - } + quoted[index] = quotePOSIXCommandArgument(argument) } return strings.Join(quoted, " ") } diff --git a/src/pkg/ai/devin_hooks_test.go b/src/pkg/ai/devin_hooks_test.go index 60fad84a..315d403b 100644 --- a/src/pkg/ai/devin_hooks_test.go +++ b/src/pkg/ai/devin_hooks_test.go @@ -11,6 +11,7 @@ import ( func TestInstallDevinHooksPreservesConfigAndIsIdempotent(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) configPath := filepath.Join(home, ".config", "devin", "config.json") if err := os.MkdirAll(filepath.Dir(configPath), 0o700); err != nil { t.Fatal(err) @@ -59,6 +60,7 @@ func TestInstallDevinHooksPreservesConfigAndIsIdempotent(t *testing.T) { func TestUninstallDevinHooksPreservesUnrelatedHooks(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) configPath := filepath.Join(home, ".config", "devin", "config.json") executable := filepath.Join(home, "phase") if err := os.WriteFile(executable, []byte("phase"), 0o700); err != nil { diff --git a/src/pkg/ai/opencode_plugin_test.go b/src/pkg/ai/opencode_plugin_test.go index 92921c05..cf3b96d7 100644 --- a/src/pkg/ai/opencode_plugin_test.go +++ b/src/pkg/ai/opencode_plugin_test.go @@ -14,6 +14,7 @@ import ( func TestInstallOpenCodePluginIsDependencyFreeAndIdempotent(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") first := filepath.Join(home, "first-phase") if err := os.WriteFile(first, []byte("first"), 0o700); err != nil { @@ -97,6 +98,7 @@ func TestInstalledOpenCodeOnlyDelegatesExplicitPhaseControlCalls(t *testing.T) { } home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, OpenCodePluginName+".js") helper := filepath.Join(home, "phase-hook-v1") helperSource := "#!/bin/sh\nprintf '%s\\n' '{\"schemaVersion\":1,\"managed\":true,\"command\":\"phase-rewritten\"}'\n" @@ -179,6 +181,7 @@ func TestInstalledOpenCodePluginAppliesPublishedEnvironmentOnlyWhileProxyIsAlive } home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, OpenCodePluginName+".js") helper := filepath.Join(home, "phase-hook-v1") kickLog := filepath.Join(home, "kicks.log") @@ -258,6 +261,7 @@ check(Object.keys(missing.env).length === 1 && missing.env.ONLY === "1", "missin func TestUninstallOpenCodePluginPreservesForeignFile(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") if err := os.MkdirAll(filepath.Dir(pluginPath), 0o700); err != nil { t.Fatal(err) @@ -276,6 +280,7 @@ func TestUninstallOpenCodePluginPreservesForeignFile(t *testing.T) { func TestUninstallOpenCodePluginRemovesOwnedFiles(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") executable := filepath.Join(home, "phase") if err := os.WriteFile(executable, []byte("phase"), 0o700); err != nil { @@ -306,6 +311,7 @@ func TestInstalledOpenCodePluginExportsOneDefaultDefinition(t *testing.T) { } home := t.TempDir() t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) pluginPath := filepath.Join(home, ".config", "opencode", "plugins", OpenCodePluginName+".js") if _, err := InstallOpenCodePlugin(pluginPath, filepath.Join(home, "phase-hook-v1"), filepath.Join(home, ".phase", "ai", "env.json")); err != nil { t.Fatal(err) diff --git a/src/pkg/ai/windows_command.go b/src/pkg/ai/windows_command.go new file mode 100644 index 00000000..1f7f06ae --- /dev/null +++ b/src/pkg/ai/windows_command.go @@ -0,0 +1,77 @@ +package ai + +import "strings" + +// shortPathName resolves a path's 8.3 short name; tests replace it. +var shortPathName = windowsShortPathName + +// WindowsShellCommand renders the WindowsPlainCommand form, or PowerShell's +// call-operator form when there is none. +func WindowsShellCommand(arguments []string) string { + if line, ok := WindowsPlainCommand(arguments); ok { + return line + } + return PowerShellCallCommand(arguments) +} + +// WindowsPlainCommand renders the one form PowerShell, cmd and Git Bash all +// run: an unquoted forward-slash path, or its 8.3 name, and plain words. It +// reports false when a word still needs quoting. +func WindowsPlainCommand(arguments []string) (string, bool) { + words := append([]string{strings.ReplaceAll(arguments[0], `\`, "/")}, arguments[1:]...) + if !windowsShellPlainWord(words[0]) { + if short, err := shortPathName(arguments[0]); err == nil && windowsShellPlainWord(strings.ReplaceAll(short, `\`, "/")) { + words[0] = strings.ReplaceAll(short, `\`, "/") + } + } + for _, word := range words { + if !windowsShellPlainWord(word) { + return "", false + } + } + return strings.Join(words, " "), true +} + +// PowerShellCallCommand renders `& 'exe' 'arg' ...`. PowerShell treats the +// typographic single quotes as quote characters too, so each is doubled. +func PowerShellCallCommand(arguments []string) string { + quoted := make([]string, len(arguments)) + for index, argument := range arguments { + quoted[index] = "'" + powerShellQuoteEscape(argument) + "'" + } + return "& " + strings.Join(quoted, " ") +} + +func powerShellQuoteEscape(value string) string { + var builder strings.Builder + for _, character := range value { + if character == '\'' || character == '\u2018' || character == '\u2019' || character == '\u201A' || character == '\u201B' { + builder.WriteRune(character) + } + builder.WriteRune(character) + } + return builder.String() +} + +// windowsExecutableForms lists the spellings of executable in a rendered command. +func windowsExecutableForms(executable string) []string { + forms := []string{executable, strings.ReplaceAll(executable, `\`, "/"), powerShellQuoteEscape(executable)} + if short, err := shortPathName(executable); err == nil { + forms = append(forms, strings.ReplaceAll(short, `\`, "/")) + } + return forms +} + +// windowsShellPlainWord reports a word every shell passes through unchanged. +func windowsShellPlainWord(word string) bool { + if word == "" || word[0] == '~' { + return false + } + for _, character := range word { + if (character < 'A' || character > 'Z') && (character < 'a' || character > 'z') && + (character < '0' || character > '9') && !strings.ContainsRune(":/._-~", character) { + return false + } + } + return true +} diff --git a/src/pkg/ai/windows_command_other.go b/src/pkg/ai/windows_command_other.go new file mode 100644 index 00000000..dc4948a4 --- /dev/null +++ b/src/pkg/ai/windows_command_other.go @@ -0,0 +1,10 @@ +//go:build !windows + +package ai + +import "errors" + +// windowsShortPathName exists only on Windows. +func windowsShortPathName(string) (string, error) { + return "", errors.New("short path names exist only on Windows") +} diff --git a/src/pkg/ai/windows_command_test.go b/src/pkg/ai/windows_command_test.go new file mode 100644 index 00000000..ce83012f --- /dev/null +++ b/src/pkg/ai/windows_command_test.go @@ -0,0 +1,80 @@ +package ai + +import ( + "encoding/json" + "errors" + "strings" + "testing" +) + +func withShortPathName(t *testing.T, resolve func(string) (string, error)) { + t.Helper() + original := shortPathName + shortPathName = resolve + t.Cleanup(func() { shortPathName = original }) +} + +func TestWindowsShellCommandRunsEverywhereOrFallsBack(t *testing.T) { + arguments := []string{`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, "ai", "__codex-event", "guard"} + withShortPathName(t, func(string) (string, error) { return "", errors.New("not needed") }) + if got, want := WindowsShellCommand(arguments), "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __codex-event guard"; got != want { + t.Fatalf("plain path command = %q, want %q", got, want) + } + spaced := append([]string{`C:\Users\First Last\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) + withShortPathName(t, func(path string) (string, error) { + if path != spaced[0] { + t.Fatalf("short name requested for %q", path) + } + return `C:\Users\FIRSTL~1\.phase\ai\bin\PHASE-~1.EXE`, nil + }) + if got, want := WindowsShellCommand(spaced), "C:/Users/FIRSTL~1/.phase/ai/bin/PHASE-~1.EXE ai __codex-event guard"; got != want { + t.Fatalf("spaced path command = %q, want %q", got, want) + } + withShortPathName(t, func(string) (string, error) { return "", errors.New("8.3 names are disabled") }) + quoted := append([]string{`C:\Users\O'Brien Smith\.phase\ai\bin\phase-hook-v1.exe`}, arguments[1:]...) + if got, want := WindowsShellCommand(quoted), `& 'C:\Users\O''Brien Smith\.phase\ai\bin\phase-hook-v1.exe' 'ai' '__codex-event' 'guard'`; got != want { + t.Fatalf("fallback command = %q, want %q", got, want) + } + if line, ok := WindowsPlainCommand(quoted); ok { + t.Fatalf("a path that still needs quoting has no plain form, got %q", line) + } + typographic := []string{"C:\\Users\\O\u2019Brien\\phase-hook-v1.exe", "ai"} + if got, want := WindowsShellCommand(typographic), "& 'C:\\Users\\O\u2019\u2019Brien\\phase-hook-v1.exe' 'ai'"; got != want { + t.Fatalf("typographic fallback = %q, want %q", got, want) + } +} + +func TestCodexWindowsHookCommandsAvoidALeadingQuotedPath(t *testing.T) { + withShortPathName(t, func(string) (string, error) { return "", errors.New("not needed") }) + command := codexPluginCommand(`C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe`, true, "ai", "__codex-event", "guard") + if strings.HasPrefix(command, `"`) || command != "C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __codex-event guard" { + t.Fatalf("Codex commandWindows = %q", command) + } +} + +func TestDevinRecognizesEveryWindowsSpellingOfTheHelper(t *testing.T) { + withShortPathName(t, func(string) (string, error) { return "", errors.New("8.3 names are disabled") }) + helper := `C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe` + group := func(command string) json.RawMessage { + payload, err := json.Marshal(devinHookGroup{Matcher: "exec", Hooks: []devinHookHandler{{Type: "command", Command: command, Timeout: 45}}}) + if err != nil { + t.Fatal(err) + } + return payload + } + hooks := map[string][]json.RawMessage{"PreToolUse": { + group(`"C:\Users\dev\.phase\ai\bin\phase-hook-v1.exe" "ai" "__devin-event" "guard"`), + group("C:/Users/dev/.phase/ai/bin/phase-hook-v1.exe ai __devin-event guard"), + group("node C:/Users/dev/hooks/audit.js"), + }} + removePhaseDevinGroups(hooks, windowsExecutableForms(helper)...) + if len(hooks["PreToolUse"]) != 1 || !strings.Contains(string(hooks["PreToolUse"][0]), "audit.js") { + t.Fatalf("Phase groups from earlier and current builds must both be removed, and only them: %s", hooks["PreToolUse"]) + } + quotedHelper := `C:\Users\O'Brien Smith\.phase\ai\bin\phase-hook-v1.exe` + hooks = map[string][]json.RawMessage{"PreToolUse": {group(WindowsShellCommand([]string{quotedHelper, "ai", "__devin-event", "guard"}))}} + removePhaseDevinGroups(hooks, windowsExecutableForms(quotedHelper)...) + if len(hooks["PreToolUse"]) != 0 { + t.Fatalf("the PowerShell fallback spelling was not recognized: %s", hooks["PreToolUse"]) + } +} diff --git a/src/pkg/ai/windows_command_windows.go b/src/pkg/ai/windows_command_windows.go new file mode 100644 index 00000000..a39ff497 --- /dev/null +++ b/src/pkg/ai/windows_command_windows.go @@ -0,0 +1,24 @@ +//go:build windows + +package ai + +import "syscall" + +// windowsShortPathName returns the 8.3 short form of an existing path. +func windowsShortPathName(path string) (string, error) { + long, err := syscall.UTF16PtrFromString(path) + if err != nil { + return "", err + } + buffer := make([]uint16, syscall.MAX_PATH) + for { + length, err := syscall.GetShortPathName(long, &buffer[0], uint32(len(buffer))) + if err != nil { + return "", err + } + if length < uint32(len(buffer)) { + return syscall.UTF16ToString(buffer[:length]), nil + } + buffer = make([]uint16, length) + } +} diff --git a/src/pkg/ai/windows_command_windows_test.go b/src/pkg/ai/windows_command_windows_test.go new file mode 100644 index 00000000..2468fbb7 --- /dev/null +++ b/src/pkg/ai/windows_command_windows_test.go @@ -0,0 +1,69 @@ +//go:build windows + +package ai + +import ( + "os" + "os/exec" + "path/filepath" + "strings" + "syscall" + "testing" +) + +// windowsTestShells runs one exact command line in each installed shell. +func windowsTestShells() map[string]func(string) (string, error) { + shells := map[string]func(string) (string, error){ + "cmd": func(line string) (string, error) { + command := exec.Command(filepath.Join(os.Getenv("SystemRoot"), "System32", "cmd.exe")) + command.SysProcAttr = &syscall.SysProcAttr{CmdLine: `cmd.exe /d /s /c "` + line + `"`} + output, err := command.CombinedOutput() + return string(output), err + }, + } + for _, name := range []string{"powershell", "pwsh"} { + if path, err := exec.LookPath(name); err == nil { + shells[name] = func(line string) (string, error) { + output, err := exec.Command(path, "-NoLogo", "-NoProfile", "-NonInteractive", "-Command", line).CombinedOutput() + return string(output), err + } + } + } + if bash := filepath.Join(os.Getenv("ProgramFiles"), "Git", "bin", "bash.exe"); func() bool { _, err := os.Stat(bash); return err == nil }() { + shells["git-bash"] = func(line string) (string, error) { + output, err := exec.Command(bash, "-c", line).CombinedOutput() + return string(output), err + } + } + return shells +} + +// curl.exe stands in for the hook executable, from a plain and a spaced path. +func TestWindowsShellCommandRunsInEveryWindowsShell(t *testing.T) { + system := filepath.Join(os.Getenv("SystemRoot"), "System32", "curl.exe") + payload, err := os.ReadFile(system) + if err != nil { + t.Skip("curl.exe is not installed") + } + directory := filepath.Join(t.TempDir(), "with space") + if err := os.MkdirAll(directory, 0o700); err != nil { + t.Fatal(err) + } + spaced := filepath.Join(directory, "curl.exe") + if err := os.WriteFile(spaced, payload, 0o700); err != nil { + t.Fatal(err) + } + shells := windowsTestShells() + for _, executable := range []string{system, spaced} { + line := WindowsShellCommand([]string{executable, "--version"}) + for name, run := range shells { + if strings.HasPrefix(line, "& ") && name != "powershell" && name != "pwsh" { + continue + } + output, err := run(line) + if err != nil || !strings.HasPrefix(strings.TrimSpace(output), "curl ") { + t.Errorf("%s could not run %q: %v\n%s", name, line, err, output) + } + } + } +} diff --git a/src/pkg/proxy/environment.go b/src/pkg/proxy/environment.go index 790b4cfd..dc407ae3 100644 --- a/src/pkg/proxy/environment.go +++ b/src/pkg/proxy/environment.go @@ -650,11 +650,12 @@ func CompileProcessEnvironment(options EnvironmentOptions) (ProcessEnvironmentPl // ApplyProcessEnvironment applies a compiled Phase-owned delta to an inherited // process environment without modifying the parent process or any files. +// Only the keys Phase sets or unsets must be shell identifiers. func ApplyProcessEnvironment(base []string, plan ProcessEnvironmentPlan) ([]string, error) { values := make(map[string]string) for _, entry := range base { key, value, ok := strings.Cut(entry, "=") - if !ok || !ValidShellEnvironmentKey(key) || containsEnvironmentKey(plan.Unset, key) { + if !ok || !validInheritedEnvironmentKey(key) || containsEnvironmentKey(plan.Unset, key) { continue } if _, phaseOwned := planValue(plan.Set, key); phaseOwned { @@ -682,6 +683,12 @@ func ApplyProcessEnvironment(base []string, plan ProcessEnvironmentPlan) ([]stri return environment, nil } +// validInheritedEnvironmentKey accepts names such as ProgramFiles(x86), but not +// the empty names Windows' hidden "=C:" entries split into. +func validInheritedEnvironmentKey(key string) bool { + return key != "" && !strings.ContainsAny(key, "=\x00") +} + // ValidShellEnvironmentKey reports whether a key can be emitted as a POSIX // shell assignment without interpreting any part of it as shell syntax. func ValidShellEnvironmentKey(key string) bool { diff --git a/src/pkg/proxy/store_test.go b/src/pkg/proxy/store_test.go index e3bad272..e001eccc 100644 --- a/src/pkg/proxy/store_test.go +++ b/src/pkg/proxy/store_test.go @@ -414,6 +414,31 @@ func TestCompileProcessEnvironmentUsesExplicitCleanupAndSanitizesNoProxy(t *test } } +func TestApplyProcessEnvironmentKeepsInheritedNamesThatAreNotShellIdentifiers(t *testing.T) { + environment, err := ApplyProcessEnvironment([]string{ + `ProgramFiles(x86)=C:\Program Files (x86)`, + `CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files`, + "BASH_FUNC_module%%=() { :\n}", + `=C:=C:\work`, + "AWS_PROFILE=ambient-admin", + "AWS_ACCESS_KEY_ID=ambient", + }, ProcessEnvironmentPlan{Set: map[string]string{"AWS_ACCESS_KEY_ID": "decoy"}, Unset: []string{"AWS_PROFILE"}}) + if err != nil { + t.Fatal(err) + } + joined := "\n" + strings.Join(environment, "\n") + "\n" + for _, entry := range []string{`ProgramFiles(x86)=C:\Program Files (x86)`, `CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files`, "BASH_FUNC_module%%=() { :\n}", "AWS_ACCESS_KEY_ID=decoy"} { + if !strings.Contains(joined, "\n"+entry+"\n") { + t.Fatalf("missing %q in %q", entry, environment) + } + } + for _, entry := range environment { + if strings.HasPrefix(entry, "=") || strings.HasPrefix(entry, "AWS_PROFILE=") || entry == "AWS_ACCESS_KEY_ID=ambient" { + t.Fatalf("hidden drive entry, unset or overridden value survived: %q", environment) + } + } +} + func TestCompileProcessEnvironmentRejectsBindingAndAdapterCollisions(t *testing.T) { base := EnvironmentOptions{ ProxyURL: "http://phase:capability@127.0.0.1:9543", CABundlePath: "/bundle.pem",