feat(connect): classify config access + macOS TCC denial remediation (US2)

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
+}

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")
+	}
+}

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) {

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 ----------