diff --git a/internal/connect/access.go b/internal/connect/access.go new file mode 100644 index 00000000..57dd6515 --- /dev/null +++ b/internal/connect/access.go @@ -0,0 +1,110 @@ +package connect + +import ( + "errors" + "fmt" + "io/fs" +) + +// AccessOutcome classifies an attempt to read or write a client config file +// (Spec 075 FR-003). It is an alias of string so it interoperates with the +// existing untyped access* constants and the ClientStatus.AccessState wire +// field, while giving classifier signatures a documented, enum-like type. +// +// Valid values are the access* constants in connect.go: accessAccessible, +// accessAbsent, accessDenied, accessMalformed. (accessUnknown is the overall, +// not-content-checked status default, not a classification outcome.) +type AccessOutcome = string + +// macOS bundle identifiers used in the tccutil reset remediation. Kept in sync +// with native/macos/MCPProxy/MCPProxy/Info.plist (prod) and the .dev variant. +const ( + bundleIDProd = "com.smartmcpproxy.mcpproxy" + bundleIDDev = "com.smartmcpproxy.mcpproxy.dev" +) + +// classifyAccess maps a file-access error to an AccessOutcome strictly from the +// error class (Spec 075 FR-011) — never from string-matching error text: +// +// - nil -> accessAccessible +// - errors.Is(err, ErrNotExist) -> accessAbsent (client not installed) +// - errors.Is(err, ErrPermission)-> accessDenied (macOS TCC App-Data block) +// - any other error -> accessMalformed (read/parse failure) +// +// syscall.EPERM and syscall.EACCES both satisfy errors.Is(err, fs.ErrPermission) +// via their Errno.Is implementation, so a real TCC denial classifies as denied. +func classifyAccess(err error) AccessOutcome { + switch { + case err == nil: + return accessAccessible + case errors.Is(err, fs.ErrNotExist): + return accessAbsent + case errors.Is(err, fs.ErrPermission): + return accessDenied + default: + return accessMalformed + } +} + +// AccessError is returned by connect/disconnect (and surfaced by GetStatus via +// the Remediation field) when a client-config access is permission-denied. It +// is errors.As-discoverable so the REST layer can map it to the right field, +// and unwraps to the underlying OS error so errors.Is(err, fs.ErrPermission) +// still holds (data-model.md AccessError). +type AccessError struct { + Client string // client id/name + Path string // config path attempted + Outcome AccessOutcome // accessDenied (could also wrap malformed) + Remediation string // actionable fix text + Err error // underlying OS cause +} + +func (e *AccessError) Error() string { + if e.Path != "" { + return fmt.Sprintf("%s (config: %s)", e.Remediation, e.Path) + } + return e.Remediation +} + +// Unwrap exposes the underlying OS error for errors.Is/errors.As. +func (e *AccessError) Unwrap() error { return e.Err } + +// remediationText builds the canonical privacy-denied message (data-model.md): +// it names the cause, the App Data settings path, and the exact tccutil reset +// command with both the prod and dev bundle identifiers (FR-005). +func remediationText(client string) string { + return fmt.Sprintf( + "macOS blocked mcpproxy from reading %s's configuration (Privacy & Security ▸ App Data).\n"+ + "Fix: System Settings ▸ Privacy & Security ▸ App Data ▸ enable mcpproxy,\n"+ + "or run: tccutil reset SystemPolicyAppData %s\n"+ + "(dev builds: %s)", + client, bundleIDProd, bundleIDDev, + ) +} + +// newAccessError constructs an *AccessError for a denied access on the given +// client/path, wrapping the OS cause. +func (s *Service) newAccessError(client *ClientDef, path string, cause error) *AccessError { + name := client.Name + return &AccessError{ + Client: name, + Path: path, + Outcome: accessDenied, + Remediation: remediationText(name), + Err: cause, + } +} + +// asAccessError wraps a connect/disconnect error as a typed *AccessError when it +// classifies as a permission denial; otherwise it returns the error unchanged so +// existing error semantics (unknown client, already-exists, parse failures) are +// preserved (FR-004). A nil error stays nil. +func (s *Service) asAccessError(client *ClientDef, path string, err error) error { + if err == nil { + return nil + } + if classifyAccess(err) == accessDenied { + return s.newAccessError(client, path, err) + } + return err +} diff --git a/internal/connect/access_test.go b/internal/connect/access_test.go new file mode 100644 index 00000000..c0cca1b3 --- /dev/null +++ b/internal/connect/access_test.go @@ -0,0 +1,158 @@ +package connect + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path/filepath" + "strings" + "syscall" + "testing" +) + +// TestClassifyAccess covers the four outcomes derived from the OS error class +// (Spec 075 FR-003/FR-011): classification comes from errors.Is, never from +// string-matching arbitrary error text. +func TestClassifyAccess(t *testing.T) { + tests := []struct { + name string + err error + want AccessOutcome + }{ + {"nil is accessible", nil, accessAccessible}, + {"ErrNotExist is absent", fs.ErrNotExist, accessAbsent}, + {"PathError ENOENT is absent", &fs.PathError{Op: "open", Path: "/x", Err: syscall.ENOENT}, accessAbsent}, + {"ErrPermission is denied", fs.ErrPermission, accessDenied}, + {"PathError EPERM is denied", &fs.PathError{Op: "open", Path: "/x", Err: syscall.EPERM}, accessDenied}, + {"PathError EACCES is denied", &fs.PathError{Op: "open", Path: "/x", Err: syscall.EACCES}, accessDenied}, + {"wrapped EPERM is denied", fmt.Errorf("read /x: %w", &fs.PathError{Err: syscall.EPERM}), accessDenied}, + {"parse error is malformed", errors.New("invalid character 'x'"), accessMalformed}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := classifyAccess(tt.err); got != tt.want { + t.Errorf("classifyAccess(%v) = %q, want %q", tt.err, got, tt.want) + } + }) + } +} + +// TestAccessError_ErrorAndUnwrap verifies the typed error carries remediation, +// is errors.As-discoverable, and unwraps to the OS permission cause so the REST +// layer can map it (data-model.md AccessError). +func TestAccessError_ErrorAndUnwrap(t *testing.T) { + cause := &fs.PathError{Op: "open", Path: "/cfg", Err: syscall.EPERM} + ae := &AccessError{ + Client: "Claude Code", + Path: "/cfg", + Outcome: accessDenied, + Remediation: remediationText("Claude Code"), + Err: cause, + } + + if !strings.Contains(ae.Error(), "tccutil reset SystemPolicyAppData") { + t.Errorf("Error() should include remediation, got: %q", ae.Error()) + } + if !errors.Is(ae, fs.ErrPermission) { + t.Error("AccessError should unwrap to fs.ErrPermission") + } + var target *AccessError + if !errors.As(fmt.Errorf("connect: %w", ae), &target) { + t.Error("AccessError should be discoverable via errors.As through a wrap") + } +} + +// TestRemediationText asserts the canonical message names the cause, the App +// Data settings path, the tccutil reset command, and both bundle ids (FR-005). +func TestRemediationText(t *testing.T) { + got := remediationText("Cursor") + for _, want := range []string{ + "Cursor", + "Privacy & Security", + "App Data", + "tccutil reset SystemPolicyAppData com.smartmcpproxy.mcpproxy", + "com.smartmcpproxy.mcpproxy.dev", + } { + if !strings.Contains(got, want) { + t.Errorf("remediationText missing %q; got:\n%s", want, got) + } + } +} + +// epermReader returns a content reader that always fails with a TCC-style +// permission denial, simulating a macOS App-Data block without a real OS denial. +func epermReader(path string) ([]byte, error) { + return nil, &fs.PathError{Op: "open", Path: path, Err: syscall.EPERM} +} + +// TestGetStatus_DeniedSurfacesRemediation: a permission-denied content read on +// an installed client resolves to access_state=denied with actionable +// remediation, and must NOT be reported as plain "not connected" (FR-004). +func TestGetStatus_DeniedSurfacesRemediation(t *testing.T) { + svc, homeDir := testService(t) + + // The config file must exist on disk so os.Stat (metadata) reports installed; + // the denial happens on the content read, mirroring macOS TCC App-Data. + cfgPath := ConfigPath("claude-code", homeDir) + 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) + } + svc.setReadFile(epermReader) + + st, err := svc.GetStatus("claude-code") + if err != nil { + t.Fatalf("GetStatus should not hard-error on denial: %v", err) + } + if st.AccessState != accessDenied { + t.Errorf("expected access_state=%q, got %q", accessDenied, st.AccessState) + } + if st.Connected { + t.Error("denied access must not be reported as connected") + } + if !strings.Contains(st.Remediation, "tccutil reset SystemPolicyAppData") { + t.Errorf("expected remediation with tccutil reset, got %q", st.Remediation) + } + if !strings.Contains(st.Remediation, "com.smartmcpproxy.mcpproxy") { + t.Errorf("expected remediation to name the bundle id, got %q", st.Remediation) + } +} + +// TestConnectDenied_ReturnsAccessError: a permission denial on the connect path +// returns a typed *AccessError carrying remediation, distinct from the +// unknown-client and not-supported errors (FR-004). +func TestConnectDenied_ReturnsAccessError(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(`{"mcpServers":{}}`), 0o644); err != nil { + t.Fatal(err) + } + svc.setReadFile(epermReader) + + _, err := svc.Connect("claude-code", "", false) + if err == nil { + t.Fatal("expected an error when the connect read is permission-denied") + } + var ae *AccessError + if !errors.As(err, &ae) { + t.Fatalf("expected *AccessError, got %T: %v", err, err) + } + if ae.Outcome != accessDenied { + t.Errorf("expected Outcome=%q, got %q", accessDenied, ae.Outcome) + } + if !strings.Contains(ae.Remediation, "tccutil reset SystemPolicyAppData") { + t.Errorf("expected remediation, got %q", ae.Remediation) + } + + // A genuine unknown-client error must NOT be classified as a denial. + if _, unknownErr := svc.Connect("does-not-exist", "", false); errors.As(unknownErr, &ae) { + t.Error("unknown-client error must not be an *AccessError") + } +} diff --git a/internal/connect/backup.go b/internal/connect/backup.go index e464c290..93a25c91 100644 --- a/internal/connect/backup.go +++ b/internal/connect/backup.go @@ -10,6 +10,11 @@ import ( // backupFile creates a timestamped backup of the given file. // Returns the backup path, or empty string if the source file does not exist. +// +// All failure paths wrap their OS cause with %w, so a permission denial here +// (e.g. macOS TCC App-Data) preserves fs.ErrPermission up the call chain and is +// classified into a typed *AccessError by the Connect/Disconnect boundary +// (Spec 075 FR-004, see Service.asAccessError). func backupFile(path string) (string, error) { info, err := os.Stat(path) if os.IsNotExist(err) { diff --git a/internal/connect/connect.go b/internal/connect/connect.go index 42491379..6326c3e5 100644 --- a/internal/connect/connect.go +++ b/internal/connect/connect.go @@ -3,9 +3,7 @@ package connect import ( "bytes" "encoding/json" - "errors" "fmt" - "io/fs" "net/url" "os" "regexp" @@ -22,7 +20,7 @@ const ( 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. + accessDenied = "denied" // blocked by OS permission (macOS TCC App-Data) ) // ConnectResult describes the outcome of a connect or disconnect operation. @@ -237,25 +235,27 @@ func (s *Service) GetStatus(clientID string) (ClientStatus, error) { name, found, outcome := s.entryAccess(*c, cfgPath) status.AccessState = outcome - if outcome == accessAccessible && found { + switch { + case outcome == accessAccessible && found: status.Connected = true status.ServerName = name + case outcome == accessDenied: + // A macOS App-Data block must surface as actionable remediation, not as + // a plain "not connected" (Spec 075 FR-004). + status.Remediation = remediationText(c.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. +// access outcome classified strictly from the error class (Spec 075 FR-011): +// a read error maps to absent/denied/malformed via classifyAccess, and a parse +// failure on otherwise-readable bytes maps to malformed. 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 + return "", false, classifyAccess(err) } name, found, parsedOK := s.findEntryFromBytes(client, raw) if !parsedOK { @@ -292,10 +292,17 @@ func (s *Service) Connect(clientID, serverName string, force bool) (*ConnectResu mcpURL := s.mcpURL() + var res *ConnectResult + var err error if client.Format == "toml" { - return s.connectTOML(client, cfgPath, serverName, mcpURL, force) - } - return s.connectJSON(client, cfgPath, serverName, mcpURL, force) + res, err = s.connectTOML(client, cfgPath, serverName, mcpURL, force) + } else { + res, err = s.connectJSON(client, cfgPath, serverName, mcpURL, force) + } + // A permission denial anywhere in the read/backup/write chain (the errors + // preserve their OS cause via %w) surfaces as a typed *AccessError with + // remediation; other errors keep their existing semantics (Spec 075 FR-004). + return res, s.asAccessError(client, cfgPath, err) } // Disconnect removes the MCPProxy entry from the specified client's configuration. @@ -317,10 +324,14 @@ func (s *Service) Disconnect(clientID, serverName string) (*ConnectResult, error return nil, fmt.Errorf("cannot determine config path for %s", clientID) } + var res *ConnectResult + var err error if client.Format == "toml" { - return s.disconnectTOML(client, cfgPath, serverName) + res, err = s.disconnectTOML(client, cfgPath, serverName) + } else { + res, err = s.disconnectJSON(client, cfgPath, serverName) } - return s.disconnectJSON(client, cfgPath, serverName) + return res, s.asAccessError(client, cfgPath, err) } // ---------- JSON helpers ----------