diff --git a/internal/connect/access.go b/internal/connect/access.go index 57dd6515..a36e7036 100644 --- a/internal/connect/access.go +++ b/internal/connect/access.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" "io/fs" + "os" ) // AccessOutcome classifies an attempt to read or write a client config file @@ -108,3 +109,36 @@ func (s *Service) asAccessError(client *ClientDef, path string, err error) error } return err } + +// DetectAppDataDenial probes installed client configs for a persisted macOS +// App-Data (TCC) permission denial, for the doctor diagnostic (Spec 075 US3, +// FR-007/008). It walks the supported clients and, for the first whose config +// file exists (os.Stat metadata only), performs a single content read through the +// seam; if that read classifies as accessDenied it reports the denial with the +// canonical, one-command remediation. It returns (false, "") when no installed +// client config is permission-denied — including when none are installed — so the +// check never raises a false positive on a machine that simply has no clients or +// has granted access (FR-008, T022). +// +// Unlike GetAllStatus this DOES read content: the doctor command is an explicit +// user action, the one place a macOS App-Data prompt may legitimately appear. +func (s *Service) DetectAppDataDenial() (denied bool, remediation string) { + for _, c := range GetAllClients() { + if !c.Supported { + continue + } + cfgPath := ConfigPath(c.ID, s.homeDir) + if cfgPath == "" { + continue + } + // Metadata-only existence gate: never read a config that is not installed, + // so a brand-new machine produces no read and no false positive. + if _, err := os.Stat(cfgPath); err != nil { + continue + } + if _, _, outcome := s.entryAccess(c, cfgPath); outcome == accessDenied { + return true, remediationText(c.Name) + } + } + return false, "" +} diff --git a/internal/connect/access_test.go b/internal/connect/access_test.go index c0cca1b3..1f8e67d7 100644 --- a/internal/connect/access_test.go +++ b/internal/connect/access_test.go @@ -156,3 +156,70 @@ func TestConnectDenied_ReturnsAccessError(t *testing.T) { t.Error("unknown-client error must not be an *AccessError") } } + +// installClientConfig makes a supported client appear installed by writing a +// config file at its resolved path under the (test-isolated) home dir. +func installClientConfig(t *testing.T, homeDir, clientID, body string) { + t.Helper() + cfgPath := ConfigPath(clientID, homeDir) + if cfgPath == "" { + t.Fatalf("no config path for %s", clientID) + } + if err := os.MkdirAll(filepath.Dir(cfgPath), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(cfgPath, []byte(body), 0o644); err != nil { + t.Fatal(err) + } +} + +// TestDetectAppDataDenial covers the doctor probe (Spec 075 US3, T020): an +// installed client whose content read is permission-denied reports a denial with +// remediation; a clean read does not; and no installed clients yields neither a +// denial nor any content read (no false positive). +func TestDetectAppDataDenial(t *testing.T) { + t.Run("denied when an installed client config read is EPERM", func(t *testing.T) { + svc, homeDir := testService(t) + installClientConfig(t, homeDir, "claude-code", `{"mcpServers":{}}`) + svc.setReadFile(epermReader) + + denied, remediation := svc.DetectAppDataDenial() + if !denied { + t.Fatal("expected denied=true for an EPERM content read on an installed client") + } + if !strings.Contains(remediation, "tccutil reset SystemPolicyAppData") { + t.Errorf("remediation must carry the exact reset command, got %q", remediation) + } + if !strings.Contains(remediation, bundleIDProd) { + t.Errorf("remediation must name the prod bundle id, got %q", remediation) + } + }) + + t.Run("not denied when installed configs read cleanly", func(t *testing.T) { + svc, homeDir := testService(t) + installClientConfig(t, homeDir, "claude-code", `{"mcpServers":{}}`) + svc.setReadFile(func(string) ([]byte, error) { + return []byte(`{"mcpServers":{}}`), nil + }) + + denied, remediation := svc.DetectAppDataDenial() + if denied || remediation != "" { + t.Fatalf("clean read must not be a denial, got denied=%v remediation=%q", denied, remediation) + } + }) + + t.Run("no false positive when no clients are installed", func(t *testing.T) { + svc, _ := testService(t) + // Fresh isolated home: nothing stat-exists, so a denial-returning reader + // must never be consulted. + svc.setReadFile(func(path string) ([]byte, error) { + t.Errorf("reader must not be called when no client config exists; read %s", path) + return nil, &fs.PathError{Op: "open", Path: path, Err: syscall.EPERM} + }) + + denied, remediation := svc.DetectAppDataDenial() + if denied || remediation != "" { + t.Fatalf("no installed clients must yield no denial, got denied=%v remediation=%q", denied, remediation) + } + }) +} diff --git a/internal/management/diagnostics.go b/internal/management/diagnostics.go index b462d4fe..d870a5a4 100644 --- a/internal/management/diagnostics.go +++ b/internal/management/diagnostics.go @@ -157,6 +157,12 @@ func (s *service) Doctor(ctx context.Context) (*contracts.Diagnostics, error) { diag.DockerStatus = s.checkDockerDaemon() } + // macOS App-Data (TCC) denial probe (Spec 075 US3): surface a persisted denial + // to read MCP client configs as an actionable runtime warning. No-op off darwin. + if warning, ok := appDataDenialWarning(); ok { + diag.RuntimeWarnings = append(diag.RuntimeWarnings, warning) + } + // Calculate total issues diag.TotalIssues = len(diag.UpstreamErrors) + len(diag.OAuthRequired) + len(diag.OAuthIssues) + len(diag.MissingSecrets) + len(diag.RuntimeWarnings) diff --git a/internal/management/tcc_appdata.go b/internal/management/tcc_appdata.go new file mode 100644 index 00000000..b7ff0f2a --- /dev/null +++ b/internal/management/tcc_appdata.go @@ -0,0 +1,24 @@ +package management + +// appDataProbe reports whether a persisted macOS App-Data (TCC) denial is +// currently blocking reads of MCP client configurations. *connect.Service +// satisfies it via DetectAppDataDenial. +type appDataProbe interface { + DetectAppDataDenial() (denied bool, remediation string) +} + +// appDataWarningPrefix introduces the doctor runtime warning; the actionable +// tccutil remediation is appended after it. +const appDataWarningPrefix = "macOS blocked mcpproxy from reading MCP client configurations (Privacy & Security ▸ App Data)." + +// appDataWarningFrom turns a probe result into a doctor runtime-warning string. +// It returns ("", false) when there is no denial, so the caller adds nothing to +// the diagnostics. It is pure and OS-independent so it can be unit-tested on +// every platform without a real macOS denial (Spec 075 US3, T021). +func appDataWarningFrom(p appDataProbe) (warning string, ok bool) { + denied, remediation := p.DetectAppDataDenial() + if !denied { + return "", false + } + return appDataWarningPrefix + "\n" + remediation, true +} diff --git a/internal/management/tcc_appdata_darwin.go b/internal/management/tcc_appdata_darwin.go new file mode 100644 index 00000000..a9f8ecbe --- /dev/null +++ b/internal/management/tcc_appdata_darwin.go @@ -0,0 +1,13 @@ +//go:build darwin + +package management + +import "github.com/smart-mcp-proxy/mcpproxy-go/internal/connect" + +// appDataDenialWarning probes the real macOS MCP-client configs for a persisted +// App-Data (TCC) denial and returns an actionable doctor runtime warning when one +// is found (Spec 075 US3, T023). It reads at most one installed client config +// (the explicit-action doctor path); GetAllStatus remains content-read-free. +func appDataDenialWarning() (string, bool) { + return appDataWarningFrom(connect.NewService("", "")) +} diff --git a/internal/management/tcc_appdata_other.go b/internal/management/tcc_appdata_other.go new file mode 100644 index 00000000..9326ff31 --- /dev/null +++ b/internal/management/tcc_appdata_other.go @@ -0,0 +1,8 @@ +//go:build !darwin + +package management + +// appDataDenialWarning is a no-op off macOS: the App-Data (TCC) privacy gate is +// macOS-specific, so non-darwin builds never probe client configs or emit this +// warning (Spec 075 US3, T023). +func appDataDenialWarning() (string, bool) { return "", false } diff --git a/internal/management/tcc_appdata_other_test.go b/internal/management/tcc_appdata_other_test.go new file mode 100644 index 00000000..2c9a86a2 --- /dev/null +++ b/internal/management/tcc_appdata_other_test.go @@ -0,0 +1,15 @@ +//go:build !darwin + +package management + +import "testing" + +// TestAppDataDenialWarning_NoOpOffDarwin asserts the doctor App-Data check is a +// no-op on non-macOS platforms: it never probes client configs and never emits a +// warning (Spec 075 US3, T021). +func TestAppDataDenialWarning_NoOpOffDarwin(t *testing.T) { + warning, ok := appDataDenialWarning() + if ok || warning != "" { + t.Fatalf("appDataDenialWarning must be a no-op off darwin, got ok=%v warning=%q", ok, warning) + } +} diff --git a/internal/management/tcc_appdata_test.go b/internal/management/tcc_appdata_test.go new file mode 100644 index 00000000..05888e88 --- /dev/null +++ b/internal/management/tcc_appdata_test.go @@ -0,0 +1,43 @@ +package management + +import ( + "strings" + "testing" +) + +// fakeAppDataProbe is a programmable appDataProbe for testing the warning +// translation without a real macOS denial. +type fakeAppDataProbe struct { + denied bool + remediation string +} + +func (f fakeAppDataProbe) DetectAppDataDenial() (bool, string) { + return f.denied, f.remediation +} + +// TestAppDataWarningFrom covers the doctor check translation (Spec 075 US3, T021): +// a probe that reports a denial yields a warning carrying the remediation; a +// probe that reports no denial yields nothing. +func TestAppDataWarningFrom(t *testing.T) { + t.Run("denied yields a warning with remediation", func(t *testing.T) { + remediation := "Fix: tccutil reset SystemPolicyAppData com.smartmcpproxy.mcpproxy" + warning, ok := appDataWarningFrom(fakeAppDataProbe{denied: true, remediation: remediation}) + if !ok { + t.Fatal("expected ok=true when a denial is reported") + } + if !strings.Contains(warning, remediation) { + t.Errorf("warning must carry the remediation, got %q", warning) + } + if !strings.Contains(warning, "App Data") { + t.Errorf("warning should name the App Data privacy gate, got %q", warning) + } + }) + + t.Run("no denial yields nothing", func(t *testing.T) { + warning, ok := appDataWarningFrom(fakeAppDataProbe{denied: false}) + if ok || warning != "" { + t.Fatalf("expected no warning when not denied, got ok=%v warning=%q", ok, warning) + } + }) +} diff --git a/specs/075-macos-tcc-connect/tasks.md b/specs/075-macos-tcc-connect/tasks.md index 8c9d9108..0a54f907 100644 --- a/specs/075-macos-tcc-connect/tasks.md +++ b/specs/075-macos-tcc-connect/tasks.md @@ -22,7 +22,7 @@ Single Go module. Connect logic in `internal/connect/`; doctor check in the diag **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. +- [x] T004 Pin the diagnostics package/registration point for the doctor check. **Decision:** the runtime doctor is `internal/management` `service.Doctor()` (`diagnostics.go`), which builds `contracts.Diagnostics`; the macOS App-Data check appends an actionable warning string to its `RuntimeWarnings []string` (rendered by `mcpproxy doctor` as the "⚠️ Runtime Warnings" section and counted in `TotalIssues`). The static `internal/diagnostics` registry is an error-CODE catalog (classification metadata), not a runtime-check registry, so it is NOT used. The build-tagged check lives in `internal/management/tcc_appdata_{darwin,other}.go` with a pure, cross-platform translator in `tcc_appdata.go`. --- @@ -69,11 +69,11 @@ Single Go module. Connect logic in `internal/connect/`; doctor check in the diag **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. +- [x] 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). +- [x] 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. +- [x] 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, ""). +- [x] 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. +- [x] 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.