fix(doctor): address review — rename shadowing var, align missing-binary message

- Rename the local `runtime` (was shadowing the stdlib `runtime` package used
  elsewhere in doctor.go) to `runtimeVersion`.
- Align the unresolvable-binary FAIL message's remediation with the npm wrapper
  (reinstall, or build from source: <repo url>) so both missing-binary paths
  give consistent guidance.

Addresses CodeRabbit review on #405.

README.md

@@ -51,6 +51,27 @@ The npm package installs a small wrapper plus the matching Zero binary for your
 platform from GitHub Releases. It supports Linux, macOS, and Windows on x64 and
 arm64.
 
+### Bun
+
+Bun does not run dependency lifecycle scripts by default, so the `postinstall`
+that fetches the Zero binary is skipped and the first run fails with
+`No native binary found next to the npm wrapper`.
+
+Install with Bun, then fetch the binary in one of two ways:
+
+```bash
+# Option A: run the installer manually
+bun add @gitlawb/zero
+node node_modules/@gitlawb/zero/scripts/postinstall.mjs
+
+# Option B: allow the postinstall to run on install
+# add to your package.json:  "trustedDependencies": ["@gitlawb/zero"]
+bun add @gitlawb/zero
+```
+
+For global installs (`bun add -g @gitlawb/zero`), use Option A against the
+global install path, or prefer the install scripts below.
+
 ### Install scripts
 
 Linux/macOS:

bin/zero.js

@@ -54,8 +54,26 @@ const nativePath = join(packageRoot, zeroBinaryName());
 const localControlHelpers = localControlHelperManifest(packageRoot);
 
 if (!existsSync(nativePath)) {
+  const postinstallScript = join(packageRoot, 'scripts', 'postinstall.mjs');
+  const ranByBun = process.execPath.includes('bun') || !!process.versions?.bun;
   console.error(
-    '[zero] No native binary found next to the npm wrapper. Reinstall the zero package or run `go run ./cmd/zero-release build` from the repository.'
+    '[zero] No native binary found next to the npm wrapper.\n' +
+      'The platform binary is fetched at install time by a postinstall script,\n' +
+      'which did not run (or was skipped) for this install.\n' +
+      '\n' +
+      'Fix it now by running the installer manually:\n' +
+      `  node "${postinstallScript}"\n` +
+      '\n' +
+      (ranByBun
+        ? 'You installed with Bun, which does not run dependency lifecycle scripts\n' +
+          'by default. To let it run on future installs, add zero to your\n' +
+          'project\'s trustedDependencies:\n' +
+          '  "trustedDependencies": ["@gitlawb/zero"]\n' +
+          'then reinstall.\n' +
+          '\n'
+        : '') +
+      'If that fails, build from source: https://github.com/Gitlawb/zero\n' +
+      '(go run ./cmd/zero, requires Go 1.25+).',
   );
   process.exit(1);
 }

docs/INSTALL.md

@@ -24,6 +24,38 @@ Requirements:
 - Node.js 18+
 - network access to npm and GitHub Releases
 
+## Bun
+
+Bun is "default-secure" and does not run lifecycle scripts of installed
+dependencies (only the installing project's own scripts), so the `postinstall`
+that fetches the Zero binary is silently skipped. The first run then fails with
+`No native binary found next to the npm wrapper`.
+
+To install with Bun, either run the installer manually after installing:
+
+```bash
+bun add @gitlawb/zero
+node node_modules/@gitlawb/zero/scripts/postinstall.mjs
+```
+
+Or allow the postinstall to run by adding the package to your project's
+`trustedDependencies` before installing:
+
+```json
+{
+  "trustedDependencies": ["@gitlawb/zero"]
+}
+```
+
+```bash
+bun add @gitlawb/zero
+```
+
+For global installs (`bun add -g @gitlawb/zero`), run the installer manually
+against the global install path, or use the install scripts below.
+
+Reference: <https://bun.sh/docs/pm/lifecycle>
+
 ## Linux And macOS Script
 
 Install the latest release:

internal/agent/loop.go

@@ -2159,6 +2159,18 @@ func buildPermissionEvent(call ToolCall, tool tools.Tool, args map[string]any, p
 		}
 	}
 
+	// A command that explicitly requests additional sandbox permissions
+	// (sandbox_permissions: with_additional_permissions) is an ELEVATION the user
+	// must consent to, even when the base command's sandbox decision was Allow.
+	// shouldRequestPermission blocks on OnPermissionRequest for exactly this case,
+	// so the event MUST be a prompt — otherwise it carried Action=allow while the
+	// loop waited on a decision, and a UI that renders only prompts (the TUI)
+	// dropped it, deadlocking the run. Keep this consistent with
+	// shouldRequestPermission's own condition.
+	if action == PermissionActionAllow && !permissionGranted && shellCommandAdditionalPermissionsRequested(args) {
+		action = PermissionActionPrompt
+	}
+
 	if safety.Permission == tools.PermissionAllow && action == PermissionActionAllow && !grantMatched && block == nil {
 		return PermissionEvent{}, false
 	}

internal/agent/permission_additional_perms_test.go

