From 27de6cb00a52bffd539cab56fd6a123f3f11923f Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Wed, 17 Jun 2026 22:05:59 +0300 Subject: [PATCH] feat(connect): stat-only overall status + on-demand content reads (US1) Related #696 Spec 075 US1. Connect overall status (GetAllStatus) now determines each client's installed state via os.Stat metadata only and performs ZERO config content reads, eliminating the macOS "wants to access data from other apps" privacy prompt storm when simply viewing status (FR-001, SC-001/SC-002). Reading a client's config contents to detect whether mcpproxy is registered now happens only on an explicit per-client action via the new GetStatus(id), which resolves Connected + AccessState (FR-002). ## Changes - Add a content-read seam: Service.readFile (default os.ReadFile) + NewServiceWithReader / setReadFile so tests inject reads/denials. - ClientStatus gains additive access_state + remediation fields (FR-006). - GetAllStatus: metadata-only existence; AccessState="unknown", Connected=false for installed clients (eager findEntry removed). - GetStatus(clientID): single on-demand content read via the seam; resolves accessible/absent/malformed and Connected. - Route all content reads (readOrCreate*, disconnect*, verifyJSONEntry, entry-finders) through the seam; entry-finders parse bytes only. - GetConnectedCount/GetConnectedIDs re-implemented to read lazily per client. - CLI `mcpproxy connect` resolves Connected on demand (explicit action), preserving the CONNECTED column. ## Testing - New TestGetAllStatus_NoContentReads (reader asserts 0 calls), TestGetStatus_ReadsSingleClientOnDemand (exactly 1 read), plus absent / malformed / unknown-client cases. - Migrated GetAllStatus connected-detection tests to GetStatus. - go test ./internal/connect/... ./internal/httpapi/... -race green; golangci-lint v2 clean; build (personal+server) ok; API E2E 65/65 pass. --- cmd/mcpproxy/connect_cmd.go | 12 + docs/api/rest-api.md | 26 ++ internal/connect/connect.go | 243 +++++++++++++----- internal/connect/connect_test.go | 60 +++-- internal/connect/status_ondemand_test.go | 154 +++++++++++ .../checklists/requirements.md | 36 +++ .../contracts/connect-status.md | 65 +++++ specs/075-macos-tcc-connect/data-model.md | 65 +++++ specs/075-macos-tcc-connect/plan.md | 83 ++++++ specs/075-macos-tcc-connect/quickstart.md | 47 ++++ specs/075-macos-tcc-connect/research.md | 43 ++++ specs/075-macos-tcc-connect/spec.md | 152 +++++++++++ specs/075-macos-tcc-connect/tasks.md | 122 +++++++++ 13 files changed, 1021 insertions(+), 87 deletions(-) create mode 100644 internal/connect/status_ondemand_test.go create mode 100644 specs/075-macos-tcc-connect/checklists/requirements.md create mode 100644 specs/075-macos-tcc-connect/contracts/connect-status.md create mode 100644 specs/075-macos-tcc-connect/data-model.md create mode 100644 specs/075-macos-tcc-connect/plan.md create mode 100644 specs/075-macos-tcc-connect/quickstart.md create mode 100644 specs/075-macos-tcc-connect/research.md create mode 100644 specs/075-macos-tcc-connect/spec.md create mode 100644 specs/075-macos-tcc-connect/tasks.md diff --git a/cmd/mcpproxy/connect_cmd.go b/cmd/mcpproxy/connect_cmd.go index 9daffd289..444549c6b 100644 --- a/cmd/mcpproxy/connect_cmd.go +++ b/cmd/mcpproxy/connect_cmd.go @@ -132,7 +132,19 @@ func runDisconnect(cmd *cobra.Command, args []string) error { } func printConnectStatus(svc *connect.Service, formatter clioutput.OutputFormatter, format string) error { + // Spec 075: GetAllStatus is content-read-free and leaves Connected/AccessState + // unresolved. Running `mcpproxy connect` is an explicit user action, so resolve + // each supported+installed client's connected state on demand via GetStatus to + // preserve the CONNECTED column. Unsupported/absent clients keep the cheap + // metadata-only listing (no content read). statuses := svc.GetAllStatus() + for i := range statuses { + if statuses[i].Supported && statuses[i].Exists { + if st, err := svc.GetStatus(statuses[i].ID); err == nil { + statuses[i] = st + } + } + } if format == "table" { headers := []string{"CLIENT", "STATUS", "CONFIG PATH", "CONNECTED"} diff --git a/docs/api/rest-api.md b/docs/api/rest-api.md index 3455485e7..b1f518466 100644 --- a/docs/api/rest-api.md +++ b/docs/api/rest-api.md @@ -634,6 +634,32 @@ Success returns `data.server` (`name`, `protocol`, `command`, `args`, `url`, Drop a registry's cached server lists. Returns `{ "registry_id": "...", "cleared": }`. +### Connect (client wizard) + +#### GET /api/v1/connect + +Lists the connection status of every known MCP client (Claude Desktop, Cursor, +VS Code, Codex, Gemini, OpenCode, …). + +As of Spec 075, the overall listing determines each client's installed state +using **file-existence metadata only** (`os.Stat`) and performs **zero config +content reads** — so simply viewing status never triggers the macOS +"wants to access data from other apps" privacy prompt. Each per-client object is +additive-compatible and gains two fields: + +| Field | Type | Meaning | +|-------|------|---------| +| `exists` | bool | Config file present (metadata only). | +| `connected` | bool | mcpproxy registered in the config. Authoritative **only** when `access_state == "accessible"`; `false`/unresolved in the overall listing. | +| `access_state` | string | `"unknown"` in the overall listing (not content-checked); resolved to `"accessible"`, `"absent"`, or `"malformed"` by an on-demand single-client read. | +| `remediation` | string | Present only when access is denied (populated by later stories). | + +A client that is installed but not yet content-checked reads as +`exists=true, connected=false, access_state="unknown"`. Resolving `connected` +requires an explicit per-client read (connect/disconnect, or the CLI +`mcpproxy connect` command), which is the only place a privacy prompt may +legitimately appear. + ### Real-time Updates #### GET /events diff --git a/internal/connect/connect.go b/internal/connect/connect.go index 279b3a7c2..424913791 100644 --- a/internal/connect/connect.go +++ b/internal/connect/connect.go @@ -3,7 +3,9 @@ package connect import ( "bytes" "encoding/json" + "errors" "fmt" + "io/fs" "net/url" "os" "regexp" @@ -12,6 +14,17 @@ import ( "github.com/BurntSushi/toml" ) +// AccessState classifies a per-client config access (Spec 075). It is left as +// accessUnknown by the content-read-free overall status and resolved by the +// on-demand GetStatus / connect / disconnect paths. +const ( + accessUnknown = "unknown" // overall status: not content-checked + accessAccessible = "accessible" // config read and parsed successfully + accessAbsent = "absent" // config file does not exist (not installed) + accessMalformed = "malformed" // config read but contents unparseable + // accessDenied ("denied", macOS TCC App-Data) is added in US2. +) + // ConnectResult describes the outcome of a connect or disconnect operation. type ConnectResult struct { Success bool `json:"success"` @@ -37,6 +50,13 @@ type ClientStatus struct { Bridge bool `json:"bridge,omitempty"` // connects via a stdio bridge; connectable even without an existing config Icon string `json:"icon"` ServerName string `json:"server_name,omitempty"` // name under which mcpproxy is registered + + // AccessState classifies the per-client content access (Spec 075, additive). + // Empty/"unknown" in the content-read-free overall status; resolved to + // "accessible"/"absent"/"malformed" (and "denied" in US2) by on-demand reads. + AccessState string `json:"access_state"` + // Remediation carries actionable fix text, populated only when access is denied. + Remediation string `json:"remediation,omitempty"` } // Service provides connect/disconnect operations for MCP client configurations. @@ -44,6 +64,9 @@ type Service struct { listenAddr string // e.g. "127.0.0.1:8080" apiKey string // optional API key homeDir string // override for testing; empty means use os.UserHomeDir + // readFile is the content-read seam (Spec 075 T003). Defaults to os.ReadFile; + // tests inject a permission-denied error or a call counter through it. + readFile func(string) ([]byte, error) } // NewService creates a Service that will inject the given listen address @@ -52,6 +75,7 @@ func NewService(listenAddr, apiKey string) *Service { return &Service{ listenAddr: listenAddr, apiKey: apiKey, + readFile: os.ReadFile, } } @@ -61,7 +85,31 @@ func NewServiceWithHome(listenAddr, apiKey, homeDir string) *Service { listenAddr: listenAddr, apiKey: apiKey, homeDir: homeDir, + readFile: os.ReadFile, + } +} + +// NewServiceWithReader creates a Service with a custom content reader (for +// testing the access-classification seam without a real OS denial). +func NewServiceWithReader(listenAddr, apiKey, homeDir string, readFile func(string) ([]byte, error)) *Service { + return &Service{ + listenAddr: listenAddr, + apiKey: apiKey, + homeDir: homeDir, + readFile: readFile, + } +} + +// setReadFile overrides the content-read seam (test helper). +func (s *Service) setReadFile(fn func(string) ([]byte, error)) { s.readFile = fn } + +// read performs a config content read through the seam, falling back to +// os.ReadFile for a zero-value Service. +func (s *Service) read(path string) ([]byte, error) { + if s.readFile != nil { + return s.readFile(path) } + return os.ReadFile(path) } // mcpURL builds the MCPProxy MCP endpoint URL. @@ -86,8 +134,14 @@ const defaultServerName = "mcpproxy" // predicate (Spec 046). func (s *Service) GetConnectedCount() int { count := 0 - for _, st := range s.GetAllStatus() { - if st.Connected { + for _, c := range GetAllClients() { + if !c.Supported { + continue + } + // On-demand per-client read: GetConnectedCount/IDs are the one internal + // caller that legitimately needs the connected truth for the wizard + // predicate, and it reads lazily per client (Spec 075 T011). + if st, err := s.GetStatus(c.ID); err == nil && st.Connected { count++ } } @@ -98,10 +152,13 @@ func (s *Service) GetConnectedCount() int { // mcpproxy is currently registered. Identifiers come from the fixed // per-client adapter table; user-entered values never appear here. func (s *Service) GetConnectedIDs() []string { - statuses := s.GetAllStatus() - ids := make([]string, 0, len(statuses)) - for _, st := range statuses { - if st.Connected { + clients := GetAllClients() + ids := make([]string, 0, len(clients)) + for _, c := range clients { + if !c.Supported { + continue + } + if st, err := s.GetStatus(c.ID); err == nil && st.Connected { ids = append(ids, st.ID) } } @@ -109,6 +166,12 @@ func (s *Service) GetConnectedIDs() []string { } // GetAllStatus returns the connection status for every known client. +// +// It determines "installed" via os.Stat metadata only and performs ZERO config +// content reads (Spec 075 FR-001): no client config file is opened, so simply +// viewing status raises no macOS App-Data privacy prompt. AccessState is left as +// "unknown" and Connected stays false for installed clients until an explicit +// per-client read via GetStatus. func (s *Service) GetAllStatus() []ClientStatus { clients := GetAllClients() statuses := make([]ClientStatus, 0, len(clients)) @@ -116,34 +179,91 @@ func (s *Service) GetAllStatus() []ClientStatus { for _, c := range clients { cfgPath := ConfigPath(c.ID, s.homeDir) status := ClientStatus{ - ID: c.ID, - Name: c.Name, - ConfigPath: cfgPath, - Supported: c.Supported, - Reason: c.Reason, - Note: c.Note, - Bridge: c.Bridge, - Icon: c.Icon, + ID: c.ID, + Name: c.Name, + ConfigPath: cfgPath, + Supported: c.Supported, + Reason: c.Reason, + Note: c.Note, + Bridge: c.Bridge, + Icon: c.Icon, + AccessState: accessUnknown, } + // Metadata-only existence check (no content read). if _, err := os.Stat(cfgPath); err == nil { status.Exists = true } - // Check if mcpproxy entry exists in the config - if status.Exists && c.Supported { - if name, found := s.findEntry(c, cfgPath); found { - status.Connected = true - status.ServerName = name - } - } - statuses = append(statuses, status) } return statuses } +// GetStatus returns the status for a single client, reading its config contents +// on demand (Spec 075 FR-002). This is the scoped, explicit-action path where a +// macOS App-Data prompt may legitimately appear. It resolves Connected and +// AccessState (accessible/absent/malformed; "denied" is added in US2). +func (s *Service) GetStatus(clientID string) (ClientStatus, error) { + c := FindClient(clientID) + if c == nil { + return ClientStatus{}, fmt.Errorf("unknown client: %s", clientID) + } + + cfgPath := ConfigPath(c.ID, s.homeDir) + status := ClientStatus{ + ID: c.ID, + Name: c.Name, + ConfigPath: cfgPath, + Supported: c.Supported, + Reason: c.Reason, + Note: c.Note, + Bridge: c.Bridge, + Icon: c.Icon, + AccessState: accessUnknown, + } + + if _, err := os.Stat(cfgPath); err == nil { + status.Exists = true + } + if !status.Exists { + status.AccessState = accessAbsent + return status, nil + } + if !c.Supported { + return status, nil + } + + name, found, outcome := s.entryAccess(*c, cfgPath) + status.AccessState = outcome + if outcome == accessAccessible && found { + status.Connected = true + status.ServerName = name + } + return status, nil +} + +// entryAccess reads the client config exactly once via the seam, then reports +// the registered server name (if any), whether mcpproxy is connected, and the +// access outcome. US1 classifies accessible/absent/malformed; US2 refines a +// permission denial (currently surfaced as accessUnknown) into accessDenied. +func (s *Service) entryAccess(client ClientDef, cfgPath string) (name string, found bool, outcome string) { + raw, err := s.read(cfgPath) + if err != nil { + if errors.Is(err, fs.ErrNotExist) { + return "", false, accessAbsent + } + // US1: unclassified read error. US2 maps fs.ErrPermission -> denied. + return "", false, accessUnknown + } + name, found, parsedOK := s.findEntryFromBytes(client, raw) + if !parsedOK { + return "", false, accessMalformed + } + return name, found, accessAccessible +} + // Connect registers MCPProxy in the specified client's configuration file. // serverName defaults to "mcpproxy" if empty. If force is false and an entry // already exists, an error is returned. @@ -208,7 +328,7 @@ func (s *Service) Disconnect(clientID, serverName string) (*ConnectResult, error // connectJSON adds or updates the mcpproxy entry in a JSON config file. func (s *Service) connectJSON(client *ClientDef, cfgPath, serverName, mcpURL string, force bool) (*ConnectResult, error) { // Read existing config or start fresh - data, perm, err := readOrCreateJSON(cfgPath) + data, perm, err := s.readOrCreateJSON(cfgPath) if err != nil { return nil, err } @@ -274,7 +394,7 @@ func (s *Service) connectJSON(client *ClientDef, cfgPath, serverName, mcpURL str } // Verify by re-reading - if err := verifyJSONEntry(cfgPath, serversKey, serverName); err != nil { + if err := s.verifyJSONEntry(cfgPath, serversKey, serverName); err != nil { return nil, fmt.Errorf("verification failed: %w", err) } @@ -291,7 +411,7 @@ func (s *Service) connectJSON(client *ClientDef, cfgPath, serverName, mcpURL str // disconnectJSON removes the mcpproxy entry from a JSON config file. func (s *Service) disconnectJSON(client *ClientDef, cfgPath, serverName string) (*ConnectResult, error) { - raw, err := os.ReadFile(cfgPath) + raw, err := s.read(cfgPath) if os.IsNotExist(err) { return &ConnectResult{ Success: false, @@ -374,7 +494,7 @@ func (s *Service) disconnectJSON(client *ClientDef, cfgPath, serverName string) // connectTOML adds or updates the mcpproxy entry in a TOML config file (Codex). func (s *Service) connectTOML(client *ClientDef, cfgPath, serverName, mcpURL string, force bool) (*ConnectResult, error) { - data, perm, err := readOrCreateTOML(cfgPath) + data, perm, err := s.readOrCreateTOML(cfgPath) if err != nil { return nil, err } @@ -441,7 +561,7 @@ func (s *Service) connectTOML(client *ClientDef, cfgPath, serverName, mcpURL str // disconnectTOML removes the mcpproxy entry from a TOML config file. func (s *Service) disconnectTOML(client *ClientDef, cfgPath, serverName string) (*ConnectResult, error) { - raw, err := os.ReadFile(cfgPath) + raw, err := s.read(cfgPath) if os.IsNotExist(err) { return &ConnectResult{ Success: false, @@ -528,10 +648,10 @@ func (s *Service) disconnectTOML(client *ClientDef, cfgPath, serverName string) // readOrCreateJSON reads a JSON config file, or returns an empty map with default permissions // if the file does not exist. -func readOrCreateJSON(path string) (map[string]interface{}, os.FileMode, error) { +func (s *Service) readOrCreateJSON(path string) (map[string]interface{}, os.FileMode, error) { perm := os.FileMode(0o644) - raw, err := os.ReadFile(path) + raw, err := s.read(path) if os.IsNotExist(err) { return make(map[string]interface{}), perm, nil } @@ -553,10 +673,10 @@ func readOrCreateJSON(path string) (map[string]interface{}, os.FileMode, error) } // readOrCreateTOML reads a TOML config file, or returns an empty map with default permissions. -func readOrCreateTOML(path string) (map[string]interface{}, os.FileMode, error) { +func (s *Service) readOrCreateTOML(path string) (map[string]interface{}, os.FileMode, error) { perm := os.FileMode(0o644) - raw, err := os.ReadFile(path) + raw, err := s.read(path) if os.IsNotExist(err) { return make(map[string]interface{}), perm, nil } @@ -588,8 +708,8 @@ func marshalJSONIndent(data interface{}) ([]byte, error) { } // verifyJSONEntry re-reads the config file and checks that the expected entry exists. -func verifyJSONEntry(path, serversKey, serverName string) error { - raw, err := os.ReadFile(path) +func (s *Service) verifyJSONEntry(path, serversKey, serverName string) error { + raw, err := s.read(path) if err != nil { return fmt.Errorf("re-read %s: %w", path, err) } @@ -630,30 +750,29 @@ func findEquivalentJSONServerName(serversMap map[string]interface{}, mcpURL, req return "", false } -// findEntry checks whether a config file contains an mcpproxy-like entry. -// It returns the server name and true if found. -func (s *Service) findEntry(client ClientDef, cfgPath string) (string, bool) { +// findEntryFromBytes checks whether already-read config bytes contain an +// mcpproxy-like entry. It returns the server name, whether it was found, and +// whether the bytes parsed successfully (parsedOK=false => malformed). All +// content reads route through s.read (Spec 075 T010); this function never +// touches the filesystem. +func (s *Service) findEntryFromBytes(client ClientDef, raw []byte) (name string, found, parsedOK bool) { if client.Format == "toml" { - return s.findEntryTOML(cfgPath) + return s.findEntryTOMLBytes(raw) } - return s.findEntryJSON(client, cfgPath) + return s.findEntryJSONBytes(client, raw) } -// findEntryJSON looks for an entry in a JSON config that points to our MCP URL. -func (s *Service) findEntryJSON(client ClientDef, cfgPath string) (string, bool) { - raw, err := os.ReadFile(cfgPath) - if err != nil { - return "", false - } - +// findEntryJSONBytes parses JSON config bytes and looks for an entry that points +// to our MCP URL. +func (s *Service) findEntryJSONBytes(client ClientDef, raw []byte) (name string, found, parsedOK bool) { var data map[string]interface{} if err := unmarshalLenientJSON(raw, &data); err != nil { - return "", false + return "", false, false } serversMap, ok := data[client.ServerKey].(map[string]interface{}) if !ok { - return "", false + return "", false, true } mcpURL := s.mcpURL() @@ -669,7 +788,7 @@ func (s *Service) findEntryJSON(client ClientDef, cfgPath string) (string, bool) for _, field := range []string{"url", "serverUrl", "httpUrl"} { if u, ok := entry[field].(string); ok { if u == mcpURL || u == baseURL || strings.HasPrefix(u, baseURL+"?") { - return name, true + return name, true, true } } } @@ -678,16 +797,16 @@ func (s *Service) findEntryJSON(client ClientDef, cfgPath string) (string, bool) // mcpproxy endpoint lives in the command args. Detect by inspecting // args so a bridge written under a custom server name is still found. if entryPointsToBridge(entry, mcpURL, baseURL) { - return name, true + return name, true, true } // Also match by server name if name == defaultServerName { - return name, true + return name, true, true } } - return "", false + return "", false, true } // entryPointsToBridge reports whether a JSON config entry is an mcp-remote @@ -724,26 +843,22 @@ func unmarshalLenientJSON(raw []byte, out interface{}) error { return json.Unmarshal(cleaned, out) } -// findEntryTOML looks for an entry in a TOML config that points to our MCP URL. -func (s *Service) findEntryTOML(cfgPath string) (string, bool) { - raw, err := os.ReadFile(cfgPath) - if err != nil { - return "", false - } - +// findEntryTOMLBytes parses TOML config bytes and looks for an entry that points +// to our MCP URL. +func (s *Service) findEntryTOMLBytes(raw []byte) (name string, found, parsedOK bool) { var data map[string]interface{} if _, err := toml.Decode(string(raw), &data); err != nil { - return "", false + return "", false, false } serversRaw, ok := data["mcp_servers"] if !ok { - return "", false + return "", false, true } serversMap, ok := serversRaw.(map[string]interface{}) if !ok { - return "", false + return "", false, true } mcpURL := s.mcpURL() @@ -756,13 +871,13 @@ func (s *Service) findEntryTOML(cfgPath string) (string, bool) { } if u, ok := entry["url"].(string); ok { if u == mcpURL || u == baseURL || strings.HasPrefix(u, baseURL+"?") { - return name, true + return name, true, true } } if name == defaultServerName { - return name, true + return name, true, true } } - return "", false + return "", false, true } diff --git a/internal/connect/connect_test.go b/internal/connect/connect_test.go index 1ddb7d786..971cd9cab 100644 --- a/internal/connect/connect_test.go +++ b/internal/connect/connect_test.go @@ -875,15 +875,14 @@ func TestGetAllStatus_ClaudeDesktop_DetectsBridgeConnection(t *testing.T) { t.Fatal(err) } - for _, s := range svc.GetAllStatus() { - if s.ID == "claude-desktop" { - if !s.Connected { - t.Error("expected claude-desktop connected=true after bridge connect") - } - return - } + // Spec 075: bridge-connected detection is now resolved on demand. + st, err := svc.GetStatus("claude-desktop") + if err != nil { + t.Fatalf("GetStatus: %v", err) + } + if !st.Connected { + t.Error("expected claude-desktop connected=true after bridge connect") } - t.Fatal("claude-desktop status not found") } // Gap 2 (Codex review): a bridge written under a custom server_name has no @@ -900,18 +899,17 @@ func TestGetAllStatus_ClaudeDesktop_DetectsBridgeUnderCustomName(t *testing.T) { t.Fatal(err) } - for _, s := range svc.GetAllStatus() { - if s.ID == "claude-desktop" { - if !s.Connected { - t.Error("expected claude-desktop connected via bridge args under a custom name") - } - if s.ServerName != "my-bridge" { - t.Errorf("expected server_name=my-bridge, got %q", s.ServerName) - } - return - } + // Spec 075: resolved on demand rather than from the overall listing. + st, err := svc.GetStatus("claude-desktop") + if err != nil { + t.Fatalf("GetStatus: %v", err) + } + if !st.Connected { + t.Error("expected claude-desktop connected via bridge args under a custom name") + } + if st.ServerName != "my-bridge" { + t.Errorf("expected server_name=my-bridge, got %q", st.ServerName) } - t.Fatal("claude-desktop status not found") } func TestConnect_UnknownClient(t *testing.T) { @@ -988,18 +986,34 @@ func TestGetAllStatus_AfterConnect(t *testing.T) { t.Fatal(err) } - statuses := svc.GetAllStatus() - for _, s := range statuses { + // Spec 075: GetAllStatus is content-read-free, so Connected is resolved via + // the on-demand GetStatus path rather than the overall listing. + overall := svc.GetAllStatus() + for _, s := range overall { if s.ID == "claude-code" { if !s.Exists { t.Error("Expected exists=true for claude-code after connect") } - if !s.Connected { - t.Error("Expected connected=true for claude-code after connect") + if s.Connected { + t.Error("Expected overall status Connected=false (content-read-free) for claude-code") + } + if s.AccessState != accessUnknown { + t.Errorf("Expected overall access_state=unknown, got %q", s.AccessState) } break } } + + st, err := svc.GetStatus("claude-code") + if err != nil { + t.Fatalf("GetStatus: %v", err) + } + if !st.Exists { + t.Error("Expected exists=true for claude-code after connect") + } + if !st.Connected { + t.Error("Expected connected=true for claude-code after connect (on-demand)") + } } // ---------- Backup utility tests ---------- diff --git a/internal/connect/status_ondemand_test.go b/internal/connect/status_ondemand_test.go new file mode 100644 index 000000000..a0c572e02 --- /dev/null +++ b/internal/connect/status_ondemand_test.go @@ -0,0 +1,154 @@ +package connect + +import ( + "fmt" + "os" + "path/filepath" + "testing" +) + +// TestGetAllStatus_NoContentReads asserts that computing the overall Connect +// status determines installed-state via metadata only and never reads any +// client config file's contents (FR-001, SC-001). The injected reader fails +// the test if invoked. +func TestGetAllStatus_NoContentReads(t *testing.T) { + svc, homeDir := testService(t) + + // Make a couple of clients appear "installed" by creating their config files. + installed := []string{"claude-code", "cursor"} + for _, id := range installed { + cfgPath := ConfigPath(id, homeDir) + if cfgPath == "" { + t.Fatalf("no config path for %s", id) + } + if err := os.MkdirAll(filepath.Dir(cfgPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cfgPath, []byte(`{"mcpServers":{}}`), 0o644); err != nil { + t.Fatal(err) + } + } + + // Any content read during overall status is a contract violation. + svc.setReadFile(func(path string) ([]byte, error) { + t.Errorf("GetAllStatus must not read config contents, but read %s", path) + return nil, fmt.Errorf("unexpected content read of %s", path) + }) + + statuses := svc.GetAllStatus() + + seen := map[string]bool{} + for _, st := range statuses { + seen[st.ID] = true + if st.ID == "claude-code" || st.ID == "cursor" { + if !st.Exists { + t.Errorf("%s: expected Exists=true via metadata", st.ID) + } + // Overall status must not claim connected without a content read. + if st.Connected { + t.Errorf("%s: Connected must be false in overall (content-read-free) status", st.ID) + } + if st.AccessState != accessUnknown { + t.Errorf("%s: expected access_state=%q, got %q", st.ID, accessUnknown, st.AccessState) + } + } + } + for _, id := range installed { + if !seen[id] { + t.Errorf("expected %s in overall status", id) + } + } +} + +// TestGetStatus_ReadsSingleClientOnDemand asserts that GetStatus(id) performs +// exactly one content read for the requested client and resolves Connected + +// AccessState (FR-002, SC-001 independent test). +func TestGetStatus_ReadsSingleClientOnDemand(t *testing.T) { + svc, _ := testService(t) + + // Register mcpproxy in claude-code so its config is "connected". + if _, err := svc.Connect("claude-code", "", false); err != nil { + t.Fatal(err) + } + + // Count reads only from this point on (Connect's own reads are excluded). + var reads int + var readPaths []string + realRead := os.ReadFile + svc.setReadFile(func(path string) ([]byte, error) { + reads++ + readPaths = append(readPaths, path) + return realRead(path) + }) + + st, err := svc.GetStatus("claude-code") + if err != nil { + t.Fatalf("GetStatus: %v", err) + } + if reads != 1 { + t.Errorf("expected exactly 1 content read, got %d (%v)", reads, readPaths) + } + if !st.Exists { + t.Error("expected Exists=true") + } + if !st.Connected { + t.Error("expected Connected=true after connect") + } + if st.AccessState != accessAccessible { + t.Errorf("expected access_state=%q, got %q", accessAccessible, st.AccessState) + } +} + +// TestGetStatus_AbsentClient resolves to absent without a content read failure. +func TestGetStatus_AbsentClient(t *testing.T) { + svc, _ := testService(t) + + st, err := svc.GetStatus("claude-code") + if err != nil { + t.Fatalf("GetStatus: %v", err) + } + if st.Exists { + t.Error("expected Exists=false for a client with no config") + } + if st.Connected { + t.Error("expected Connected=false for absent client") + } + if st.AccessState != accessAbsent { + t.Errorf("expected access_state=%q, got %q", accessAbsent, st.AccessState) + } +} + +// TestGetStatus_Malformed reports malformed when the config cannot be parsed. +func TestGetStatus_Malformed(t *testing.T) { + svc, homeDir := testService(t) + + cfgPath := ConfigPath("claude-code", homeDir) + if err := os.MkdirAll(filepath.Dir(cfgPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cfgPath, []byte(`{ this is not valid json `), 0o644); err != nil { + t.Fatal(err) + } + + st, err := svc.GetStatus("claude-code") + if err != nil { + t.Fatalf("GetStatus: %v", err) + } + if !st.Exists { + t.Error("expected Exists=true for present-but-malformed config") + } + if st.Connected { + t.Error("expected Connected=false for malformed config") + } + if st.AccessState != accessMalformed { + t.Errorf("expected access_state=%q, got %q", accessMalformed, st.AccessState) + } +} + +// TestGetStatus_UnknownClient errors for an unknown client id. +func TestGetStatus_UnknownClient(t *testing.T) { + svc, _ := testService(t) + if _, err := svc.GetStatus("does-not-exist"); err == nil { + t.Error("expected error for unknown client") + } +} diff --git a/specs/075-macos-tcc-connect/checklists/requirements.md b/specs/075-macos-tcc-connect/checklists/requirements.md new file mode 100644 index 000000000..6d203f5bc --- /dev/null +++ b/specs/075-macos-tcc-connect/checklists/requirements.md @@ -0,0 +1,36 @@ +# Specification Quality Checklist: macOS TCC-safe Connect wizard & App-Data denial diagnostics + +**Purpose**: Validate specification completeness and quality before proceeding to planning +**Created**: 2026-06-17 +**Feature**: [spec.md](../spec.md) + +## Content Quality + +- [x] No implementation details (languages, frameworks, APIs) +- [x] Focused on user value and business needs +- [x] Written for non-technical stakeholders +- [x] All mandatory sections completed + +## Requirement Completeness + +- [x] No [NEEDS CLARIFICATION] markers remain +- [x] Requirements are testable and unambiguous +- [x] Success criteria are measurable +- [x] Success criteria are technology-agnostic (no implementation details) +- [x] All acceptance scenarios are defined +- [x] Edge cases are identified +- [x] Scope is clearly bounded +- [x] Dependencies and assumptions identified + +## Feature Readiness + +- [x] All functional requirements have clear acceptance criteria +- [x] User scenarios cover primary flows +- [x] Feature meets measurable outcomes defined in Success Criteria +- [x] No implementation details leak into specification + +## Notes + +- The spec deliberately keeps "stat/metadata vs content-read" framed as user-observable behavior (no privacy prompt on status) rather than naming syscalls, satisfying the technology-agnostic bar while preserving the testable distinction. +- FR-011 references the OS permission-denied signal generically; the concrete error class is left to planning. +- No outstanding clarifications; informed assumptions recorded in the Assumptions section. diff --git a/specs/075-macos-tcc-connect/contracts/connect-status.md b/specs/075-macos-tcc-connect/contracts/connect-status.md new file mode 100644 index 000000000..8955ec049 --- /dev/null +++ b/specs/075-macos-tcc-connect/contracts/connect-status.md @@ -0,0 +1,65 @@ +# Contract: Connect status REST payload (delta) + +Endpoints are UNCHANGED in path/method/auth. Only the per-client status object +gains additive fields. Existing fields and their meaning are preserved (FR-006). + +## GET /api/v1/connect + +Returns the list of client statuses. + +### Before (per-client object — preserved) + +```json +{ + "id": "claude-desktop", + "name": "Claude Desktop", + "config_path": "/Users/x/Library/Application Support/Claude/claude_desktop_config.json", + "exists": true, + "connected": true, + "supported": true, + "icon": "…", + "server_name": "mcpproxy" +} +``` + +### After (additive — `access_state`, `remediation`) + +```json +{ + "id": "claude-desktop", + "name": "Claude Desktop", + "config_path": "…/claude_desktop_config.json", + "exists": true, + "connected": false, + "access_state": "unknown", + "supported": true, + "icon": "…" +} +``` + +Behavioral contract: +- Overall status performs **no content reads**: `exists` reflects `os.Stat`; `access_state` is `"unknown"` for installed clients; `connected` is `false` until a per-client check runs. +- `connected` is authoritative **only** when `access_state == "accessible"`. +- When a content access is blocked: `access_state == "denied"` and `remediation` is populated. +- Old consumers ignoring `access_state`/`remediation` keep working; `connected=false` + `exists=true` reads as "installed, not (yet) connected." + +## GET /api/v1/connect/{client} (per-client, on-demand — NEW or status-refresh semantics) + +> If a single-client status route does not already exist, it is introduced here; +> otherwise the existing per-client status path gains the on-demand content read. + +- Performs the single client's content read at request time (the only place a + macOS App-Data prompt may legitimately appear, scoped to user action). +- Response: full `ClientStatus` with resolved `access_state` (`accessible|absent|denied|malformed`), + `connected`, and `remediation` when denied. + +## POST /api/v1/connect/{client} and DELETE /api/v1/connect/{client} + +- Unchanged contract. On a permission-denied file access, the error response + MUST carry the remediation text (HTTP error body), distinct from a generic + failure or a "not found". + +## Non-goals + +- No change to request bodies, auth headers, or status codes for success paths. +- No removed/renamed fields. diff --git a/specs/075-macos-tcc-connect/data-model.md b/specs/075-macos-tcc-connect/data-model.md new file mode 100644 index 000000000..64bbca1bf --- /dev/null +++ b/specs/075-macos-tcc-connect/data-model.md @@ -0,0 +1,65 @@ +# Data Model: macOS TCC-safe Connect wizard & App-Data denial diagnostics + +No persistent storage changes. These are in-memory/DTO shapes. + +## AccessOutcome (new enum) + +Classification of an attempt to access (read or write) a client config file. + +| Value | Meaning | Derivation | +|-------|---------|------------| +| `accessible` | File read/written successfully | no error | +| `absent` | File does not exist → client "not installed" | `errors.Is(err, fs.ErrNotExist)` | +| `denied` | Blocked by OS permission (macOS TCC App Data) | `errors.Is(err, fs.ErrPermission)` | +| `malformed` | Read OK but contents unparseable | parse error after a successful read | + +Rules: +- Classification MUST come from the error class, never from substring matching (FR-011). +- `accessible` is the only outcome that yields a trustworthy `Connected` value. + +## ClientStatus (extended — additive only) + +Existing fields preserved exactly (FR-006). New fields: + +| Field | JSON | Type | Notes | +|-------|------|------|-------| +| *(existing)* `Exists` | `exists` | bool | Set via `os.Stat` (metadata) — unchanged | +| *(existing)* `Connected` | `connected` | bool | Now only authoritative when `AccessState == "accessible"`; remains `false` when `unknown` | +| **AccessState** | `access_state` | string | `""`/`unknown` (overall status, not content-checked), `accessible`, `absent`, `denied`, `malformed` | +| **Remediation** | `remediation,omitempty` | string | Present only when `AccessState == denied`; the macOS App-Data fix text | + +State transitions for `AccessState`: +- Overall `GetAllStatus`: every installed client → `unknown` (no content read). Absent clients → `Exists=false` (`AccessState` may be `absent` or left empty). +- `GetStatus(id)` / connect / disconnect: performs the content access → resolves to `accessible | absent | denied | malformed`. + +## AccessError (new typed error) + +Returned by connect/disconnect when the underlying file access is `denied`. + +| Field | Type | Notes | +|-------|------|-------| +| Client | string | client id/name | +| Path | string | config path attempted | +| Outcome | AccessOutcome | `denied` (could also wrap `malformed`) | +| Remediation | string | actionable fix text | + +- `Error()` returns a human-readable message including cause + remediation. +- `errors.Is`/`errors.As` friendly so the REST layer can map it to the right field. + +## DoctorCheckResult (uses existing diagnostics result shape) + +| Field | Type | Notes | +|-------|------|-------| +| Status | pass/warn | warn when a persisted App-Data denial is detected | +| Summary | string | "macOS blocked mcpproxy from reading other apps' data" | +| Remediation | string | `tccutil reset SystemPolicyAppData com.smartmcpproxy.mcpproxy` + Settings path | +| Platform gate | — | darwin-only; no-op (not emitted) on other platforms | + +## Remediation text (canonical) + +``` +macOS blocked mcpproxy from reading 's configuration (Privacy & Security ▸ App Data). +Fix: System Settings ▸ Privacy & Security ▸ App Data ▸ enable mcpproxy, +or run: tccutil reset SystemPolicyAppData com.smartmcpproxy.mcpproxy +(dev builds: com.smartmcpproxy.mcpproxy.dev) +``` diff --git a/specs/075-macos-tcc-connect/plan.md b/specs/075-macos-tcc-connect/plan.md new file mode 100644 index 000000000..10a84539b --- /dev/null +++ b/specs/075-macos-tcc-connect/plan.md @@ -0,0 +1,83 @@ +# Implementation Plan: macOS TCC-safe Connect wizard & App-Data denial diagnostics + +**Branch**: `075-macos-tcc-connect` | **Date**: 2026-06-17 | **Spec**: [spec.md](./spec.md) +**Input**: Feature specification from `/specs/075-macos-tcc-connect/spec.md` + +## Summary + +The Connect feature eagerly reads the **contents** of every installed MCP client config in `Service.GetAllStatus()` (`internal/connect/connect.go:135` → `findEntry` → `os.ReadFile`) to compute the `Connected` flag. On macOS 14+ this fires the "App Data" privacy prompt ("wants to access data from other apps") for every client, and a denial makes the reads fail silently forever. + +Approach: +1. **Stat-only overall status**: `GetAllStatus` keeps using `os.Stat` for `Exists` but stops calling `findEntry`. `Connected` becomes a tri-state (`unknown` until explicitly checked). Add `GetStatus(clientID)` that performs the single-client content read on demand. +2. **Access-outcome classification**: a small helper classifies a config access into `accessible | absent | denied | malformed`, derived from `errors.Is(err, fs.ErrPermission)` / `syscall.EPERM`/`EACCES` (not string matching). Reads/writes in `connect`/`disconnect` and `findEntry` route through it. +3. **Actionable surfacing**: denied outcomes populate a new `AccessState` + `Remediation` field on `ClientStatus` and produce a typed error from connect/disconnect carrying the remediation string (System Settings path + `tccutil reset SystemPolicyAppData `). +4. **Doctor check**: a macOS-only diagnostics check that detects a persisted App-Data denial affecting Connect and reports remediation; no-op on other platforms. + +All TDD; the denied path is tested by injecting a permission error through a seam (a `configReader` func var / interface), no real OS denial required. + +## Technical Context + +**Language/Version**: Go 1.24 (toolchain go1.24.10) +**Primary Dependencies**: stdlib (`os`, `io/fs`, `errors`, `syscall`, `runtime`), `BurntSushi/toml` (existing), Cobra (existing, doctor command), Chi (existing, Connect REST). No new deps. +**Storage**: None new. Reads/writes existing on-disk client config files; no BBolt schema change. +**Testing**: `go test` unit tests in `internal/connect` and the diagnostics package; permission-denied path via injected reader/seam; existing Connect REST contract tests must still pass. +**Target Platform**: macOS 13+ (primary for the privacy behavior), Linux, Windows (behavior preserved). +**Project Type**: Single Go module (core server). Optional FE follow-up to render the new tri-state, tracked but not required for the backend MVP. +**Performance Goals**: Overall status must do **zero** content reads (only metadata) — strictly faster than today. +**Constraints**: Backward-compatible Connect REST payload (add fields only). Classification must use OS error class, not text. macOS check must be a no-op elsewhere. +**Scale/Scope**: ~8 supported clients; status called on Connect page load / wizard predicate (`GetConnectedCount`). + +## Constitution Check + +*GATE: Must pass before Phase 0 research. Re-check after Phase 1 design.* + +- **I. Performance at Scale**: PASS — change strictly reduces I/O on the status path (removes per-client content reads). No hot-path regression. +- **II. Actor-Based Concurrency**: PASS — no new concurrency; pure function changes + on-demand reads. No new locks. +- **III. Configuration-Driven Architecture**: PASS — no new config keys required; behavior is automatic. Tray remains a UI controller reading core REST. +- **IV. Security by Default**: PASS / reinforces — reduces unnecessary access to other apps' data (least privilege); no weakening of quarantine/isolation. The Docker probe (security-relevant) is untouched. +- **V. Test-Driven Development**: PASS — every change is red→green; denied path injected via seam; REST contract tests preserved. +- **VI. Documentation Hygiene**: PASS — docs updated (CLAUDE.md REST notes if payload fields added; a docs note on the macOS App-Data prompt + remediation). +- **Upstream Client Modularity / DDD layering**: N/A to `internal/connect`, which is a self-contained adapter package; change stays within it + the diagnostics package. + +**Result**: No violations. Complexity Tracking not required. + +## Project Structure + +### Documentation (this feature) + +```text +specs/075-macos-tcc-connect/ +├── plan.md # This file +├── research.md # Phase 0 output +├── data-model.md # Phase 1 output +├── quickstart.md # Phase 1 output +├── contracts/ # Phase 1 output (REST status payload delta) +│ └── connect-status.md +└── tasks.md # Phase 2 (/speckit.tasks) +``` + +### Source Code (repository root) + +```text +internal/connect/ +├── connect.go # GetAllStatus (stat-only), new GetStatus(clientID), connect/disconnect route through classifier +├── access.go # NEW: AccessOutcome classification (accessible/absent/denied/malformed) + remediation text +├── access_test.go # NEW: classification unit tests incl. injected EPERM +├── connect_test.go # extend: GetAllStatus does no content reads; GetStatus on-demand; denied surfacing +├── clients.go # unchanged (path table); bundle IDs constant may live here or access.go +└── backup.go # route os.Open through classifier so backup denial surfaces too + +internal/diagnostics/ # (or internal/management/diagnostics.go — confirm in tasks) +├── tcc_appdata_darwin.go # NEW: macOS App-Data denial check +├── tcc_appdata_other.go # NEW: no-op build-tagged stub for !darwin +└── tcc_appdata_test.go # NEW: check reports warn/pass; no-op off darwin + +internal/httpapi/ +└── connect.go # GET /connect unchanged contract; map new ClientStatus fields into response +``` + +**Structure Decision**: Single Go module. All Connect logic stays inside the self-contained `internal/connect` adapter package; the doctor check lands in the existing diagnostics registry (exact package pinned during tasks). The REST handler (`internal/httpapi/connect.go`) only widens the JSON it maps from `ClientStatus`. An optional frontend follow-up (render tri-state + remediation banner) is noted but not part of the backend MVP and can ship separately. + +## Complexity Tracking + +> No constitution violations — section intentionally empty. diff --git a/specs/075-macos-tcc-connect/quickstart.md b/specs/075-macos-tcc-connect/quickstart.md new file mode 100644 index 000000000..2d5f3be0d --- /dev/null +++ b/specs/075-macos-tcc-connect/quickstart.md @@ -0,0 +1,47 @@ +# Quickstart: macOS TCC-safe Connect wizard & App-Data denial diagnostics + +## What changes for a user + +- Opening the Connect page on macOS **no longer triggers** a "wants to access data from other apps" prompt for every installed MCP client. Status shows which clients are installed (file present) without reading their contents. +- A client's "connected to mcpproxy?" detail is computed **when you act on that client** (open its detail / connect / disconnect). That is the only moment macOS may prompt — for that one app, in context. +- If macOS has blocked access, you get a clear message with the exact fix instead of a silent "not connected." + +## Recover a wrong "Don't Allow" + +```bash +# Reset the App Data decision for mcpproxy (release build): +tccutil reset SystemPolicyAppData com.smartmcpproxy.mcpproxy +# Dev build: +tccutil reset SystemPolicyAppData com.smartmcpproxy.mcpproxy.dev +# Or: System Settings ▸ Privacy & Security ▸ App Data ▸ enable mcpproxy +``` + +## Verify (developer) + +```bash +# Unit tests — status does no content reads; on-demand read; denied surfacing; classifier. +go test ./internal/connect/ -run 'Status|Access|Denied|Connect' -v + +# Doctor check (macOS-only logic; no-op build on Linux/Windows). +go test ./internal/diagnostics/... -run 'TCC|AppData' -v # (package pinned in tasks) + +# Doctor end-to-end: +go build -o mcpproxy ./cmd/mcpproxy +./mcpproxy doctor # On macOS with a denial present: warns + prints tccutil command. + +# Existing Connect REST contract still green: +./scripts/test-api-e2e.sh +``` + +## Manual macOS check (optional) + +1. With several MCP clients installed, open the Connect page → confirm NO privacy prompt appears. +2. Click a single client's connect → a prompt may appear for that one app; allow it → connects. +3. Deny it → UI shows the remediation banner; `./mcpproxy doctor` flags it. + +## Acceptance mapping + +- US1/SC-001/SC-002 → `internal/connect` status test asserts zero content reads. +- US2/SC-003/SC-004 → classifier + surfacing tests (accessible/absent/denied/malformed). +- US3/SC-005 → doctor check tests (warn/pass on darwin, no-op elsewhere). +- SC-006 → existing Connect REST tests + `test-api-e2e.sh`. diff --git a/specs/075-macos-tcc-connect/research.md b/specs/075-macos-tcc-connect/research.md new file mode 100644 index 000000000..1f03cc06a --- /dev/null +++ b/specs/075-macos-tcc-connect/research.md @@ -0,0 +1,43 @@ +# Research: macOS TCC-safe Connect wizard & App-Data denial diagnostics + +## R1 — What triggers the "wants to access data from other apps" dialog + +- **Decision**: Treat the prompt as macOS TCC **App Data** (`kTCCServiceSystemPolicyAppData`, Sonoma 14+). It gates **content reads** (`open`+`read` / `file-read-data`), **not** metadata (`stat`/`lstat`/`access`). +- **Rationale**: Confirmed by multiple sources (eclecticlight.co container articles; mjtsai "TCC doesn't prevent protected folders from being listed" — *"denies file-read-data … does not block anything else such as metadata, which is what lets us stat these files"*). Therefore `os.Stat` never prompts; `os.ReadFile`/`os.Open` of another app's data does. +- **Alternatives considered**: Full Disk Access (heavy, user-granted, rejected as default), new entitlement (none exists for reading another app's data; rejected), App Group entitlement (only for legitimate shared group containers; N/A). + +## R2 — Which mcpproxy code actually triggers it + +- **Decision**: The Connect wizard is the sole trigger. `Service.GetAllStatus()` (`internal/connect/connect.go:135`) calls `findEntry` → `os.ReadFile` for **every** installed+supported client to compute `Connected`, on every status request. The Docker probe is metadata-only (`os.Stat`) and is NOT a trigger. +- **Rationale**: Code audit found `os.ReadFile`/`os.Open` on other apps' configs only in `internal/connect` (connect.go:294/444/534/559/644/729, backup.go:25). Docker resolution (`shellwrap.probeWellKnownDocker`, `secureenv.discoverUnixPaths`) is `os.Stat` only. +- **Alternatives considered**: login-shell hydration sourcing rc files (indirect, low likelihood); ruled out as the primary cause. + +## R3 — Detecting a permission denial robustly in Go + +- **Decision**: Classify via `errors.Is(err, fs.ErrPermission)` (covers `EPERM` and `EACCES`, which both map to `fs.ErrPermission` in the stdlib). Optionally tighten to `errors.Is(err, syscall.EPERM)` for the TCC-specific case, but `fs.ErrPermission` is the portable, idiomatic gate. +- **Rationale**: Constitution + FR-011 forbid string-matching error text. `os.Open`/`os.ReadFile` return `*fs.PathError` wrapping the syscall errno; `errors.Is(err, fs.ErrPermission)` is the documented way to test it. TCC content denial surfaces as `EPERM` ("operation not permitted"). +- **Alternatives considered**: string match on "operation not permitted" (rejected — brittle, FR-011); shelling out to read TCC.db (SIP-protected, rejected). + +## R4 — `Connected` tri-state without breaking the REST contract + +- **Decision**: Keep `Connected bool` (defaults false) for backward compatibility and ADD an `AccessState string` enum field (`""`=unknown/not-checked, `accessible`, `absent`, `denied`, `malformed`) plus `Remediation string`. Overall `GetAllStatus` sets `Exists` (stat) and leaves `AccessState` empty/`unknown` (no content read). Per-client `GetStatus(id)` fills `Connected` + `AccessState` via the on-demand read. +- **Rationale**: FR-006 forbids removing/repurposing existing fields. Additive enum is safe; old clients ignore unknown fields. `Connected=false` with `AccessState=unknown` reads as "not yet checked," not "disconnected." +- **Alternatives considered**: changing `Connected` to a string (breaking, rejected); a separate endpoint only (still need the field to carry denied state). + +## R5 — Where the doctor check lives and how it detects a persisted denial + +- **Decision**: Add a macOS-only diagnostics check (build-tagged `_darwin.go` + `_other.go` no-op) in the existing diagnostics registry. Detection is **best-effort**: attempt a representative content read of a known-present client config that exists (via stat) but, if a read returns `fs.ErrPermission`, report a warning with the remediation command. Never warn when reads succeed or when no client config exists. +- **Rationale**: There is no SIP-free API to read the TCC database; inferring from an actual `EPERM` on a real protected path is the only reliable, false-positive-free signal. Build tags keep it a true no-op off darwin (FR-008). +- **Alternatives considered**: parsing `~/Library/Application Support/com.apple.TCC/TCC.db` (SIP-protected, would itself need FDA; rejected); calling `tccutil` (no read/query verb; rejected). + +## R6 — Remediation message content & bundle identifiers + +- **Decision**: Message = cause + two remediations: (a) System Settings → Privacy & Security → App Data → enable mcpproxy; (b) `tccutil reset SystemPolicyAppData `. Bundle IDs: release `com.smartmcpproxy.mcpproxy`, dev `com.smartmcpproxy.mcpproxy.dev` (from `native/macos/MCPProxy/MCPProxy/Info.plist`). Emit the release id by default and mention the dev suffix. +- **Rationale**: `tccutil reset [bundle-id]` is the documented reset; `SystemPolicyAppData` is the service token (kTCCService prefix dropped). +- **Alternatives considered**: omitting bundle id (resets all apps — broader than needed; keep as fallback note). + +## R7 — Testing the denied path without a real OS denial + +- **Decision**: Introduce a seam: a package-level `readConfigFile func(string) ([]byte, error)` (default `os.ReadFile`) or a small `configReader` the Service holds, so tests inject a func returning `&fs.PathError{Op:"open", Path:p, Err: syscall.EPERM}`. Classification + surfacing assert on the injected error. +- **Rationale**: FR-012 requires the denied path be testable without a real denial; matches the existing `homeDir` override and `wellKnownDockerPathsFn` seam patterns in the codebase. +- **Alternatives considered**: chmod 000 a temp file (root can still read; flaky in CI; rejected in favor of the injected error). diff --git a/specs/075-macos-tcc-connect/spec.md b/specs/075-macos-tcc-connect/spec.md new file mode 100644 index 000000000..1c65c069f --- /dev/null +++ b/specs/075-macos-tcc-connect/spec.md @@ -0,0 +1,152 @@ +# Feature Specification: macOS TCC-safe Connect wizard & App-Data denial diagnostics + +**Feature Branch**: `075-macos-tcc-connect` +**Created**: 2026-06-17 +**Status**: Draft +**Input**: User description: "macOS TCC-safe Connect wizard and App-Data denial diagnostics — make Connect status determine 'installed' via os.Stat metadata only, defer config content-reads to explicit per-client action, surface EPERM/TCC denials with actionable remediation, and add a `mcpproxy doctor` check for persisted macOS App-Data TCC denial." + +## Context (why this exists) + +On macOS 14+ (Sonoma / Sequoia / Tahoe), the "App Data" privacy permission (`kTCCServiceSystemPolicyAppData`) shows the dialog **" wants to access data from other apps"** when an app *reads the content* of another app's data. It gates content reads (`open`+read), **not** metadata operations (`stat`/`access`). + +Today, the Connect feature — which links MCP clients (Claude Desktop, Cursor, VS Code, Codex, Gemini, OpenCode, etc.) to mcpproxy — reads the **full contents** of every installed client's config file every time the Connect status is requested, just to decide whether each client is already connected. On macOS this fires the privacy prompt for apps the user never asked to touch. If the user clicks **"Don't Allow"**, the decision is remembered by the OS: every future read fails silently, Connect shows everything as "not connected," and connect/disconnect actions fail without a clear reason. + +## User Scenarios & Testing *(mandatory)* + +### User Story 1 - Viewing Connect status without a privacy storm (Priority: P1) + +A macOS user opens the Connect page (or the status is polled). They want to see which MCP clients are installed and which are connected to mcpproxy, **without** macOS popping a "wants to access data from other apps" prompt for every client they have installed. + +**Why this priority**: This is the root cause of the reported problem and affects every macOS user with multiple MCP clients installed. It is the difference between Connect being usable out-of-the-box and Connect triggering a wall of privacy prompts (and silent breakage after a denial). + +**Independent Test**: Load Connect status on a machine with several client configs present and confirm that determining the "installed" state performs only metadata checks (no content reads), so no privacy prompt is raised by simply viewing status. + +**Acceptance Scenarios**: + +1. **Given** several MCP client config files exist on disk, **When** the user requests overall Connect status, **Then** each client is reported as installed/not-installed using only file-existence (metadata) checks and no client config content is read. +2. **Given** the user has not acted on any specific client, **When** Connect status is computed, **Then** no client config file is opened for reading. +3. **Given** a client is installed, **When** the user explicitly requests that one client's detailed status (or connects/disconnects it), **Then** that single client's config is read at that moment, scoped to the user's action. + +--- + +### User Story 2 - Clear, actionable message when macOS blocks access (Priority: P2) + +A user (who previously denied the prompt, or whose OS silently blocks the background process) tries to connect or refresh a specific client. The access is blocked by macOS. They want to understand *why* and *exactly how to fix it*, instead of a silent "not connected" or a confusing generic failure. + +**Why this priority**: Once access is denied, the feature is broken in a way that is invisible and undiagnosable to the user. Turning a silent failure into an actionable message is high value but only matters after Story 1 reduces how often the denial happens. + +**Independent Test**: Simulate a permission-denied error on a client config read/write and verify the surfaced status/error names the privacy cause and gives the precise remediation steps. + +**Acceptance Scenarios**: + +1. **Given** reading a client config is blocked by a macOS permission denial, **When** that client's status is computed or a connect/disconnect is attempted, **Then** the result is reported as a distinct "blocked by macOS privacy" state — not "not connected" and not a generic error. +2. **Given** a privacy-denied result, **When** the message is shown, **Then** it states the cause ("macOS blocked access to 's data") and the remediation (enable mcpproxy under System Settings → Privacy & Security → App Data, or the exact `tccutil reset` command). +3. **Given** a config file is simply absent, **When** status is computed, **Then** it is reported as "not installed" and is clearly distinguished from a privacy denial. +4. **Given** a config file exists but is malformed, **When** it is read on an explicit action, **Then** it is reported as "unreadable/malformed" and clearly distinguished from a privacy denial. + +--- + +### User Story 3 - Doctor flags a persisted privacy denial (Priority: P3) + +A user whose Connect feature mysteriously shows nothing runs the health-check command. They want it to tell them, in one line, that a macOS privacy denial is the cause and how to reset it. + +**Why this priority**: A diagnostic convenience that turns a confusing state into a one-command fix. Valuable, but the in-feature message (Story 2) already covers the primary path. + +**Independent Test**: Run the health check on macOS in a state representing a persisted App-Data denial and confirm it reports the issue with the remediation command; run it on a non-macOS platform and confirm it is a no-op. + +**Acceptance Scenarios**: + +1. **Given** the platform is macOS and a persisted App-Data denial affecting Connect is present, **When** the health check runs, **Then** it reports a warning naming the privacy denial and the exact remediation command. +2. **Given** the platform is not macOS, **When** the health check runs, **Then** the App-Data check is skipped (no-op) and does not appear as a failure. +3. **Given** macOS with no relevant denial, **When** the health check runs, **Then** the App-Data check passes (or is silent) and does not produce a false warning. + +--- + +### Edge Cases + +- A client config path is a symlink into a protected location (e.g. a sandbox container): existence is still determined by metadata only; content is only read on explicit action. +- The same status request covers a mix of: installed+readable, installed+denied, installed+malformed, and absent clients — each must be reported in its correct distinct state. +- The background (non-GUI) process cannot raise a prompt and is silently denied: the denied state must still be detectable and surfaced (no infinite silent failure). +- Connect/disconnect *writes* (not just reads) are blocked by the denial: the write failure must surface the same actionable privacy message. +- A denial that the user later resets externally: subsequent explicit actions must work again without restarting mcpproxy. +- Non-macOS platforms: behavior is unchanged; existence + on-demand reads remain functionally equivalent and no privacy-specific messaging appears. + +## Requirements *(mandatory)* + +### Functional Requirements + +- **FR-001**: Connect overall status MUST determine each client's "installed" state using file-existence/metadata checks only, and MUST NOT read any client config file's contents. +- **FR-002**: Reading a client's config contents (to detect whether mcpproxy is already configured in it) MUST occur only in response to an explicit, per-client user action (connect, disconnect, or an explicit single-client detailed-status request). +- **FR-003**: The system MUST classify a client-config access into distinct outcomes: accessible, absent (not installed), permission-denied (privacy), and malformed/unreadable. +- **FR-004**: When a client-config read or write fails due to a permission denial, the system MUST surface a distinct "blocked by macOS privacy" outcome rather than reporting "not connected" or a generic error. +- **FR-005**: The privacy-denied message MUST name the cause and provide remediation: enabling mcpproxy under System Settings → Privacy & Security → App Data, and the exact reset command including the relevant bundle identifier(s). +- **FR-006**: The existing Connect REST API surface (overall status, per-client connect, per-client disconnect) MUST remain backward compatible; the status payload MAY add fields but MUST NOT remove or repurpose existing ones. +- **FR-007**: The health-check command MUST include a macOS-only check that detects a persisted App-Data privacy denial affecting Connect and reports it with the remediation command. +- **FR-008**: The health-check App-Data check MUST be a no-op on non-macOS platforms and MUST NOT produce false warnings when no relevant denial exists. +- **FR-009**: Behavior on Linux and Windows MUST be functionally preserved (existence via metadata, content read on explicit action); no privacy-specific prompts or messaging apply there. +- **FR-010**: The Docker binary discovery probe MUST remain unchanged (it already uses metadata-only checks and is out of scope). +- **FR-011**: The privacy-denied classification MUST be derived from the operating system's permission-denied signal for a file access (not from string-matching arbitrary error text). +- **FR-012**: All new behavior MUST be covered by automated tests, including a test that injects a permission-denied error for a config access without requiring a real OS denial. + +### Key Entities *(include if feature involves data)* + +- **Client status**: Per MCP client — identity/name, installed (exists) flag, connected flag, and an access-state describing whether its config was readable, absent, privacy-denied, or malformed, plus any remediation hint. +- **Access outcome**: The classification of an attempt to read/write a client config — one of accessible, absent, permission-denied, malformed. +- **Doctor check result**: A health-check finding for the macOS App-Data denial — pass/warn, human-readable summary, and remediation command. + +## Success Criteria *(mandatory)* + +### Measurable Outcomes + +- **SC-001**: Viewing overall Connect status reads **zero** client config file contents (100% of installed/not-installed determinations come from metadata), verified by test. +- **SC-002**: On macOS, simply viewing Connect status raises **no** "access data from other apps" prompts for clients the user has not explicitly acted on. +- **SC-003**: 100% of permission-denied config accesses are reported as the distinct privacy state with remediation text, not as "not connected" or a generic error. +- **SC-004**: The four access outcomes (accessible, absent, privacy-denied, malformed) are each reported correctly and distinctly in tests covering all four. +- **SC-005**: The health-check App-Data check correctly reports denial-present (warn) and denial-absent (pass) on macOS and is a no-op on other platforms, verified by test. +- **SC-006**: The existing Connect REST endpoints continue to pass their current contract/integration tests with no breaking changes. + +## Assumptions + +- "Installed" for a client is adequately determined by the presence of its known config file path(s); no content read is required to know a client exists. +- "Connected" detection requires reading config content and is therefore acceptable to defer to an explicit per-client action; overall status may show "connected: unknown / needs check" for installed-but-not-yet-read clients until the user acts, rather than eagerly reading. +- The relevant bundle identifiers for the remediation command are mcpproxy's release and development identifiers; the message will include the applicable one(s). +- Permission denials surface as the OS "operation not permitted / permission denied" error class on a file open/read or write; this is the signal used for classification. +- The health check's detection of a *persisted* denial is best-effort (it may infer from a representative blocked access) and must never produce a false positive when access is fine. + +## Out of Scope + +- Changes to the Docker well-known-path probe (already metadata-only and TCC-safe). +- Changes to app signing, notarization, or entitlements. +- Automating the granting of Full Disk Access or any privacy permission. +- Changing how clients other than the existing supported set are discovered. + +## Commit Message Conventions *(mandatory)* + +### Issue References +- ✅ **Use**: `Related #696` - Links the commit to the issue without auto-closing +- ❌ **Do NOT use**: `Fixes #`, `Closes #`, `Resolves #` + +### Co-Authorship +- ❌ **Do NOT include** `Co-Authored-By: Claude` or "Generated with Claude Code" trailers. + +### Example Commit Message +``` +feat(connect): stat-only status + on-demand reads + TCC denial surfacing + +Related #696 + +Connect status now determines installed-state via metadata only and reads +client config contents only on explicit per-client action, eliminating the +macOS "access data from other apps" prompt storm. Permission denials are +classified distinctly and surfaced with remediation. + +## Changes +- Connect status: existence via stat; content read deferred to explicit action +- Access-outcome classification (accessible/absent/denied/malformed) +- EPERM/TCC-aware error surfacing with remediation +- doctor: macOS App-Data denial check (no-op elsewhere) + +## Testing +- Unit tests incl. injected permission-denied path +- Existing Connect REST contract tests pass +``` diff --git a/specs/075-macos-tcc-connect/tasks.md b/specs/075-macos-tcc-connect/tasks.md new file mode 100644 index 000000000..8c9d91087 --- /dev/null +++ b/specs/075-macos-tcc-connect/tasks.md @@ -0,0 +1,122 @@ +# Tasks: macOS TCC-safe Connect wizard & App-Data denial diagnostics + +**Input**: Design documents from `/specs/075-macos-tcc-connect/` +**Prerequisites**: plan.md, spec.md, research.md, data-model.md, contracts/connect-status.md +**Tests**: INCLUDED — TDD is required (FR-012 + Constitution V). Each behavior gets a failing test first. + +## Path Conventions + +Single Go module. Connect logic in `internal/connect/`; doctor check in the diagnostics package (pinned in T004); REST mapping in `internal/httpapi/connect.go`. + +--- + +## Phase 1: Setup (Shared Infrastructure) + +- [ ] T001 Confirm working on branch `075-macos-tcc-connect` and that `go build ./cmd/mcpproxy` + `go test ./internal/connect/...` are green as a baseline (capture current `GetAllStatus` behavior). +- [ ] T002 [P] Record the canonical remediation string and bundle IDs (`com.smartmcpproxy.mcpproxy`, `com.smartmcpproxy.mcpproxy.dev`) from `native/macos/MCPProxy/MCPProxy/Info.plist` for reuse in code + tests. + +--- + +## Phase 2: Foundational (Blocking Prerequisites) + +**Purpose**: The access-classification primitive every story depends on. MUST complete before US1–US3. + +- [ ] T003 [P] Add a content-read seam to `internal/connect`: a `Service`-held `readFile func(string) ([]byte, error)` (default `os.ReadFile`) and a `NewServiceWithReader`/test setter, mirroring the existing `homeDir` override, so tests can inject a permission-denied error. File: `internal/connect/connect.go`. +- [ ] T004 Pin the diagnostics package/registration point for the doctor check: inspect `internal/diagnostics/registry.go` and `internal/management/diagnostics.go`, document in tasks.md which registry the new macOS check registers into and the existing check-result type to reuse. + +--- + +## Phase 3: User Story 1 — Stat-only overall status, no privacy storm (Priority: P1) 🎯 MVP + +**Goal**: `GetAllStatus` determines installed via `os.Stat` only and performs **zero** content reads; per-client content read moves to an on-demand path. + +**Independent Test**: With several client configs present, `GetAllStatus` reads no file contents (injected reader asserts 0 calls); `GetStatus(id)` reads exactly one. + +- [ ] T005 [P] [US1] Write failing test `TestGetAllStatus_NoContentReads` in `internal/connect/connect_test.go`: inject a `readFile` that fails the test if called; create temp client configs via `NewServiceWithHome`; assert `GetAllStatus` sets `Exists` correctly and never invokes the reader. +- [ ] T006 [P] [US1] Write failing test `TestGetStatus_ReadsSingleClientOnDemand` in `internal/connect/connect_test.go`: assert `GetStatus(id)` triggers exactly one content read for that client and resolves `Connected`/`AccessState`. +- [ ] T007 [US1] Add `AccessState` (`json:"access_state"`) and `Remediation` (`json:"remediation,omitempty"`) fields to `ClientStatus` in `internal/connect/connect.go` (additive only; preserve all existing fields per FR-006). +- [ ] T008 [US1] Refactor `GetAllStatus` (`internal/connect/connect.go:112`) to set `Exists` via `os.Stat` and leave `AccessState="unknown"` / `Connected=false` for installed clients — REMOVE the eager `findEntry` call at line ~135. +- [ ] T009 [US1] Add `func (s *Service) GetStatus(clientID string) (ClientStatus, error)` that does the single-client content read via the seam, calls `findEntry`, and sets `Connected` + `AccessState=accessible|absent|malformed`. File: `internal/connect/connect.go`. +- [ ] T010 [US1] Route `findEntry`/`findEntryJSON`/`findEntryTOML` (and `readOrCreate*`) through `s.readFile` instead of calling `os.ReadFile` directly, so the seam covers all content reads. File: `internal/connect/connect.go`. +- [ ] T011 [US1] Update `GetConnectedCount`/`GetConnectedIDs` (connect.go:87/100) which currently rely on `GetAllStatus().Connected`: re-implement them to use per-client on-demand `GetStatus` (they legitimately need the connected truth for the wizard predicate) — document that this is the one internal caller allowed to content-read, and it does so lazily per client. +- [ ] T012 [US1] Run `go test ./internal/connect/ -run 'Status' -race` → green. Verify T005/T006 pass. + +**Checkpoint**: Overall status is content-read-free; connected detection is on-demand. SC-001/SC-002 satisfied. + +--- + +## Phase 4: User Story 2 — Actionable message when macOS blocks access (Priority: P2) + +**Goal**: Classify access into accessible/absent/denied/malformed and surface a distinct, actionable "blocked by macOS privacy" outcome on reads AND writes. + +**Independent Test**: Injected `fs.ErrPermission` on a config access yields `AccessState=denied` + remediation; absent → `absent`; bad JSON → `malformed`; each distinct. + +- [ ] T013 [P] [US2] Write failing tests in new `internal/connect/access_test.go` for `classifyAccess(err)`: nil→accessible, `fs.ErrNotExist`→absent, `fs.ErrPermission`(wrap `syscall.EPERM` and `EACCES`)→denied, parse-error→malformed. Use `&fs.PathError{Err: syscall.EPERM}`. +- [ ] T014 [P] [US2] Write failing test `TestGetStatus_DeniedSurfacesRemediation` in `internal/connect/connect_test.go`: inject reader returning EPERM; assert `GetStatus` returns `AccessState=denied`, non-empty `Remediation` containing `tccutil reset SystemPolicyAppData` and the bundle id, and that `Connected` stays false (not reported as plain "not connected"). +- [ ] T015 [P] [US2] Write failing test `TestConnectDenied_ReturnsAccessError` in `internal/connect/connect_test.go`: inject EPERM on the connect read/write path; assert a typed `*AccessError` (errors.As) carrying remediation is returned (distinct from unknown-client / already-exists errors). +- [ ] T016 [US2] Implement `internal/connect/access.go`: `AccessOutcome` enum, `classifyAccess(err) AccessOutcome` (via `errors.Is(err, fs.ErrPermission)` / `fs.ErrNotExist`), `AccessError` type (Client/Path/Outcome/Remediation, `Error()` + `Unwrap`), and `remediationText(client)` building the canonical message from data-model.md. +- [ ] T017 [US2] Wire `classifyAccess` into `GetStatus` (T009) so denied/malformed/absent set `AccessState` + `Remediation`. +- [ ] T018 [US2] Wire `classifyAccess` into `Connect`/`Disconnect` (connect.go) and `backupFile` (`internal/connect/backup.go`): on `denied`, return an `*AccessError` with remediation; preserve existing error semantics otherwise. +- [ ] T019 [US2] Run `go test ./internal/connect/ -run 'Access|Denied|Connect' -race` → green (T013/T014/T015). + +**Checkpoint**: All four access outcomes are distinct and surfaced; denials are actionable. SC-003/SC-004 satisfied. + +--- + +## Phase 5: User Story 3 — Doctor flags a persisted privacy denial (Priority: P3) + +**Goal**: macOS-only diagnostics check that detects a persisted App-Data denial affecting Connect and reports remediation; no-op elsewhere. + +**Independent Test**: On darwin with an injected denial → warn + remediation; darwin with access OK → pass; non-darwin → check absent/no-op. + +- [ ] T020 [P] [US3] Write failing test `internal/connect/...` or diagnostics test for a `Service` helper `DetectAppDataDenial() (denied bool, remediation string)`: inject reader returning EPERM on an existing (stat-true) client → denied=true + remediation; reader OK → denied=false; no installed clients → denied=false (no false positive). +- [ ] T021 [P] [US3] Write failing test `tcc_appdata_test.go` (build-tagged darwin) asserting the diagnostics check returns warn when `DetectAppDataDenial` reports denied, and pass otherwise; plus a `!darwin` test asserting the check is not registered / is a no-op. +- [ ] T022 [US3] Implement `Service.DetectAppDataDenial()` in `internal/connect/access.go`: iterate clients, for the first that `os.Stat`-exists, attempt a content read via the seam; if `classifyAccess`==denied return (true, remediation); else (false, ""). +- [ ] T023 [US3] Implement `internal//tcc_appdata_darwin.go` (`//go:build darwin`): a check that calls `DetectAppDataDenial` and emits warn+remediation; register it in the registry pinned in T004. Add `tcc_appdata_other.go` (`//go:build !darwin`) no-op stub. +- [ ] T024 [US3] Run `go test ./internal/connect/... ./internal//... -run 'AppData|TCC|Denial' -race` → green. Build & smoke `./mcpproxy doctor` on darwin. + +**Checkpoint**: Doctor surfaces persisted denials with a one-command fix. SC-005 satisfied. + +--- + +## Phase 6: Polish & Cross-Cutting + +- [ ] T025 [P] Map new `ClientStatus` fields (`access_state`, `remediation`) into the REST response in `internal/httpapi/connect.go`; ensure GET `/connect` payload is additive only. Confirm/append a per-client GET `/connect/{client}` on-demand status route per contracts (or document that connect/disconnect carry the denied error). +- [ ] T026 [P] Run the existing Connect REST contract/integration tests and `./scripts/test-api-e2e.sh`; confirm no regression (SC-006). +- [ ] T027 [P] Run CI linter locally: `/opt/homebrew/bin/golangci-lint run --config .github/.golangci.yml internal/connect/... internal/httpapi/... internal//...` → 0 issues (Constitution V; CI uses v2). +- [ ] T028 [P] Docs: add a short macOS "App Data privacy & Connect" note (cause + `tccutil reset` remediation) to the Connect/troubleshooting docs and update CLAUDE.md REST-payload notes for the new fields (Constitution VI). +- [ ] T029 Full verification: `go build ./cmd/mcpproxy && go build -tags server ./cmd/mcpproxy && go test -race ./internal/connect/... ./internal/httpapi/... ./internal//...`. Update execution_log.md. +- [ ] T030 [P] (Optional, separable) Frontend: render the `unknown`/`denied` tri-state + remediation banner in the Connect view (`frontend/src/`), verified via the Playwright sweep in CLAUDE.md. Not required for backend MVP. + +--- + +## Dependencies & Execution Order + +- **Setup (T001–T002)** → **Foundational (T003–T004)** → user stories. +- **US1 (P1, T005–T012)**: depends only on Foundational. This is the MVP — kills the prompt storm. +- **US2 (P2, T013–T019)**: depends on US1 (`GetStatus` + seam). Adds classification/surfacing. +- **US3 (P3, T020–T024)**: depends on US2 (`classifyAccess`, `DetectAppDataDenial`). +- **Polish (T025–T030)**: after the stories it touches; T025/T026 need US1+US2. + +``` +Setup → Foundational → US1 → US2 → US3 → Polish + │ │ + └─MVP──┘ (US1 alone delivers SC-001/SC-002) +``` + +## Parallel Opportunities + +- T002 ∥ within setup. +- All test-authoring tasks marked [P] within a phase (T005/T006; T013/T014/T015; T020/T021) — different test funcs, write first, watch fail. +- Polish T025/T026/T027/T028 ∥ (different files), then T029 gate. + +## Implementation Strategy + +1. **MVP = US1** (T001–T012): stat-only status removes the prompt storm — ship-able alone, addresses the root cause for issue #696's sibling TCC report. +2. **US2** makes denials diagnosable; **US3** adds the doctor convenience. +3. Each phase ends green under `-race` + linter before the next. Frontend (T030) ships separately. + +## Notes + +- Issue link: `Related #696` (per spec commit conventions). Do NOT use auto-closing keywords; no Claude co-author trailer. +- Out of scope (do not touch): Docker well-known probe (`shellwrap`/`secureenv`), entitlements, signing/notarization.