diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c1efce..c2c86b22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ See [VERSIONING.md](VERSIONING.md) for why the version starts at 1.8.1. ### Fixed +- Skip protected browser profile reads on macOS 27 when protected-directory scanning is disabled, while preserving browser extension inventory on macOS 26. - Suppress Windows scheduler-registration probe console flashes during heartbeat, telemetry initialization, and scheduler diagnostics; bound each probe to three seconds. - **Plugin-catalog templates are no longer counted as MCP servers.** The MCP walk matches on basename anywhere under `$HOME`, and an agent plugin marketplace is a clone of a catalog repo where every entry ships a template `.mcp.json` — on one machine that turned 7 real configs into 53, and in enterprise mode 36 of them carried an `mcpServers` block, so the backend recorded stripe, slack, gmail and notion as servers on a device that had installed none of them. A hit inside a plugin package (marked by a `.claude-plugin`/`.codex-plugin` manifest) is now classified rather than guessed at: the package's own `.mcp.json` is kept when the package is installed, while catalog entries and MCP-shaped files vendored elsewhere in a payload are dropped. Configs outside a plugin package are untouched. Fixes #201. - **Globally installed packages report the directory they live in.** The disk-based global scan built its result without a project path, and that field is the only place the backend learns where a global package lives, so since 1.13.0 every globally installed package reached the dashboard with a blank "Project Paths" column — a customer triaging the compromised `chalk`/`ansi-*` versions had no directory to go clean up. The scan now emits one result per global root, reporting the `node_modules` directory it actually read (exact even for pnpm's content-addressed store), so a package installed under two prefixes lists both. Roots are deduplicated on package manager and directory, and a root that has emptied is still emitted, so its packages are retracted instead of standing forever. diff --git a/internal/detector/browserext/detector.go b/internal/detector/browserext/detector.go index c6de4d05..c9921751 100644 --- a/internal/detector/browserext/detector.go +++ b/internal/detector/browserext/detector.go @@ -7,6 +7,7 @@ import ( "os/user" "path/filepath" "sort" + "strconv" "strings" "time" "unicode/utf8" @@ -21,8 +22,9 @@ import ( // reads the browsers' own state files and nothing else: no browser is launched, // no store is asked what it published, and no extension's code is opened. type Detector struct { - exec executor.Executor - skipper *tcc.Skipper + exec executor.Executor + skipper *tcc.Skipper + osVersion string // serviceSession reports whether this process runs with no interactive user // behind it. A function field because the answer comes from the process rather @@ -41,6 +43,12 @@ func (d *Detector) WithSkipper(s *tcc.Skipper) *Detector { return d } +// WithOSVersion uses the OS version already gathered for this run. +func (d *Detector) WithOSVersion(version string) *Detector { + d.osVersion = version + return d +} + // Detect returns the inventory for one run, for the account named by target. // // It returns nil, the "did not run" sentinel, when there is no interactive account @@ -152,21 +160,28 @@ func isServiceIdentity(platform string, u *user.User) bool { // consentGuard is what the resolver asks before it touches a path, answering in this // phase's own reason code so a refusal reads like every other one. // -// The macOS skipper declines ~/Library wholesale, which is right for a walk and wrong -// for this detector: the browsers keep their data directories under it. Their own -// directories are exempt, along with the directories above them a descent passes -// through, matched against the cleaned path so -// "Library/Application Support/Google/Chrome/../Mail" cannot ride the exemption. What -// stays protected is the one path class this detector does not fix itself: a profile -// directory named by a browser's own config file, which is a string an attacker can -// write. +// Known browser roots keep their historical exemption before macOS 27. +// On macOS 27 and unknown versions, the ordinary protected-path policy applies. +// Configured profile paths outside those roots remain guarded on every version. func (d *Detector) consentGuard(platform, home string) safepath.Guard { if d.skipper == nil { return nil } var exempt []string - for _, spec := range catalog { - exempt = append(exempt, spec.roots(platform, home)...) + parts := strings.Split(d.osVersion, ".") + validVersion := len(parts) <= 3 + for _, part := range parts { + if _, err := strconv.ParseUint(part, 10, 32); err != nil { + validVersion = false + break + } + } + major, _ := strconv.ParseUint(parts[0], 10, 32) + // macOS 27 protects these browser roots. Unknown versions fail closed. + if platform != model.PlatformDarwin || (validVersion && major > 0 && major < 27) { + for _, spec := range catalog { + exempt = append(exempt, spec.roots(platform, home)...) + } } return func(path string) string { cleaned := filepath.Clean(path) diff --git a/internal/detector/browserext/detector_test.go b/internal/detector/browserext/detector_test.go index 53baa375..6d080d25 100644 --- a/internal/detector/browserext/detector_test.go +++ b/internal/detector/browserext/detector_test.go @@ -496,7 +496,7 @@ func TestDetect_GuardExemptsTheBrowsersOwnDirectories(t *testing.T) { securePrefs(t, root, "Default", `"`+idA+`": {"location": 1, "active_permissions": {}, "manifest": {"name": "Example", "version": "1.0"}}`) - d := newDetector(model.PlatformDarwin).WithSkipper(tcc.New(home)) + d := newDetector(model.PlatformDarwin).WithOSVersion("26.5.1").WithSkipper(tcc.New(home)) info := d.Detect(context.Background(), testUser(home)) if info == nil { t.Fatal("Detect returned the did-not-run sentinel") diff --git a/internal/detector/browserext/live_darwin_test.go b/internal/detector/browserext/live_darwin_test.go new file mode 100644 index 00000000..6d994033 --- /dev/null +++ b/internal/detector/browserext/live_darwin_test.go @@ -0,0 +1,52 @@ +//go:build darwin + +package browserext + +import ( + "context" + "encoding/json" + "os" + "os/user" + "strings" + "testing" + "time" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +// Opt-in validation under a one-shot launchd job. It never grants permissions, +// launches a browser, changes a profile, or invokes enterprise telemetry. +func TestLiveProtectedBrowserInventory(t *testing.T) { + if os.Getenv("DMG_TCC_VALIDATE_LIVE") != "1" { + t.Skip("explicit live validation only") + } + exec := executor.NewReal() + version, stderr, code, err := exec.RunWithTimeout(t.Context(), 3*time.Second, "/usr/bin/sw_vers", "-productVersion") + if err != nil || code != 0 { + t.Fatalf("sw_vers failed: exit=%d stderr=%q err=%v", code, stderr, err) + } + u, err := user.Current() + if err != nil { + t.Fatal(err) + } + result := New(exec).WithOSVersion(strings.TrimSpace(version)).WithSkipper(tcc.New(u.HomeDir)).Detect(context.Background(), u) + if result == nil { + t.Fatal("missing inventory coverage") + } + assertPayloadInvariants(t, result) + summary := struct { + Version string `json:"version"` + Findings int `json:"findings"` + Complete bool `json:"complete"` + Coverage any `json:"coverage"` + }{strings.TrimSpace(version), len(result.Findings), result.ScanComplete, result.Browsers} + data, err := json.Marshal(summary) + if err != nil { + t.Fatal(err) + } + t.Log(string(data)) + if strings.HasPrefix(version, "27.") && (len(result.Findings) != 0 || result.ScanComplete) { + t.Fatal("macOS 27 protected inventory unexpectedly scanned") + } +} diff --git a/internal/detector/browserext/tcc_compat_test.go b/internal/detector/browserext/tcc_compat_test.go new file mode 100644 index 00000000..004bd7cd --- /dev/null +++ b/internal/detector/browserext/tcc_compat_test.go @@ -0,0 +1,61 @@ +package browserext + +import ( + "context" + "path/filepath" + "runtime" + "testing" + + "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +func TestDetect_OSVersionConsent(t *testing.T) { + if runtime.GOOS != "darwin" { + t.Skip("real Darwin skipper") + } + for _, tc := range []struct { + name, version string + include *bool + wantScan bool + }{ + {"26-default", "26.5.1", nil, true}, + {"26-off", "26.5.1", new(false), true}, + {"27-default", "27.0", nil, false}, + {"27-off", "27.0", new(false), false}, + {"27-explicit-include", "27.0", new(true), true}, + {"28-default", "28.0", nil, false}, + {"unknown-default", "", nil, false}, + {"malformed-default", "not-a-version", nil, false}, + {"malformed-minor", "26.invalid", nil, false}, + {"malformed-patch", "26.5.invalid", nil, false}, + {"empty-component", "26..1", nil, false}, + {"trailing-dot", "26.", nil, false}, + {"signed-major", "+26.5.1", nil, false}, + {"extra-component", "26.5.1.2", nil, false}, + } { + t.Run(tc.name, func(t *testing.T) { + home := tempHome(t) + root := filepath.Join(home, "Library", "Application Support", "Google", "Chrome") + localState(t, root, "Default") + securePrefs(t, root, "Default", `"`+idA+`": {"location": 1, "active_permissions": {}, "manifest": {"name": "Example", "version": "1.0"}}`) + var skipper *tcc.Skipper + if tcc.Enabled(tc.include) { + skipper = tcc.New(home) + } + info := newDetector(model.PlatformDarwin).WithOSVersion(tc.version).WithSkipper(skipper).Detect(context.Background(), testUser(home)) + if info == nil { + t.Fatal("missing coverage") + } + assertPayloadInvariants(t, info) + got := coverageFor(t, info, browserChrome) + if tc.wantScan { + if got.Status != model.BrowserCoverageScanned || len(findingsFor(info, browserChrome)) != 1 { + t.Fatalf("got %s/%s with %d findings", got.Status, got.ReasonCode, len(findingsFor(info, browserChrome))) + } + } else if got.Status != model.BrowserCoverageFailed || got.ReasonCode != model.BrowserExtReasonRefusedTCC || info.ScanComplete || len(info.Findings) != 0 { + t.Fatalf("unexpected coverage %s/%s complete=%v findings=%d", got.Status, got.ReasonCode, info.ScanComplete, len(info.Findings)) + } + }) + } +} diff --git a/internal/scan/scanner.go b/internal/scan/scanner.go index 251f0273..326659e5 100644 --- a/internal/scan/scanner.go +++ b/internal/scan/scanner.go @@ -292,7 +292,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { log.StepStart("Inventorying browser extensions") start = time.Now() browserTarget, _ := exec.LoggedInUser() - browserExtensionScan := browserext.New(exec).WithSkipper(tccSkipper).Detect(ctx, browserTarget) + browserExtensionScan := browserext.New(exec).WithOSVersion(dev.OSVersion).WithSkipper(tccSkipper).Detect(ctx, browserTarget) log.StepDone(time.Since(start)) // npm config audit — surface-only inventory of every .npmrc on the host diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 772474c0..4e938916 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -1068,7 +1068,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err phaseCtx, phaseCancel = startPhase(ctx, tracker, "browser_extensions_scan") log.Progress("Inventorying browser extensions...") browserTarget, _ := exec.LoggedInUser() - browserExtensionScan := browserext.New(userExec).WithSkipper(tccSkipper).Detect(phaseCtx, browserTarget) + browserExtensionScan := browserext.New(userExec).WithOSVersion(dev.OSVersion).WithSkipper(tccSkipper).Detect(phaseCtx, browserTarget) if browserExtensionScan == nil { log.Progress(" Skipped: no interactive user to describe") } else {