@@ -0,0 +1,70 @@
+package agent
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Gitlawb/zero/internal/sandbox"
+	"github.com/Gitlawb/zero/internal/tools"
+)
+
+// promptShellTool is a minimal shell-style tool whose Safety requires a prompt,
+// standing in for bash so buildPermissionEvent can be exercised directly.
+type promptShellTool struct{}
+
+func (promptShellTool) Name() string        { return "bash" }
+func (promptShellTool) Description() string { return "run a shell command" }
+func (promptShellTool) Parameters() tools.Schema {
+	return tools.Schema{Type: "object", AdditionalProperties: false}
+}
+func (promptShellTool) Safety() tools.Safety {
+	return tools.Safety{SideEffect: tools.SideEffectShell, Permission: tools.PermissionPrompt, Reason: "runs a shell command"}
+}
+func (promptShellTool) Run(context.Context, map[string]any) tools.Result {
+	return tools.Result{Status: tools.StatusOK}
+}
+
+// A command whose sandbox decision is Allow but that explicitly requests
+// additional sandbox permissions is an ELEVATION: it must be surfaced as a
+// prompt, never Action=allow. Previously it carried allow while the loop still
+// blocked on OnPermissionRequest, and the TUI (which renders only prompts)
+// dropped it → the run deadlocked forever. This is the exact gpt-5.5
+// folder-creation shape captured in the reported hang.
+func TestBuildPermissionEventPromptsForAdditionalPermissions(t *testing.T) {
+	call := ToolCall{ID: "call_1", Name: "bash", Arguments: `{}`}
+	args := map[string]any{
+		"command":             "mkdir foo && cd foo && go mod init foo",
+		"sandbox_permissions": string(tools.SandboxPermissionsWithAdditionalPermissions),
+	}
+	// Base command allowed by the sandbox…
+	decision := &sandbox.Decision{Action: sandbox.ActionAllow}
+
+	event, ok := buildPermissionEvent(call, promptShellTool{}, args, false, PermissionModeAsk, Options{}, decision)
+	if !ok {
+		t.Fatal("buildPermissionEvent returned ok=false; the elevation request must produce an event")
+	}
+	if event.Action != PermissionActionPrompt {
+		t.Fatalf("Action = %q, want prompt — an additional-permissions elevation must ask the user, not auto-allow", event.Action)
+	}
+	// Sanity: shouldRequestPermission agrees the loop blocks on this.
+	if !shouldRequestPermission(promptShellTool{}, args, false, decision) {
+		t.Fatal("shouldRequestPermission must be true for an additional-permissions request")
+	}
+}
+
+// Without the additional-permissions flag, a sandbox-allowed command is NOT a
+// prompt (shouldRequestPermission is false; it just runs) — the override must
+// not over-reach and prompt for ordinary allowed commands.
+func TestBuildPermissionEventKeepsAllowForOrdinaryAllowedCommand(t *testing.T) {
+	call := ToolCall{ID: "call_2", Name: "bash", Arguments: `{}`}
+	args := map[string]any{"command": "ls"}
+	decision := &sandbox.Decision{Action: sandbox.ActionAllow}
+
+	event, _ := buildPermissionEvent(call, promptShellTool{}, args, false, PermissionModeAsk, Options{}, decision)
+	if event.Action == PermissionActionPrompt {
+		t.Fatalf("Action = prompt for an ordinary allowed command; want allow (no spurious prompt)")
+	}
+	if shouldRequestPermission(promptShellTool{}, args, false, decision) {
+		t.Fatal("shouldRequestPermission must be false for an ordinary sandbox-allowed command")
+	}
+}

internal/cli/observability.go

