From 9b9071ac456ac55f52c5c8e3a5a28b286b447dda Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Thu, 18 Jun 2026 09:29:34 +0300 Subject: [PATCH] feat(connect,doctor): macOS App-Data (TCC) denial diagnostic (spec 075 US3) Adds a doctor check that flags a persisted macOS App-Data (TCC) denial blocking mcpproxy from reading MCP client configs, with the exact tccutil remediation. No-op off macOS. - connect.Service.DetectAppDataDenial(): probes installed client configs (an os.Stat existence gate, then one content read via the US1 seam) and reports the first accessDenied as (true, remediation). No false positive when no client is installed or access is granted. - internal/management Doctor() appends the warning to RuntimeWarnings (rendered by `mcpproxy doctor`, counted in TotalIssues). Build-tagged tcc_appdata_{darwin,other}.go with a pure, cross-platform translator. - T004 registry pinned: management.Doctor -> contracts.Diagnostics, not the static internal/diagnostics error-code catalog. TDD: DetectAppDataDenial (denied / clean / no-install) + the warning translator + a !darwin no-op test. tasks.md T004/T020-T024 checked. Co-Authored-By: Paperclip --- internal/connect/access.go | 34 ++++++++++ internal/connect/access_test.go | 67 +++++++++++++++++++ internal/management/diagnostics.go | 6 ++ internal/management/tcc_appdata.go | 24 +++++++ internal/management/tcc_appdata_darwin.go | 13 ++++ internal/management/tcc_appdata_other.go | 8 +++ internal/management/tcc_appdata_other_test.go | 15 +++++ internal/management/tcc_appdata_test.go | 43 ++++++++++++ specs/075-macos-tcc-connect/tasks.md | 12 ++-- 9 files changed, 216 insertions(+), 6 deletions(-) create mode 100644 internal/management/tcc_appdata.go create mode 100644 internal/management/tcc_appdata_darwin.go create mode 100644 internal/management/tcc_appdata_other.go create mode 100644 internal/management/tcc_appdata_other_test.go create mode 100644 internal/management/tcc_appdata_test.go 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.