diff --git a/internal/cli/mcp_install.go b/internal/cli/mcp_install.go index 41e468f9..9ddd7550 100644 --- a/internal/cli/mcp_install.go +++ b/internal/cli/mcp_install.go @@ -317,6 +317,18 @@ func RunMcpInstallWizard(ctx context.Context, cmd mcpInstallFlagGetter, ui Insta // installs. The collector still resolves the public URL afterwards via // the Resolve public URL step. w.tunnelSteps = buildMcpTunnelSteps(realCmd) + + // Restart the managed service after the operator replaces the MCP + // password so the running endpoint reloads the new MCP_AUTH_TOKEN from + // its env file. Only fires when this install is actually backed by a + // managed service (effectiveManagedService); an explicit --service=false + // (operator runs their own foreground server) skips it. + w.restartHTTPService = func(ctx context.Context, s *InstallState) error { + if s == nil || s.Service == nil || !effectiveManagedService(realCmd.IsSet("service"), s.UseService) { + return nil + } + return mcpadapter.RestartManagedService(ctx, realCmd, s.Service) + } } // Bind the shared prompt channel so the spliced tunnel-config steps diff --git a/internal/cli/mcp_install_test.go b/internal/cli/mcp_install_test.go index 47fb960f..0adeed19 100644 --- a/internal/cli/mcp_install_test.go +++ b/internal/cli/mcp_install_test.go @@ -3,9 +3,11 @@ package cli import ( "context" "encoding/json" + "fmt" "os" "path/filepath" "reflect" + "runtime" "strconv" "strings" "sync" @@ -58,9 +60,12 @@ type MockInstallUI struct { SelectTransportErr error ConfirmHTTPResult bool ConfirmHTTPErr error + SetMCPPasswordResult string + SetMCPPasswordErr error - ReportWrittenCalls []writtenReport - ReportBuildCalls []buildReport + ReportWrittenCalls []writtenReport + ReportBuildCalls []buildReport + SetMCPPasswordCalls []string // current values passed to each call } type writtenReport struct { @@ -110,6 +115,14 @@ func (m *MockInstallUI) ConfirmHTTP(_ []install.AgentKey) (bool, error) { return m.ConfirmHTTPResult, m.ConfirmHTTPErr } +func (m *MockInstallUI) SetMCPPassword(current string) (string, error) { + m.RecordCall("SetMCPPassword") + m.mu.Lock() + defer m.mu.Unlock() + m.SetMCPPasswordCalls = append(m.SetMCPPasswordCalls, current) + return m.SetMCPPasswordResult, m.SetMCPPasswordErr +} + func (m *MockInstallUI) ReportWritten(agent install.AgentKey, path string, local bool) error { m.mu.Lock() defer m.mu.Unlock() @@ -299,9 +312,9 @@ func TestMcpInstallNonInteractiveClaudeCodeStdio(t *testing.T) { // --service=false is honored as the opt-out; --service=true is honored too. func TestEffectiveManagedService(t *testing.T) { cases := []struct { - name string - flagSet bool - useService bool + name string + flagSet bool + useService bool wantWantService bool }{ {"unset defaults on (interactive & non-interactive)", false, false, true}, @@ -430,11 +443,23 @@ func TestMcpInstallHTTPCompositeWritesRemoteEntry(t *testing.T) { w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) // Inject the fake collector: the real tunnel is not exercised in this test. w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "test-auth-token") + // The MCP Password step (always run for interactive http installs) is + // prompted with the collected token and the operator keeps it. + ui.SetMCPPasswordResult = "test-auth-token" if _, err := w.Run(ctx); err != nil { t.Fatalf("wizard run failed: %v", err) } + // The operator must have been given the chance to confirm the password, + // even though the collector already sourced an auth token. + ui.mu.Lock() + pwCalls := append([]string(nil), ui.SetMCPPasswordCalls...) + ui.mu.Unlock() + if len(pwCalls) != 1 || pwCalls[0] != "test-auth-token" { + t.Errorf("SetMCPPassword calls = %v, want single call with current=%q", pwCalls, "test-auth-token") + } + // The remote (http) entry must carry type=http, url, and the Bearer auth // header that the wizard builds from AuthToken. entry := readGlobalJSON(t, root, install.AgentClaudeCode) @@ -472,6 +497,9 @@ func TestMcpInstallHTTPCompositeSkipsStdioOnlyAgent(t *testing.T) { w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "test-auth-token") + // The MCP Password step runs because claude-code supports http; keep the + // collected token. + ui.SetMCPPasswordResult = "test-auth-token" if _, err := w.Run(ctx); err != nil { t.Fatalf("wizard run failed: %v", err) @@ -1486,3 +1514,315 @@ func TestMcpInstallRunsAsDelegateSubWizard(t *testing.T) { t.Errorf("expected a command path written by the embedded install") } } + +// TestMcpInstallHTTPAlwaysPromptsForPassword guards the core edge case: an +// interactive http install must ALWAYS ask the operator for the MCP password, +// even when an auth token was already inherited from the tunnel/env collector. +// The operator's chosen password (if they replace it) must be what ends up in +// the written Authorization header — never a silently-sourced token the user +// did not see. +func TestMcpInstallHTTPAlwaysPromptsForPassword(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + projectDir := t.TempDir() + ui := newMockInstallUI() + + state := &InstallState{ + Agents: []install.AgentKey{install.AgentClaudeCode}, + Scope: scopeGlobal, + Transport: install.TransportHTTP, + UseService: true, + } + + w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) + // The collector sources an inherited token (e.g. from MCP_AUTH_TOKEN env). + w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "inherited-token") + // The operator is prompted and chooses a fresh password. + ui.SetMCPPasswordResult = "operator-chosen-password" + + if _, err := w.Run(ctx); err != nil { + t.Fatalf("wizard run failed: %v", err) + } + + // The prompt must have fired exactly once, showing the inherited token as + // the current value the operator is keeping-or-replacing. + ui.mu.Lock() + pwCalls := append([]string(nil), ui.SetMCPPasswordCalls...) + ui.mu.Unlock() + if len(pwCalls) != 1 || pwCalls[0] != "inherited-token" { + t.Errorf("SetMCPPassword calls = %v, want single call with current=%q", pwCalls, "inherited-token") + } + + // The operator's choice, not the inherited token, must be written. + entry := readGlobalJSON(t, root, install.AgentClaudeCode) + headers, _ := entry["headers"].(map[string]any) + auth, _ := headers["Authorization"] + if auth != "Bearer operator-chosen-password" { + t.Errorf("entry headers[Authorization] = %v, want 'Bearer operator-chosen-password'", auth) + } +} + +// TestMcpInstallHTTPPasswordPersistsToServiceEnv guards the follow-on: when an +// http install has a backing managed service and the operator replaces the MCP +// password, the new token must ALSO be persisted to the service env file +// (MCP_AUTH_TOKEN) so the running endpoint validates against it. If it is not +// persisted, the endpoint keeps enforcing the inherited token and the agent +// config points at a credential the server rejects. +func TestMcpInstallHTTPPasswordPersistsToServiceEnv(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + projectDir := t.TempDir() + + // A real service env file on disk, as the managed service would leave it. + envFile := filepath.Join(t.TempDir(), "mcp.env") + if err := os.WriteFile(envFile, []byte("MCP_AUTH_TOKEN=inherited-token\nMCP_PUBLIC_URL=https://mcp.example.com\n"), 0o600); err != nil { + t.Fatalf("write env file: %v", err) + } + + ui := newMockInstallUI() + state := &InstallState{ + Agents: []install.AgentKey{install.AgentClaudeCode}, + Scope: scopeGlobal, + Transport: install.TransportHTTP, + UseService: true, + // A backing service whose env file already carries the inherited token. + Service: &mcpadapter.ServiceInstallState{ + EnvFile: envFile, + AuthToken: "inherited-token", + PublicURL: "https://mcp.example.com", + Provider: tunnel.TunnelProviderNgrok, + TunnelName: "pinner-mcp", + }, + } + + w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) + // The collector folds the persisted env into the install state for the + // agent entry; the operator then replaces the password. + w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "inherited-token") + ui.SetMCPPasswordResult = "operator-chosen-password" + // Record the restart seam (the production wiring calls the real managed + // service restart; here we only assert it fires after a password change). + var restarts int + w.restartHTTPService = func(_ context.Context, _ *InstallState) error { + restarts++ + return nil + } + + if _, err := w.Run(ctx); err != nil { + t.Fatalf("wizard run failed: %v", err) + } + + // Replacing the password must trigger a service restart so the running + // endpoint reloads the new token; otherwise it keeps the old one. + if restarts != 1 { + t.Errorf("restartHTTPService called %d times, want 1 (after password change)", restarts) + } + + // The service env file on disk must now carry the operator's password so + // the running endpoint validates against the same token the agent uses. + envData, err := os.ReadFile(envFile) + if err != nil { + t.Fatalf("read env file: %v", err) + } + if !strings.Contains(string(envData), "MCP_AUTH_TOKEN=operator-chosen-password") { + t.Errorf("service env file must persist the operator's password, got:\n%s", envData) + } + + // The agent config header uses the same token. + entry := readGlobalJSON(t, root, install.AgentClaudeCode) + headers, _ := entry["headers"].(map[string]any) + auth, _ := headers["Authorization"] + if auth != "Bearer operator-chosen-password" { + t.Errorf("entry headers[Authorization] = %v, want 'Bearer operator-chosen-password'", auth) + } + if state.Service.AuthToken != "operator-chosen-password" { + t.Errorf("service state AuthToken = %q, want operator-chosen-password", state.Service.AuthToken) + } +} + +// TestMcpInstallHTTPPasswordRestartFailureRollsBack guards that a failed +// service restart does not leave the env file or in-memory state holding the +// new password while the (un-restarted) endpoint still enforces the old one. +// On restart failure the wizard must roll the env file and state back to the +// previous token so disk, memory, and the live endpoint agree. +func TestMcpInstallHTTPPasswordRestartFailureRollsBack(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + projectDir := t.TempDir() + + envFile := filepath.Join(t.TempDir(), "mcp.env") + if err := os.WriteFile(envFile, []byte("MCP_AUTH_TOKEN=inherited-token\n"), 0o600); err != nil { + t.Fatalf("write env file: %v", err) + } + + ui := newMockInstallUI() + state := &InstallState{ + Agents: []install.AgentKey{install.AgentClaudeCode}, + Scope: scopeGlobal, + Transport: install.TransportHTTP, + UseService: true, + Service: &mcpadapter.ServiceInstallState{ + EnvFile: envFile, + AuthToken: "inherited-token", + PublicURL: "https://mcp.example.com", + Provider: tunnel.TunnelProviderNgrok, + TunnelName: "pinner-mcp", + }, + } + + w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) + w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "inherited-token") + ui.SetMCPPasswordResult = "operator-chosen-password" + w.restartHTTPService = func(_ context.Context, _ *InstallState) error { + return fmt.Errorf("systemctl restart failed") + } + + if _, err := w.Run(ctx); err == nil { + t.Fatalf("expected the wizard to fail when the service restart fails, got nil") + } + + // The env file must be rolled back to the token the endpoint still enforces. + envData, err := os.ReadFile(envFile) + if err != nil { + t.Fatalf("read env file: %v", err) + } + if !strings.Contains(string(envData), "MCP_AUTH_TOKEN=inherited-token") { + t.Errorf("env file must be rolled back to the previous token on restart failure, got:\n%s", envData) + } + if strings.Contains(string(envData), "operator-chosen-password") { + t.Errorf("env file must not retain the uncommitted new password, got:\n%s", envData) + } + + // In-memory state must also point at the old (live) token. + if state.AuthToken != "inherited-token" { + t.Errorf("state.AuthToken = %q, want inherited-token (rolled back)", state.AuthToken) + } + if state.Service.AuthToken != "inherited-token" { + t.Errorf("service.AuthToken = %q, want inherited-token (rolled back)", state.Service.AuthToken) + } +} + +// TestMcpInstallHTTPPasswordRestoreFailureSurfaced guards that a failed +// restore-write during rollback is surfaced (not swallowed): if the service +// restart fails AND the env-file rollback write also fails, the wizard must +// report the restore failure so the on-disk/state disagreement is not masked. +func TestMcpInstallHTTPPasswordRestoreFailureSurfaced(t *testing.T) { + // This test forces a failed restore-write by making the env file's + // directory unwritable (os.Chmod on a dir). Windows does not honor POSIX + // directory write bits, so the trigger is POSIX-only; the production + // surfacing behavior itself is OS-independent and covered by the + // restart-failure rollback test on all platforms. + if runtime.GOOS == "windows" { + t.Skip("dir-permission write-failure trigger is POSIX-only") + } + ctx := context.Background() + root := t.TempDir() + projectDir := t.TempDir() + + envFile := filepath.Join(t.TempDir(), "mcp.env") + if err := os.WriteFile(envFile, []byte("MCP_AUTH_TOKEN=inherited-token\n"), 0o600); err != nil { + t.Fatalf("write env file: %v", err) + } + + ui := newMockInstallUI() + state := &InstallState{ + Agents: []install.AgentKey{install.AgentClaudeCode}, + Scope: scopeGlobal, + Transport: install.TransportHTTP, + UseService: true, + Service: &mcpadapter.ServiceInstallState{ + EnvFile: envFile, + AuthToken: "inherited-token", + PublicURL: "https://mcp.example.com", + Provider: tunnel.TunnelProviderNgrok, + TunnelName: "pinner-mcp", + }, + } + + w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) + w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "inherited-token") + ui.SetMCPPasswordResult = "operator-chosen-password" + // The restart fails AND renders the env file's directory read-only so the + // follow-up restore write fails too — both errors must be reported, not + // masked. (WriteEnvironment writes atomically via temp+rename, so chmodding + // the file itself would not block it; the directory must be unwritable.) + envDir := filepath.Dir(envFile) + t.Cleanup(func() { _ = os.Chmod(envDir, 0o700) }) // let TempDir cleanup remove it + w.restartHTTPService = func(_ context.Context, _ *InstallState) error { + if err := os.Chmod(envDir, 0o500); err != nil { + t.Fatalf("chmod env dir: %v", err) + } + return fmt.Errorf("systemctl restart failed") + } + + _, err := w.Run(ctx) + if err == nil { + t.Fatalf("expected an error when the service restart fails, got nil") + } + if !strings.Contains(err.Error(), "restore MCP password") { + t.Errorf("error must surface the restore-write failure, got: %v", err) + } +} + +// TestMcpInstallHTTPNonInteractiveSkipsPassword guards that a non-interactive +// http install does NOT prompt for the password (it is sourced from flags/env) +// and does not error at the prompt. The sourced token is used as-is. +func TestMcpInstallHTTPNonInteractiveSkipsPassword(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + projectDir := t.TempDir() + ui := newMockInstallUI() + + state := &InstallState{ + Agents: []install.AgentKey{install.AgentClaudeCode}, + Scope: scopeGlobal, + Transport: install.TransportHTTP, + UseService: true, + NonInteractive: true, + } + + w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) + w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "env-token") + + if _, err := w.Run(ctx); err != nil { + t.Fatalf("wizard run failed: %v", err) + } + + // No interactive prompt may fire in non-interactive mode. + if ui.WasCalled("SetMCPPassword") { + t.Errorf("SetMCPPassword must not be called in non-interactive mode") + } + + // The env-sourced token is written unchanged. + entry := readGlobalJSON(t, root, install.AgentClaudeCode) + headers, _ := entry["headers"].(map[string]any) + auth, _ := headers["Authorization"] + if auth != "Bearer env-token" { + t.Errorf("entry headers[Authorization] = %v, want 'Bearer env-token'", auth) + } +} + +// TestMcpInstallHTTPPasswordRequired guards that an interactive http install +// fails if the operator supplies no password and none was inherited. +func TestMcpInstallHTTPPasswordRequired(t *testing.T) { + ctx := context.Background() + root := t.TempDir() + projectDir := t.TempDir() + ui := newMockInstallUI() + + state := &InstallState{ + Agents: []install.AgentKey{install.AgentClaudeCode}, + Scope: scopeGlobal, + Transport: install.TransportHTTP, + UseService: true, + } + + w := NewInstallWizard(ui, state, tempPathResolver(root, projectDir)) + // No token is collected; the mock returns empty (operator typed nothing). + w.collectHTTP = fakeHTTPCollector("https://mcp.example.com", "") + ui.SetMCPPasswordErr = fmt.Errorf("an MCP password is required for a public HTTP endpoint") + + if _, err := w.Run(ctx); err == nil { + t.Fatalf("expected an error when no MCP password is provided, got nil") + } +} diff --git a/internal/cli/mcp_install_ui.go b/internal/cli/mcp_install_ui.go index 7756fdc2..668ca466 100644 --- a/internal/cli/mcp_install_ui.go +++ b/internal/cli/mcp_install_ui.go @@ -2,6 +2,7 @@ package cli import ( "fmt" + "strings" "atomicgo.dev/keyboard/keys" "github.com/pterm/pterm" @@ -32,6 +33,13 @@ type InstallUI interface { SelectTransport(agents []install.AgentKey) (install.Transport, error) // ConfirmHTTP confirms an http install before it is written. ConfirmHTTP(agents []install.AgentKey) (bool, error) + // SetMCPPassword prompts for the shared auth token ("MCP password") that + // protects the public HTTP endpoint. current is the password already + // resolved from flags/env/tunnel collection (may be empty); it is shown + // masked so the operator can keep it or replace it. Returns the password + // to use. Always called for http/remote installs in interactive mode so + // the operator is never silently skipped past setting the credential. + SetMCPPassword(current string) (string, error) // ReportWritten reports a written config entry for an agent. ReportWritten(agent install.AgentKey, path string, local bool) error @@ -202,6 +210,36 @@ func (ui *PTermInstallUI) ConfirmHTTP(agents []install.AgentKey) (bool, error) { return ok, nil } +// SetMCPPassword prompts for the shared auth token ("MCP password") that +// protects the public HTTP endpoint. current is the auth token already +// resolved from flags/env/tunnel collection (may be empty). The operator is +// always given the chance to set or replace the credential in interactive +// mode: an existing value is kept unless a new one is typed. The secret +// itself is never displayed or echoed (masked input, no pre-filled default). +func (ui *PTermInstallUI) SetMCPPassword(current string) (string, error) { + if fieldform.NonInteractive { + return "", fmt.Errorf("MCP password prompt requires an interactive terminal") + } + if current != "" { + pterm.Info.Println("A shared auth token (MCP password) already protects this endpoint. Press Enter to keep it, or type a new password to replace it.") + } else { + pterm.Warning.Println("A public HTTP MCP endpoint needs an MCP password (shared auth token) so it is not left open.") + } + val, err := pterm.DefaultInteractiveTextInput.WithDefaultText("MCP password (shared auth token for the public endpoint)").WithMask("*").Show() + if err != nil { + return "", handleInterrupt(err) + } + val = strings.TrimSpace(val) + if val == "" { + // Empty input keeps the existing token; only error when there is none. + if current == "" { + return "", fmt.Errorf("an MCP password is required for a public HTTP endpoint") + } + return current, nil + } + return val, nil +} + // ReportWritten reports a written config entry for an agent. func (ui *PTermInstallUI) ReportWritten(agent install.AgentKey, path string, local bool) error { where := "global" diff --git a/internal/cli/mcp_install_wizard.go b/internal/cli/mcp_install_wizard.go index 60112049..5eba3429 100644 --- a/internal/cli/mcp_install_wizard.go +++ b/internal/cli/mcp_install_wizard.go @@ -10,6 +10,7 @@ import ( "go.lumeweb.com/pinner-cli/internal/cli/wizard" "go.lumeweb.com/pinner-cli/internal/mcp/install" mcpadapter "go.lumeweb.com/pinner-cli/internal/mcp/services" + "go.lumeweb.com/pinner-cli/internal/service" ) // defaultServerName is the server entry name written for pinner. @@ -105,6 +106,12 @@ type InstallWizard struct { resolvePath pathResolver collectHTTP httpCollector + // restartHTTPService, when non-nil (production), restarts the managed MCP + // service after the operator replaces the MCP password so the running + // endpoint reloads the new MCP_AUTH_TOKEN from its env file. Tests leave + // it nil so no live service is ever touched. + restartHTTPService func(ctx context.Context, s *InstallState) error + // tunnelSteps, when non-empty (production), is the wrapped, VISIBLE // tunnel-config host steps (provider, credentials, env write) that getSteps // splices in between "Choose Transport" and "Write Config". Each wraps a @@ -256,6 +263,40 @@ func (w *InstallWizard) getSteps() []wizard.Step[*InstallState] { }, }) + // The MCP password (the shared auth token that protects the public HTTP + // endpoint) is a first-class, always-asked credential in interactive + // installs. Even when one was inherited from MCP_AUTH_TOKEN env/flags or + // the tunnel collection above, the operator is given the chance to keep + // or replace it, so it is never silently written past the user. Skipped in + // non-interactive mode (--non-interactive; token sourced from flags/env) + // and for non-http transports (stdio needs no credential). + steps = append(steps, wizard.StepFunc[*InstallState]{ + Name_: "MCP Password", + SkipFunc: func(s *InstallState) bool { + return s.NonInteractive || + s.Transport != install.TransportHTTP || + !anySupportsTransport(s.Agents, install.TransportHTTP) + }, + ExecuteFunc: func(ctx context.Context, s *InstallState) error { + pw, err := w.ui.SetMCPPassword(s.AuthToken) + if err != nil { + return err + } + // The agent config's Authorization header is built from s.AuthToken, + // but the running HTTP endpoint enforces MCP_AUTH_TOKEN from the + // service env file. When the operator replaces the password, persist + // it to that file too so the endpoint and the agent config agree — + // otherwise the endpoint keeps the old credential and the connection + // breaks. Keeping the existing token needs no propagation. + if pw != s.AuthToken { + if err := w.persistAuthToken(ctx, s, pw); err != nil { + return err + } + } + return nil + }, + }) + steps = append(steps, wizard.StepFunc[*InstallState]{ Name_: "Resolve Binary", @@ -386,6 +427,64 @@ func (w *InstallWizard) writeConfig(s *InstallState) error { return nil } +// persistAuthToken records the operator-chosen MCP password as the new shared +// auth token. It always updates s.AuthToken (the value the agent config's +// Authorization header is built from). When the install has a backing service +// (http + managed service), it also persists MCP_AUTH_TOKEN to the service env +// file and mirrors it on the service state so the running endpoint and the +// agent config validate against the SAME credential — without this the endpoint +// keeps enforcing the inherited token and the agent connection breaks. +func (w *InstallWizard) persistAuthToken(ctx context.Context, s *InstallState, pw string) error { + // No backing service (e.g. --service=false, operator-run foreground + // server, or tests with a fake collector): only the agent config consumes + // the token, so just record it in memory. + if s.Service == nil { + s.AuthToken = pw + return nil + } + + env, err := service.LoadEnvironment(s.Service.EnvFile) + if err != nil { + return fmt.Errorf("load MCP service environment %q to persist the MCP password: %w", s.Service.EnvFile, err) + } + prev, hadPrev := env["MCP_AUTH_TOKEN"] + + // The running service reads MCP_AUTH_TOKEN from this file at process + // start, so the new token must be written to disk BEFORE the restart that + // reloads it. If the restart then fails, roll the file (and state) back to + // the token the still-running endpoint enforces so disk and memory agree + // with what is actually live. + env["MCP_AUTH_TOKEN"] = pw + if err := service.WriteEnvironment(s.Service.EnvFile, env); err != nil { + return fmt.Errorf("persist MCP password to %q: %w", s.Service.EnvFile, err) + } + + if w.restartHTTPService != nil { + if rerr := w.restartHTTPService(ctx, s); rerr != nil { + // Restore the previous token the live endpoint still enforces. + if hadPrev { + env["MCP_AUTH_TOKEN"] = prev + } else { + delete(env, "MCP_AUTH_TOKEN") + } + s.AuthToken = prev + s.Service.AuthToken = prev + // Surface a failed restore write too: if the rollback cannot be + // persisted, disk still holds the uncommitted new password while + // state/endpoint use the old one — that disagreement must not be + // silently masked. + if werr := service.WriteEnvironment(s.Service.EnvFile, env); werr != nil { + return fmt.Errorf("restore MCP password after failed restart: %v (restart: %w)", werr, rerr) + } + return fmt.Errorf("restart MCP service to load the new MCP password: %w", rerr) + } + } + + s.AuthToken = pw + s.Service.AuthToken = pw + return nil +} + // writeOne writes a single server entry and reports it. func (w *InstallWizard) writeOne(s *InstallState, agentCfg install.Agent, serverCfg install.McpServerConfig, local bool) error { path := w.resolvePath(agentCfg, local, s.ProjectDir) diff --git a/internal/mcp/services/service_command.go b/internal/mcp/services/service_command.go index 860590f4..79039b0f 100644 --- a/internal/mcp/services/service_command.go +++ b/internal/mcp/services/service_command.go @@ -582,6 +582,23 @@ func newManagedService(cmd *cli.Command, envFile string, provider tunnel.TunnelP return service.New(cfg) } +// RestartManagedService restarts the managed MCP service so a freshly written +// MCP_AUTH_TOKEN in its env file takes effect on the running endpoint. It is a +// no-op when the install state carries no backing service to restart (no env +// file or provider). Callers gate on whether a managed service was actually +// started (e.g. effectiveManagedService) before invoking it. The caller's ctx +// is honored so an interrupted install (Ctrl-C) can cancel the restart. +func RestartManagedService(ctx context.Context, cmd *cli.Command, s *ServiceInstallState) error { + if s == nil || s.EnvFile == "" || s.Provider == "" { + return nil + } + svc, err := newManagedService(cmd, s.EnvFile, s.Provider) + if err != nil { + return err + } + return svc.Restart(ctx) +} + // serviceConfigForInstall builds the service.Config for the managed MCP // service: the pinner executable run as `pinner mcp`, referencing the tunnel // credentials via envFile (a path). Each platform backend chooses its own diff --git a/internal/mcp/services/service_command_test.go b/internal/mcp/services/service_command_test.go index f06e8225..2782e0dd 100644 --- a/internal/mcp/services/service_command_test.go +++ b/internal/mcp/services/service_command_test.go @@ -828,3 +828,18 @@ func TestServiceConfigForInstallPassesEnvFileUntouched(t *testing.T) { require.Nil(t, cfg.EnvVars) require.FileExists(t, path) } + +// TestRestartManagedServiceNoOp guards that RestartManagedService is a safe +// no-op when the install state carries no backing service to restart (nil +// state, or no env file / provider). This keeps the MCP install password path +// from touching a live service when there is none to reload. +func TestRestartManagedServiceNoOp(t *testing.T) { + cmd := &cli.Command{Flags: []cli.Flag{&cli.StringFlag{Name: serviceEnvFileFlag}}} + ctx := context.Background() + require.NoError(t, RestartManagedService(ctx, cmd, nil)) + require.NoError(t, RestartManagedService(ctx, cmd, &ServiceInstallState{})) + // An env file without a provider, or a provider without an env file, is + // still a no-op — neither can identify a managed service to restart. + require.NoError(t, RestartManagedService(ctx, cmd, &ServiceInstallState{EnvFile: "mcp.env"})) + require.NoError(t, RestartManagedService(ctx, cmd, &ServiceInstallState{Provider: tunnel.TunnelProviderNgrok})) +}