@@ -4,6 +4,7 @@ import (
 	"encoding/json"
 	"fmt"
 	"io"
+	"runtime"
 	"strconv"
 	"strings"
 
@@ -65,7 +66,7 @@ func runDoctor(args []string, stdout io.Writer, stderr io.Writer, deps appDeps)
 
 	report := doctor.Run(doctor.Options{
 		Now:            deps.now,
-		Runtime:        "go",
+		Runtime:        runtime.Version(),
 		UserConfig:     userConfig,
 		ProjectConfig:  projectConfig,
 		Provider:       provider,

internal/doctor/doctor.go

@@ -57,6 +57,11 @@ type Options struct {
 	// LSP-server checks. Nil means exec.LookPath; tests inject a stub so the
 	// checks are deterministic regardless of the host's installed tooling.
 	LookupExecutable func(string) (string, error)
+	// Executable resolves the running Zero binary's path (nil => os.Executable)
+	// and StatExecutable stats it (nil => os.Stat). Injected so the native-binary
+	// runtime check is testable without depending on a real on-disk layout.
+	Executable     func() (string, error)
+	StatExecutable func(string) (os.FileInfo, error)
 }
 
 func Run(options Options) Report {
@@ -65,7 +70,7 @@ func Run(options Options) Report {
 		now = time.Now
 	}
 	checks := []Check{
-		runtimeCheck(options.Runtime),
+		runtimeCheck(options),
 		configFilesCheck(options.UserConfig, options.ProjectConfig),
 		configValidationCheck(options.UserConfig, options.ProjectConfig),
 	}
@@ -114,12 +119,48 @@ func Format(report Report) string {
 	return strings.Join(lines, "\n")
 }
 
-func runtimeCheck(runtime string) Check {
-	runtime = strings.TrimSpace(runtime)
-	if runtime == "" {
-		runtime = "go"
+// runtimeCheck verifies the running Zero native binary is actually present and
+// runnable, and reports its resolved path — instead of the former hardcoded
+// always-pass, which claimed "Go runtime is available" while checking nothing
+// (#405). doctor executes INSIDE the native binary, so this only fails if
+// os.Executable can't resolve or the running file has vanished; the missing-
+// binary-at-install case is caught earlier by the npm wrapper, which refuses to
+// launch without the platform binary. Surfacing the binary PATH is the useful
+// part: it lets a user see which binary answered (e.g. an npm-installed one vs a
+// stale/shadowing copy on PATH) rather than a meaningless green.
+func runtimeCheck(options Options) Check {
+	runtimeVersion := strings.TrimSpace(options.Runtime)
+	if runtimeVersion == "" {
+		runtimeVersion = "go"
 	}
-	return check("runtime.go", "Go runtime", StatusPass, fmt.Sprintf("Zero Go runtime is available (%s).", runtime), map[string]any{"runtime": runtime})
+	details := map[string]any{"runtime": runtimeVersion}
+
+	executable := options.Executable
+	if executable == nil {
+		executable = os.Executable
+	}
+	path, err := executable()
+	if err != nil {
+		return check("runtime.go", "Zero native binary", StatusFail,
+			"Could not resolve the running Zero binary: "+err.Error(), details)
+	}
+	details["binaryPath"] = path
+
+	stat := options.StatExecutable
+	if stat == nil {
+		stat = os.Stat
+	}
+	info, err := stat(path)
+	if err != nil {
+		return check("runtime.go", "Zero native binary", StatusFail,
+			fmt.Sprintf("The running Zero binary is not accessible at %s: %s. Reinstall the zero package, or build from source: https://github.com/Gitlawb/zero", path, err.Error()), details)
+	}
+	if info.IsDir() {
+		return check("runtime.go", "Zero native binary", StatusFail,
+			fmt.Sprintf("The resolved Zero binary path %s is a directory, not an executable.", path), details)
+	}
+	return check("runtime.go", "Zero native binary", StatusPass,
+		fmt.Sprintf("Zero native binary is present and runnable at %s (%s).", path, runtimeVersion), details)
 }
 
 func configFilesCheck(userPath string, projectPath string) Check {

internal/doctor/runtime_check_test.go

@@ -0,0 +1,88 @@
+package doctor
+
+import (
+	"errors"
+	"io/fs"
+	"os"
+	"strings"
+	"testing"
+	"time"
+)
+
+// fakeFileInfo is a minimal os.FileInfo for the runtime-check stat injection.
+type fakeFileInfo struct{ dir bool }
+
+func (f fakeFileInfo) Name() string       { return "zero" }
+func (f fakeFileInfo) Size() int64        { return 1 }
+func (f fakeFileInfo) Mode() fs.FileMode  { return 0o755 }
+func (f fakeFileInfo) ModTime() time.Time { return time.Unix(0, 0) }
+func (f fakeFileInfo) IsDir() bool        { return f.dir }
+func (f fakeFileInfo) Sys() any           { return nil }
+
+func runtimeCheckFor(t *testing.T, exe func() (string, error), stat func(string) (os.FileInfo, error)) Check {
+	t.Helper()
+	return runtimeCheck(Options{Runtime: "go1.26", Executable: exe, StatExecutable: stat})
+}
+
+// A present, runnable binary passes AND reports its resolved path — the check is
+// no longer a hardcoded green; it names which binary answered.
+func TestRuntimeCheckPassesAndReportsBinaryPath(t *testing.T) {
+	c := runtimeCheckFor(t,
+		func() (string, error) { return "/opt/homebrew/lib/node_modules/@gitlawb/zero/zero", nil },
+		func(string) (os.FileInfo, error) { return fakeFileInfo{}, nil })
+	if c.Status != StatusPass {
+		t.Fatalf("status = %s, want pass", c.Status)
+	}
+	if c.Details["binaryPath"] != "/opt/homebrew/lib/node_modules/@gitlawb/zero/zero" {
+		t.Fatalf("binaryPath = %v, want the resolved path in details", c.Details["binaryPath"])
+	}
+	if !strings.Contains(c.Message, "/opt/homebrew/lib/node_modules/@gitlawb/zero/zero") {
+		t.Fatalf("message should surface the binary path, got %q", c.Message)
+	}
+}
+
+// The vanished-binary case FAILS loudly instead of the old unconditional pass —
+// this is the #405 regression: a broken runtime no longer gets a green check.
+func TestRuntimeCheckFailsWhenBinaryMissing(t *testing.T) {
+	c := runtimeCheckFor(t,
+		func() (string, error) { return "/path/to/zero", nil },
+		func(string) (os.FileInfo, error) { return nil, os.ErrNotExist })
+	if c.Status != StatusFail {
+		t.Fatalf("#405: a missing/inaccessible binary must FAIL, got %s (%q)", c.Status, c.Message)
+	}
+	if !strings.Contains(c.Message, "/path/to/zero") {
+		t.Fatalf("failure message should name the path, got %q", c.Message)
+	}
+}
+
+// os.Executable resolution failure fails loudly too.
+func TestRuntimeCheckFailsWhenExecutableUnresolvable(t *testing.T) {
+	c := runtimeCheckFor(t,
+		func() (string, error) { return "", errors.New("no executable path") },
+		func(string) (os.FileInfo, error) { return fakeFileInfo{}, nil })
+	if c.Status != StatusFail {
+		t.Fatalf("unresolvable executable must FAIL, got %s", c.Status)
+	}
+}
+
+// A path that resolves to a directory is not a runnable binary → FAIL.
+func TestRuntimeCheckFailsWhenPathIsDirectory(t *testing.T) {
+	c := runtimeCheckFor(t,
+		func() (string, error) { return "/some/dir", nil },
+		func(string) (os.FileInfo, error) { return fakeFileInfo{dir: true}, nil })
+	if c.Status != StatusFail {
+		t.Fatalf("a directory path must FAIL, got %s", c.Status)
+	}
+}
+
+// Default injection (nil Executable/StatExecutable) resolves the real running
+// test binary — proving the production default path works and passes.
+func TestRuntimeCheckDefaultsToRealBinary(t *testing.T) {
+	c := runtimeCheck(Options{})
+	if c.Status != StatusPass {
+		t.Fatalf("default runtime check should pass for the running test binary, got %s (%q)", c.Status, c.Message)
+	}
+	if strings.TrimSpace(c.Details["binaryPath"].(string)) == "" {
+		t.Fatal("default runtime check should report the running binary path")
+	}
+}

internal/mcp/client_test.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -211,6 +212,85 @@ func TestHTTPClientListsAndCallsTools(t *testing.T) {
 	}
 }
 
+func TestHTTPClientFollowsSameOriginRedirect(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	testServer := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		if request.URL.Path == "/mcp" {
+			http.Redirect(response, request, "/redirected", http.StatusTemporaryRedirect)
+			return
+		}
+		if request.Method != http.MethodPost || request.URL.Path != "/redirected" {
+			t.Errorf("request = %s %s, want POST /redirected", request.Method, request.URL.Path)
+			http.Error(response, "bad request", http.StatusNotFound)
+			return
+		}
+		if got := request.Header.Get("X-Api-Key"); got != "secret" {
+			t.Errorf("X-Api-Key = %q, want secret", got)
+			http.Error(response, "missing auth", http.StatusUnauthorized)
+			return
+		}
+
+		message := readHTTPRPCMessage(t, request)
+		switch message.Method {
+		case "initialize":
+			writeHTTPRPCResponse(t, response, message.ID, map[string]any{"protocolVersion": "2024-11-05"})
+		case "notifications/initialized":
+			response.WriteHeader(http.StatusAccepted)
+		default:
+			writeHTTPRPCResponse(t, response, message.ID, map[string]any{})
+		}
+	}))
+	defer testServer.Close()
+
+	client, err := Connect(ctx, Server{
+		Name:    "docs",
+		Type:    ServerTypeHTTP,
+		URL:     testServer.URL + "/mcp",
+		Headers: map[string]string{"X-Api-Key": "secret"},
+	})
+	if err != nil {
+		t.Fatalf("Connect() error = %v", err)
+	}
+	if err := client.Close(); err != nil {
+		t.Fatalf("Close() error = %v", err)
+	}
+}
+
+func TestHTTPClientRejectsCrossOriginRedirectBeforeSendingHeaders(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	var targetHits int32
+	target := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		atomic.AddInt32(&targetHits, 1)
+		if got := request.Header.Get("X-Api-Key"); got != "" {
+			t.Errorf("redirect target received X-Api-Key = %q", got)
+		}
+		response.WriteHeader(http.StatusOK)
+	}))
+	defer target.Close()
+
+	redirector := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		http.Redirect(response, request, target.URL+"/mcp", http.StatusTemporaryRedirect)
+	}))
+	defer redirector.Close()
+
+	_, err := Connect(ctx, Server{
+		Name:    "docs",
+		Type:    ServerTypeHTTP,
+		URL:     redirector.URL + "/mcp",
+		Headers: map[string]string{"X-Api-Key": "secret"},
+	})
+	if err == nil || !strings.Contains(err.Error(), "cross-origin redirect") {
+		t.Fatalf("Connect() error = %v, want cross-origin redirect error", err)
+	}
+	if got := atomic.LoadInt32(&targetHits); got != 0 {
+		t.Fatalf("redirect target hits = %d, want 0", got)
+	}
+}
+
 func TestSSEClientListsAndCallsToolsFromRemoteStream(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
@@ -340,6 +420,53 @@ func TestSSEClientListsAndCallsToolsFromRemoteStream(t *testing.T) {
 	}
 }
 
+func TestSSEClientRejectsCrossOriginEndpointBeforeSendingHeaders(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+
+	var targetHits int32
+	target := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		atomic.AddInt32(&targetHits, 1)
+		if got := request.Header.Get("X-Api-Key"); got != "" {
+			t.Errorf("SSE endpoint target received X-Api-Key = %q", got)
+		}
+		response.WriteHeader(http.StatusAccepted)
+	}))
+	defer target.Close()
+
+	stream := httptest.NewServer(http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+		if got := request.Header.Get("X-Api-Key"); got != "secret" {
+			t.Errorf("X-Api-Key = %q, want secret on configured SSE server", got)
+			http.Error(response, "missing auth", http.StatusUnauthorized)
+			return
+		}
+		if request.Method != http.MethodGet || request.URL.Path != "/sse" {
+			t.Errorf("request = %s %s, want GET /sse", request.Method, request.URL.Path)
+			http.Error(response, "bad request", http.StatusNotFound)
+			return
+		}
+		response.Header().Set("Content-Type", "text/event-stream")
+		_, _ = fmt.Fprintf(response, "event: endpoint\ndata: %s/messages\n\n", target.URL)
+		if flusher, ok := response.(http.Flusher); ok {
+			flusher.Flush()
+		}
+	}))
+	defer stream.Close()
+
+	_, err := Connect(ctx, Server{
+		Name:    "docs",
+		Type:    ServerTypeSSE,
+		URL:     stream.URL + "/sse",
+		Headers: map[string]string{"X-Api-Key": "secret"},
+	})
+	if err == nil || !strings.Contains(err.Error(), "endpoint origin") {
+		t.Fatalf("Connect() error = %v, want cross-origin endpoint error", err)
+	}
+	if got := atomic.LoadInt32(&targetHits); got != 0 {
+		t.Fatalf("SSE endpoint target hits = %d, want 0", got)
+	}
+}
+
 func TestHTTPClientReportsNonOKStatus(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()

internal/mcp/network_client.go

@@ -733,7 +733,11 @@ func resolveSSEEndpointURL(baseValue string, endpointValue string) (string, erro
 	if err != nil {
 		return "", fmt.Errorf("parse MCP SSE endpoint URL: %w", err)
 	}
-	return baseURL.ResolveReference(endpointURL).String(), nil
+	resolvedURL := baseURL.ResolveReference(endpointURL)
+	if !sameMCPOrigin(baseURL, resolvedURL) {
+		return "", fmt.Errorf("MCP SSE endpoint origin %s differs from configured server origin %s", mcpOrigin(resolvedURL), mcpOrigin(baseURL))
+	}
+	return resolvedURL.String(), nil
 }
 
 func rpcResponseKey(id any) string {
@@ -873,10 +877,10 @@ func (source *storeTokenSource) Refresh(ctx context.Context) (string, error) {
 
 // oauthHTTPClient returns an HTTP client whose transport attaches OAuth bearer
 // tokens and refreshes them on 401 for OAuth-configured servers. Servers that do
-// not declare OAuth get the default client unchanged.
+// not declare OAuth use the default transport with the same MCP redirect guard.
 func oauthHTTPClient(server Server) (*http.Client, error) {
 	if !strings.EqualFold(strings.TrimSpace(server.Auth), ServerAuthOAuth) {
-		return http.DefaultClient, nil
+		return mcpHTTPClient(server, nil), nil
 	}
 	store, err := NewTokenStore(TokenStoreOptions{})
 	if err != nil {
@@ -888,5 +892,5 @@ func oauthHTTPClient(server Server) (*http.Client, error) {
 		httpClient: http.DefaultClient,
 		now:        time.Now,
 	}
-	return &http.Client{Transport: newOAuthRoundTripper(http.DefaultTransport, source, server.Name)}, nil
+	return mcpHTTPClient(server, newOAuthRoundTripper(http.DefaultTransport, source, server.Name)), nil
 }

internal/mcp/network_client_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"net/http"
 	"net/http/httptest"
+	"net/url"
 	"strings"
 	"sync/atomic"
 	"testing"
@@ -149,6 +150,141 @@ func TestOAuthRoundTripperDoesNotRetryAfterSuccessfulNon401(t *testing.T) {
 	}
 }
 
+func TestOAuthRoundTripperRejectsCrossOriginRedirectBeforeBearerLeak(t *testing.T) {
+	source := &fakeTokenSource{}
+	source.access.Store("access-secret")
+	source.refreshFunc = func() (string, error) { return "access-fresh", nil }
+
+	var targetHits int32
+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		atomic.AddInt32(&targetHits, 1)
+		if got := r.Header.Get("Authorization"); got != "" {
+			t.Errorf("redirect target received Authorization = %q", got)
+		}
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer target.Close()
+
+	redirector := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, target.URL+"/mcp", http.StatusTemporaryRedirect)
+	}))
+	defer redirector.Close()
+
+	client := mcpHTTPClient(
+		Server{Name: "oauth-docs", Type: ServerTypeHTTP},
+		newOAuthRoundTripper(http.DefaultTransport, source, "oauth-docs"),
+	)
+	resp, err := client.Get(redirector.URL + "/mcp")
+	if resp != nil {
+		_ = resp.Body.Close()
+	}
+	if err == nil || !strings.Contains(err.Error(), "cross-origin redirect") {
+		t.Fatalf("Get() error = %v, want cross-origin redirect error", err)
+	}
+	if got := atomic.LoadInt32(&targetHits); got != 0 {
+		t.Fatalf("redirect target hits = %d, want 0", got)
+	}
+}
+
+func TestSameMCPOriginNormalizesDefaultPorts(t *testing.T) {
+	for _, tc := range []struct {
+		name  string
+		left  string
+		right string
+		want  bool
+	}{
+		{
+			name:  "https default port",
+			left:  "https://example.test/mcp",
+			right: "https://EXAMPLE.test:443/other",
+			want:  true,
+		},
+		{
+			name:  "http default port",
+			left:  "http://example.test/mcp",
+			right: "http://example.test:80/other",
+			want:  true,
+		},
+		{
+			name:  "different scheme",
+			left:  "https://example.test/mcp",
+			right: "http://example.test:443/other",
+			want:  false,
+		},
+		{
+			name:  "different port",
+			left:  "https://example.test/mcp",
+			right: "https://example.test:8443/other",
+			want:  false,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			left, err := url.Parse(tc.left)
+			if err != nil {
+				t.Fatal(err)
+			}
+			right, err := url.Parse(tc.right)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if got := sameMCPOrigin(left, right); got != tc.want {
+				t.Fatalf("sameMCPOrigin(%q, %q) = %v, want %v", tc.left, tc.right, got, tc.want)
+			}
+		})
+	}
+}
+
+func TestResolveSSEEndpointURLRestrictsOrigin(t *testing.T) {
+	for _, tc := range []struct {
+		name     string
+		base     string
+		endpoint string
+		want     string
+		wantErr  string
+	}{
+		{
+			name:     "relative endpoint",
+			base:     "https://example.test/sse",
+			endpoint: "/messages",
+			want:     "https://example.test/messages",
+		},
+		{
+			name:     "absolute same origin",
+			base:     "https://example.test/sse",
+			endpoint: "https://example.test/messages",
+			want:     "https://example.test/messages",
+		},
+		{
+			name:     "default port equivalent",
+			base:     "https://example.test/sse",
+			endpoint: "https://example.test:443/messages",
+			want:     "https://example.test:443/messages",
+		},
+		{
+			name:     "cross origin",
+			base:     "https://example.test/sse",
+			endpoint: "https://evil.test/messages",
+			wantErr:  "differs from configured server origin",
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			got, err := resolveSSEEndpointURL(tc.base, tc.endpoint)
+			if tc.wantErr != "" {
+				if err == nil || !strings.Contains(err.Error(), tc.wantErr) {
+					t.Fatalf("resolveSSEEndpointURL() error = %v, want %q", err, tc.wantErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("resolveSSEEndpointURL() error = %v", err)
+			}
+			if got != tc.want {
+				t.Fatalf("resolveSSEEndpointURL() = %q, want %q", got, tc.want)
+			}
+		})
+	}
+}
+
 func TestNonOAuthServerIsUnaffected(t *testing.T) {
 	// Regression: a server without auth must not get a bearer header or any
 	// OAuth round tripper, and must connect normally.

internal/mcp/network_redirect.go

@@ -0,0 +1,74 @@
+package mcp
+
+import (
+	"fmt"
+	"net"
+	"net/http"
+	"net/url"
+	"strings"
+)
+
+const maxMCPRedirects = 10
+
+func mcpHTTPClient(server Server, transport http.RoundTripper) *http.Client {
+	return &http.Client{
+		Transport:     transport,
+		CheckRedirect: checkMCPRedirect(server),
+	}
+}
+
+func checkMCPRedirect(server Server) func(*http.Request, []*http.Request) error {
+	return func(request *http.Request, via []*http.Request) error {
+		if len(via) >= maxMCPRedirects {
+			return fmt.Errorf("MCP %s server %s stopped after %d redirects", server.Type, server.Name, maxMCPRedirects)
+		}
+		if len(via) == 0 {
+			return nil
+		}
+		if !sameMCPOrigin(via[0].URL, request.URL) {
+			return fmt.Errorf("MCP %s server %s refused cross-origin redirect to %s", server.Type, server.Name, mcpOrigin(request.URL))
+		}
+		return nil
+	}
+}
+
+func sameMCPOrigin(left *url.URL, right *url.URL) bool {
+	if left == nil || right == nil {
+		return false
+	}
+	return strings.EqualFold(left.Scheme, right.Scheme) &&
+		strings.EqualFold(left.Hostname(), right.Hostname()) &&
+		effectiveMCPPort(left) == effectiveMCPPort(right)
+}
+
+func effectiveMCPPort(value *url.URL) string {
+	if value == nil {
+		return ""
+	}
+	if port := value.Port(); port != "" {
+		return port
+	}
+	switch strings.ToLower(value.Scheme) {
+	case "http":
+		return "80"
+	case "https":
+		return "443"
+	default:
+		return ""
+	}
+}
+
+func mcpOrigin(value *url.URL) string {
+	if value == nil {
+		return "<nil>"
+	}
+	host := value.Hostname()
+	port := effectiveMCPPort(value)
+	if port != "" {
+		host = net.JoinHostPort(host, port)
+	}
+	if value.Scheme == "" {
+		return host
+	}
+	return strings.ToLower(value.Scheme) + "://" + host
+}

internal/tui/model.go

@@ -1669,17 +1669,38 @@ func (m model) updateModel(msg tea.Msg) (tea.Model, tea.Cmd) {
 		// when the loop is already running or there is nothing to animate.
 		return m, m.ensureSpinnerTick()
 	case permissionRequestMsg:
+		// The agent goroutine that raised this request is BLOCKED waiting on the
+		// decision callback, so every branch below must resolve it exactly once —
+		// or store it in pendingPermission, which resolves it on the user's reply.
+		// Dropping a request without resolving parks the run forever (the reported
+		// "stuck for 33 minutes" deadlock): the agent waits on a decision channel
+		// nothing will ever signal, with no visible prompt and no network activity.
 		if msg.runID != m.activeRunID {
+			// A superseded/stale run: unblock its parked goroutine now rather than
+			// relying on that run's context cancel to fire first.
+			if msg.decide != nil {
+				msg.decide(agent.PermissionDecision{Action: agent.PermissionDecisionCancel, Reason: "run superseded"})
+			}
+			return m, nil
+		}
+		if msg.request.Action != agent.PermissionActionPrompt {
+			// Not a user-facing prompt (e.g. a sandbox-allowed command that still
+			// blocked because it requested additional permissions). The UI has
+			// nothing to ask, so resolve immediately and FAIL CLOSED — never
+			// silently grant access that was never surfaced to the user. (The agent
+			// now marks such elevation requests as prompts, so in practice this is a
+			// defensive backstop; matches the ACP handler's fail-closed contract.)
+			if msg.decide != nil {
+				msg.decide(autoResolvedPermissionDecision(msg.request.Action))
+			}
 			return m, nil
 		}
 		promptRow := permissionTranscriptRow(permissionEventFromRequest(msg.request))
 		promptRow.runID = msg.runID
 		m.transcript = appendTranscriptRow(m.transcript, promptRow)
-		if msg.request.Action == agent.PermissionActionPrompt {
-			m.pendingPermission = &pendingPermissionPrompt{
-				request: msg.request,
-				decide:  msg.decide,
-			}
+		m.pendingPermission = &pendingPermissionPrompt{
+			request: msg.request,
+			decide:  msg.decide,
 		}
 		return m, nil
 	case askUserRequestMsg:
@@ -4525,6 +4546,21 @@ func (m model) sendPermissionRequest(runID int, request agent.PermissionRequest,
 	m.runtimeMessageSink(permissionRequestMsg{runID: runID, request: request, decide: decide})
 }
 
+// autoResolvedPermissionDecision resolves a permission request the TUI cannot
+// turn into a user prompt (Action != prompt). The agent is blocked awaiting a
+// decision, so one must ALWAYS be produced. Only an explicit Cancel is honored
+// as such; every other non-prompt action — including allow — is DENIED, so the
+// UI never silently grants access it did not surface for approval.
+func autoResolvedPermissionDecision(action agent.PermissionAction) agent.PermissionDecision {
+	if action == agent.PermissionActionCancel {
+		return agent.PermissionDecision{Action: agent.PermissionDecisionCancel, Reason: "run cancelled"}
+	}
+	return agent.PermissionDecision{
+		Action: agent.PermissionDecisionDeny,
+		Reason: "permission request could not be surfaced for approval",
+	}
+}
+
 func (m model) sendAskUserRequest(runID int, request agent.AskUserRequest, answer func([]string)) {
 	if m.runtimeMessageSink == nil {
 		return

internal/tui/model_picker_display_test.go

@@ -0,0 +1,57 @@
+package tui
+
+import "testing"
+
+// The model picker must title each row with the model NAME, not the models.dev
+// marketing description. Regression: Ollama Cloud models resolved full-sentence
+// descriptions from the remote catalog, so rows showed sentences and several
+// models sharing one description looked like exact duplicates (issue: /model
+// list rendered descriptions as titles).
+func TestModelPickerDisplayNamePrefersModelName(t *testing.T) {
+	sentence := "DeepSeek chat model for instruction following, coding, and analysis"
+	cases := []struct {
+		id          string
+		description string
+		want        string
+	}{
+		{"deepseek-v3.2", sentence, "Deepseek V3.2"},
+		{"deepseek-v3.2-thinking", sentence, "Deepseek V3.2 Thinking"},
+		{"glm-5.2", "GLM flagship model", "GLM 5.2"},
+		{"qwen3-coder:480b", "Qwen coding agent model for repository tasks", "Qwen3 Coder 480b"},
+		{"anthropic/claude-sonnet-4.6", "some blurb", "Claude Sonnet 4.6"},
+	}
+	for _, tc := range cases {
+		if got := modelPickerDisplayName(tc.id, tc.description); got != tc.want {
+			t.Errorf("modelPickerDisplayName(%q, %q) = %q, want %q", tc.id, tc.description, got, tc.want)
+		}
+	}
+}
+
+// Two distinct model ids that share the same catalog description must render as
+// distinct rows (the "duplicate-looking rows" symptom), never as the shared
+// sentence.
+func TestModelPickerDisplayNameDistinctForSharedDescription(t *testing.T) {
+	shared := "Mistral coding agent model for repository tasks and software engineering"
+	a := modelPickerDisplayName("codestral-2", shared)
+	b := modelPickerDisplayName("devstral-2", shared)
+	if a == b {
+		t.Fatalf("shared description collapsed rows to the same title: %q", a)
+	}
+	if a == shared || b == shared {
+		t.Fatalf("row title used the description sentence: a=%q b=%q", a, b)
+	}
+}
+
+// With no id to name the row, a non-generic description is still an acceptable
+// fallback; a generic placeholder (or nothing) falls back to "model".
+func TestModelPickerDisplayNameFallsBackToDescriptionOnlyWithoutID(t *testing.T) {
+	if got := modelPickerDisplayName("", "Friendly Name"); got != "Friendly Name" {
+		t.Fatalf("empty id should fall back to the description, got %q", got)
+	}
+	if got := modelPickerDisplayName("", "catalog default"); got != "model" {
+		t.Fatalf("generic description with no id should yield %q, got %q", "model", got)
+	}
+	if got := modelPickerDisplayName("", ""); got != "model" {
+		t.Fatalf("empty id and description should yield %q, got %q", "model", got)
+	}
+}

internal/tui/permission_deadlock_test.go

@@ -0,0 +1,109 @@
+package tui
+
+import (
+	"context"
+	"testing"
+
+	"github.com/Gitlawb/zero/internal/agent"
+)
+
+func permissionModel(t *testing.T) model {
+	t.Helper()
+	m := newModel(context.Background(), Options{})
+	m.pending = true
+	m.activeRunID = 7
+	m.width = 96
+	return m
+}
+
+// The deadlock: a permission request whose Action is NOT prompt (e.g. a
+// sandbox-allowed command that still blocked to request additional permissions)
+// must be resolved immediately — the agent goroutine is parked awaiting a
+// decision. Before the fix the handler dropped it, and the run hung forever
+// (the reported "stuck for 33 minutes"). Fail closed: a non-prompt allow denies.
+func TestPermissionRequestNonPromptResolvesFailClosed(t *testing.T) {
+	m := permissionModel(t)
+	var got *agent.PermissionDecision
+	decide := func(d agent.PermissionDecision) { got = &d }
+
+	updated, _ := m.Update(permissionRequestMsg{
+		runID:   7,
+		request: agent.PermissionRequest{ToolCallID: "c1", ToolName: "bash", Action: agent.PermissionActionAllow},
+		decide:  decide,
+	})
+	next := updated.(model)
+
+	if got == nil {
+		t.Fatal("DEADLOCK: decide was never called for a non-prompt request — the agent goroutine would park forever")
+	}
+	if got.Action != agent.PermissionDecisionDeny {
+		t.Fatalf("decision = %q, want deny (fail closed — never silently grant an unsurfaced permission)", got.Action)
+	}
+	if next.pendingPermission != nil {
+		t.Fatal("a non-prompt request must not open a pending prompt")
+	}
+}
+
+// A superseded/stale run's request (runID mismatch) must also be resolved
+// (cancelled) so its parked goroutine unblocks instead of depending on
+// context-cancel ordering.
+func TestPermissionRequestStaleRunResolvesCancel(t *testing.T) {
+	m := permissionModel(t) // activeRunID = 7
+	var got *agent.PermissionDecision
+	decide := func(d agent.PermissionDecision) { got = &d }
+
+	m.Update(permissionRequestMsg{
+		runID:   3, // != activeRunID
+		request: agent.PermissionRequest{ToolCallID: "c1", ToolName: "bash", Action: agent.PermissionActionPrompt},
+		decide:  decide,
+	})
+
+	if got == nil {
+		t.Fatal("stale-run request was dropped without resolving — its agent goroutine would park")
+	}
+	if got.Action != agent.PermissionDecisionCancel {
+		t.Fatalf("decision = %q, want cancel for a superseded run", got.Action)
+	}
+}
+
+// An explicit-cancel action is honored as a cancel (not denied).
+func TestPermissionRequestCancelActionResolvesCancel(t *testing.T) {
+	m := permissionModel(t)
+	var got *agent.PermissionDecision
+	decide := func(d agent.PermissionDecision) { got = &d }
+
+	m.Update(permissionRequestMsg{
+		runID:   7,
+		request: agent.PermissionRequest{ToolCallID: "c1", ToolName: "bash", Action: agent.PermissionActionCancel},
+		decide:  decide,
+	})
+	if got == nil || got.Action != agent.PermissionDecisionCancel {
+		t.Fatalf("cancel action must resolve as cancel, got %#v", got)
+	}
+}
+
+// A genuine prompt request stores the pending prompt and does NOT resolve yet —
+// the decision comes from the user's reply. (Regression guard: the fix must not
+// accidentally auto-resolve real prompts.)
+func TestPermissionRequestPromptDefersToUser(t *testing.T) {
+	m := permissionModel(t)
+	resolved := false
+	decide := func(agent.PermissionDecision) { resolved = true }
+
+	updated, _ := m.Update(permissionRequestMsg{
+		runID: 7,
+		request: agent.PermissionRequest{
+			ToolCallID: "c1", ToolName: "bash", Action: agent.PermissionActionPrompt,
+			Reason: "runs a shell command", AvailableDecisions: []agent.PermissionDecisionAction{agent.PermissionDecisionAllow},
+		},
+		decide: decide,
+	})
+	next := updated.(model)
+
+	if resolved {
+		t.Fatal("a real prompt must not be auto-resolved; it awaits the user")
+	}
+	if next.pendingPermission == nil {
+		t.Fatal("a prompt request must open a pending permission prompt")
+	}
+}

internal/tui/picker.go

@@ -452,11 +452,17 @@ func registryModelPickerMeta(entry modelregistry.ModelEntry) string {
 }
 
 func modelPickerDisplayName(id string, description string) string {
-	if description = strings.TrimSpace(description); description != "" && !providerWizardGenericModelDescription(description) {
-		return description
-	}
+	// The row title is the model NAME, not its marketing blurb. Catalog/discovery
+	// descriptions (from models.dev) are full sentences, and several models share
+	// the same one — using them as titles showed sentence-long rows that looked like
+	// exact duplicates and hid the actual id (which only survived in the meta line).
+	// Prefer the (prettified) id; fall back to a non-generic description only when
+	// there is no id to name the row.
 	id = strings.TrimSpace(id)
 	if id == "" {
+		if description = strings.TrimSpace(description); description != "" && !providerWizardGenericModelDescription(description) {
+			return description
+		}
 		return "model"
 	}
 	name := id