From 5578ac9f7b6c69e8ec68627caa715e5a6d310823 Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Wed, 23 Sep 2026 21:55:11 +0530 Subject: [PATCH 1/5] fix(tcc): guard protected reads in non-browser scanners --- CHANGELOG.md | 2 + internal/detector/agent.go | 7 + internal/detector/aicli.go | 1 + internal/detector/aicli_agents_test.go | 5 + internal/detector/configaudit/bunfig.go | 24 +- internal/detector/configaudit/files.go | 57 ++++ internal/detector/configaudit/npmrc.go | 34 ++- .../detector/configaudit/npmrc_stat_unix.go | 4 + .../configaudit/npmrc_stat_windows.go | 4 + internal/detector/configaudit/pipconfig.go | 32 ++- internal/detector/configaudit/pnpm.go | 34 ++- .../protected_reads_darwin_test.go | 92 ++++++ internal/detector/configaudit/yarn.go | 24 +- internal/detector/credentials/detector.go | 2 +- internal/detector/extension.go | 7 + internal/detector/framework.go | 7 + internal/detector/ide.go | 19 ++ internal/detector/jetbrains.go | 7 + internal/detector/jetbrains_plugins.go | 18 +- internal/detector/mcp.go | 1 + internal/detector/mcp_discovery.go | 4 +- internal/detector/mcp_discovery_test.go | 7 +- internal/detector/mcp_plugins.go | 10 +- internal/detector/mcp_plugins_test.go | 9 +- internal/detector/mcp_test.go | 22 +- internal/detector/nodedist.go | 9 +- internal/detector/nodedist_modules.go | 13 +- internal/detector/nodepm.go | 8 +- internal/detector/nodepm_fallback.go | 4 + internal/detector/nodeproject.go | 3 +- internal/detector/nodescan.go | 19 +- .../detector/protected_reads_darwin_test.go | 266 ++++++++++++++++++ internal/detector/pythondist.go | 57 ++-- internal/detector/pythondist_test.go | 5 + internal/detector/pythonpm.go | 8 + internal/detector/pythonproject.go | 3 +- internal/detector/pythonscan.go | 6 +- internal/detector/rules/engine.go | 25 +- .../rules/protected_reads_darwin_test.go | 29 ++ internal/detector/rules/roots.go | 14 +- internal/detector/skills.go | 1 + internal/execguard/execguard.go | 5 + internal/execguard/execguard_test.go | 12 + internal/executor/executor.go | 32 ++- internal/executor/guarded_files_test.go | 71 +++++ internal/executor/guarded_glob.go | 76 +++++ internal/executor/mock.go | 10 + internal/executor/user_aware.go | 7 + internal/safepath/open_unix.go | 14 +- internal/safepath/open_windows.go | 2 +- internal/safepath/reader.go | 25 ++ internal/safepath/safepath.go | 3 +- internal/scan/scanner.go | 38 +-- internal/tcc/reader.go | 87 ++++++ internal/tcc/reader_darwin_test.go | 64 +++++ internal/tcc/tcc.go | 12 +- internal/telemetry/telemetry.go | 22 +- internal/versionmeta/versionmeta.go | 21 +- internal/versionmeta/versionmeta_test.go | 31 ++ 59 files changed, 1289 insertions(+), 146 deletions(-) create mode 100644 internal/detector/configaudit/files.go create mode 100644 internal/detector/configaudit/protected_reads_darwin_test.go create mode 100644 internal/detector/protected_reads_darwin_test.go create mode 100644 internal/detector/rules/protected_reads_darwin_test.go create mode 100644 internal/executor/guarded_files_test.go create mode 100644 internal/executor/guarded_glob.go create mode 100644 internal/tcc/reader.go create mode 100644 internal/tcc/reader_darwin_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index a2c1efce..7551adc3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,8 @@ See [VERSIONING.md](VERSIONING.md) for why the version starts at 1.8.1. ### Fixed +- Guard direct scanner reads, directory discovery and symbolic-link targets before filesystem access. Keep ordinary targeted Library inventory, and use existing failure results when protected reads prevent a complete package or rule scan. +- Use static package-manager metadata and config files when protected-directory scanning is disabled, avoiding config-loading command probes. - 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/agent.go b/internal/detector/agent.go index b9efd439..4eef2517 100644 --- a/internal/detector/agent.go +++ b/internal/detector/agent.go @@ -12,6 +12,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" "github.com/step-security/dev-machine-guard/internal/versionmeta" ) @@ -213,3 +214,9 @@ func isCoworkVersion(version string) bool { } return major == 0 && minor >= 7 } + +// WithSkipper protects direct and redirected inventory reads. +func (d *AgentDetector) WithSkipper(s *tcc.Skipper) *AgentDetector { + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize) + return d +} diff --git a/internal/detector/aicli.go b/internal/detector/aicli.go index 66314831..c225fd87 100644 --- a/internal/detector/aicli.go +++ b/internal/detector/aicli.go @@ -317,6 +317,7 @@ func (d *AICLIDetector) WithLogger(log *progress.Logger) *AICLIDetector { // everything", the same contract every walking detector honors. func (d *AICLIDetector) WithSkipper(skipper *tcc.Skipper) *AICLIDetector { d.skipper = skipper + d.exec = tcc.GuardedFiles(d.exec, skipper, maxLockfileSize, "pnpm", "Application Support/fnm") return d } diff --git a/internal/detector/aicli_agents_test.go b/internal/detector/aicli_agents_test.go index e58b5310..6f7804ff 100644 --- a/internal/detector/aicli_agents_test.go +++ b/internal/detector/aicli_agents_test.go @@ -3051,3 +3051,8 @@ func TestAICLIAgents2_EmptyFixture(t *testing.T) { }) } } + +// Keep the recording mock attached when production selects a guarded reader. +func (r *recExec) GuardedFiles(_ []string, _ func(string) string, _ int64) executor.Executor { + return r +} diff --git a/internal/detector/configaudit/bunfig.go b/internal/detector/configaudit/bunfig.go index b08db982..f5eec145 100644 --- a/internal/detector/configaudit/bunfig.go +++ b/internal/detector/configaudit/bunfig.go @@ -71,6 +71,13 @@ func (d *BunDetector) WithLogger(log *progress.Logger) *BunDetector { // WithSkipper attaches a TCC skipper so discovery skips macOS-protected dirs. func (d *BunDetector) WithSkipper(s *tcc.Skipper) *BunDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxConfigFileSize) + if tcc.ProtectedReadsDisabled(d.exec, s) { + d.ownerLookup = guardedOwner(d.exec) + d.inGitRepo = guardedInGitRepo(d.exec) + // Git loads user-controlled config and includes in its own process. + d.gitTracked = nil + } return d } @@ -144,7 +151,7 @@ func (d *BunDetector) Detect(ctx context.Context, searchDirs []string, loggedInU // searchDirs walk). The .npmrc walk overlaps with the npm + pnpm audits; if // scan time becomes a concern, share results across detectors. func (d *BunDetector) discoverAuthSideChannel(ctx context.Context, searchDirs []string, loggedInUser *user.User) []model.NPMRCFile { - side := NewNPMRCDetector(d.exec) + side := NewNPMRCDetector(d.exec).WithSkipper(d.skipper) side.skipper = d.skipper side.ownerLookup = d.ownerLookup side.gitTracked = d.gitTracked @@ -167,7 +174,7 @@ func (d *BunDetector) findProjectBunfigs(dir string) []string { return nil } var results []string - _ = filepath.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { + _ = d.exec.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { if err != nil { return nil } @@ -197,7 +204,7 @@ func (d *BunDetector) findProjectBunfigs(dir string) []string { func (d *BunDetector) collectFile(ctx context.Context, path, scope string) model.BunConfigFile { f := model.BunConfigFile{Path: path, Scope: scope} - linfo, err := os.Lstat(path) + linfo, err := auditLstat(d.exec, d.skipper, path) if err != nil { if os.IsNotExist(err) { f.Exists = false @@ -209,13 +216,13 @@ func (d *BunDetector) collectFile(ctx context.Context, path, scope string) model } f.Exists = true - if linfo.Mode()&os.ModeSymlink != 0 { - if target, err := os.Readlink(path); err == nil { + if linfo.Mode()&os.ModeSymlink != 0 || tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if target, err := d.exec.Readlink(path); err == nil { f.SymlinkTo = target } } - info, err := os.Stat(path) + info, err := auditStat(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "stat: " + err.Error() @@ -241,7 +248,7 @@ func (d *BunDetector) collectFile(ctx context.Context, path, scope string) model // #nosec G304 -- path comes from the detector's own candidate enumeration // (user-scope well-known locations + project walk). - data, err := os.ReadFile(path) + data, err := auditReadFile(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "read: " + err.Error() @@ -285,6 +292,9 @@ func (d *BunDetector) bunVersion(ctx context.Context) string { } target = path } + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return "unknown" + } d.log.Progress("exec fallback: running %s --version (no metadata version source)", target) stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, target, "--version") if exit != 0 { diff --git a/internal/detector/configaudit/files.go b/internal/detector/configaudit/files.go new file mode 100644 index 00000000..c7288bc3 --- /dev/null +++ b/internal/detector/configaudit/files.go @@ -0,0 +1,57 @@ +package configaudit + +import ( + "os" + "path/filepath" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +const maxConfigFileSize = 32 << 20 +const protectedCommandReason = "not collected: protected-directory scanning is disabled" + +func auditLstat(exec executor.Executor, s *tcc.Skipper, path string) (os.FileInfo, error) { + if tcc.ProtectedReadsDisabled(exec, s) { + return exec.Stat(path) + } + return os.Lstat(path) +} + +func auditStat(exec executor.Executor, s *tcc.Skipper, path string) (os.FileInfo, error) { + if tcc.ProtectedReadsDisabled(exec, s) { + return exec.Stat(path) + } + return os.Stat(path) +} + +func auditReadFile(exec executor.Executor, s *tcc.Skipper, path string) ([]byte, error) { + if tcc.ProtectedReadsDisabled(exec, s) { + return exec.ReadFile(path) + } + // #nosec G304 -- Existing unguarded mode reads scanner-selected config files. + return os.ReadFile(path) +} + +func guardedOwner(exec executor.Executor) func(string) ownerInfo { + return func(path string) ownerInfo { + info, err := exec.Stat(path) + if err != nil { + return ownerInfo{} + } + return ownerFromInfo(info) + } +} + +func guardedInGitRepo(exec executor.Executor) func(string) bool { + return func(path string) bool { + for dir := filepath.Dir(path); ; dir = filepath.Dir(dir) { + if _, err := exec.Stat(filepath.Join(dir, ".git")); err == nil { + return true + } + if filepath.Dir(dir) == dir { + return false + } + } + } +} diff --git a/internal/detector/configaudit/npmrc.go b/internal/detector/configaudit/npmrc.go index d1298e57..b41bda69 100644 --- a/internal/detector/configaudit/npmrc.go +++ b/internal/detector/configaudit/npmrc.go @@ -17,6 +17,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/tcc" + "github.com/step-security/dev-machine-guard/internal/versionmeta" ) // maxNPMRCFiles caps the number of .npmrc files we report. Even on big @@ -82,6 +83,13 @@ func NewNPMRCDetector(exec executor.Executor) *NPMRCDetector { // directories. A nil skipper is a no-op. Returns the detector for chaining. func (d *NPMRCDetector) WithSkipper(s *tcc.Skipper) *NPMRCDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxConfigFileSize) + if tcc.ProtectedReadsDisabled(d.exec, s) { + d.ownerLookup = guardedOwner(d.exec) + d.inGitRepo = guardedInGitRepo(d.exec) + // Git loads user-controlled config and includes in its own process. + d.gitTracked = nil + } return d } @@ -170,7 +178,7 @@ func (d *NPMRCDetector) findProjectNPMRCs(dir string) []string { return nil } var results []string - _ = filepath.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { + _ = d.exec.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { if err != nil { return nil } @@ -228,7 +236,7 @@ func (d *NPMRCDetector) collectFile(ctx context.Context, path, scope string) mod } // Lstat first so a symlink doesn't get followed silently. - linfo, err := os.Lstat(path) + linfo, err := auditLstat(d.exec, d.skipper, path) if err != nil { // Distinguish "not found" from "not readable" so the user can act. if os.IsNotExist(err) { @@ -241,14 +249,14 @@ func (d *NPMRCDetector) collectFile(ctx context.Context, path, scope string) mod } f.Exists = true - if linfo.Mode()&os.ModeSymlink != 0 { - if target, err := os.Readlink(path); err == nil { + if linfo.Mode()&os.ModeSymlink != 0 || tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if target, err := d.exec.Readlink(path); err == nil { f.SymlinkTo = target } } // Stat (follows symlinks) for size/mtime/mode. - info, err := os.Stat(path) + info, err := auditStat(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "stat: " + err.Error() @@ -275,7 +283,7 @@ func (d *NPMRCDetector) collectFile(ctx context.Context, path, scope string) mod // #nosec G304 -- path comes from the detector's own candidate // enumeration of well-known npmrc locations (built-in/global/user/ // project); not from external input. - data, err := os.ReadFile(path) + data, err := auditReadFile(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "read: " + err.Error() @@ -301,6 +309,9 @@ func (d *NPMRCDetector) collectFile(ctx context.Context, path, scope string) mod // captureEffective runs `npm config ls -l --json` and `npm config ls -l` for // source attribution. Returns nil when npm is unavailable. func (d *NPMRCDetector) captureEffective(ctx context.Context) *model.NPMRCEffective { + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return &model.NPMRCEffective{Error: protectedCommandReason} + } if _, err := d.exec.LookPath("npm"); err != nil { return nil } @@ -371,6 +382,14 @@ func parseSourceAttribution(text string) map[string]string { // npmVersion returns the npm CLI's version string, "unknown" on failure. func (d *NPMRCDetector) npmVersion(ctx context.Context) string { + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if path, err := d.exec.LookPath("npm"); err == nil { + if v := versionmeta.FromBinary(ctx, d.exec, path); v != "" { + return v + } + } + return "unknown" + } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "npm", "--version") if exit != 0 { return "unknown" @@ -386,6 +405,9 @@ func (d *NPMRCDetector) npmVersion(ctx context.Context) string { // empty if the call failed or the value is "undefined" (npm's literal output // for an unset key). func (d *NPMRCDetector) npmConfigGet(ctx context.Context, key string) string { + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return "" + } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "npm", "config", "get", key) if exit != 0 { return "" diff --git a/internal/detector/configaudit/npmrc_stat_unix.go b/internal/detector/configaudit/npmrc_stat_unix.go index b1f725e4..ca2c4569 100644 --- a/internal/detector/configaudit/npmrc_stat_unix.go +++ b/internal/detector/configaudit/npmrc_stat_unix.go @@ -20,6 +20,10 @@ func statOwner(path string) ownerInfo { if err != nil { return ownerInfo{} } + return ownerFromInfo(info) +} + +func ownerFromInfo(info os.FileInfo) ownerInfo { st, ok := info.Sys().(*syscall.Stat_t) if !ok { return ownerInfo{} diff --git a/internal/detector/configaudit/npmrc_stat_windows.go b/internal/detector/configaudit/npmrc_stat_windows.go index 69e5c154..f0458018 100644 --- a/internal/detector/configaudit/npmrc_stat_windows.go +++ b/internal/detector/configaudit/npmrc_stat_windows.go @@ -2,9 +2,13 @@ package configaudit +import "os" + // statOwner is a no-op on Windows: getting a meaningful owner string from a // SID is non-trivial and not actionable for the audit's first cut. The // detector handles ownerInfo.OK == false by leaving owner fields empty. func statOwner(_ string) ownerInfo { return ownerInfo{} } + +func ownerFromInfo(_ os.FileInfo) ownerInfo { return ownerInfo{} } diff --git a/internal/detector/configaudit/pipconfig.go b/internal/detector/configaudit/pipconfig.go index 75cebbe0..578daa78 100644 --- a/internal/detector/configaudit/pipconfig.go +++ b/internal/detector/configaudit/pipconfig.go @@ -17,6 +17,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" ) // devNullPaths are values of $PIP_CONFIG_FILE that disable all config-file @@ -87,7 +88,8 @@ var pipConfigDebugFileRE = regexp.MustCompile(`^\s+(.+),\s+exists:\s+(True|False // PipConfigDetector performs the read-only pip config audit. type PipConfigDetector struct { - exec executor.Executor + skipper *tcc.Skipper + exec executor.Executor // Hooks for tests; default to platform-specific impls. Owner lookup // uses syscall.Stat_t on Unix and is a no-op on Windows. @@ -175,6 +177,9 @@ func (d *PipConfigDetector) detectPip(ctx context.Context) (string, []string, st // invoking --version against them pops a GUI install prompt. continue } + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return path, cand.args, cand.display, "unknown", true + } args := append([]string(nil), cand.args...) args = append(args, "--version") stdout, _, exit, err := d.exec.RunWithTimeout(ctx, 5*time.Second, cand.binary, args...) @@ -465,7 +470,7 @@ func (d *PipConfigDetector) discoverFiles(ctx context.Context, pipAvailable bool // Preferred: `pip config debug`. Falls back to manual path enumeration // when pip isn't installed or the output is unparseable. usedPipDebug := false - if pipAvailable { + if pipAvailable && !tcc.ProtectedReadsDisabled(d.exec, d.skipper) { if discovered, ok := d.discoverViaPipDebug(ctx); ok { usedPipDebug = true for _, e := range discovered { @@ -574,7 +579,7 @@ func pipConfigFilename(goos string) string { // --- per-file metadata ------------------------------------------------------ func (d *PipConfigDetector) populateFileMetadata(ctx context.Context, f *model.PipConfigFile) { - info, err := os.Lstat(f.Path) + info, err := auditLstat(d.exec, d.skipper, f.Path) if err != nil { if os.IsNotExist(err) { f.Exists = false @@ -590,7 +595,7 @@ func (d *PipConfigDetector) populateFileMetadata(ctx context.Context, f *model.P // the symlink itself exists; a broken symlink target shouldn't crash // the audit). if info.Mode()&os.ModeSymlink != 0 { - stat, statErr := os.Stat(f.Path) + stat, statErr := auditStat(d.exec, d.skipper, f.Path) if statErr != nil { f.Readable = false f.ParseError = "stat (followed symlink): " + statErr.Error() @@ -615,7 +620,7 @@ func (d *PipConfigDetector) populateFileMetadata(ctx context.Context, f *model.P } } - data, err := os.ReadFile(f.Path) + data, err := auditReadFile(d.exec, d.skipper, f.Path) if err != nil { f.Readable = false f.ParseError = "read: " + err.Error() @@ -647,6 +652,9 @@ func (d *PipConfigDetector) populateFileMetadata(ctx context.Context, f *model.P var pipConfigListPrefix = regexp.MustCompile(`^([A-Za-z0-9_\-]+)\.([A-Za-z0-9_\-]+)='`) func (d *PipConfigDetector) captureEffective(ctx context.Context) (*model.PipEffective, error) { + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return nil, errors.New(protectedCommandReason) + } stdout, exit, ok := d.runPip(ctx, 10*time.Second, "config", "list", "-v") if !ok || exit != 0 { return nil, fmt.Errorf("pip config list -v exited %d", exit) @@ -747,7 +755,7 @@ func (d *PipConfigDetector) probeNetrc(loggedInUser *user.User) *model.PipNetrcS path = filepath.Join(homeDir, "_netrc") } out := &model.PipNetrcStatus{Path: path} - info, err := os.Stat(path) + info, err := auditStat(d.exec, d.skipper, path) if err != nil { if !os.IsNotExist(err) { out.Exists = true // probe error; surface that we tried @@ -769,3 +777,15 @@ var _ = func() fs.WalkDirFunc { return nil } // formatModeOctal is unused today (mode is rendered via fmt.Sprintf in // populateFileMetadata) but kept for tests; suppress unused warning. var _ = strconv.FormatUint + +func (d *PipConfigDetector) WithSkipper(s *tcc.Skipper) *PipConfigDetector { + d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxConfigFileSize, "Application Support/pip/pip.conf") + if tcc.ProtectedReadsDisabled(d.exec, s) { + d.ownerLookup = guardedOwner(d.exec) + d.inGitRepo = guardedInGitRepo(d.exec) + // Git loads user-controlled config and includes in its own process. + d.gitTracked = nil + } + return d +} diff --git a/internal/detector/configaudit/pnpm.go b/internal/detector/configaudit/pnpm.go index f7b0fd88..d50660af 100644 --- a/internal/detector/configaudit/pnpm.go +++ b/internal/detector/configaudit/pnpm.go @@ -16,6 +16,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/tcc" + "github.com/step-security/dev-machine-guard/internal/versionmeta" ) // pnpmEnvVars: pnpm-specific names plus the npm_config_* lowercase variants @@ -63,6 +64,13 @@ func NewPnpmDetector(exec executor.Executor) *PnpmDetector { // directories. nil is a no-op. Returns the detector for chaining. func (d *PnpmDetector) WithSkipper(s *tcc.Skipper) *PnpmDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxConfigFileSize) + if tcc.ProtectedReadsDisabled(d.exec, s) { + d.ownerLookup = guardedOwner(d.exec) + d.inGitRepo = guardedInGitRepo(d.exec) + // Git loads user-controlled config and includes in its own process. + d.gitTracked = nil + } return d } @@ -131,7 +139,7 @@ func (d *PnpmDetector) findProjectNPMRCs(dir string) []string { return nil } var results []string - _ = filepath.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { + _ = d.exec.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { if err != nil { return nil } @@ -161,7 +169,7 @@ func (d *PnpmDetector) findProjectNPMRCs(dir string) []string { func (d *PnpmDetector) collectFile(ctx context.Context, path, scope string) model.NPMRCFile { f := model.NPMRCFile{Path: path, Scope: scope} - linfo, err := os.Lstat(path) + linfo, err := auditLstat(d.exec, d.skipper, path) if err != nil { if os.IsNotExist(err) { f.Exists = false @@ -173,13 +181,13 @@ func (d *PnpmDetector) collectFile(ctx context.Context, path, scope string) mode } f.Exists = true - if linfo.Mode()&os.ModeSymlink != 0 { - if target, err := os.Readlink(path); err == nil { + if linfo.Mode()&os.ModeSymlink != 0 || tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if target, err := d.exec.Readlink(path); err == nil { f.SymlinkTo = target } } - info, err := os.Stat(path) + info, err := auditStat(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "stat: " + err.Error() @@ -205,7 +213,7 @@ func (d *PnpmDetector) collectFile(ctx context.Context, path, scope string) mode // #nosec G304 -- path comes from the detector's own candidate enumeration // of well-known npmrc locations; not external input. - data, err := os.ReadFile(path) + data, err := auditReadFile(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "read: " + err.Error() @@ -229,6 +237,9 @@ func (d *PnpmDetector) collectFile(ctx context.Context, path, scope string) mode // captureEffective runs `pnpm config list --json`. SourceByKey stays empty — // pnpm doesn't emit per-key source attribution like `npm config ls -l` does. func (d *PnpmDetector) captureEffective(ctx context.Context) *model.PnpmEffective { + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return &model.PnpmEffective{Error: protectedCommandReason} + } eff := &model.PnpmEffective{ SourceByKey: map[string]string{}, Config: map[string]any{}, @@ -249,6 +260,14 @@ func (d *PnpmDetector) captureEffective(ctx context.Context) *model.PnpmEffectiv // pnpmVersion returns the pnpm CLI's version string, "unknown" on failure. func (d *PnpmDetector) pnpmVersion(ctx context.Context) string { + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if path, err := d.exec.LookPath("pnpm"); err == nil { + if v := versionmeta.FromBinary(ctx, d.exec, path); v != "" { + return v + } + } + return "unknown" + } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "pnpm", "--version") if exit != 0 { return "unknown" @@ -263,6 +282,9 @@ func (d *PnpmDetector) pnpmVersion(ctx context.Context) string { // pnpmConfigGet runs `pnpm config get ` and returns the trimmed value, // or empty if the call failed or the value is pnpm's literal "undefined". func (d *PnpmDetector) pnpmConfigGet(ctx context.Context, key string) string { + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return "" + } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "pnpm", "config", "get", key) if exit != 0 { return "" diff --git a/internal/detector/configaudit/protected_reads_darwin_test.go b/internal/detector/configaudit/protected_reads_darwin_test.go new file mode 100644 index 00000000..e57192b4 --- /dev/null +++ b/internal/detector/configaudit/protected_reads_darwin_test.go @@ -0,0 +1,92 @@ +//go:build darwin + +package configaudit + +import ( + "context" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +func TestProtectedConfigReads(t *testing.T) { + for _, blocked := range []bool{false, true} { + home := t.TempDir() + target := filepath.Join(home, "Library", "Containers", "test-app", "config") + if err := os.MkdirAll(filepath.Dir(target), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(target, []byte("registry=https://registry.npmjs.org/\n"), 0600); err != nil { + t.Fatal(err) + } + path := filepath.Join(home, "config") + if blocked { + if err := os.Symlink(target, path); err != nil { + t.Fatal(err) + } + } else if err := os.WriteFile(path, []byte("registry=https://registry.npmjs.org/\n"), 0600); err != nil { + t.Fatal(err) + } + e, s := executor.NewReal(), tcc.New(home) + ctx := context.Background() + npm := NewNPMRCDetector(e).WithSkipper(s).collectFile(ctx, path, "user") + pnpm := NewPnpmDetector(e).WithSkipper(s).collectFile(ctx, path, "user") + bun := NewBunDetector(e).WithSkipper(s).collectFile(ctx, path, "user") + yarn := NewYarnDetector(e).WithSkipper(s).collectFile(ctx, path, "user", "classic") + pip := model.PipConfigFile{Path: path, Layer: "user"} + NewPipConfigDetector(e).WithSkipper(s).populateFileMetadata(ctx, &pip) + for _, got := range []struct { + name string + readable bool + err string + }{{"npm", npm.Readable, npm.ParseError}, {"pnpm", pnpm.Readable, pnpm.ParseError}, {"bun", bun.Readable, bun.ParseError}, {"yarn", yarn.Readable, yarn.ParseError}, {"pip", pip.Readable, pip.ParseError}} { + if got.readable == blocked || (blocked && !strings.Contains(got.err, "tcc_protected")) { + t.Errorf("%s blocked=%v readable=%v error=%s", got.name, blocked, got.readable, got.err) + } + } + } +} + +func TestProtectedConfigCommandsAreSkipped(t *testing.T) { + e, s := executor.NewMock(), tcc.New("/Users/test-user") + ctx := context.Background() + if got := NewNPMRCDetector(e).WithSkipper(s).captureEffective(ctx); got == nil || got.Error != protectedCommandReason { + t.Fatalf("npm effective=%+v", got) + } + if got := NewPnpmDetector(e).WithSkipper(s).captureEffective(ctx); got == nil || got.Error != protectedCommandReason { + t.Fatalf("pnpm effective=%+v", got) + } + if _, err := NewPipConfigDetector(e).WithSkipper(s).captureEffective(ctx); err == nil || err.Error() != protectedCommandReason { + t.Fatalf("pip err=%v", err) + } +} + +func TestProtectedConfigOrdinarySymlinkMetadata(t *testing.T) { + home := t.TempDir() + target, link := filepath.Join(home, "config"), filepath.Join(home, ".npmrc") + if err := os.WriteFile(target, []byte("registry=https://registry.npmjs.org/\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + e, s := executor.NewReal(), tcc.New(home) + ctx := context.Background() + npm := NewNPMRCDetector(e).WithSkipper(s).collectFile(ctx, link, "user") + pnpm := NewPnpmDetector(e).WithSkipper(s).collectFile(ctx, link, "user") + bun := NewBunDetector(e).WithSkipper(s).collectFile(ctx, link, "user") + yarn := NewYarnDetector(e).WithSkipper(s).collectFile(ctx, link, "user", "classic") + for _, got := range []struct { + readable bool + target string + }{{npm.Readable, npm.SymlinkTo}, {pnpm.Readable, pnpm.SymlinkTo}, {bun.Readable, bun.SymlinkTo}, {yarn.Readable, yarn.SymlinkTo}} { + if !got.readable || got.target != target { + t.Errorf("ordinary symlink metadata = %+v", got) + } + } +} diff --git a/internal/detector/configaudit/yarn.go b/internal/detector/configaudit/yarn.go index 36ddd118..19b7f3af 100644 --- a/internal/detector/configaudit/yarn.go +++ b/internal/detector/configaudit/yarn.go @@ -77,6 +77,13 @@ func (d *YarnDetector) WithLogger(log *progress.Logger) *YarnDetector { // WithSkipper attaches a TCC skipper so discovery skips macOS-protected dirs. func (d *YarnDetector) WithSkipper(s *tcc.Skipper) *YarnDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxConfigFileSize) + if tcc.ProtectedReadsDisabled(d.exec, s) { + d.ownerLookup = guardedOwner(d.exec) + d.inGitRepo = guardedInGitRepo(d.exec) + // Git loads user-controlled config and includes in its own process. + d.gitTracked = nil + } return d } @@ -145,7 +152,7 @@ func (d *YarnDetector) Detect(ctx context.Context, searchDirs []string, loggedIn // for auth. builtin/global belong to npm proper and are dropped. See the // bun-side note about overlapping work — same caveat applies. func (d *YarnDetector) discoverAuthSideChannel(ctx context.Context, searchDirs []string, loggedInUser *user.User) []model.NPMRCFile { - side := NewNPMRCDetector(d.exec) + side := NewNPMRCDetector(d.exec).WithSkipper(d.skipper) side.skipper = d.skipper side.ownerLookup = d.ownerLookup side.gitTracked = d.gitTracked @@ -168,7 +175,7 @@ func (d *YarnDetector) findProjectYarnConfigs(dir string) []string { return nil } var results []string - _ = filepath.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { + _ = d.exec.WalkDir(dir, func(path string, entry fs.DirEntry, err error) error { if err != nil { return nil } @@ -198,7 +205,7 @@ func (d *YarnDetector) findProjectYarnConfigs(dir string) []string { func (d *YarnDetector) collectFile(ctx context.Context, path, scope, flavor string) model.YarnConfigFile { f := model.YarnConfigFile{Path: path, Scope: scope, Flavor: flavor} - linfo, err := os.Lstat(path) + linfo, err := auditLstat(d.exec, d.skipper, path) if err != nil { if os.IsNotExist(err) { f.Exists = false @@ -210,13 +217,13 @@ func (d *YarnDetector) collectFile(ctx context.Context, path, scope, flavor stri } f.Exists = true - if linfo.Mode()&os.ModeSymlink != 0 { - if target, err := os.Readlink(path); err == nil { + if linfo.Mode()&os.ModeSymlink != 0 || tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if target, err := d.exec.Readlink(path); err == nil { f.SymlinkTo = target } } - info, err := os.Stat(path) + info, err := auditStat(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "stat: " + err.Error() @@ -241,7 +248,7 @@ func (d *YarnDetector) collectFile(ctx context.Context, path, scope, flavor stri } // #nosec G304 -- path comes from the detector's own candidate enumeration. - data, err := os.ReadFile(path) + data, err := auditReadFile(d.exec, d.skipper, path) if err != nil { f.Readable = false f.ParseError = "read: " + err.Error() @@ -291,6 +298,9 @@ func (d *YarnDetector) yarnVersion(ctx context.Context) string { } target = path } + if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + return "unknown" + } d.log.Progress("exec fallback: running %s --version (no metadata version source)", target) stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, target, "--version") if exit != 0 { diff --git a/internal/detector/credentials/detector.go b/internal/detector/credentials/detector.go index 577f9e5f..cf89a330 100644 --- a/internal/detector/credentials/detector.go +++ b/internal/detector/credentials/detector.go @@ -376,7 +376,7 @@ func (d *Detector) buildFinding(ctx context.Context, scan *scanState, s source, finding.BroadReadAllowACEPresent = broadReadAllowACE(resolved) } finding.InGitRepo = scan.inGitRepo(resolved) - if finding.InGitRepo { + if finding.InGitRepo && !tcc.ProtectedReadsDisabled(d.exec, d.skipper) { finding.GitTracked = gitTracked(ctx, d.exec, resolved) } return finding diff --git a/internal/detector/extension.go b/internal/detector/extension.go index 7f1cb19e..24f82490 100644 --- a/internal/detector/extension.go +++ b/internal/detector/extension.go @@ -8,6 +8,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" ) type ideExtensionSpec struct { @@ -164,3 +165,9 @@ func (d *ExtensionDetector) loadObsolete(extDir string) map[string]bool { } return obsoleteMap } + +// WithSkipper protects direct and redirected inventory reads. +func (d *ExtensionDetector) WithSkipper(s *tcc.Skipper) *ExtensionDetector { + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Application Support/JetBrains", "Application Support/Google/AndroidStudio*") + return d +} diff --git a/internal/detector/framework.go b/internal/detector/framework.go index dd29e7c9..1da696da 100644 --- a/internal/detector/framework.go +++ b/internal/detector/framework.go @@ -9,6 +9,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" "github.com/step-security/dev-machine-guard/internal/versionmeta" ) @@ -46,6 +47,12 @@ func NewFrameworkDetector(exec executor.Executor) *FrameworkDetector { return &FrameworkDetector{exec: exec, log: progress.NewNoop()} } +// WithSkipper protects static metadata and suppresses command fallbacks. +func (d *FrameworkDetector) WithSkipper(s *tcc.Skipper) *FrameworkDetector { + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize) + return d +} + // WithLogger injects a logger (used to surface exec fallbacks when metadata // version resolution misses). Chainable, mirrors configaudit's WithSkipper. func (d *FrameworkDetector) WithLogger(log *progress.Logger) *FrameworkDetector { diff --git a/internal/detector/ide.go b/internal/detector/ide.go index dc7118de..439e44b4 100644 --- a/internal/detector/ide.go +++ b/internal/detector/ide.go @@ -10,6 +10,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/execguard" "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" "github.com/step-security/dev-machine-guard/internal/versionmeta" "howett.net/plist" ) @@ -677,6 +678,19 @@ func readEclipseProductVersion(exec executor.Executor, filePath string) string { // readPlistVersion reads CFBundleShortVersionString from an Info.plist (macOS). func readPlistVersion(ctx context.Context, exec executor.Executor, plistPath string) string { + if tcc.HasGuard(exec) { + data, err := exec.ReadFile(plistPath) + if err != nil { + return "unknown" + } + var info struct { + Version string `plist:"CFBundleShortVersionString"` + } + if _, err := plist.Unmarshal(data, &info); err == nil && info.Version != "" { + return info.Version + } + return "unknown" + } if !exec.FileExists(plistPath) { return "unknown" } @@ -875,3 +889,8 @@ func resolveInstallDirFromBinary(binPath string) string { // Simple /bin/ layout: /usr/share/code/bin/code -> /usr/share/code return parent } + +func (d *IDEDetector) WithSkipper(s *tcc.Skipper) *IDEDetector { + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize) + return d +} diff --git a/internal/detector/jetbrains.go b/internal/detector/jetbrains.go index e1b0f54a..f7687af1 100644 --- a/internal/detector/jetbrains.go +++ b/internal/detector/jetbrains.go @@ -8,6 +8,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" ) // jetbrainsProductInfo holds the fields we read from product-info.json. @@ -248,3 +249,9 @@ func (d *JetBrainsPluginDetector) parsePluginVersion(libDir, pluginDirName strin return "unknown" } + +// WithSkipper protects direct and redirected inventory reads. +func (d *JetBrainsPluginDetector) WithSkipper(s *tcc.Skipper) *JetBrainsPluginDetector { + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Application Support/JetBrains", "Application Support/Google/AndroidStudio*") + return d +} diff --git a/internal/detector/jetbrains_plugins.go b/internal/detector/jetbrains_plugins.go index cd2cdc9d..69fe1782 100644 --- a/internal/detector/jetbrains_plugins.go +++ b/internal/detector/jetbrains_plugins.go @@ -223,7 +223,7 @@ func (d *ExtensionDetector) readPluginXMLFromJars(pluginDir string) []byte { continue } jarPath := filepath.Join(libDir, entry.Name()) - data := readFileFromZip(jarPath, "META-INF/plugin.xml") + data := d.readFileFromZip(jarPath, "META-INF/plugin.xml") if data != nil { return data } @@ -232,12 +232,20 @@ func (d *ExtensionDetector) readPluginXMLFromJars(pluginDir string) []byte { } // readFileFromZip extracts a single file from a zip/jar archive. -func readFileFromZip(zipPath, targetFile string) []byte { - r, err := zip.OpenReader(zipPath) +func (d *ExtensionDetector) readFileFromZip(zipPath, targetFile string) []byte { + file, err := d.exec.Open(zipPath) + if err != nil { + return nil + } + defer func() { _ = file.Close() }() + info, err := file.Stat() + if err != nil { + return nil + } + r, err := zip.NewReader(file, info.Size()) if err != nil { return nil } - defer func() { _ = r.Close() }() for _, f := range r.File { if f.Name == targetFile { @@ -246,7 +254,7 @@ func readFileFromZip(zipPath, targetFile string) []byte { return nil } defer func() { _ = rc.Close() }() - data, err := io.ReadAll(rc) + data, err := io.ReadAll(io.LimitReader(rc, 1<<20)) if err != nil { return nil } diff --git a/internal/detector/mcp.go b/internal/detector/mcp.go index 833c69c1..e5ea3d06 100644 --- a/internal/detector/mcp.go +++ b/internal/detector/mcp.go @@ -64,6 +64,7 @@ func NewMCPDetector(exec executor.Executor) *MCPDetector { // for chaining. func (d *MCPDetector) WithSkipper(s *tcc.Skipper) *MCPDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Application Support/Claude/claude_desktop_config.json", "Application Support/Code/User/mcp.json", "Application Support/Code - Insiders/User/mcp.json", "Application Support/Cursor/User/mcp.json", "Application Support/Windsurf/User/mcp.json", "Application Support/VSCodium/User/mcp.json") return d } diff --git a/internal/detector/mcp_discovery.go b/internal/detector/mcp_discovery.go index fbe11298..270f7014 100644 --- a/internal/detector/mcp_discovery.go +++ b/internal/detector/mcp_discovery.go @@ -134,7 +134,7 @@ func (d *MCPDetector) discoverWalkedMCPConfigs(searchDirs []string, homeDir stri if root == "" || filesVisited > maxMCPWalkFiles { continue } - _ = filepath.WalkDir(root, func(path string, ent fs.DirEntry, err error) error { + _ = d.exec.WalkDir(root, func(path string, ent fs.DirEntry, err error) error { if err != nil { // Unreadable dir: skip its subtree, continue elsewhere. if ent != nil && ent.IsDir() { @@ -164,7 +164,7 @@ func (d *MCPDetector) discoverWalkedMCPConfigs(searchDirs []string, homeDir stri c := filepath.Clean(path) if !seen[c] { seen[c] = true - if source, vendor, keep := classifyWalkedMCPConfig(c, root); keep { + if source, vendor, keep := classifyWalkedMCPConfig(d.exec, c, root); keep { specs = append(specs, mcpConfigSpec{ SourceName: source, ConfigPath: c, diff --git a/internal/detector/mcp_discovery_test.go b/internal/detector/mcp_discovery_test.go index 7151021e..d1d94224 100644 --- a/internal/detector/mcp_discovery_test.go +++ b/internal/detector/mcp_discovery_test.go @@ -1,6 +1,7 @@ package detector import ( + "github.com/step-security/dev-machine-guard/internal/executor" "os" "path/filepath" "runtime" @@ -40,7 +41,7 @@ func TestDiscoverWalkedMCPConfigs(t *testing.T) { writeFile(t, root, "proj/dist/mcp.json") // excluded dir writeFile(t, root, "proj/readme.txt") // not a config - d := &MCPDetector{} // no skipper + d := NewMCPDetector(executor.NewReal()) // no skipper got := gotPathSet(d.discoverWalkedMCPConfigs([]string{root}, "")) if !got[want1] { @@ -122,7 +123,7 @@ func TestDiscoverWalkedMCPConfigs_OpenCode(t *testing.T) { writeFile(t, root, "proj-c/node_modules/pkg/opencode.json") // excluded dir writeFile(t, root, "proj-d/opencode.json.bak") // not a config - d := &MCPDetector{} // no skipper + d := NewMCPDetector(executor.NewReal()) // no skipper specs := d.discoverWalkedMCPConfigs([]string{root}, "") got := gotPathSet(specs) @@ -156,7 +157,7 @@ func TestDiscoverWalkedMCPConfigs_TCCSkip(t *testing.T) { protected := writeFile(t, home, "Library/Application Support/App/mcp.json") allowed := writeFile(t, home, "proj/.mcp.json") - d := &MCPDetector{skipper: tcc.New(home)} + d := NewMCPDetector(executor.NewReal()).WithSkipper(tcc.New(home)) got := gotPathSet(d.discoverWalkedMCPConfigs([]string{home}, "")) if got[protected] { diff --git a/internal/detector/mcp_plugins.go b/internal/detector/mcp_plugins.go index 790de5ed..05449be5 100644 --- a/internal/detector/mcp_plugins.go +++ b/internal/detector/mcp_plugins.go @@ -1,7 +1,7 @@ package detector import ( - "os" + "github.com/step-security/dev-machine-guard/internal/executor" "path/filepath" "runtime" "strings" @@ -34,9 +34,9 @@ const maxPluginRootLookup = 8 // per catalog entry, none of which any agent loads (issue #201). So a // plugin-scoped config counts only when it is the package's own .mcp.json and // the package is installed. -func classifyWalkedMCPConfig(path, root string) (sourceName, vendor string, keep bool) { +func classifyWalkedMCPConfig(exec executor.Executor, path, root string) (sourceName, vendor string, keep bool) { dir := filepath.Dir(path) - pluginRoot, manifest, isPlugin := pluginPackageRoot(dir, root) + pluginRoot, manifest, isPlugin := pluginPackageRoot(exec, dir, root) if !isPlugin { return "discovered_mcp", mcpVendorForPath(path), true } @@ -51,11 +51,11 @@ func classifyWalkedMCPConfig(path, root string) (sourceName, vendor string, keep // pluginPackageRoot walks up from dir looking for a plugin manifest, stopping at // the walk root. -func pluginPackageRoot(dir, root string) (string, mcpPluginManifest, bool) { +func pluginPackageRoot(exec executor.Executor, dir, root string) (string, mcpPluginManifest, bool) { cleanRoot := filepath.Clean(root) for i := 0; i < maxPluginRootLookup; i++ { for _, m := range mcpPluginManifests { - if info, err := os.Stat(filepath.Join(dir, m.dir, "plugin.json")); err == nil && !info.IsDir() { + if info, err := exec.Stat(filepath.Join(dir, m.dir, "plugin.json")); err == nil && !info.IsDir() { return dir, m, true } } diff --git a/internal/detector/mcp_plugins_test.go b/internal/detector/mcp_plugins_test.go index b2767374..2c28cd25 100644 --- a/internal/detector/mcp_plugins_test.go +++ b/internal/detector/mcp_plugins_test.go @@ -1,6 +1,7 @@ package detector import ( + "github.com/step-security/dev-machine-guard/internal/executor" "path/filepath" "testing" ) @@ -36,7 +37,7 @@ func TestDiscoverWalkedMCPConfigs_PluginPackages(t *testing.T) { // Ordinary project config, no plugin manifest anywhere above it. project := writeFile(t, root, "proj/.mcp.json") - d := &MCPDetector{} + d := NewMCPDetector(executor.NewReal()) got := gotSpecMap(d.discoverWalkedMCPConfigs([]string{root}, "")) for _, p := range []string{catalogClaude, catalogCodex, vendored} { @@ -66,7 +67,7 @@ func TestDiscoverWalkedMCPConfigs_CodexInstalledPlugin(t *testing.T) { writeFile(t, root, ".codex/plugins/cache/openai-curated-remote/github/.codex-plugin/plugin.json") installed := writeFile(t, root, ".codex/plugins/cache/openai-curated-remote/github/.mcp.json") - d := &MCPDetector{} + d := NewMCPDetector(executor.NewReal()) got := gotSpecMap(d.discoverWalkedMCPConfigs([]string{root}, "")) s, ok := got[installed] @@ -85,7 +86,7 @@ func TestPluginPackageRoot_NestedAndBounded(t *testing.T) { writeFile(t, root, "pkg/.claude-plugin/plugin.json") nested := writeFile(t, root, "pkg/config/deep/mcp.json") - pluginRoot, manifest, ok := pluginPackageRoot(filepath.Dir(nested), root) + pluginRoot, manifest, ok := pluginPackageRoot(executor.NewReal(), filepath.Dir(nested), root) if !ok { t.Fatal("expected to find package root") } @@ -96,7 +97,7 @@ func TestPluginPackageRoot_NestedAndBounded(t *testing.T) { t.Errorf("manifest = %q, want claude_plugin", manifest.sourceName) } - if _, _, ok := pluginPackageRoot(filepath.Join(root, "other"), root); ok { + if _, _, ok := pluginPackageRoot(executor.NewReal(), filepath.Join(root, "other"), root); ok { t.Error("expected no package root outside a plugin package") } } diff --git a/internal/detector/mcp_test.go b/internal/detector/mcp_test.go index 96860db8..f9f12ff9 100644 --- a/internal/detector/mcp_test.go +++ b/internal/detector/mcp_test.go @@ -184,7 +184,7 @@ func TestFilterMCPContent_StripsSecrets(t *testing.T) { } func TestExtractMCPServers_ClaudeCodeProjectScoped(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) // Claude Code ~/.claude.json with project-scoped mcpServers content := []byte(`{ @@ -257,7 +257,7 @@ func TestExtractMCPServers_ClaudeCodeProjectScoped(t *testing.T) { } func TestExtractMCPServers_VSCodeFormat(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) content := []byte(`{ "servers": { @@ -483,7 +483,7 @@ func TestMCPDetector_OpenCode_GoldenPayload(t *testing.T) { // and oauth all carry live credentials and are outside the allowlist, so none // of them — nor their values — reach the wire. func TestFilterMCPContent_OpenCode_DropsSecretBearingFields(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) input := []byte(`{ "mcp": { @@ -578,7 +578,7 @@ func TestFilterMCPContent_OpenCode_JSONC(t *testing.T) { {"project-level, discovered source", "/Users/testuser/proj/opencode.jsonc", both}, } - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { // A project-level config arrives labelled discovered_mcp, so the @@ -615,7 +615,7 @@ func TestFilterMCPContent_OpenCode_FailsClosed(t *testing.T) { for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) filtered, ok := det.filterMCPContent("opencode", openCodeGlobalDir+"opencode.json", []byte(tc.content)) if ok { t.Errorf("expected filtering to fail, got ok with %q", filtered) @@ -653,7 +653,7 @@ func TestFilterMCPContent_ScalarMCPKeyDoesNotEvictSiblings(t *testing.T) { for _, mcpValue := range []string{`true`, `"enabled"`, `["a"]`, `42`} { t.Run("keeps siblings when mcp is "+mcpValue, func(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) content := fmt.Sprintf(siblings, mcpValue) filtered, ok := det.filterMCPContent("discovered_mcp", "/Users/testuser/proj/.mcp.json", []byte(content)) if !ok { @@ -668,7 +668,7 @@ func TestFilterMCPContent_ScalarMCPKeyDoesNotEvictSiblings(t *testing.T) { }) t.Run("fails closed when only mcp is "+mcpValue, func(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) content := fmt.Sprintf(`{"mcp":%s}`, mcpValue) filtered, ok := det.filterMCPContent("opencode", openCodeGlobalDir+"opencode.json", []byte(content)) if ok || filtered != nil { @@ -690,7 +690,7 @@ func TestFilterMCPContent_ScalarMCPKeyDoesNotEvictSiblings(t *testing.T) { // documented convention into the opposite behaviour. const wantNull = `{"mcp":{},"mcpServers":{"fs":{"args":["-y","s"],"command":"npx"}}}` - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) filtered, ok := det.filterMCPContent("discovered_mcp", "/Users/testuser/proj/.mcp.json", fmt.Appendf(nil, siblings, `null`)) if !ok { t.Fatalf("expected content, got ok=false") @@ -709,7 +709,7 @@ func TestFilterMCPContent_ScalarMCPKeyDoesNotEvictSiblings(t *testing.T) { // Control: a well-formed mcp map is still emitted, so the guard rejects only // what it cannot filter. - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) filtered, ok := det.filterMCPContent("opencode", openCodeGlobalDir+"opencode.json", []byte(openCodeGoldenConfig)) if !ok || string(filtered) != openCodeGoldenEmitted { t.Errorf("valid mcp map regressed: ok=%v %s", ok, filtered) @@ -804,7 +804,7 @@ func TestFilterMCPContent_NonOpenCodeUnchanged(t *testing.T) { }, } - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { filtered, ok := det.filterMCPContent(tc.source, tc.path, []byte(tc.content)) @@ -825,7 +825,7 @@ func TestFilterMCPContent_NonOpenCodeUnchanged(t *testing.T) { // args. Those values must still be redacted before upload, not passed through // verbatim just because the field name isn't "env" or "headers". func TestFilterServerFields_RedactsSecretsInKeptFields(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) content := `{"mcpServers":{"pipeboard":{ "url":"https://meta-ads.mcp.pipeboard.co/?token=abcdEFGH12345678opaqueTokenValue", "command":"npx", diff --git a/internal/detector/nodedist.go b/internal/detector/nodedist.go index 82f60d3a..fd1c316d 100644 --- a/internal/detector/nodedist.go +++ b/internal/detector/nodedist.go @@ -22,12 +22,13 @@ // Security context: all reads go through the Executor (so the user-aware // executor and test mocks both apply) and are size-bounded via maxLockfileSize // before the bytes are pulled into memory. The node_modules walk uses -// filepath.WalkDir directly (matching nodeproject.go) and never follows +// the executor's guarded WalkDir and never follows // directory symlinks, so a symlinked dependency can't redirect the walk out of // the project tree. package detector import ( + "os" "path/filepath" "sort" @@ -49,6 +50,7 @@ type NodeDistDetector struct { log *progress.Logger skipper *tcc.Skipper maxFileSize int64 + readFailed bool } func NewNodeDistDetector(exec executor.Executor) *NodeDistDetector { @@ -60,6 +62,7 @@ func NewNodeDistDetector(exec executor.Executor) *NodeDistDetector { // for chaining. func (d *NodeDistDetector) WithSkipper(s *tcc.Skipper) *NodeDistDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "pnpm", "Application Support/fnm") return d } @@ -80,6 +83,7 @@ func (d *NodeDistDetector) WithLogger(log *progress.Logger) *NodeDistDetector { // node_modules. The result is de-duplicated by (name, version) and sorted by // name then version for stable output. func (d *NodeDistDetector) ScanProject(projectDir, pm string) []model.NodePackage { + d.readFailed = false var pkgs []model.NodePackage switch pm { @@ -139,6 +143,9 @@ func (d *NodeDistDetector) readBounded(path string) (data []byte, ok bool) { } b, err := d.exec.ReadFile(path) if err != nil { + if !os.IsNotExist(err) { + d.readFailed = true + } return nil, false } if d.maxFileSize > 0 && int64(len(b)) > d.maxFileSize { diff --git a/internal/detector/nodedist_modules.go b/internal/detector/nodedist_modules.go index dc826f86..050ebb28 100644 --- a/internal/detector/nodedist_modules.go +++ b/internal/detector/nodedist_modules.go @@ -33,12 +33,20 @@ func (d *NodeDistDetector) walkNodeModules(projectDir string) []model.NodePackag // symlink farm is read via the real .pnpm store dirs, not by chasing links out // of the tree. func (d *NodeDistDetector) scanModulesTree(root string, isDirect func(name, path string) bool) []model.NodePackage { - if !d.exec.DirExists(root) { + info, err := d.exec.Stat(root) + if err != nil { + if !os.IsNotExist(err) { + d.readFailed = true + } + return nil + } + if !info.IsDir() { return nil } var pkgs []model.NodePackage - _ = filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + _ = d.exec.WalkDir(root, func(path string, entry os.DirEntry, err error) error { if err != nil { + d.readFailed = true return nil } if entry.IsDir() { @@ -75,6 +83,7 @@ func (d *NodeDistDetector) scanModulesTree(root string, isDirect func(name, path // the user installed with `-g`); anything below a further node_modules is a // transitive dependency of one of those. func (d *NodeDistDetector) ScanGlobalModules(nmRoot string) []model.NodePackage { + d.readFailed = false clean := filepath.Clean(nmRoot) return d.scanModulesTree(clean, func(_, path string) bool { rel := strings.TrimPrefix(filepath.ToSlash(path), filepath.ToSlash(clean)) diff --git a/internal/detector/nodepm.go b/internal/detector/nodepm.go index 3b517eff..488490e5 100644 --- a/internal/detector/nodepm.go +++ b/internal/detector/nodepm.go @@ -9,6 +9,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" "github.com/step-security/dev-machine-guard/internal/versionmeta" ) @@ -60,7 +61,7 @@ func (d *NodePMDetector) DetectManagers(ctx context.Context) []model.PkgManager // the version without launching anything. version = versionmeta.FromBinary(ctx, d.exec, path) } - if path != "" && version == "" { + if path != "" && version == "" && !tcc.HasGuard(d.exec) { if safe, reason := execguard.SafeToExec(ctx, d.exec, path); !safe { d.log.Warn("skipping %s version probe: %s", path, reason) } else { @@ -107,3 +108,8 @@ func (d *NodePMDetector) DetectManagers(ctx context.Context) []model.PkgManager return results } + +func (d *NodePMDetector) WithSkipper(s *tcc.Skipper) *NodePMDetector { + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "pnpm", "Application Support/fnm") + return d +} diff --git a/internal/detector/nodepm_fallback.go b/internal/detector/nodepm_fallback.go index 04c94507..7e5f5a39 100644 --- a/internal/detector/nodepm_fallback.go +++ b/internal/detector/nodepm_fallback.go @@ -11,6 +11,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" "github.com/step-security/dev-machine-guard/internal/versionmeta" ) @@ -171,6 +172,9 @@ func runPMVersion(ctx context.Context, exec executor.Executor, log *progress.Log if v := versionmeta.FromBinary(ctx, exec, binPath); v != "" { return v } + if tcc.HasGuard(exec) { + return "" + } if safe, reason := execguard.SafeToExec(ctx, exec, binPath); !safe { log.Warn("skipping %s version probe: %s", binPath, reason) return "" diff --git a/internal/detector/nodeproject.go b/internal/detector/nodeproject.go index 176f4ece..25f20319 100644 --- a/internal/detector/nodeproject.go +++ b/internal/detector/nodeproject.go @@ -32,6 +32,7 @@ func NewNodeProjectDetector(exec executor.Executor) *NodeProjectDetector { // directories. A nil skipper is a no-op. Returns the detector for chaining. func (d *NodeProjectDetector) WithSkipper(s *tcc.Skipper) *NodeProjectDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "pnpm", "Application Support/fnm") return d } @@ -64,7 +65,7 @@ func (d *NodeProjectDetector) ListProjects(searchDirs []string) []model.ProjectI func (d *NodeProjectDetector) listInDir(dir string) []model.ProjectInfo { var projects []model.ProjectInfo - _ = filepath.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { + _ = d.exec.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { if err != nil { return nil } diff --git a/internal/detector/nodescan.go b/internal/detector/nodescan.go index bd11ce07..30028d3b 100644 --- a/internal/detector/nodescan.go +++ b/internal/detector/nodescan.go @@ -109,6 +109,7 @@ func (s *NodeScanner) binaryAvailable(ctx context.Context, name string) error { // macOS-protected directories. A nil skipper is a no-op. func (s *NodeScanner) WithSkipper(skipper *tcc.Skipper) *NodeScanner { s.skipper = skipper + s.exec = tcc.GuardedFiles(s.exec, skipper, maxLockfileSize, "pnpm", "Application Support/fnm") return s } @@ -435,7 +436,7 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, kno var projects []projectEntry for _, dir := range searchDirs { s.log.Progress(" Searching in: %s", dir) - _ = filepath.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { + _ = s.exec.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { if err != nil { return nil } @@ -767,7 +768,11 @@ func (s *NodeScanner) scanProject(ctx context.Context, projectDir, pm string) (m // (the backend reads Packages directly), and PMVersion is omitted — resolving // it would mean running the binary we are deliberately not invoking. func (s *NodeScanner) scanProjectFromDisk(projectDir, pm string) (model.NodeScanResult, bool) { - pkgs := s.dist.ScanProject(projectDir, pm) + dist := *s.dist + pkgs := dist.ScanProject(projectDir, pm) + if dist.readFailed { + return model.NodeScanResult{ProjectPath: projectDir, PackageManager: pm, WorkingDirectory: projectDir, ExitCode: 1, Error: "package metadata could not be read completely"}, true + } return model.NodeScanResult{ ProjectPath: projectDir, PackageManager: pm, @@ -784,7 +789,11 @@ func (s *NodeScanner) scanProjectFromDisk(projectDir, pm string) (model.NodeScan // separate so a package installed under two prefixes lists both; the delta // layer reconciles them back to one record per PM (globalRecordsFromNode). func (s *NodeScanner) scanGlobalPackagesFromDisk() []model.NodeScanResult { + before := tcc.Refusals(s.exec) roots := NodeGlobalRoots(s.exec) + if tcc.Refusals(s.exec) != before { + return []model.NodeScanResult{{PackageManager: "npm", ExitCode: 1, Error: "global package roots include protected paths"}, {PackageManager: "pnpm", ExitCode: 1, Error: "global package roots include protected paths"}, {PackageManager: "yarn", ExitCode: 1, Error: "global package roots include protected paths"}, {PackageManager: "bun", ExitCode: 1, Error: "global package roots include protected paths"}} + } if len(roots) == 0 { s.log.Debug("node global disk scan: no global node_modules roots found") return nil @@ -793,12 +802,16 @@ func (s *NodeScanner) scanGlobalPackagesFromDisk() []model.NodeScanResult { for _, r := range roots { s.emitProgress("global: " + r.pm) pkgs := s.dist.ScanGlobalModules(r.dir) - if len(pkgs) == 0 { + if len(pkgs) == 0 && !s.dist.readFailed { // pnpm symlinks its global node_modules into a content-addressed // store the walk can't traverse. The install dir holds the // lockfile with the resolved graph — parse that instead. pkgs = s.dist.ScanProject(filepath.Dir(r.dir), r.pm) } + if s.dist.readFailed { + results = append(results, model.NodeScanResult{ProjectPath: r.dir, PackageManager: r.pm, WorkingDirectory: r.dir, ExitCode: 1, Error: "package metadata could not be read completely"}) + continue + } // A root that has gone empty is still reported. Dropping it would // leave the PM out of the delta records entirely once its last root // empties, so nothing would mark the PM changed and the previously diff --git a/internal/detector/protected_reads_darwin_test.go b/internal/detector/protected_reads_darwin_test.go new file mode 100644 index 00000000..725a9584 --- /dev/null +++ b/internal/detector/protected_reads_darwin_test.go @@ -0,0 +1,266 @@ +//go:build darwin + +package detector + +import ( + "archive/zip" + "bytes" + "context" + "os" + "os/user" + "path/filepath" + "sync" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +type protectedFixtureExecutor struct { + executor.Executor + home string +} + +func (e protectedFixtureExecutor) CurrentUser() (*user.User, error) { + return &user.User{Uid: "501", Username: "test-user", HomeDir: e.home}, nil +} +func (e protectedFixtureExecutor) LoggedInUser() (*user.User, error) { return e.CurrentUser() } +func (e protectedFixtureExecutor) GuardedFiles(roots []string, guard func(string) string, max int64) executor.Executor { + e.Executor = e.Executor.GuardedFiles(roots, guard, max) + return e +} +func (e protectedFixtureExecutor) Getenv(key string) string { + if key == "HOME" { + return e.home + } + return "" +} + +func TestProtectedReadsKeepOrdinaryInventory(t *testing.T) { + for _, blocked := range []bool{false, true} { + name := "ordinary" + if blocked { + name = "protected-redirect" + } + t.Run(name, func(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + exec := protectedFixtureExecutor{Executor: executor.NewReal(), home: home} + skipper := tcc.New(home) + put := func(path, data string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(data), 0600); err != nil { + t.Fatal(err) + } + } + target := filepath.Join(home, "Library", "Containers", "test-app", "Data") + file := func(path, data string) { + t.Helper() + if blocked { + dest := filepath.Join(target, filepath.Base(path)) + put(dest, data) + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(dest, path); err != nil { + t.Fatal(err) + } + } else { + put(path, data) + } + } + directory := func(path string) string { + t.Helper() + if !blocked { + return path + } + dest := filepath.Join(target, filepath.Base(path)) + if err := os.MkdirAll(dest, 0700); err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(dest, path); err != nil { + t.Fatal(err) + } + return dest + } + want := 1 + if blocked { + want = 0 + } + mcp := filepath.Join(home, "Library", "Application Support", "Claude", "claude_desktop_config.json") + file(mcp, `{"mcpServers":{"example":{"command":"example-tool"}}}`) + if got := len(NewMCPDetector(exec).WithSkipper(skipper).DetectEnterprise(context.Background(), nil)); got != want { + t.Errorf("MCP count = %d, want %d", got, want) + } + ideRoot := filepath.Join(home, ".vscode", "extensions") + put(filepath.Join(directory(ideRoot), "example.extension-1.0.0", "package.json"), `{}`) + if got := len(NewExtensionDetector(exec).WithSkipper(skipper).collectFromDir(ideRoot, "vscode")); got != want { + t.Errorf("IDE count = %d, want %d", got, want) + } + jbRoot := filepath.Join(home, "Library", "Application Support", "JetBrains", "GoLand2026.1", "plugins") + put(filepath.Join(directory(jbRoot), "example", "lib", "example-1.0.0.jar"), "fixture") + if got := len(NewJetBrainsPluginDetector(exec).WithSkipper(skipper).collectPlugins(jbRoot, "goland")); got != want { + t.Errorf("JetBrains count = %d, want %d", got, want) + } + pyRoot := filepath.Join(home, "Library", "Python", "3.12", "site-packages") + file(filepath.Join(pyRoot, "example-1.0.0.dist-info", "METADATA"), "Name: example\nVersion: 1.0.0\n") + py := NewPythonDistDetector(exec).WithSkipper(skipper).ScanRoots([]string{pyRoot}) + if len(py) != want || (blocked && py != nil) { + t.Errorf("Python count = %d, want %d, nil=%v", len(py), want, py == nil) + } + nodeRoot := filepath.Join(home, "Library", "pnpm", "global", "node_modules") + file(filepath.Join(nodeRoot, "example", "package.json"), `{"name":"example","version":"1.0.0"}`) + node := NewNodeDistDetector(exec).WithSkipper(skipper) + if got := len(node.ScanGlobalModules(nodeRoot)); got != want || node.readFailed != blocked { + t.Errorf("Node count=%d failed=%v, want %d/%v", got, node.readFailed, want, blocked) + } + }) + } +} + +func TestProtectedPluginArchiveAndManifest(t *testing.T) { + home := t.TempDir() + protected := filepath.Join(home, "Library", "Containers", "test-app") + safe := filepath.Join(home, "plugins") + for _, p := range []string{protected, filepath.Join(safe, ".claude-plugin")} { + if err := os.MkdirAll(p, 0700); err != nil { + t.Fatal(err) + } + } + var archive bytes.Buffer + z := zip.NewWriter(&archive) + f, err := z.Create("META-INF/plugin.xml") + if err != nil { + t.Fatal(err) + } + if _, err := f.Write([]byte("")); err != nil { + t.Fatal(err) + } + if err := z.Close(); err != nil { + t.Fatal(err) + } + target := filepath.Join(protected, "plugin.jar") + if err := os.WriteFile(target, archive.Bytes(), 0600); err != nil { + t.Fatal(err) + } + jar := filepath.Join(safe, "plugin.jar") + if err := os.Symlink(target, jar); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, filepath.Join(safe, ".claude-plugin", "plugin.json")); err != nil { + t.Fatal(err) + } + exec := executor.NewReal() + skipper := tcc.New(home) + if got := NewExtensionDetector(exec).WithSkipper(skipper).readFileFromZip(jar, "META-INF/plugin.xml"); got != nil { + t.Fatal("read protected archive") + } + if _, _, ok := pluginPackageRoot(tcc.GuardedFiles(exec, skipper, 1024), safe, safe); ok { + t.Fatal("read protected manifest") + } +} + +// Only the fixed build is used here. Targets are existing protected locations; +// the test creates links in its own temporary directory and never writes targets. +func TestLiveProtectedScannerRedirects(t *testing.T) { + if os.Getenv("DMG_TCC_VALIDATE_LIVE") != "1" { + t.Skip("explicit live validation only") + } + u, err := user.Current() + if err != nil { + t.Fatal(err) + } + scratch := t.TempDir() + target := filepath.Join(u.HomeDir, "Library", "Containers", "com.apple.TextEdit", "Data") + link := filepath.Join(scratch, "redirect") + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + skipper := tcc.New(u.HomeDir) + e := executor.NewReal() + mcp := NewMCPDetector(e).WithSkipper(skipper) + if mcp.exec.FileExists(filepath.Join(link, ".mcp.json")) || tcc.Refusals(mcp.exec) == 0 { + t.Fatal("MCP did not refuse target") + } + ide := NewExtensionDetector(e).WithSkipper(skipper) + if got := ide.collectFromDir(link, "vscode"); len(got) != 0 || tcc.Refusals(ide.exec) == 0 { + t.Fatal("IDE did not refuse target") + } + jb := NewJetBrainsPluginDetector(e).WithSkipper(skipper) + if got := jb.collectPlugins(link, "goland"); len(got) != 0 || tcc.Refusals(jb.exec) == 0 { + t.Fatal("JetBrains did not refuse target") + } + py := NewPythonDistDetector(e).WithSkipper(skipper) + if got := py.ScanRoots([]string{link}); got != nil || tcc.Refusals(py.exec) == 0 { + t.Fatal("Python did not refuse target") + } + node := NewNodeDistDetector(e).WithSkipper(skipper) + if got := node.ScanGlobalModules(link); len(got) != 0 || !node.readFailed || tcc.Refusals(node.exec) == 0 { + t.Fatal("Node did not refuse target") + } + t.Log("MCP, IDE extensions, JetBrains, Python and Node refused before protected access") +} + +func TestProtectedNodeConcurrentResults(t *testing.T) { + home := t.TempDir() + safe, denied := filepath.Join(home, "project"), filepath.Join(home, "Documents", "project") + if err := os.MkdirAll(safe, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(safe, "package-lock.json"), []byte(`{"lockfileVersion":3,"packages":{"node_modules/example":{"version":"1.0.0"}}}`), 0600); err != nil { + t.Fatal(err) + } + e := executor.NewReal() + scanner := NewNodeScanner(e, progress.NewNoop(), "").WithDiskScan(NewNodeDistDetector(e).WithSkipper(tcc.New(home))) + var wg sync.WaitGroup + for i := range 16 { + wg.Go(func() { + path := safe + if i%2 != 0 { + path = denied + } + result, _ := scanner.scanProjectFromDisk(path, "npm") + if path == denied { + if result.ExitCode == 0 || result.Error == "" { + t.Error("refused project reported success") + } + } else if result.ExitCode != 0 || result.PackagesCount != 1 { + t.Errorf("ordinary project lost: %+v", result) + } + }) + } + wg.Wait() +} + +func TestProtectedPythonVenvDiscoveryIsNotEmptySuccess(t *testing.T) { + for _, intermediate := range []bool{false, true} { + home := t.TempDir() + venv := filepath.Join(home, "project", ".venv") + site := filepath.Join(venv, "lib", "python3.12", "site-packages") + target := filepath.Join(home, "Library", "Containers", "test-app", "site-packages") + if intermediate { + site = filepath.Dir(site) + } + for _, path := range []string{filepath.Dir(site), target} { + if err := os.MkdirAll(path, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.Symlink(target, site); err != nil { + t.Fatal(err) + } + d := NewPythonDistDetector(executor.NewReal()).WithSkipper(tcc.New(home)) + if got := d.ScanVenv(venv); got != nil { + t.Fatalf("intermediate=%v: refused venv returned successful inventory: %v", intermediate, got) + } + } +} diff --git a/internal/detector/pythondist.go b/internal/detector/pythondist.go index 1270ba3f..b331ec43 100644 --- a/internal/detector/pythondist.go +++ b/internal/detector/pythondist.go @@ -52,6 +52,7 @@ func NewPythonDistDetector(exec executor.Executor) *PythonDistDetector { // directories. A nil skipper is a no-op. Returns the detector for chaining. func (d *PythonDistDetector) WithSkipper(s *tcc.Skipper) *PythonDistDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxMetadataFileSize, "Python") return d } @@ -69,7 +70,12 @@ func (d *PythonDistDetector) WithLogger(log *progress.Logger) *PythonDistDetecto // lib/python*/site-packages or Lib/site-packages). Replaces the per-venv // `pip list` call. func (d *PythonDistDetector) ScanVenv(venvPath string) []model.PackageDetail { - return d.ScanRoots(venvSitePackages(venvPath)) + before := tcc.Refusals(d.exec) + roots := venvSitePackages(d.exec, venvPath) + if tcc.Refusals(d.exec) != before { + return nil + } + return d.ScanRoots(roots) } // venvSitePackages returns the site-packages directories inside a venv — @@ -77,13 +83,13 @@ func (d *PythonDistDetector) ScanVenv(venvPath string) []model.PackageDetail { // only these avoids walking bin/include/share, which never hold install // metadata. Falls back to the venv root if no site-packages dir is found, so // a non-standard layout is still scanned. -func venvSitePackages(venvPath string) []string { +func venvSitePackages(exec executor.Executor, venvPath string) []string { var roots []string for _, pattern := range []string{ filepath.Join(venvPath, "lib", "python*", "site-packages"), filepath.Join(venvPath, "Lib", "site-packages"), } { - if matches, err := filepath.Glob(pattern); err == nil { + if matches, err := exec.Glob(pattern); err == nil { roots = append(roots, matches...) } } @@ -109,7 +115,7 @@ func (d *PythonDistDetector) ScanRoots(roots []string) []model.PackageDetail { walkFailed := false for _, root := range roots { - _ = filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + _ = d.exec.WalkDir(root, func(path string, entry os.DirEntry, err error) error { if err != nil { walkFailed = true return nil @@ -124,8 +130,12 @@ func (d *PythonDistDetector) ScanRoots(roots []string) []model.PackageDetail { return nil } - name, version, ok := d.parseMetadataFile(path, entry.Name()) - if !ok { + name, version, readErr := d.parseMetadataFile(path, entry.Name()) + if readErr != nil { + walkFailed = true + return nil + } + if name == "" || version == "" { return nil } key := strings.ToLower(name) + "\x00" + version @@ -153,7 +163,15 @@ func (d *PythonDistDetector) ScanRoots(roots []string) []model.PackageDetail { // ScanGlobalPackages walks the host's global / user site-packages roots and // returns the installed packages, replacing the `pip3 list` global scan. func (d *PythonDistDetector) ScanGlobalPackages() []model.PythonPackage { - details := d.ScanRoots(GlobalPythonRoots(d.exec, d.log)) + before := tcc.Refusals(d.exec) + roots := GlobalPythonRoots(d.exec, d.log) + if tcc.Refusals(d.exec) != before { + return nil + } + details := d.ScanRoots(roots) + if details == nil { + return nil + } out := make([]model.PythonPackage, len(details)) for i, p := range details { out[i] = model.PythonPackage(p) @@ -163,30 +181,30 @@ func (d *PythonDistDetector) ScanGlobalPackages() []model.PythonPackage { // parseMetadataFile returns the package name and version if path is a // recognised metadata file (*.dist-info/METADATA or *.egg-info/PKG-INFO). -func (d *PythonDistDetector) parseMetadataFile(path, base string) (name, version string, ok bool) { +func (d *PythonDistDetector) parseMetadataFile(path, base string) (name, version string, err error) { switch base { case "METADATA": if !isDistInfoMetadata(path) { - return "", "", false + return "", "", nil } case "PKG-INFO": if !isEggInfoPKGInfo(path) { - return "", "", false + return "", "", nil } default: - return "", "", false + return "", "", nil } data, err := d.readBounded(path) if err != nil { - return "", "", false + return "", "", err } name, version = parseRFC822NameVersion(data) if name == "" || version == "" { d.log.Debug("python dist scan: %s missing Name/Version header — skipping", path) - return "", "", false + return "", "", nil } - return name, version, true + return name, version, nil } // readBounded reads path through the executor and rejects files over the size @@ -286,7 +304,7 @@ func PythonGlobalRoots(exec executor.Executor) []string { var candidates []string add := func(paths ...string) { candidates = append(candidates, paths...) } addGlob := func(pattern string) { - if matches, err := filepath.Glob(pattern); err == nil { + if matches, err := exec.Glob(pattern); err == nil { add(matches...) } } @@ -331,16 +349,9 @@ func PythonGlobalRoots(exec executor.Executor) []string { continue } seen[c] = struct{}{} - if exec.FileExists(c) || isDir(c) { + if exec.DirExists(c) { roots = append(roots, c) } } return roots } - -// isDir reports whether path is an existing directory. exec.FileExists rejects -// directories, so global roots (which are dirs) are confirmed here. -func isDir(path string) bool { - info, err := os.Stat(path) - return err == nil && info.IsDir() -} diff --git a/internal/detector/pythondist_test.go b/internal/detector/pythondist_test.go index a74064ea..68c1a4b7 100644 --- a/internal/detector/pythondist_test.go +++ b/internal/detector/pythondist_test.go @@ -67,6 +67,11 @@ func TestPythonDistDetector_ScanVenv_ScopedToSitePackages(t *testing.T) { mustWriteMeta(t, mock, filepath.Join(venv, "bin", "stray-1.0.0.dist-info", "METADATA"), "Name: stray\nVersion: 1.0.0\n\nbody") + matches, err := filepath.Glob(filepath.Join(venv, "lib", "python*", "site-packages")) + if err != nil { + t.Fatal(err) + } + mock.SetGlob(filepath.Join(venv, "lib", "python*", "site-packages"), matches) pkgs := NewPythonDistDetector(mock).ScanVenv(venv) if len(pkgs) != 1 || pkgs[0].Name != "requests" { t.Fatalf("expected only requests from site-packages, got %+v", pkgs) diff --git a/internal/detector/pythonpm.go b/internal/detector/pythonpm.go index 79ee1b84..24a03e8f 100644 --- a/internal/detector/pythonpm.go +++ b/internal/detector/pythonpm.go @@ -11,6 +11,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" "github.com/step-security/dev-machine-guard/internal/versionmeta" ) @@ -62,6 +63,8 @@ func (d *PythonPMDetector) DetectManagers(ctx context.Context) []model.PkgManage // layouts carry the version in the install path. if v := versionmeta.FromBinary(ctx, d.exec, path); v != "" { version = v + } else if tcc.HasGuard(d.exec) { + // Preserve detection with an unknown version when executing could load protected config. } else if safe, reason := execguard.SafeToExec(ctx, d.exec, path); !safe { d.log.Warn("skipping %s version probe: %s", path, reason) } else { @@ -144,3 +147,8 @@ func parsePythonVersion(name, stdout string) string { } return strings.TrimSpace(line) } + +func (d *PythonPMDetector) WithSkipper(s *tcc.Skipper) *PythonPMDetector { + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Python") + return d +} diff --git a/internal/detector/pythonproject.go b/internal/detector/pythonproject.go index b9296070..52c70f24 100644 --- a/internal/detector/pythonproject.go +++ b/internal/detector/pythonproject.go @@ -35,6 +35,7 @@ func NewPythonProjectDetector(exec executor.Executor) *PythonProjectDetector { // directories. A nil skipper is a no-op. Returns the detector for chaining. func (d *PythonProjectDetector) WithSkipper(s *tcc.Skipper) *PythonProjectDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Python") return d } @@ -246,7 +247,7 @@ var pythonPMFromMarker = map[string]string{ // reorder via state before any pip list is run. func (d *PythonProjectDetector) discoverInDir(dir string) []venvCandidate { var found []venvCandidate - _ = filepath.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { + _ = d.exec.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { if err != nil { return nil } diff --git a/internal/detector/pythonscan.go b/internal/detector/pythonscan.go index 19caf19f..aa2ffba0 100644 --- a/internal/detector/pythonscan.go +++ b/internal/detector/pythonscan.go @@ -100,7 +100,11 @@ func (s *PythonScanner) ScanGlobalPackages(ctx context.Context) []model.PythonSc // so the existing backend decoder needs no change. Returns nil when no global // site-packages roots exist on the host. func (s *PythonScanner) ScanGlobalPackagesFromDisk(skipper *tcc.Skipper) []model.PythonScanResult { - roots := GlobalPythonRoots(s.exec, s.log) + guarded := tcc.GuardedFiles(s.exec, skipper, maxMetadataFileSize, "Python") + roots := GlobalPythonRoots(guarded, s.log) + if tcc.Refusals(guarded) > 0 { + return []model.PythonScanResult{{PackageManager: "pip", ExitCode: 1, Error: "global package roots include protected paths"}} + } if len(roots) == 0 { s.log.Debug("python global disk scan: no site-packages roots found") return nil diff --git a/internal/detector/rules/engine.go b/internal/detector/rules/engine.go index f1aa8952..046b78c9 100644 --- a/internal/detector/rules/engine.go +++ b/internal/detector/rules/engine.go @@ -2,6 +2,7 @@ package rules import ( "context" + "os" "time" "github.com/step-security/dev-machine-guard/internal/executor" @@ -63,15 +64,17 @@ func NewEngine(exec executor.Executor, skipper *tcc.Skipper, caps Caps, log *pro if log == nil { log = progress.NewNoop() } + exec = tcc.GuardedFiles(exec, skipper, caps.MaxFileSize) return &Engine{exec: exec, skipper: skipper, caps: caps, log: log} } // ruleState accumulates one rule's matches during a scan. type ruleState struct { - rule *Rule - matches []model.RuleFileMatch - seen map[string]bool // dedupe candidate paths per rule - truncated bool // hit MaxMatchesPerRule + rule *Rule + matches []model.RuleFileMatch + seen map[string]bool // dedupe candidate paths per rule + incomplete bool + truncated bool // hit MaxMatchesPerRule } // scanState is the mutable bookkeeping shared across the absolute-resolution @@ -116,7 +119,10 @@ func (e *Engine) Scan(ctx context.Context, rs RuleSet, searchDirs []string) mode res := model.RuleScan{ScanComplete: !st.globalStop} matchedRules, matchedFiles, incompleteRules := 0, 0, 0 for _, rstate := range st.states { - complete := res.ScanComplete && !rstate.truncated + complete := !st.globalStop && !rstate.truncated && !rstate.incomplete + if rstate.incomplete { + res.ScanComplete = false + } if !complete { incompleteRules++ } @@ -166,7 +172,13 @@ func (e *Engine) evaluate(st *scanState, rstate *ruleState, path, matchedGlob st st.filesScanned++ info, err := st.cache.stat(e.exec, path) - if err != nil || info.IsDir() { + if err != nil { + if !os.IsNotExist(err) { + rstate.incomplete = true + } + return false + } + if info.IsDir() { return false } @@ -190,6 +202,7 @@ func (e *Engine) evaluate(st *scanState, rstate *ruleState, path, matchedGlob st data, hash, ok := st.cache.read(e.exec, path) if !ok { + rstate.incomplete = true // Unreadable: still report existence + metadata, no content-derived fields. rstate.matches = append(rstate.matches, fm) return false diff --git a/internal/detector/rules/protected_reads_darwin_test.go b/internal/detector/rules/protected_reads_darwin_test.go new file mode 100644 index 00000000..21969eb9 --- /dev/null +++ b/internal/detector/rules/protected_reads_darwin_test.go @@ -0,0 +1,29 @@ +//go:build darwin + +package rules + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +func TestProtectedAbsoluteRule(t *testing.T) { + home := t.TempDir() + target := writeFile(t, home, "Library/Containers/test-app/marker.txt", "fixture") + link := filepath.Join(home, "redirect") + if err := os.Symlink(filepath.Dir(target), link); err != nil { + t.Fatal(err) + } + for _, pattern := range []string{target, filepath.Join(link, "*.txt")} { + rs := prep(t, RuleSet{Rules: []Rule{{ID: "test-rule", Revision: "1", FileGlobs: []string{pattern}}}}) + got := NewEngine(executor.NewReal(), tcc.New(home), DefaultCaps(), nil).Scan(context.Background(), rs, nil) + if got.ScanComplete || len(got.Results) != 0 || len(got.EvaluatedRules) != 1 || got.EvaluatedRules[0].Complete { + t.Fatalf("protected rule result=%+v", got) + } + } +} diff --git a/internal/detector/rules/roots.go b/internal/detector/rules/roots.go index 111c8a9f..a34e3b7f 100644 --- a/internal/detector/rules/roots.go +++ b/internal/detector/rules/roots.go @@ -28,7 +28,7 @@ func (e *Engine) resolveAbsolute(ctx context.Context, st *scanState) { } paths, err := e.exec.Glob(filepath.FromSlash(cg.raw)) if err != nil { - continue + rstate.incomplete = true } for _, p := range paths { if rstate.truncated { @@ -158,6 +158,9 @@ func (e *Engine) walkOneRoot(ctx context.Context, st *scanState, root string, id cleanRoot := filepath.Clean(root) err := e.walkCandidates(ctx, root, idx, func(filePath string, d fs.DirEntry, err error) error { if err != nil { + for _, state := range st.states { + state.incomplete = true + } return nil } if idx.activeRules == 0 { @@ -229,7 +232,7 @@ func (e *Engine) walkCandidates(ctx context.Context, root string, idx *relativeI } info, err := e.exec.Stat(root) if err != nil { - return nil + return visit(root, nil, err) } var walk func(string, fs.DirEntry) error walk = func(name string, entry fs.DirEntry) error { @@ -243,7 +246,12 @@ func (e *Engine) walkCandidates(ctx context.Context, root string, idx *relativeI return nil } // As with WalkDir, keep any entries returned before a read error. - entries, _ := e.exec.ReadDir(name) + entries, readErr := e.exec.ReadDir(name) + if readErr != nil { + if err := visit(name, entry, readErr); err != nil { + return err + } + } for _, child := range entries { if idx.activeRules == 0 { return fs.SkipAll diff --git a/internal/detector/skills.go b/internal/detector/skills.go index 1f60e487..4b4a2921 100644 --- a/internal/detector/skills.go +++ b/internal/detector/skills.go @@ -89,6 +89,7 @@ func NewSkillsDetector(exec executor.Executor) *SkillsDetector { // the --include-tcc-protected opt-in. Returns the detector for chaining. func (d *SkillsDetector) WithSkipper(s *tcc.Skipper) *SkillsDetector { d.skipper = s + d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize) return d } diff --git a/internal/execguard/execguard.go b/internal/execguard/execguard.go index 090ba948..2f4d17c8 100644 --- a/internal/execguard/execguard.go +++ b/internal/execguard/execguard.go @@ -24,6 +24,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" ) const probeTimeout = 5 * time.Second @@ -50,6 +51,10 @@ const probeTimeout = 5 * time.Second // On Linux it answers whether the binary is an Electron app's GUI entry point, // purely from stats. func SafeToExec(ctx context.Context, exec executor.Executor, binaryPath string) (bool, string) { + // Child processes can load configs beyond the guarded reader. + if tcc.HasGuard(exec) { + return false, "protected-directory scanning is disabled" + } if binaryPath == "" { return true, "" } diff --git a/internal/execguard/execguard_test.go b/internal/execguard/execguard_test.go index 99268bf7..ec8365d6 100644 --- a/internal/execguard/execguard_test.go +++ b/internal/execguard/execguard_test.go @@ -7,6 +7,7 @@ import ( "time" "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/tcc" ) const ( @@ -243,3 +244,14 @@ func TestSafeToExec_ReasonMatchesPlatform(t *testing.T) { } }) } + +func TestSafeToExecRefusesGuardedFallback(t *testing.T) { + skipper := tcc.New(t.TempDir()) + if skipper == nil { + t.Skip("macOS protection only") + } + exec := tcc.GuardedFiles(executor.NewMock(), skipper, 1024) + if safe, reason := SafeToExec(context.Background(), exec, "/usr/local/bin/example"); safe || reason != "protected-directory scanning is disabled" { + t.Fatalf("guarded fallback = %v, %q", safe, reason) + } +} diff --git a/internal/executor/executor.go b/internal/executor/executor.go index c6aa1237..e774e174 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "fmt" + "io/fs" "os" "os/exec" "os/user" @@ -20,7 +21,7 @@ import ( // Executor defines the interface for all OS interactions. // Every detector depends on this interface, enabling full unit-test coverage via mocks. type Executor interface { - // GuardedFiles restricts ReadFile, ReadDir, Stat and EvalSymlinks to roots + // GuardedFiles restricts filesystem reads, existence checks and Glob to roots // and guard. Other operations retain their original behavior. GuardedFiles(roots []string, guard func(string) string, maxReadBytes int64) Executor // Run executes a command and returns stdout, stderr, and exit code. @@ -45,6 +46,8 @@ type Executor interface { FileExists(path string) bool // DirExists checks if a directory exists. DirExists(path string) bool + // Open opens a file read-only. The caller must close it. + Open(path string) (*os.File, error) // ReadFile reads a file's contents. ReadFile(path string) ([]byte, error) // ReadDir lists directory entries. @@ -61,6 +64,8 @@ type Executor interface { CurrentUser() (*user.User, error) // HomeDir returns the home directory for a given username. HomeDir(username string) (string, error) + // WalkDir walks without following directory entries that are symlinks. + WalkDir(root string, fn fs.WalkDirFunc) error // Glob returns filenames matching a pattern. Glob(pattern string) ([]string, error) // EvalSymlinks resolves symbolic links in a path. Returns the resolved @@ -107,6 +112,23 @@ type guardedFiles struct { maxReadBytes int64 } +func (g *guardedFiles) FileExists(path string) bool { + info, err := g.Stat(path) + return err == nil && !info.IsDir() +} + +func (g *guardedFiles) DirExists(path string) bool { + info, err := g.Stat(path) + return err == nil && info.IsDir() +} + +func (g *guardedFiles) Readlink(path string) (string, error) { + if _, err := g.resolver.Resolve(path); err != nil { + return "", err + } + return g.Executor.Readlink(path) +} + func (g *guardedFiles) EvalSymlinks(path string) (string, error) { return g.resolver.Resolve(path) } @@ -372,3 +394,11 @@ func HardenCommand(cmd *exec.Cmd) { winproc.HideWindow(cmd) setupKillgroupOnCancel(cmd) } + +func (r *Real) WalkDir(root string, fn fs.WalkDirFunc) error { return filepath.WalkDir(root, fn) } + +func (r *Real) Open(path string) (*os.File, error) { + // #nosec G304 -- OS boundary; protected scanners use guardedFiles.Open. + return os.Open(path) +} +func (g *guardedFiles) Open(path string) (*os.File, error) { return g.resolver.Open(path) } diff --git a/internal/executor/guarded_files_test.go b/internal/executor/guarded_files_test.go new file mode 100644 index 00000000..4a7f5f50 --- /dev/null +++ b/internal/executor/guarded_files_test.go @@ -0,0 +1,71 @@ +package executor + +import ( + "os" + "path/filepath" + "testing" +) + +func TestGuardedFilesRefusesRedirectsAndGlob(t *testing.T) { + home := t.TempDir() + home, _ = filepath.EvalSymlinks(home) + safe, protected := filepath.Join(home, "safe"), filepath.Join(home, "protected") + for _, dir := range []string{safe, protected} { + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(protected, "metadata.json"), []byte("secret"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(protected, filepath.Join(safe, "redirect")); err != nil { + t.Skip(err) + } + guarded := NewReal().GuardedFiles([]string{home}, func(path string) string { + if path == protected { + return "tcc_protected" + } + return "" + }, 1024) + for _, path := range []string{filepath.Join(protected, "metadata.json"), filepath.Join(safe, "redirect", "metadata.json")} { + if guarded.FileExists(path) { + t.Errorf("FileExists accepted protected path %s", path) + } + if _, err := guarded.ReadFile(path); err == nil { + t.Errorf("ReadFile accepted protected path %s", path) + } + } + if guarded.DirExists(filepath.Join(safe, "redirect")) { + t.Error("DirExists followed protected redirect") + } + if matches, err := guarded.Glob(filepath.Join(safe, "*", "*.json")); err == nil || len(matches) != 0 { + t.Errorf("Glob = %v, %v, want refused traversal", matches, err) + } +} + +func TestGuardedFilesFilesystemRoot(t *testing.T) { + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + path := filepath.Join(dir, "metadata.json") + if err := os.WriteFile(path, []byte("ordinary"), 0600); err != nil { + t.Fatal(err) + } + root := filepath.VolumeName(path) + string(filepath.Separator) + guarded := NewReal().GuardedFiles([]string{root}, nil, 1024) + if data, err := guarded.ReadFile(path); err != nil || string(data) != "ordinary" { + t.Fatalf("filesystem-root reader = %q, %v", data, err) + } +} + +func TestGuardedFilesRootListingAndGlob(t *testing.T) { + root := filepath.VolumeName(t.TempDir()) + string(filepath.Separator) + guarded := NewReal().GuardedFiles([]string{root}, nil, 1024) + if _, err := guarded.ReadDir(root); err != nil { + t.Fatal(err) + } + if matches, err := guarded.Glob(filepath.Join(root, "*")); err != nil || len(matches) == 0 { + t.Fatalf("root glob = %v, %v", matches, err) + } +} diff --git a/internal/executor/guarded_glob.go b/internal/executor/guarded_glob.go new file mode 100644 index 00000000..08d83eb9 --- /dev/null +++ b/internal/executor/guarded_glob.go @@ -0,0 +1,76 @@ +package executor + +import ( + "io/fs" + "os" + "path/filepath" + "strings" +) + +// guardedGlobFS lets the standard glob implementation enumerate through the +// same reader as direct accesses. Filtering matches after Glob is too late. +type guardedGlobFS struct { + exec Executor + root string + err error +} + +func (g *guardedGlobFS) Open(string) (fs.File, error) { return nil, fs.ErrInvalid } + +func (g *guardedGlobFS) Stat(name string) (fs.FileInfo, error) { + info, err := g.exec.Stat(filepath.Join(g.root, filepath.FromSlash(name))) + g.record(err) + return info, err +} + +func (g *guardedGlobFS) ReadDir(name string) ([]fs.DirEntry, error) { + entries, err := g.exec.ReadDir(filepath.Join(g.root, filepath.FromSlash(name))) + g.record(err) + return entries, err +} + +func (g *guardedGlobFS) record(err error) { + // fs.Glob ignores directory errors. Preserve refusals for callers so a + // denied inventory cannot become a successful empty inventory. + if err != nil && !os.IsNotExist(err) && g.err == nil { + g.err = err + } +} + +func (g *guardedFiles) Glob(pattern string) ([]string, error) { + abs, err := filepath.Abs(pattern) + if err != nil { + return nil, err + } + root := filepath.VolumeName(abs) + string(filepath.Separator) + reader := &guardedGlobFS{exec: g, root: root} + matches, err := fs.Glob(reader, filepath.ToSlash(strings.TrimPrefix(abs, root))) + if err != nil { + return nil, err + } + for i, match := range matches { + matches[i] = filepath.Join(root, filepath.FromSlash(match)) + if !filepath.IsAbs(pattern) { + cwd, err := os.Getwd() + if err != nil { + return nil, err + } + matches[i], err = filepath.Rel(cwd, matches[i]) + if err != nil { + return nil, err + } + } + } + return matches, reader.err +} + +func (g *guardedFiles) WalkDir(root string, fn fs.WalkDirFunc) error { + abs, err := filepath.Abs(root) + if err != nil { + return fn(root, nil, err) + } + reader := &guardedGlobFS{exec: g, root: abs} + return fs.WalkDir(reader, ".", func(name string, entry fs.DirEntry, err error) error { + return fn(filepath.Join(root, filepath.FromSlash(name)), entry, err) + }) +} diff --git a/internal/executor/mock.go b/internal/executor/mock.go index 7dbac530..9645303a 100644 --- a/internal/executor/mock.go +++ b/internal/executor/mock.go @@ -3,6 +3,7 @@ package executor import ( "context" "fmt" + "io/fs" "os" "os/user" "path/filepath" @@ -504,3 +505,12 @@ func (e *mockDirEntry) Type() os.FileMode { func (e *mockDirEntry) Info() (os.FileInfo, error) { return &mockFileInfo{name: e.name, dir: e.dir}, nil } + +// WalkDir retains the real fixture walks used by detector tests. +func (m *Mock) WalkDir(root string, fn fs.WalkDirFunc) error { return filepath.WalkDir(root, fn) } + +// Open supports real ZIP fixtures; mocked metadata uses ReadFile. +func (m *Mock) Open(path string) (*os.File, error) { + // #nosec G304 -- Only used for test-owned archive fixtures. + return os.Open(path) +} diff --git a/internal/executor/user_aware.go b/internal/executor/user_aware.go index 09d5a7e7..7d7021af 100644 --- a/internal/executor/user_aware.go +++ b/internal/executor/user_aware.go @@ -3,6 +3,7 @@ package executor import ( "context" "fmt" + "io/fs" "os" "os/user" "strings" @@ -256,3 +257,9 @@ func (e *UserAwareExecutor) IsAppleCLTStub(ctx context.Context, binPath string) func (e *UserAwareExecutor) DiskCapacityBytes(path string) uint64 { return e.inner.DiskCapacityBytes(path) } + +func (e *UserAwareExecutor) WalkDir(root string, fn fs.WalkDirFunc) error { + return e.inner.WalkDir(root, fn) +} + +func (e *UserAwareExecutor) Open(path string) (*os.File, error) { return e.inner.Open(path) } diff --git a/internal/safepath/open_unix.go b/internal/safepath/open_unix.go index 3fe9d244..2171644d 100644 --- a/internal/safepath/open_unix.go +++ b/internal/safepath/open_unix.go @@ -25,7 +25,7 @@ import ( // unresolvable path while one that tolerates none sees the symlink it refuses. func openVerified(resolved string, wantDir, noFollow bool) (*os.File, os.FileInfo, error) { _, comps := split(resolved) - if len(comps) == 0 { + if len(comps) == 0 && !wantDir { return nil, nil, refuse(ReasonUnresolved) } @@ -43,6 +43,18 @@ func openVerified(resolved string, wantDir, noFollow bool) (*os.File, os.FileInf } }() + if len(comps) == 0 { + // #nosec G115 -- Openat returned a valid non-negative descriptor. + f := os.NewFile(uintptr(dirfd), resolved) + dirfd = -1 + info, err := f.Stat() + if err != nil { + _ = f.Close() + return nil, nil, refuse(ReasonDenied) + } + return f, info, nil + } + for _, comp := range comps[:len(comps)-1] { next, oerr := unix.Openat(dirfd, comp, dirFlags, 0) if oerr != nil { diff --git a/internal/safepath/open_windows.go b/internal/safepath/open_windows.go index 31a3eb6c..fe4faa21 100644 --- a/internal/safepath/open_windows.go +++ b/internal/safepath/open_windows.go @@ -26,7 +26,7 @@ import ( // follows nothing has already refused a reparse point it saw during resolution, // and this reports the ones that appear after it. func openVerified(resolved string, wantDir, noFollow bool) (*os.File, os.FileInfo, error) { - if _, comps := split(resolved); len(comps) == 0 { + if _, comps := split(resolved); len(comps) == 0 && !wantDir { return nil, nil, refuse(ReasonUnresolved) } diff --git a/internal/safepath/reader.go b/internal/safepath/reader.go index 3988c46e..422ef714 100644 --- a/internal/safepath/reader.go +++ b/internal/safepath/reader.go @@ -69,6 +69,13 @@ func (r *Reader) resolveChain(path string) (string, os.FileInfo, error) { } prefix := volume + string(filepath.Separator) + if len(comps) == 0 { + if !r.contains(prefix, false) { + return "", nil, refuse(ReasonOutsideRoots) + } + info, err := os.Lstat(prefix) + return prefix, info, err + } redirected := false var leaf os.FileInfo for i, comp := range comps { @@ -208,3 +215,21 @@ func splitPendingPath(path string) (volume string, comps []string) { } return volume, comps } + +// Open returns a verified regular file for seekable readers such as archive/zip. +// The caller owns the handle and must bound any content it reads from it. +func (r *Reader) Open(path string) (*os.File, error) { + resolved, err := r.Resolve(path) + if err != nil { + return nil, err + } + f, info, err := openVerified(resolved, false, false) + if err != nil { + return nil, err + } + if !info.Mode().IsRegular() { + _ = f.Close() + return nil, refuse(ReasonDenied) + } + return f, nil +} diff --git a/internal/safepath/safepath.go b/internal/safepath/safepath.go index cf89fe62..6b9af4f1 100644 --- a/internal/safepath/safepath.go +++ b/internal/safepath/safepath.go @@ -172,7 +172,8 @@ func (r *Resolver) containmentRoots() []string { func (r *Resolver) Contains(path string) bool { cleaned := filepath.Clean(path) for _, root := range r.containmentRoots() { - if pathEqual(cleaned, root) { + if pathEqual(cleaned, root) || (strings.HasSuffix(root, string(filepath.Separator)) && + len(cleaned) > len(root) && pathEqual(cleaned[:len(root)], root)) { return true } if len(cleaned) > len(root) && cleaned[len(root)] == filepath.Separator && diff --git a/internal/scan/scanner.go b/internal/scan/scanner.go index 251f0273..a6de1b1c 100644 --- a/internal/scan/scanner.go +++ b/internal/scan/scanner.go @@ -37,13 +37,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { // Resolve search directories searchDirs := resolveSearchDirs(exec, cfg.SearchDirs) log.Debug("search directories resolved: %v", searchDirs) - for _, d := range searchDirs { - if info, err := os.Stat(d); err != nil { - log.Warn("search directory %q is not accessible: %v — it will be skipped", d, err) - } else if !info.IsDir() { - log.Warn("search directory %q is not a directory — it will be skipped", d) - } - } // Build the TCC skipper so directory walks avoid macOS-protected dirs // (Documents, Downloads, ~/Library/Mail, ...) and don't trigger system @@ -51,6 +44,15 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { // network volumes are walked (the default); every Skipper method is // nil-safe so downstream callers don't branch. tccSkipper := tcc.ForRun(executor.ResolveHome(exec), cfg.IncludeTCCProtected, cfg.IncludeNetworkVolumes) + rootReader := tcc.GuardedFiles(exec, tccSkipper, 64<<20) + for _, d := range searchDirs { + if info, err := rootReader.Stat(d); err != nil { + log.Warn("search directory %q is not accessible: %v, it will be skipped", d, err) + } else if !info.IsDir() { + log.Warn("search directory %q is not a directory, it will be skipped", d) + } + } + if cands := tccSkipper.Candidates(); len(cands) > 0 { log.Warn("macOS TCC: skipping %d protected dirs (Documents, Downloads, ~/Library/Mail, ...) to avoid permission prompts. Pass --include-tcc-protected to scan them.", len(cands)) log.Debug("tcc skip list: %v", cands) @@ -78,7 +80,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { // Detect IDE installations log.StepStart("Detecting IDE installations") start = time.Now() - ideDetector := detector.NewIDEDetector(exec) + ideDetector := detector.NewIDEDetector(exec).WithSkipper(tccSkipper) ides := ideDetector.Detect(ctx) log.StepDone(time.Since(start)) @@ -87,9 +89,9 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { start = time.Now() cliDetector := detector.NewAICLIDetector(exec).WithLogger(log).WithSkipper(tccSkipper) cliTools := cliDetector.Detect(ctx) - agentDetector := detector.NewAgentDetector(exec).WithLogger(log) + agentDetector := detector.NewAgentDetector(exec).WithSkipper(tccSkipper).WithLogger(log) agents := agentDetector.Detect(ctx, searchDirs) - fwDetector := detector.NewFrameworkDetector(exec).WithLogger(log) + fwDetector := detector.NewFrameworkDetector(exec).WithLogger(log).WithSkipper(tccSkipper) frameworks := fwDetector.Detect(ctx) aiTools := mergeAITools(cliTools, agents, frameworks) log.StepDone(time.Since(start)) @@ -104,11 +106,11 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { // Collect IDE extensions log.StepStart("Collecting IDE extensions") start = time.Now() - extDetector := detector.NewExtensionDetector(exec) + extDetector := detector.NewExtensionDetector(exec).WithSkipper(tccSkipper) extensions := extDetector.Detect(ctx, searchDirs, ides) // Collect JetBrains plugins - jbDetector := detector.NewJetBrainsPluginDetector(exec) + jbDetector := detector.NewJetBrainsPluginDetector(exec).WithSkipper(tccSkipper) jbPlugins := jbDetector.Detect(ctx, ides) extensions = append(extensions, jbPlugins...) @@ -138,14 +140,14 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { if npmEnabled { log.StepStart("Detecting package managers") start = time.Now() - npmDetector := detector.NewNodePMDetector(exec).WithLogger(log) + npmDetector := detector.NewNodePMDetector(exec).WithSkipper(tccSkipper).WithLogger(log) pkgManagers = npmDetector.DetectManagers(ctx) log.StepDone(time.Since(start)) log.StepStart("Scanning Node.js projects") start = time.Now() projectDetector := detector.NewNodeProjectDetector(exec).WithSkipper(tccSkipper) - if !config.UseLegacyNodeScan { + if !config.UseLegacyNodeScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { projectDetector = projectDetector.WithDiskScan( detector.NewNodeDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } @@ -230,13 +232,13 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { if pythonEnabled { log.StepStart("Detecting Python package managers") start = time.Now() - pyDetector := detector.NewPythonPMDetector(exec).WithLogger(log) + pyDetector := detector.NewPythonPMDetector(exec).WithSkipper(tccSkipper).WithLogger(log) pythonPkgManagers = pyDetector.DetectManagers(ctx) log.StepDone(time.Since(start)) log.StepStart("Listing Python packages") start = time.Now() - if config.UseLegacyPythonScan { + if config.UseLegacyPythonScan && !tcc.ProtectedReadsDisabled(exec, tccSkipper) { pythonPackages = pyDetector.ListPackages(ctx) } else { pythonPackages = detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log).ScanGlobalPackages() @@ -246,7 +248,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { log.StepStart("Scanning Python projects") start = time.Now() pyProjectDetector := detector.NewPythonProjectDetector(exec).WithSkipper(tccSkipper).WithLogger(log) - if !config.UseLegacyPythonScan { + if !config.UseLegacyPythonScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { pyProjectDetector = pyProjectDetector.WithDiskScan( detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } @@ -315,7 +317,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { if featuregate.IsEnabled(featuregate.FeaturePipConfigAudit) { log.StepStart("Auditing pip configuration") start = time.Now() - pipAudit = configaudit.NewPipConfigDetector(exec).Detect(ctx, loggedInUser) + pipAudit = configaudit.NewPipConfigDetector(exec).WithSkipper(tccSkipper).Detect(ctx, loggedInUser) log.StepDone(time.Since(start)) } diff --git a/internal/tcc/reader.go b/internal/tcc/reader.go new file mode 100644 index 00000000..44018f16 --- /dev/null +++ b/internal/tcc/reader.go @@ -0,0 +1,87 @@ +package tcc + +import ( + "path/filepath" + "strings" + "sync/atomic" + + "github.com/step-security/dev-machine-guard/internal/executor" +) + +// GuardedFiles applies the skipper before direct reads and symlink traversal. +// libraryPaths are fixed, scanner-owned paths relative to ~/Library. They +// retain targeted inventory there without admitting arbitrary Library data. +func GuardedFiles(exec executor.Executor, s *Skipper, maxReadBytes int64, libraryPaths ...string) executor.Executor { + if exec.GOOS() != "darwin" || s == nil { + return exec + } + var allowed []string + for _, relative := range libraryPaths { + allowed = append(allowed, filepath.Join(s.home, "Library", relative)) + } + guarded := &protectedFiles{} + guarded.Executor = exec.GuardedFiles([]string{"/"}, func(path string) string { + // A Library exception must never override the independent volume opt-out. + if s.withinNetworkVolume(filepath.Clean(path)) { + guarded.refusals.Add(1) + return "tcc_protected" + } + for _, root := range allowed { + if strings.HasSuffix(root, "*") { + prefix := canonicalProtectionPath(strings.TrimSuffix(root, "*")) + if strings.HasPrefix(canonicalProtectionPath(path), prefix) || hasDirPrefix(prefix, canonicalProtectionPath(path)) { + return "" + } + continue + } + if hasDirPrefix(canonicalProtectionPath(path), canonicalProtectionPath(root)) || hasDirPrefix(canonicalProtectionPath(root), canonicalProtectionPath(path)) { + return "" + } + } + if !s.WithinProtected(path) { + return "" + } + guarded.refusals.Add(1) + return "tcc_protected" + }, maxReadBytes) + return guarded +} + +// ProtectedReadsDisabled also gates commands that load configuration themselves; +// the executor's file guard cannot intercept a child process's filesystem reads. +func ProtectedReadsDisabled(exec executor.Executor, s *Skipper) bool { + return exec.GOOS() == "darwin" && s != nil && (len(s.paths) != 0 || len(s.prefixes) != 0 || len(s.volumes) != 0) +} + +// canonicalProtectionPath covers the system Data-volume alias and the standard +// /private aliases without asking the kernel to resolve an untrusted path. +func canonicalProtectionPath(path string) string { + path = strings.ToLower(filepath.Clean(path)) + if hasDirPrefix(path, "/system/volumes/data") { + path = strings.TrimPrefix(path, "/system/volumes/data") + } + for _, prefix := range []string{"/private/var", "/private/tmp", "/private/etc"} { + if hasDirPrefix(path, prefix) { + path = strings.TrimPrefix(path, "/private") + break + } + } + // Refuse case variants on macOS, including on its default case-insensitive FS. + return path +} + +type protectedFiles struct { + executor.Executor + refusals atomic.Uint64 +} + +// Refusals lets discovery preserve a prior inventory when some roots were refused. +func Refusals(exec executor.Executor) uint64 { + if guarded, ok := exec.(*protectedFiles); ok { + return guarded.refusals.Load() + } + return 0 +} + +// HasGuard also gates helper commands that would read outside this reader. +func HasGuard(exec executor.Executor) bool { _, ok := exec.(*protectedFiles); return ok } diff --git a/internal/tcc/reader_darwin_test.go b/internal/tcc/reader_darwin_test.go new file mode 100644 index 00000000..9b308075 --- /dev/null +++ b/internal/tcc/reader_darwin_test.go @@ -0,0 +1,64 @@ +//go:build darwin + +package tcc + +import ( + "os" + "path/filepath" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" +) + +func TestProtectionAliases(t *testing.T) { + s := New("/Users/test-user") + for _, path := range []string{"/Users/test-user/Library/Containers/x", "/SYSTEM/VOLUMES/DATA/Users/test-user/library/Containers/x", "/System/Volumes/Data/Users/test-user/Documents/x"} { + if !s.WithinProtected(path) { + t.Errorf("not protected: %s", path) + } + } + if s.WithinProtected("/System/Volumes/DataBackup/Users/test-user/Library/x") { + t.Error("matched unrelated DataBackup path") + } +} + +func TestLibraryExceptionsDoNotAllowBrowserSiblings(t *testing.T) { + home := t.TempDir() + ordinary := filepath.Join(home, "Library", "Application Support", "Google", "AndroidStudio2026.1", "plugins") + chrome := filepath.Join(home, "Library", "Application Support", "Google", "Chrome") + for _, path := range []string{ordinary, chrome} { + if err := os.MkdirAll(path, 0700); err != nil { + t.Fatal(err) + } + } + s := New(home) + guarded := GuardedFiles(executor.NewReal(), s, 1024, "Application Support/Google/AndroidStudio*") + if _, err := guarded.ReadDir(ordinary); err != nil { + t.Fatal(err) + } + if len(s.Hits()) != 0 { + t.Fatalf("ordinary Library read recorded skip: %v", s.Hits()) + } + if _, err := guarded.ReadDir(chrome); err == nil { + t.Fatal("Chrome sibling admitted") + } + if Refusals(guarded) == 0 { + t.Fatal("refusal not recorded") + } +} + +func TestExplicitProtectedRootRequiresInclude(t *testing.T) { + home := t.TempDir() + root := filepath.Join(home, "Documents") + if err := os.MkdirAll(root, 0700); err != nil { + t.Fatal(err) + } + guarded := GuardedFiles(executor.NewReal(), ForRun(home, new(false), nil), 1024) + if guarded.DirExists(root) { + t.Fatal("explicit root bypassed protection") + } + included := GuardedFiles(executor.NewReal(), ForRun(home, new(true), nil), 1024) + if !included.DirExists(root) { + t.Fatal("explicit include refused ordinary fixture") + } +} diff --git a/internal/tcc/tcc.go b/internal/tcc/tcc.go index 610030a6..57cb3a23 100644 --- a/internal/tcc/tcc.go +++ b/internal/tcc/tcc.go @@ -69,6 +69,7 @@ func SkipNetworkVolumes(override *bool) bool { // Hits are tracked so callers can prove from logs which protected paths // were actually encountered during the walks. type Skipper struct { + home string paths map[string]struct{} prefixes []string volumes []string @@ -83,6 +84,7 @@ type Skipper struct { // the agent runs without a console user. func New(home string) *Skipper { return &Skipper{ + home: home, paths: buildProtectedPaths(home), prefixes: protectedPrefixes(), } @@ -109,7 +111,7 @@ func build(home string, protected bool, mounts []string) *Skipper { if !protected && len(mounts) == 0 { return nil } - s := &Skipper{volumes: mounts} + s := &Skipper{home: home, volumes: mounts} if protected { s.paths = buildProtectedPaths(home) s.prefixes = protectedPrefixes() @@ -171,19 +173,19 @@ func (s *Skipper) WithinProtected(path string) bool { if s == nil { return false } - cleaned := filepath.Clean(path) + cleaned := canonicalProtectionPath(path) for p := range s.paths { // Home-anchored protected dirs match on equality or a "/" boundary only. // hasPathPrefix (used for the prefixes below) also treats "." as a // boundary, which is correct for Time Machine names but here would let // ~/Documents swallow a sibling like ~/Documents.backup. - if hasDirPrefix(cleaned, p) { + if hasDirPrefix(cleaned, canonicalProtectionPath(p)) { s.recordHit(p) return true } } for _, p := range s.prefixes { - if hasPathPrefix(cleaned, p) { + if hasPathPrefix(cleaned, canonicalProtectionPath(p)) { s.recordHit(p) return true } @@ -197,7 +199,7 @@ func (s *Skipper) WithinProtected(path string) bool { // this is a no-op loop on the default path. func (s *Skipper) withinNetworkVolume(cleaned string) bool { for _, v := range s.volumes { - if hasDirPrefix(cleaned, v) { + if hasDirPrefix(canonicalProtectionPath(cleaned), canonicalProtectionPath(v)) { s.recordHit(v) return true } diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 772474c0..b3fbdc63 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -620,7 +620,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err // Detect IDEs phaseCtx, phaseCancel = startPhase(ctx, tracker, "ide_scan") log.Progress("Detecting IDE and AI desktop app installations...") - ideDetector := detector.NewIDEDetector(exec) + ideDetector := detector.NewIDEDetector(exec).WithSkipper(tccSkipper) ides := ideDetector.Detect(phaseCtx) for _, ide := range ides { log.Progress(" Found: %s (%s) v%s at %s", ideDisplayName(ide.IDEType), ide.Vendor, ide.Version, ide.InstallPath) @@ -635,11 +635,11 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err // Collect extensions phaseCtx, phaseCancel = startPhase(ctx, tracker, "extension_scan") log.Progress("Scanning extensions...") - extDetector := detector.NewExtensionDetector(exec) + extDetector := detector.NewExtensionDetector(exec).WithSkipper(tccSkipper) extensions := extDetector.Detect(phaseCtx, searchDirs, ides) // Collect JetBrains plugins - jbDetector := detector.NewJetBrainsPluginDetector(exec) + jbDetector := detector.NewJetBrainsPluginDetector(exec).WithSkipper(tccSkipper) jbPlugins := jbDetector.Detect(phaseCtx, ides) extensions = append(extensions, jbPlugins...) @@ -671,7 +671,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err fmt.Fprintln(os.Stderr) log.Progress("Detecting general-purpose AI agents...") - agents := detector.NewAgentDetector(userExec).WithLogger(log).Detect(phaseCtx, searchDirs) + agents := detector.NewAgentDetector(userExec).WithSkipper(tccSkipper).WithLogger(log).Detect(phaseCtx, searchDirs) for _, a := range agents { log.Progress(" Found: %s (%s) at %s", a.Name, a.Vendor, a.InstallPath) } @@ -681,7 +681,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err fmt.Fprintln(os.Stderr) log.Progress("Detecting AI frameworks and runtimes...") - frameworks := detector.NewFrameworkDetector(userExec).WithLogger(log).Detect(phaseCtx) + frameworks := detector.NewFrameworkDetector(userExec).WithLogger(log).WithSkipper(tccSkipper).Detect(phaseCtx) for _, f := range frameworks { running := "false" if f.IsRunning != nil && *f.IsRunning { @@ -815,7 +815,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err if pythonEnabled { phaseCtx, phaseCancel = startPhase(ctx, tracker, "python_scan") log.Progress("Detecting Python package managers...") - pyDetector := detector.NewPythonPMDetector(userExec).WithLogger(log) + pyDetector := detector.NewPythonPMDetector(userExec).WithSkipper(tccSkipper).WithLogger(log) pythonPkgManagers = pyDetector.DetectManagers(phaseCtx) for _, pm := range pythonPkgManagers { log.Progress(" Found: %s v%s at %s", pm.Name, pm.Version, pm.Path) @@ -830,7 +830,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err // "scanning uv") into the phase tracker so heartbeats surface where // inside the python phase a slow pip3 list is stuck. pyScanner.ProgressHook = func(detail string) { tracker.UpdateDetail(detail) } - if config.UseLegacyPythonScan { + if config.UseLegacyPythonScan && !tcc.ProtectedReadsDisabled(exec, tccSkipper) { pythonGlobalPkgs = pyScanner.ScanGlobalPackages(phaseCtx) } else { pythonGlobalPkgs = pyScanner.ScanGlobalPackagesFromDisk(tccSkipper) @@ -839,7 +839,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Searching for Python projects...") pyProjectDetector := detector.NewPythonProjectDetector(exec).WithSkipper(tccSkipper).WithLogger(log) - if !config.UseLegacyPythonScan { + if !config.UseLegacyPythonScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { pyProjectDetector = pyProjectDetector.WithDiskScan( detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } @@ -941,7 +941,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Node.js package scanning is ENABLED") log.Progress("Detecting Node.js package managers...") - npmDetector := detector.NewNodePMDetector(userExec).WithLogger(log) + npmDetector := detector.NewNodePMDetector(userExec).WithSkipper(tccSkipper).WithLogger(log) pkgManagers = npmDetector.DetectManagers(phaseCtx) for _, pm := range pkgManagers { log.Progress(" Found: %s v%s at %s", pm.Name, pm.Version, pm.Path) @@ -960,7 +960,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Scanning globally installed packages...") nodeScanner := detector.NewNodeScanner(exec, log, loggedInUsername).WithSkipper(tccSkipper) - if !config.UseLegacyNodeScan { + if !config.UseLegacyNodeScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { nodeScanner = nodeScanner.WithDiskScan( detector.NewNodeDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } @@ -1091,7 +1091,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err fmt.Fprintln(os.Stderr) log.Progress("Auditing pip configuration...") - pipAudit := configaudit.NewPipConfigDetector(userExec).Detect(ctx, npmrcLoggedIn) + pipAudit := configaudit.NewPipConfigDetector(userExec).WithSkipper(tccSkipper).Detect(ctx, npmrcLoggedIn) log.Progress(" pip available: %v, files discovered: %d, findings: %d", pipAudit.Available, len(pipAudit.Files), len(pipAudit.Findings)) fmt.Fprintln(os.Stderr) diff --git a/internal/versionmeta/versionmeta.go b/internal/versionmeta/versionmeta.go index d8439bbd..6bd3f7ed 100644 --- a/internal/versionmeta/versionmeta.go +++ b/internal/versionmeta/versionmeta.go @@ -19,6 +19,8 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/tcc" + "howett.net/plist" ) // FromBinary returns the version of the tool installed at binaryPath, derived @@ -180,11 +182,24 @@ func versionFromAppBundle(ctx context.Context, exec executor.Executor, resolved if idx < 0 { return "" } - plist := resolved[:idx+4] + "/Contents/Info.plist" - if !exec.FileExists(plist) { + plistPath := resolved[:idx+4] + "/Contents/Info.plist" + if tcc.HasGuard(exec) { + data, err := exec.ReadFile(plistPath) + var info struct { + Version string `plist:"CFBundleShortVersionString"` + } + if err != nil { + return "" + } + if _, err := plist.Unmarshal(data, &info); err != nil || !IsVersionLike(info.Version) { + return "" + } + return info.Version + } + if !exec.FileExists(plistPath) { return "" } - stdout, _, _, err := exec.RunWithTimeout(ctx, 10*time.Second, "/usr/libexec/PlistBuddy", "-c", "Print :CFBundleShortVersionString", plist) + stdout, _, _, err := exec.RunWithTimeout(ctx, 10*time.Second, "/usr/libexec/PlistBuddy", "-c", "Print :CFBundleShortVersionString", plistPath) if err != nil { return "" } diff --git a/internal/versionmeta/versionmeta_test.go b/internal/versionmeta/versionmeta_test.go index 2c3d7c86..e01a0ee4 100644 --- a/internal/versionmeta/versionmeta_test.go +++ b/internal/versionmeta/versionmeta_test.go @@ -2,9 +2,13 @@ package versionmeta import ( "context" + "os" + "path/filepath" "testing" "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/tcc" + "howett.net/plist" ) func TestFromBinary_NPMPackageManifest(t *testing.T) { @@ -243,3 +247,30 @@ func TestNodeModulesPackageRoot(t *testing.T) { } } } + +func TestGuardedAppBundleVersion(t *testing.T) { + home := t.TempDir() + skipper := tcc.New(home) + if skipper == nil { + t.Skip("macOS protection only") + } + app := filepath.Join(home, "Example.app", "Contents") + if err := os.MkdirAll(app, 0700); err != nil { + t.Fatal(err) + } + data, err := plist.Marshal(map[string]string{"CFBundleShortVersionString": "1.2.3"}, plist.BinaryFormat) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(app, "Info.plist"), data, 0600); err != nil { + t.Fatal(err) + } + exec := tcc.GuardedFiles(executor.NewReal(), skipper, 1<<20) + if got := versionFromAppBundle(context.Background(), exec, filepath.Join(app, "MacOS", "example")); got != "1.2.3" { + t.Fatalf("ordinary version = %q", got) + } + protected := filepath.Join(home, "Library", "Containers", "Example.app", "Contents", "MacOS", "example") + if got := versionFromAppBundle(context.Background(), exec, protected); got != "" { + t.Fatalf("protected version = %q", got) + } +} From a5993777d2fca39a562a6be1955d4403801b0f47 Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Thu, 1 Oct 2026 14:27:20 +0530 Subject: [PATCH 2/5] fix(tcc): preserve readable inventory and existing command behavior --- internal/detector/configaudit/bunfig.go | 5 - internal/detector/configaudit/files.go | 1 - internal/detector/configaudit/npmrc.go | 17 --- internal/detector/configaudit/pipconfig.go | 10 +- internal/detector/configaudit/pnpm.go | 17 --- .../protected_reads_darwin_test.go | 14 --- .../configaudit/tcc_command_darwin_test.go | 22 ++++ internal/detector/configaudit/yarn.go | 5 - internal/detector/credentials/detector.go | 2 +- .../credentials/tcc_command_darwin_test.go | 26 +++++ internal/detector/extension.go | 3 + internal/detector/jetbrains.go | 3 + internal/detector/mcp.go | 12 +- .../mixed_global_roots_darwin_test.go | 56 ++++++++++ .../mixed_python_roots_darwin_test.go | 103 ++++++++++++++++++ internal/detector/nodedist_global.go | 8 +- internal/detector/nodepm.go | 2 +- internal/detector/nodepm_fallback.go | 8 +- internal/detector/nodescan.go | 18 ++- .../detector/protected_reads_darwin_test.go | 25 +++++ internal/detector/pythondist.go | 33 +++--- internal/detector/pythonenv.go | 5 +- internal/detector/pythonpm.go | 2 - internal/detector/pythonproject.go | 9 ++ internal/detector/pythonscan.go | 7 +- internal/execguard/execguard.go | 5 - internal/execguard/execguard_test.go | 12 -- internal/execguard/tcc_command_darwin_test.go | 21 ++++ internal/model/model.go | 14 +++ internal/scan/scanner.go | 13 ++- internal/tcc/reader.go | 5 +- internal/telemetry/delta.go | 6 +- internal/telemetry/telemetry.go | 12 +- 33 files changed, 365 insertions(+), 136 deletions(-) create mode 100644 internal/detector/configaudit/tcc_command_darwin_test.go create mode 100644 internal/detector/credentials/tcc_command_darwin_test.go create mode 100644 internal/detector/mixed_global_roots_darwin_test.go create mode 100644 internal/detector/mixed_python_roots_darwin_test.go create mode 100644 internal/execguard/tcc_command_darwin_test.go diff --git a/internal/detector/configaudit/bunfig.go b/internal/detector/configaudit/bunfig.go index f5eec145..ff82363a 100644 --- a/internal/detector/configaudit/bunfig.go +++ b/internal/detector/configaudit/bunfig.go @@ -75,8 +75,6 @@ func (d *BunDetector) WithSkipper(s *tcc.Skipper) *BunDetector { if tcc.ProtectedReadsDisabled(d.exec, s) { d.ownerLookup = guardedOwner(d.exec) d.inGitRepo = guardedInGitRepo(d.exec) - // Git loads user-controlled config and includes in its own process. - d.gitTracked = nil } return d } @@ -292,9 +290,6 @@ func (d *BunDetector) bunVersion(ctx context.Context) string { } target = path } - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return "unknown" - } d.log.Progress("exec fallback: running %s --version (no metadata version source)", target) stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, target, "--version") if exit != 0 { diff --git a/internal/detector/configaudit/files.go b/internal/detector/configaudit/files.go index c7288bc3..2cf21cee 100644 --- a/internal/detector/configaudit/files.go +++ b/internal/detector/configaudit/files.go @@ -9,7 +9,6 @@ import ( ) const maxConfigFileSize = 32 << 20 -const protectedCommandReason = "not collected: protected-directory scanning is disabled" func auditLstat(exec executor.Executor, s *tcc.Skipper, path string) (os.FileInfo, error) { if tcc.ProtectedReadsDisabled(exec, s) { diff --git a/internal/detector/configaudit/npmrc.go b/internal/detector/configaudit/npmrc.go index b41bda69..1c57831a 100644 --- a/internal/detector/configaudit/npmrc.go +++ b/internal/detector/configaudit/npmrc.go @@ -17,7 +17,6 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/tcc" - "github.com/step-security/dev-machine-guard/internal/versionmeta" ) // maxNPMRCFiles caps the number of .npmrc files we report. Even on big @@ -87,8 +86,6 @@ func (d *NPMRCDetector) WithSkipper(s *tcc.Skipper) *NPMRCDetector { if tcc.ProtectedReadsDisabled(d.exec, s) { d.ownerLookup = guardedOwner(d.exec) d.inGitRepo = guardedInGitRepo(d.exec) - // Git loads user-controlled config and includes in its own process. - d.gitTracked = nil } return d } @@ -309,9 +306,6 @@ func (d *NPMRCDetector) collectFile(ctx context.Context, path, scope string) mod // captureEffective runs `npm config ls -l --json` and `npm config ls -l` for // source attribution. Returns nil when npm is unavailable. func (d *NPMRCDetector) captureEffective(ctx context.Context) *model.NPMRCEffective { - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return &model.NPMRCEffective{Error: protectedCommandReason} - } if _, err := d.exec.LookPath("npm"); err != nil { return nil } @@ -382,14 +376,6 @@ func parseSourceAttribution(text string) map[string]string { // npmVersion returns the npm CLI's version string, "unknown" on failure. func (d *NPMRCDetector) npmVersion(ctx context.Context) string { - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - if path, err := d.exec.LookPath("npm"); err == nil { - if v := versionmeta.FromBinary(ctx, d.exec, path); v != "" { - return v - } - } - return "unknown" - } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "npm", "--version") if exit != 0 { return "unknown" @@ -405,9 +391,6 @@ func (d *NPMRCDetector) npmVersion(ctx context.Context) string { // empty if the call failed or the value is "undefined" (npm's literal output // for an unset key). func (d *NPMRCDetector) npmConfigGet(ctx context.Context, key string) string { - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return "" - } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "npm", "config", "get", key) if exit != 0 { return "" diff --git a/internal/detector/configaudit/pipconfig.go b/internal/detector/configaudit/pipconfig.go index 578daa78..cabfb416 100644 --- a/internal/detector/configaudit/pipconfig.go +++ b/internal/detector/configaudit/pipconfig.go @@ -177,9 +177,6 @@ func (d *PipConfigDetector) detectPip(ctx context.Context) (string, []string, st // invoking --version against them pops a GUI install prompt. continue } - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return path, cand.args, cand.display, "unknown", true - } args := append([]string(nil), cand.args...) args = append(args, "--version") stdout, _, exit, err := d.exec.RunWithTimeout(ctx, 5*time.Second, cand.binary, args...) @@ -470,7 +467,7 @@ func (d *PipConfigDetector) discoverFiles(ctx context.Context, pipAvailable bool // Preferred: `pip config debug`. Falls back to manual path enumeration // when pip isn't installed or the output is unparseable. usedPipDebug := false - if pipAvailable && !tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if pipAvailable { if discovered, ok := d.discoverViaPipDebug(ctx); ok { usedPipDebug = true for _, e := range discovered { @@ -652,9 +649,6 @@ func (d *PipConfigDetector) populateFileMetadata(ctx context.Context, f *model.P var pipConfigListPrefix = regexp.MustCompile(`^([A-Za-z0-9_\-]+)\.([A-Za-z0-9_\-]+)='`) func (d *PipConfigDetector) captureEffective(ctx context.Context) (*model.PipEffective, error) { - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return nil, errors.New(protectedCommandReason) - } stdout, exit, ok := d.runPip(ctx, 10*time.Second, "config", "list", "-v") if !ok || exit != 0 { return nil, fmt.Errorf("pip config list -v exited %d", exit) @@ -784,8 +778,6 @@ func (d *PipConfigDetector) WithSkipper(s *tcc.Skipper) *PipConfigDetector { if tcc.ProtectedReadsDisabled(d.exec, s) { d.ownerLookup = guardedOwner(d.exec) d.inGitRepo = guardedInGitRepo(d.exec) - // Git loads user-controlled config and includes in its own process. - d.gitTracked = nil } return d } diff --git a/internal/detector/configaudit/pnpm.go b/internal/detector/configaudit/pnpm.go index d50660af..ddc2d611 100644 --- a/internal/detector/configaudit/pnpm.go +++ b/internal/detector/configaudit/pnpm.go @@ -16,7 +16,6 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/tcc" - "github.com/step-security/dev-machine-guard/internal/versionmeta" ) // pnpmEnvVars: pnpm-specific names plus the npm_config_* lowercase variants @@ -68,8 +67,6 @@ func (d *PnpmDetector) WithSkipper(s *tcc.Skipper) *PnpmDetector { if tcc.ProtectedReadsDisabled(d.exec, s) { d.ownerLookup = guardedOwner(d.exec) d.inGitRepo = guardedInGitRepo(d.exec) - // Git loads user-controlled config and includes in its own process. - d.gitTracked = nil } return d } @@ -237,9 +234,6 @@ func (d *PnpmDetector) collectFile(ctx context.Context, path, scope string) mode // captureEffective runs `pnpm config list --json`. SourceByKey stays empty — // pnpm doesn't emit per-key source attribution like `npm config ls -l` does. func (d *PnpmDetector) captureEffective(ctx context.Context) *model.PnpmEffective { - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return &model.PnpmEffective{Error: protectedCommandReason} - } eff := &model.PnpmEffective{ SourceByKey: map[string]string{}, Config: map[string]any{}, @@ -260,14 +254,6 @@ func (d *PnpmDetector) captureEffective(ctx context.Context) *model.PnpmEffectiv // pnpmVersion returns the pnpm CLI's version string, "unknown" on failure. func (d *PnpmDetector) pnpmVersion(ctx context.Context) string { - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - if path, err := d.exec.LookPath("pnpm"); err == nil { - if v := versionmeta.FromBinary(ctx, d.exec, path); v != "" { - return v - } - } - return "unknown" - } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "pnpm", "--version") if exit != 0 { return "unknown" @@ -282,9 +268,6 @@ func (d *PnpmDetector) pnpmVersion(ctx context.Context) string { // pnpmConfigGet runs `pnpm config get ` and returns the trimmed value, // or empty if the call failed or the value is pnpm's literal "undefined". func (d *PnpmDetector) pnpmConfigGet(ctx context.Context, key string) string { - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return "" - } stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, "pnpm", "config", "get", key) if exit != 0 { return "" diff --git a/internal/detector/configaudit/protected_reads_darwin_test.go b/internal/detector/configaudit/protected_reads_darwin_test.go index e57192b4..efc8790c 100644 --- a/internal/detector/configaudit/protected_reads_darwin_test.go +++ b/internal/detector/configaudit/protected_reads_darwin_test.go @@ -52,20 +52,6 @@ func TestProtectedConfigReads(t *testing.T) { } } -func TestProtectedConfigCommandsAreSkipped(t *testing.T) { - e, s := executor.NewMock(), tcc.New("/Users/test-user") - ctx := context.Background() - if got := NewNPMRCDetector(e).WithSkipper(s).captureEffective(ctx); got == nil || got.Error != protectedCommandReason { - t.Fatalf("npm effective=%+v", got) - } - if got := NewPnpmDetector(e).WithSkipper(s).captureEffective(ctx); got == nil || got.Error != protectedCommandReason { - t.Fatalf("pnpm effective=%+v", got) - } - if _, err := NewPipConfigDetector(e).WithSkipper(s).captureEffective(ctx); err == nil || err.Error() != protectedCommandReason { - t.Fatalf("pip err=%v", err) - } -} - func TestProtectedConfigOrdinarySymlinkMetadata(t *testing.T) { home := t.TempDir() target, link := filepath.Join(home, "config"), filepath.Join(home, ".npmrc") diff --git a/internal/detector/configaudit/tcc_command_darwin_test.go b/internal/detector/configaudit/tcc_command_darwin_test.go new file mode 100644 index 00000000..11b81147 --- /dev/null +++ b/internal/detector/configaudit/tcc_command_darwin_test.go @@ -0,0 +1,22 @@ +//go:build darwin + +package configaudit + +import ( + "context" + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/tcc" + "testing" +) + +func TestTCCPreservesEffectiveNPMConfig(t *testing.T) { + e := executor.NewMock() + e.SetGOOS("darwin") + e.SetPath("npm", "/usr/local/bin/npm") + e.SetCommand(`{"registry":"https://registry.npmjs.org/"}`, "", 0, "npm", "config", "ls", "-l", "--json") + e.SetCommand("registry = https://registry.npmjs.org/", "", 0, "npm", "config", "ls", "-l") + got := NewNPMRCDetector(e).WithSkipper(tcc.New(t.TempDir())).captureEffective(context.Background()) + if got == nil || got.Error != "" || got.Config["registry"] != "https://registry.npmjs.org/" { + t.Fatalf("lost effective npm config: %+v", got) + } +} diff --git a/internal/detector/configaudit/yarn.go b/internal/detector/configaudit/yarn.go index 19b7f3af..4e9db69b 100644 --- a/internal/detector/configaudit/yarn.go +++ b/internal/detector/configaudit/yarn.go @@ -81,8 +81,6 @@ func (d *YarnDetector) WithSkipper(s *tcc.Skipper) *YarnDetector { if tcc.ProtectedReadsDisabled(d.exec, s) { d.ownerLookup = guardedOwner(d.exec) d.inGitRepo = guardedInGitRepo(d.exec) - // Git loads user-controlled config and includes in its own process. - d.gitTracked = nil } return d } @@ -298,9 +296,6 @@ func (d *YarnDetector) yarnVersion(ctx context.Context) string { } target = path } - if tcc.ProtectedReadsDisabled(d.exec, d.skipper) { - return "unknown" - } d.log.Progress("exec fallback: running %s --version (no metadata version source)", target) stdout, _, exit, _ := d.exec.RunWithTimeout(ctx, 5*time.Second, target, "--version") if exit != 0 { diff --git a/internal/detector/credentials/detector.go b/internal/detector/credentials/detector.go index cf89a330..577f9e5f 100644 --- a/internal/detector/credentials/detector.go +++ b/internal/detector/credentials/detector.go @@ -376,7 +376,7 @@ func (d *Detector) buildFinding(ctx context.Context, scan *scanState, s source, finding.BroadReadAllowACEPresent = broadReadAllowACE(resolved) } finding.InGitRepo = scan.inGitRepo(resolved) - if finding.InGitRepo && !tcc.ProtectedReadsDisabled(d.exec, d.skipper) { + if finding.InGitRepo { finding.GitTracked = gitTracked(ctx, d.exec, resolved) } return finding diff --git a/internal/detector/credentials/tcc_command_darwin_test.go b/internal/detector/credentials/tcc_command_darwin_test.go new file mode 100644 index 00000000..bc557831 --- /dev/null +++ b/internal/detector/credentials/tcc_command_darwin_test.go @@ -0,0 +1,26 @@ +//go:build darwin + +package credentials + +import ( + "context" + "github.com/step-security/dev-machine-guard/internal/tcc" + "os" + "path/filepath" + "testing" +) + +func TestTCCPreservesCredentialGitTracking(t *testing.T) { + home := testHome(t) + writeTree(t, home, awsTree) + if err := os.MkdirAll(filepath.Join(home, ".git"), 0700); err != nil { + t.Fatal(err) + } + e := newMock(t, home) + e.SetCommand("credentials", "", 0, "git", "-C", filepath.Join(home, ".aws"), "ls-files", "--error-unmatch", "credentials") + got := New(e).WithSkipper(tcc.New(home)).withEnv(staticEnv(nil)).Detect(context.Background()) + f, ok := findingFor(got, sourceAWSCredentials) + if !ok || !f.InGitRepo || !f.GitTracked { + t.Fatalf("tracked credential became untracked: found=%v in_repo=%v tracked=%v", ok, f.InGitRepo, f.GitTracked) + } +} diff --git a/internal/detector/extension.go b/internal/detector/extension.go index 24f82490..092c7243 100644 --- a/internal/detector/extension.go +++ b/internal/detector/extension.go @@ -171,3 +171,6 @@ func (d *ExtensionDetector) WithSkipper(s *tcc.Skipper) *ExtensionDetector { d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Application Support/JetBrains", "Application Support/Google/AndroidStudio*") return d } + +// Incomplete reports a protected read refused during this scan. +func (d *ExtensionDetector) Incomplete() bool { return tcc.Refusals(d.exec) > 0 } diff --git a/internal/detector/jetbrains.go b/internal/detector/jetbrains.go index f7687af1..3e96c579 100644 --- a/internal/detector/jetbrains.go +++ b/internal/detector/jetbrains.go @@ -255,3 +255,6 @@ func (d *JetBrainsPluginDetector) WithSkipper(s *tcc.Skipper) *JetBrainsPluginDe d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Application Support/JetBrains", "Application Support/Google/AndroidStudio*") return d } + +// Incomplete reports a protected read refused during this scan. +func (d *JetBrainsPluginDetector) Incomplete() bool { return tcc.Refusals(d.exec) > 0 } diff --git a/internal/detector/mcp.go b/internal/detector/mcp.go index 4a943469..63157596 100644 --- a/internal/detector/mcp.go +++ b/internal/detector/mcp.go @@ -13,6 +13,7 @@ import ( "github.com/step-security/dev-machine-guard/internal/aiagents/redact" "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" + "github.com/step-security/dev-machine-guard/internal/safepath" "github.com/step-security/dev-machine-guard/internal/tcc" ) @@ -52,8 +53,9 @@ var mcpConfigDefinitions = []mcpConfigSpec{ // MCPDetector collects MCP configuration files. type MCPDetector struct { - exec executor.Executor - skipper *tcc.Skipper + readRefused bool + exec executor.Executor + skipper *tcc.Skipper } func NewMCPDetector(exec executor.Executor) *MCPDetector { @@ -102,6 +104,9 @@ func (d *MCPDetector) DetectEnterprise(_ context.Context, searchDirs []string) [ }, maxJSONConfigBytes) } content, err := reader.ReadFile(loc.ConfigPath) + if safepath.ReasonOf(err) != "" { + d.readRefused = true + } if err != nil || len(content) == 0 { continue } @@ -393,3 +398,6 @@ func stripJSONCComments(input []byte) []byte { } return out } + +// Incomplete reports a protected read refused during this scan. +func (d *MCPDetector) Incomplete() bool { return d.readRefused || tcc.Refusals(d.exec) > 0 } diff --git a/internal/detector/mixed_global_roots_darwin_test.go b/internal/detector/mixed_global_roots_darwin_test.go new file mode 100644 index 00000000..b2cd56e7 --- /dev/null +++ b/internal/detector/mixed_global_roots_darwin_test.go @@ -0,0 +1,56 @@ +//go:build darwin + +package detector + +import ( + "os" + "path/filepath" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +func TestMixedNVMGlobalRoots(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + versions := filepath.Join(home, ".nvm", "versions", "node") + safe := filepath.Join(versions, "safe", "lib", "node_modules") + protected := filepath.Join(home, "Documents", "node") + for _, p := range []string{filepath.Join(safe, "widgets"), filepath.Join(versions, "bad"), protected} { + if err := os.MkdirAll(p, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(safe, "widgets", "package.json"), []byte(`{"name":"widgets","version":"1.0.0"}`), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(protected, filepath.Join(versions, "bad", "lib")); err != nil { + t.Fatal(err) + } + e := isolatedGlobalExecutor{protectedFixtureExecutor{Executor: executor.NewReal(), home: home}} + s := tcc.New(home) + guarded := tcc.GuardedFiles(e, s, maxLockfileSize, "pnpm", "Application Support/fnm") + matches, globErr := guarded.Glob(filepath.Join(versions, "*", "lib", "node_modules")) + t.Logf("guarded Glob: matches=%v err=%v", matches, globErr) + if len(matches) != 1 || matches[0] != safe || globErr == nil { + t.Fatalf("fixture did not produce safe match plus refusal: %v %v", matches, globErr) + } + results := NewNodeScanner(e, progress.NewNoop(), "").WithSkipper(s).WithDiskScan(NewNodeDistDetector(e).WithSkipper(s)).scanGlobalPackagesFromDisk() + readable, incomplete := false, false + for _, r := range results { + t.Logf("manager=%s packages=%d exit=%d error=%q", r.PackageManager, r.PackagesCount, r.ExitCode, r.Error) + if r.PackageManager == "npm" && r.ExitCode == 0 && len(r.Packages) == 1 && r.Packages[0].Name == "widgets" { + readable = true + } + if r.PackageManager == "npm" && r.ExitCode != 0 { + incomplete = true + } + } + if !readable || !incomplete { + t.Fatalf("readable=%v incomplete=%v, want both", readable, incomplete) + } +} diff --git a/internal/detector/mixed_python_roots_darwin_test.go b/internal/detector/mixed_python_roots_darwin_test.go new file mode 100644 index 00000000..44f7232a --- /dev/null +++ b/internal/detector/mixed_python_roots_darwin_test.go @@ -0,0 +1,103 @@ +//go:build darwin + +package detector + +import ( + "encoding/base64" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +func TestMixedPythonGlobalRoots(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + safe := filepath.Join(home, ".local", "lib", "python3.12", "site-packages") + metadata := filepath.Join(safe, "widgets-1.0.0.dist-info", "METADATA") + link := filepath.Join(home, ".local", "share", "pipx", "venvs") + protected := filepath.Join(home, "Documents", "pipx") + for _, path := range []string{filepath.Dir(metadata), filepath.Dir(link), protected} { + if err := os.MkdirAll(path, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(metadata, []byte("Name: widgets\nVersion: 1.0.0\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(protected, link); err != nil { + t.Fatal(err) + } + e := isolatedGlobalExecutor{protectedFixtureExecutor{Executor: executor.NewReal(), home: home}} + s := tcc.New(home) + control := NewPythonDistDetector(e).WithSkipper(s).ScanRoots([]string{safe}) + if len(control) != 1 || control[0].Name != "widgets" { + t.Fatalf("safe control missing package: %+v", control) + } + guarded := tcc.GuardedFiles(e, s, maxMetadataFileSize, "Python") + roots := GlobalPythonRoots(guarded, progress.NewNoop()) + t.Logf("roots=%v refusals=%d", roots, tcc.Refusals(guarded)) + if len(roots) != 1 || (roots[0] != safe && roots[0] != filepath.Dir(safe)) || tcc.Refusals(guarded) == 0 { + t.Fatal("fixture must preserve safe root and refuse pipx redirect") + } + t.Run("enterprise", func(t *testing.T) { + results := NewPythonScanner(e, progress.NewNoop()).ScanGlobalPackagesFromDisk(s) + for _, r := range results { + data, _ := base64.StdEncoding.DecodeString(r.RawStdoutBase64) + t.Logf("exit=%d error=%q packages=%s", r.ExitCode, r.Error, data) + if strings.Contains(string(data), "widgets") && r.Partial && r.ExitCode == 0 { + return + } + } + t.Fatal("protected pipx root swallowed ordinary Python package") + }) + t.Run("community", func(t *testing.T) { + dist := NewPythonDistDetector(e).WithSkipper(s) + packages := dist.ScanGlobalPackages() + if !dist.Incomplete() { + t.Fatal("partial community scan reported complete") + } + for _, p := range packages { + if p.Name == "widgets" { + return + } + } + t.Fatal("protected pipx root swallowed ordinary Python package") + }) +} + +func TestPartialPythonMetadataRead(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + root := filepath.Join(home, ".local", "lib", "python3.12", "site-packages") + safe := filepath.Join(root, "widgets-1.0.0.dist-info", "METADATA") + link := filepath.Join(root, "blocked-1.0.0.dist-info", "METADATA") + target := filepath.Join(home, "Documents", "METADATA") + for _, p := range []string{filepath.Dir(safe), filepath.Dir(link), filepath.Dir(target)} { + if err := os.MkdirAll(p, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(safe, []byte("Name: widgets\nVersion: 1.0.0\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, link); err != nil { + t.Fatal(err) + } + e := isolatedGlobalExecutor{protectedFixtureExecutor{Executor: executor.NewReal(), home: home}} + dist := NewPythonDistDetector(e).WithSkipper(tcc.New(home)) + if packages := dist.scanRoots([]string{root}); len(packages) != 1 || !dist.Incomplete() { + t.Fatalf("partial metadata packages=%v incomplete=%v", packages, dist.Incomplete()) + } + if packages := dist.ScanRoots([]string{root}); packages != nil { + t.Fatal("project inventory must not cache partial metadata as complete") + } +} diff --git a/internal/detector/nodedist_global.go b/internal/detector/nodedist_global.go index a47aecd4..dd0c87b0 100644 --- a/internal/detector/nodedist_global.go +++ b/internal/detector/nodedist_global.go @@ -60,14 +60,12 @@ func nodeGlobalRoots(exec executor.Executor) ([]nodeGlobalRoot, map[string]bool) } addGlob := func(pm, pattern string) { before := tcc.Refusals(exec) - matches, err := exec.Glob(pattern) + matches, _ := exec.Glob(pattern) if tcc.Refusals(exec) != before { refused[pm] = true } - if err == nil { - for _, m := range matches { - add(pm, m) - } + for _, m := range matches { + add(pm, m) } } home := nodeHomeDir(exec) diff --git a/internal/detector/nodepm.go b/internal/detector/nodepm.go index 488490e5..bf441bc6 100644 --- a/internal/detector/nodepm.go +++ b/internal/detector/nodepm.go @@ -61,7 +61,7 @@ func (d *NodePMDetector) DetectManagers(ctx context.Context) []model.PkgManager // the version without launching anything. version = versionmeta.FromBinary(ctx, d.exec, path) } - if path != "" && version == "" && !tcc.HasGuard(d.exec) { + if path != "" && version == "" { if safe, reason := execguard.SafeToExec(ctx, d.exec, path); !safe { d.log.Warn("skipping %s version probe: %s", path, reason) } else { diff --git a/internal/detector/nodepm_fallback.go b/internal/detector/nodepm_fallback.go index 7e5f5a39..197fc209 100644 --- a/internal/detector/nodepm_fallback.go +++ b/internal/detector/nodepm_fallback.go @@ -11,7 +11,6 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" "github.com/step-security/dev-machine-guard/internal/progress" - "github.com/step-security/dev-machine-guard/internal/tcc" "github.com/step-security/dev-machine-guard/internal/versionmeta" ) @@ -99,8 +98,8 @@ func pmBinaryCandidateDirs(exec executor.Executor) []string { // binary, not strict semver ordering). func nvmNodeBinDirs(exec executor.Executor, home string) []string { pattern := filepath.Join(home, ".nvm", "versions", "node", "*", "bin") - matches, err := exec.Glob(pattern) - if err != nil || len(matches) == 0 { + matches, _ := exec.Glob(pattern) + if len(matches) == 0 { return nil } sort.Sort(sort.Reverse(sort.StringSlice(matches))) @@ -172,9 +171,6 @@ func runPMVersion(ctx context.Context, exec executor.Executor, log *progress.Log if v := versionmeta.FromBinary(ctx, exec, binPath); v != "" { return v } - if tcc.HasGuard(exec) { - return "" - } if safe, reason := execguard.SafeToExec(ctx, exec, binPath); !safe { log.Warn("skipping %s version probe: %s", binPath, reason) return "" diff --git a/internal/detector/nodescan.go b/internal/detector/nodescan.go index 9c8f078c..6f8fea12 100644 --- a/internal/detector/nodescan.go +++ b/internal/detector/nodescan.go @@ -34,6 +34,7 @@ func getMaxProjectScanBytes() int64 { // NodeScanner performs enterprise-mode node scanning (raw output, base64 encoded). type NodeScanner struct { + unobserved []string exec executor.Executor log *progress.Logger loggedInUser string // when non-empty and running as root, commands run as this user @@ -434,19 +435,19 @@ type projectEntry struct { // the cap" when comparing against prior state. func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, knownLastVerified map[string]time.Time) (results []model.NodeScanResult, discovered []string) { var projects []projectEntry - var unobserved []string + s.unobserved = nil for _, dir := range searchDirs { s.log.Progress(" Searching in: %s", dir) _ = s.exec.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { if err != nil { if !os.IsNotExist(err) { - unobserved = append(unobserved, path) + s.unobserved = append(s.unobserved, path) } return nil } if entry.IsDir() { if s.skipper.ShouldSkip(path, dir) { - unobserved = append(unobserved, path) + s.unobserved = append(s.unobserved, path) return filepath.SkipDir } name := entry.Name() @@ -479,11 +480,14 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, kno discovered = append(discovered, p.dir) } - discovered = retainUnobservedProjects(discovered, knownLastVerified, unobserved) + discovered = retainUnobservedProjects(discovered, knownLastVerified, s.unobserved) projects = orderScanProjects(projects, knownLastVerified) if len(projects) > maxNodeProjects { s.log.Warn("Node project scan truncated at %d projects (total discovered: %d) — lowest-priority projects were skipped", maxNodeProjects, len(projects)) + for _, p := range projects[maxNodeProjects:] { + s.unobserved = append(s.unobserved, p.dir) + } projects = projects[:maxNodeProjects] } @@ -505,6 +509,9 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, kno totalSize += resultSize capped = append(capped, r) } + for _, r := range results[len(capped):] { + s.unobserved = append(s.unobserved, r.ProjectPath) + } return capped, discovered } @@ -857,3 +864,6 @@ func isInsideNodeModules(projectDir string) bool { normalized := strings.ReplaceAll(projectDir, "\\", "/") return strings.Contains(normalized, "/node_modules/") } + +// UnobservedProjects includes subtrees this run could not inspect. +func (s *NodeScanner) UnobservedProjects() []string { return s.unobserved } diff --git a/internal/detector/protected_reads_darwin_test.go b/internal/detector/protected_reads_darwin_test.go index 725a9584..530773ed 100644 --- a/internal/detector/protected_reads_darwin_test.go +++ b/internal/detector/protected_reads_darwin_test.go @@ -264,3 +264,28 @@ func TestProtectedPythonVenvDiscoveryIsNotEmptySuccess(t *testing.T) { } } } + +func TestMCPNestedReaderRefusalIsIncomplete(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + config := filepath.Join(home, ".codex", "config.toml") + target := filepath.Join(home, "work", "config.toml") + for _, p := range []string{filepath.Dir(config), filepath.Dir(target)} { + if err := os.MkdirAll(p, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(target, []byte("[mcp_servers.widgets]\ncommand='example-tool'\n"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, config); err != nil { + t.Fatal(err) + } + e := protectedFixtureExecutor{Executor: executor.NewReal(), home: home} + d := NewMCPDetector(e).WithSkipper(tcc.New(home)) + if got := d.DetectEnterprise(context.Background(), nil); len(got) != 0 || !d.Incomplete() { + t.Fatalf("nested-reader results=%v incomplete=%v", got, d.Incomplete()) + } +} diff --git a/internal/detector/pythondist.go b/internal/detector/pythondist.go index b331ec43..65bf2a01 100644 --- a/internal/detector/pythondist.go +++ b/internal/detector/pythondist.go @@ -38,6 +38,7 @@ const maxMetadataFileSize = 32 << 20 // 32 MiB // PythonDistDetector discovers installed Python packages from install // metadata on disk, with no package-manager subprocess. type PythonDistDetector struct { + readFailed bool exec executor.Executor log *progress.Logger skipper *tcc.Skipper @@ -89,9 +90,8 @@ func venvSitePackages(exec executor.Executor, venvPath string) []string { filepath.Join(venvPath, "lib", "python*", "site-packages"), filepath.Join(venvPath, "Lib", "site-packages"), } { - if matches, err := exec.Glob(pattern); err == nil { - roots = append(roots, matches...) - } + matches, _ := exec.Glob(pattern) + roots = append(roots, matches...) } if len(roots) == 0 { return []string{venvPath} @@ -107,6 +107,15 @@ func venvSitePackages(exec executor.Executor, venvPath string) []string { // A successful empty walk returns a non-nil slice. A walk failure returns nil // so delta cannot replace previously uploaded inventory with a partial result. func (d *PythonDistDetector) ScanRoots(roots []string) []model.PackageDetail { + pkgs := d.scanRoots(roots) + if d.readFailed { + return nil + } + return pkgs +} + +func (d *PythonDistDetector) scanRoots(roots []string) []model.PackageDetail { + d.readFailed = false if len(roots) == 0 { return nil } @@ -148,9 +157,7 @@ func (d *PythonDistDetector) ScanRoots(roots []string) []model.PackageDetail { }) } - if walkFailed { - return nil - } + d.readFailed = walkFailed sort.Slice(pkgs, func(i, j int) bool { if pkgs[i].Name == pkgs[j].Name { return pkgs[i].Version < pkgs[j].Version @@ -163,12 +170,8 @@ func (d *PythonDistDetector) ScanRoots(roots []string) []model.PackageDetail { // ScanGlobalPackages walks the host's global / user site-packages roots and // returns the installed packages, replacing the `pip3 list` global scan. func (d *PythonDistDetector) ScanGlobalPackages() []model.PythonPackage { - before := tcc.Refusals(d.exec) roots := GlobalPythonRoots(d.exec, d.log) - if tcc.Refusals(d.exec) != before { - return nil - } - details := d.ScanRoots(roots) + details := d.scanRoots(roots) if details == nil { return nil } @@ -304,9 +307,8 @@ func PythonGlobalRoots(exec executor.Executor) []string { var candidates []string add := func(paths ...string) { candidates = append(candidates, paths...) } addGlob := func(pattern string) { - if matches, err := exec.Glob(pattern); err == nil { - add(matches...) - } + matches, _ := exec.Glob(pattern) + add(matches...) } // Anchor per-user paths on the console (GUI) user, not the process user: @@ -355,3 +357,6 @@ func PythonGlobalRoots(exec executor.Executor) []string { } return roots } + +// Incomplete reports refused discovery or unreadable package metadata. +func (d *PythonDistDetector) Incomplete() bool { return d.readFailed || tcc.Refusals(d.exec) > 0 } diff --git a/internal/detector/pythonenv.go b/internal/detector/pythonenv.go index 0eb16d09..4842fcfc 100644 --- a/internal/detector/pythonenv.go +++ b/internal/detector/pythonenv.go @@ -100,9 +100,8 @@ func DiscoverPythonInstallRoots(exec executor.Executor, log *progress.Logger) [] func expandGlobs(exec executor.Executor, patterns []string) []string { var out []string for _, pat := range patterns { - if m, err := exec.Glob(pat); err == nil { - out = append(out, m...) - } + m, _ := exec.Glob(pat) + out = append(out, m...) } return out } diff --git a/internal/detector/pythonpm.go b/internal/detector/pythonpm.go index 24a03e8f..5824d212 100644 --- a/internal/detector/pythonpm.go +++ b/internal/detector/pythonpm.go @@ -63,8 +63,6 @@ func (d *PythonPMDetector) DetectManagers(ctx context.Context) []model.PkgManage // layouts carry the version in the install path. if v := versionmeta.FromBinary(ctx, d.exec, path); v != "" { version = v - } else if tcc.HasGuard(d.exec) { - // Preserve detection with an unknown version when executing could load protected config. } else if safe, reason := execguard.SafeToExec(ctx, d.exec, path); !safe { d.log.Warn("skipping %s version probe: %s", path, reason) } else { diff --git a/internal/detector/pythonproject.go b/internal/detector/pythonproject.go index 5c00bce6..dcff952f 100644 --- a/internal/detector/pythonproject.go +++ b/internal/detector/pythonproject.go @@ -108,6 +108,9 @@ func (d *PythonProjectDetector) ListProjects(searchDirs []string, knownLastVerif if len(candidates) > maxPythonProjects { d.log.Warn("Python project scan truncated at %d venvs (total discovered: %d) — lowest-priority venvs were skipped", maxPythonProjects, len(candidates)) + for _, c := range candidates[maxPythonProjects:] { + d.unobserved = append(d.unobserved, c.path) + } candidates = candidates[:maxPythonProjects] } @@ -129,6 +132,9 @@ func (d *PythonProjectDetector) ListProjects(searchDirs []string, knownLastVerif // so delta retries instead of caching an unverified empty result. d.log.Debug("python venv has no pip — skipping package list: %s (%s)", c.path, c.pm) } + if pkgs == nil { + d.unobserved = append(d.unobserved, c.path) + } projects = append(projects, model.ProjectInfo{ Path: c.path, PackageManager: c.pm, @@ -313,3 +319,6 @@ func (d *PythonProjectDetector) detectPM(projectDir string) string { } return "pip" } + +// UnobservedProjects includes subtrees this run could not inspect. +func (d *PythonProjectDetector) UnobservedProjects() []string { return d.unobserved } diff --git a/internal/detector/pythonscan.go b/internal/detector/pythonscan.go index aa2ffba0..b155e851 100644 --- a/internal/detector/pythonscan.go +++ b/internal/detector/pythonscan.go @@ -102,7 +102,8 @@ func (s *PythonScanner) ScanGlobalPackages(ctx context.Context) []model.PythonSc func (s *PythonScanner) ScanGlobalPackagesFromDisk(skipper *tcc.Skipper) []model.PythonScanResult { guarded := tcc.GuardedFiles(s.exec, skipper, maxMetadataFileSize, "Python") roots := GlobalPythonRoots(guarded, s.log) - if tcc.Refusals(guarded) > 0 { + partial := tcc.Refusals(guarded) > 0 + if len(roots) == 0 && partial { return []model.PythonScanResult{{PackageManager: "pip", ExitCode: 1, Error: "global package roots include protected paths"}} } if len(roots) == 0 { @@ -115,7 +116,8 @@ func (s *PythonScanner) ScanGlobalPackagesFromDisk(skipper *tcc.Skipper) []model start := time.Now() dist := NewPythonDistDetector(s.exec).WithLogger(s.log).WithSkipper(skipper) - pkgs := dist.ScanRoots(roots) + pkgs := dist.scanRoots(roots) + partial = partial || dist.Incomplete() duration := time.Since(start).Milliseconds() if pkgs == nil { return []model.PythonScanResult{{ @@ -139,6 +141,7 @@ func (s *PythonScanner) ScanGlobalPackagesFromDisk(skipper *tcc.Skipper) []model PackageManager: "pip", RawStdoutBase64: base64.StdEncoding.EncodeToString(raw), ExitCode: 0, + Partial: partial, ScanDurationMs: duration, }} } diff --git a/internal/execguard/execguard.go b/internal/execguard/execguard.go index 2f4d17c8..090ba948 100644 --- a/internal/execguard/execguard.go +++ b/internal/execguard/execguard.go @@ -24,7 +24,6 @@ import ( "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" - "github.com/step-security/dev-machine-guard/internal/tcc" ) const probeTimeout = 5 * time.Second @@ -51,10 +50,6 @@ const probeTimeout = 5 * time.Second // On Linux it answers whether the binary is an Electron app's GUI entry point, // purely from stats. func SafeToExec(ctx context.Context, exec executor.Executor, binaryPath string) (bool, string) { - // Child processes can load configs beyond the guarded reader. - if tcc.HasGuard(exec) { - return false, "protected-directory scanning is disabled" - } if binaryPath == "" { return true, "" } diff --git a/internal/execguard/execguard_test.go b/internal/execguard/execguard_test.go index ec8365d6..99268bf7 100644 --- a/internal/execguard/execguard_test.go +++ b/internal/execguard/execguard_test.go @@ -7,7 +7,6 @@ import ( "time" "github.com/step-security/dev-machine-guard/internal/executor" - "github.com/step-security/dev-machine-guard/internal/tcc" ) const ( @@ -244,14 +243,3 @@ func TestSafeToExec_ReasonMatchesPlatform(t *testing.T) { } }) } - -func TestSafeToExecRefusesGuardedFallback(t *testing.T) { - skipper := tcc.New(t.TempDir()) - if skipper == nil { - t.Skip("macOS protection only") - } - exec := tcc.GuardedFiles(executor.NewMock(), skipper, 1024) - if safe, reason := SafeToExec(context.Background(), exec, "/usr/local/bin/example"); safe || reason != "protected-directory scanning is disabled" { - t.Fatalf("guarded fallback = %v, %q", safe, reason) - } -} diff --git a/internal/execguard/tcc_command_darwin_test.go b/internal/execguard/tcc_command_darwin_test.go new file mode 100644 index 00000000..9435cfb9 --- /dev/null +++ b/internal/execguard/tcc_command_darwin_test.go @@ -0,0 +1,21 @@ +//go:build darwin + +package execguard + +import ( + "context" + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/tcc" + "testing" +) + +func TestTCCPreservesSafeCommandFallback(t *testing.T) { + e := executor.NewMock() + e.SetGOOS("darwin") + e.SetCommand("", "", 1, "/usr/bin/xattr", "-p", "com.apple.quarantine", "/usr/local/bin/widgets") + e.SetCommand("", "", 1, "/usr/bin/xattr", "-p", "com.apple.quarantine", "/usr/local/bin") + guarded := tcc.GuardedFiles(e, tcc.New(t.TempDir()), 1024) + if safe, reason := SafeToExec(context.Background(), guarded, "/usr/local/bin/widgets"); !safe { + t.Fatalf("ordinary CLI fallback suppressed: %s", reason) + } +} diff --git a/internal/model/model.go b/internal/model/model.go index 31fd43d4..932b51fa 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -2,6 +2,8 @@ package model // ScanResult is the community-mode JSON output structure. type ScanResult struct { + InventoryCoverage *InventoryCoverage `json:"inventory_coverage,omitempty"` + AgentVersion string `json:"agent_version"` AgentURL string `json:"agent_url"` ScanTimestamp int64 `json:"scan_timestamp"` @@ -375,6 +377,8 @@ type BrewScanResult struct { // PythonScanResult holds raw Python scan output for enterprise telemetry. type PythonScanResult struct { + // Partial preserves readable packages while preventing removal of unseen packages. + Partial bool `json:"partial,omitempty"` PackageManager string `json:"package_manager"` PMVersion string `json:"package_manager_version"` BinaryPath string `json:"binary_path"` // Resolved path to the package manager binary @@ -1184,3 +1188,13 @@ type GoConfigFinding struct { Key string `json:"key"` Detail string `json:"detail"` } + +// InventoryCoverage reports observations that cannot establish removals. +// Absent coverage preserves the full-snapshot contract of older agents. +type InventoryCoverage struct { + PythonGlobalsIncomplete bool `json:"python_globals_incomplete,omitempty"` + IDEExtensionsIncomplete bool `json:"ide_extensions_incomplete,omitempty"` + MCPConfigsIncomplete bool `json:"mcp_configs_incomplete,omitempty"` + NodeProjectsUnobserved []string `json:"node_projects_unobserved,omitempty"` + PythonProjectsUnobserved []string `json:"python_projects_unobserved,omitempty"` +} diff --git a/internal/scan/scanner.go b/internal/scan/scanner.go index 1b6f4915..1ccb6d78 100644 --- a/internal/scan/scanner.go +++ b/internal/scan/scanner.go @@ -113,6 +113,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { jbDetector := detector.NewJetBrainsPluginDetector(exec).WithSkipper(tccSkipper) jbPlugins := jbDetector.Detect(ctx, ides) extensions = append(extensions, jbPlugins...) + coverage := &model.InventoryCoverage{IDEExtensionsIncomplete: extDetector.Incomplete() || jbDetector.Incomplete(), MCPConfigsIncomplete: mcpDetector.Incomplete()} // On Windows, filter out bundled/platform plugins (e.g., Eclipse's 500+ OSGi // bundles) unless explicitly requested. macOS detection doesn't produce bundled @@ -147,7 +148,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { log.StepStart("Scanning Node.js projects") start = time.Now() projectDetector := detector.NewNodeProjectDetector(exec).WithSkipper(tccSkipper) - if !config.UseLegacyNodeScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { + if !config.UseLegacyNodeScan { projectDetector = projectDetector.WithDiskScan( detector.NewNodeDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } @@ -238,21 +239,24 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { log.StepStart("Listing Python packages") start = time.Now() - if config.UseLegacyPythonScan && !tcc.ProtectedReadsDisabled(exec, tccSkipper) { + if config.UseLegacyPythonScan { pythonPackages = pyDetector.ListPackages(ctx) } else { - pythonPackages = detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log).ScanGlobalPackages() + dist := detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log) + pythonPackages = dist.ScanGlobalPackages() + coverage.PythonGlobalsIncomplete = dist.Incomplete() } log.StepDone(time.Since(start)) log.StepStart("Scanning Python projects") start = time.Now() pyProjectDetector := detector.NewPythonProjectDetector(exec).WithSkipper(tccSkipper).WithLogger(log) - if !config.UseLegacyPythonScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { + if !config.UseLegacyPythonScan { pyProjectDetector = pyProjectDetector.WithDiskScan( detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } pythonProjects, _ = pyProjectDetector.ListProjects(searchDirs, nil) + coverage.PythonProjectsUnobserved = pyProjectDetector.UnobservedProjects() log.StepDone(time.Since(start)) } else { log.StepStart("Python package scanning") @@ -404,6 +408,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { Device: dev, AIAgentsAndTools: aiTools, IDEInstallations: ides, + InventoryCoverage: coverage, IDEExtensions: extensions, MCPConfigs: mcpConfigsToCommunity(mcpConfigs), NodePkgManagers: pkgManagers, diff --git a/internal/tcc/reader.go b/internal/tcc/reader.go index 44018f16..806ece12 100644 --- a/internal/tcc/reader.go +++ b/internal/tcc/reader.go @@ -47,8 +47,7 @@ func GuardedFiles(exec executor.Executor, s *Skipper, maxReadBytes int64, librar return guarded } -// ProtectedReadsDisabled also gates commands that load configuration themselves; -// the executor's file guard cannot intercept a child process's filesystem reads. +// ProtectedReadsDisabled selects guarded direct file reads. func ProtectedReadsDisabled(exec executor.Executor, s *Skipper) bool { return exec.GOOS() == "darwin" && s != nil && (len(s.paths) != 0 || len(s.prefixes) != 0 || len(s.volumes) != 0) } @@ -83,5 +82,5 @@ func Refusals(exec executor.Executor) uint64 { return 0 } -// HasGuard also gates helper commands that would read outside this reader. +// HasGuard identifies an executor with protected direct reads. func HasGuard(exec executor.Executor) bool { _, ok := exec.(*protectedFiles); return ok } diff --git a/internal/telemetry/delta.go b/internal/telemetry/delta.go index 5de1a963..32f43ca8 100644 --- a/internal/telemetry/delta.go +++ b/internal/telemetry/delta.go @@ -181,7 +181,11 @@ func globalRecordsFromPython(results []model.PythonScanResult) []state.GlobalRec continue } hash, _ := state.CanonicalHashJSON(decodeBase64OrRaw(r.RawStdoutBase64)) - out = append(out, state.GlobalRecord{PM: r.PackageManager, Hash: hash, ExitCode: r.ExitCode}) + exitCode := r.ExitCode + if r.Partial { + exitCode = 1 + } + out = append(out, state.GlobalRecord{PM: r.PackageManager, Hash: hash, ExitCode: exitCode}) } return out } diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 6440e2fc..e8c5f396 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -51,6 +51,7 @@ const CurrentPayloadSchemaVersion = 1 // Payload is the enterprise telemetry JSON structure. type Payload struct { + InventoryCoverage *model.InventoryCoverage `json:"inventory_coverage,omitempty"` // PayloadSchemaVersion gates the delta-protocol sibling fields below // (NodeProjectsUnchanged etc.). Zero/absent = legacy snapshot, every // scanned project ships its full body in NodeProjects/PythonProjects. @@ -634,6 +635,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err jbDetector := detector.NewJetBrainsPluginDetector(exec).WithSkipper(tccSkipper) jbPlugins := jbDetector.Detect(phaseCtx, ides) extensions = append(extensions, jbPlugins...) + coverage := &model.InventoryCoverage{IDEExtensionsIncomplete: extDetector.Incomplete() || jbDetector.Incomplete()} // On Windows, filter out bundled/platform plugins (e.g., Eclipse's 500+ OSGi // bundles) unless explicitly requested. macOS is unaffected. @@ -695,6 +697,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Collecting MCP configuration files...") mcpDetector := detector.NewMCPDetector(exec).WithSkipper(tccSkipper) mcpConfigs := mcpDetector.DetectEnterprise(phaseCtx, searchDirs) + coverage.MCPConfigsIncomplete = mcpDetector.Incomplete() for _, c := range mcpConfigs { log.Progress(" Found: %s config (%s)", c.ConfigSource, c.Vendor) } @@ -822,7 +825,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err // "scanning uv") into the phase tracker so heartbeats surface where // inside the python phase a slow pip3 list is stuck. pyScanner.ProgressHook = func(detail string) { tracker.UpdateDetail(detail) } - if config.UseLegacyPythonScan && !tcc.ProtectedReadsDisabled(exec, tccSkipper) { + if config.UseLegacyPythonScan { pythonGlobalPkgs = pyScanner.ScanGlobalPackages(phaseCtx) } else { pythonGlobalPkgs = pyScanner.ScanGlobalPackagesFromDisk(tccSkipper) @@ -831,7 +834,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Searching for Python projects...") pyProjectDetector := detector.NewPythonProjectDetector(exec).WithSkipper(tccSkipper).WithLogger(log) - if !config.UseLegacyPythonScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { + if !config.UseLegacyPythonScan { pyProjectDetector = pyProjectDetector.WithDiskScan( detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } @@ -844,6 +847,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err } } pythonProjects, pythonDiscovered = pyProjectDetector.ListProjects(searchDirs, knownPython) + coverage.PythonProjectsUnobserved = pyProjectDetector.UnobservedProjects() log.Progress(" Found %d Python projects", len(pythonProjects)) fmt.Fprintln(os.Stderr) endPhase(phaseCtx, phaseCancel, tracker, log, "python_scan") @@ -953,7 +957,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Scanning globally installed packages...") nodeScanner := detector.NewNodeScanner(exec, log, loggedInUsername).WithSkipper(tccSkipper) - if !config.UseLegacyNodeScan || tcc.ProtectedReadsDisabled(exec, tccSkipper) { + if !config.UseLegacyNodeScan { nodeScanner = nodeScanner.WithDiskScan( detector.NewNodeDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } @@ -976,6 +980,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err } } nodeProjects, nodeDiscovered = nodeScanner.ScanProjects(phaseCtx, searchDirs, knownNPM) + coverage.NodeProjectsUnobserved = nodeScanner.UnobservedProjects() nodeScanMs = time.Since(scanStart).Milliseconds() log.Progress(" Found %d Node.js projects", len(nodeProjects)) log.Progress(" Scan duration: %dms", nodeScanMs) @@ -1212,6 +1217,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err InvocationMethod: invocationMethod, StatusInfo: &finalStatusInfo, + InventoryCoverage: coverage, IDEExtensions: extensions, IDEInstallations: ides, NodePkgManagers: pkgManagers, From c1925a1e611f793728c5ae25adc5b6361a299f93 Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Thu, 1 Oct 2026 17:42:55 +0530 Subject: [PATCH 3/5] fix(tcc): keep scanner remediation scoped to agent --- internal/detector/extension.go | 3 --- internal/detector/jetbrains.go | 3 --- internal/detector/mcp.go | 12 ++---------- internal/detector/nodescan.go | 18 ++++-------------- .../detector/protected_reads_darwin_test.go | 6 +++--- internal/detector/pythonproject.go | 9 --------- internal/model/model.go | 16 ++-------------- internal/scan/scanner.go | 7 +------ internal/telemetry/telemetry.go | 6 ------ 9 files changed, 12 insertions(+), 68 deletions(-) diff --git a/internal/detector/extension.go b/internal/detector/extension.go index 092c7243..24f82490 100644 --- a/internal/detector/extension.go +++ b/internal/detector/extension.go @@ -171,6 +171,3 @@ func (d *ExtensionDetector) WithSkipper(s *tcc.Skipper) *ExtensionDetector { d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Application Support/JetBrains", "Application Support/Google/AndroidStudio*") return d } - -// Incomplete reports a protected read refused during this scan. -func (d *ExtensionDetector) Incomplete() bool { return tcc.Refusals(d.exec) > 0 } diff --git a/internal/detector/jetbrains.go b/internal/detector/jetbrains.go index 3e96c579..f7687af1 100644 --- a/internal/detector/jetbrains.go +++ b/internal/detector/jetbrains.go @@ -255,6 +255,3 @@ func (d *JetBrainsPluginDetector) WithSkipper(s *tcc.Skipper) *JetBrainsPluginDe d.exec = tcc.GuardedFiles(d.exec, s, maxLockfileSize, "Application Support/JetBrains", "Application Support/Google/AndroidStudio*") return d } - -// Incomplete reports a protected read refused during this scan. -func (d *JetBrainsPluginDetector) Incomplete() bool { return tcc.Refusals(d.exec) > 0 } diff --git a/internal/detector/mcp.go b/internal/detector/mcp.go index 63157596..4a943469 100644 --- a/internal/detector/mcp.go +++ b/internal/detector/mcp.go @@ -13,7 +13,6 @@ import ( "github.com/step-security/dev-machine-guard/internal/aiagents/redact" "github.com/step-security/dev-machine-guard/internal/executor" "github.com/step-security/dev-machine-guard/internal/model" - "github.com/step-security/dev-machine-guard/internal/safepath" "github.com/step-security/dev-machine-guard/internal/tcc" ) @@ -53,9 +52,8 @@ var mcpConfigDefinitions = []mcpConfigSpec{ // MCPDetector collects MCP configuration files. type MCPDetector struct { - readRefused bool - exec executor.Executor - skipper *tcc.Skipper + exec executor.Executor + skipper *tcc.Skipper } func NewMCPDetector(exec executor.Executor) *MCPDetector { @@ -104,9 +102,6 @@ func (d *MCPDetector) DetectEnterprise(_ context.Context, searchDirs []string) [ }, maxJSONConfigBytes) } content, err := reader.ReadFile(loc.ConfigPath) - if safepath.ReasonOf(err) != "" { - d.readRefused = true - } if err != nil || len(content) == 0 { continue } @@ -398,6 +393,3 @@ func stripJSONCComments(input []byte) []byte { } return out } - -// Incomplete reports a protected read refused during this scan. -func (d *MCPDetector) Incomplete() bool { return d.readRefused || tcc.Refusals(d.exec) > 0 } diff --git a/internal/detector/nodescan.go b/internal/detector/nodescan.go index 6f8fea12..9c8f078c 100644 --- a/internal/detector/nodescan.go +++ b/internal/detector/nodescan.go @@ -34,7 +34,6 @@ func getMaxProjectScanBytes() int64 { // NodeScanner performs enterprise-mode node scanning (raw output, base64 encoded). type NodeScanner struct { - unobserved []string exec executor.Executor log *progress.Logger loggedInUser string // when non-empty and running as root, commands run as this user @@ -435,19 +434,19 @@ type projectEntry struct { // the cap" when comparing against prior state. func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, knownLastVerified map[string]time.Time) (results []model.NodeScanResult, discovered []string) { var projects []projectEntry - s.unobserved = nil + var unobserved []string for _, dir := range searchDirs { s.log.Progress(" Searching in: %s", dir) _ = s.exec.WalkDir(dir, func(path string, entry os.DirEntry, err error) error { if err != nil { if !os.IsNotExist(err) { - s.unobserved = append(s.unobserved, path) + unobserved = append(unobserved, path) } return nil } if entry.IsDir() { if s.skipper.ShouldSkip(path, dir) { - s.unobserved = append(s.unobserved, path) + unobserved = append(unobserved, path) return filepath.SkipDir } name := entry.Name() @@ -480,14 +479,11 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, kno discovered = append(discovered, p.dir) } - discovered = retainUnobservedProjects(discovered, knownLastVerified, s.unobserved) + discovered = retainUnobservedProjects(discovered, knownLastVerified, unobserved) projects = orderScanProjects(projects, knownLastVerified) if len(projects) > maxNodeProjects { s.log.Warn("Node project scan truncated at %d projects (total discovered: %d) — lowest-priority projects were skipped", maxNodeProjects, len(projects)) - for _, p := range projects[maxNodeProjects:] { - s.unobserved = append(s.unobserved, p.dir) - } projects = projects[:maxNodeProjects] } @@ -509,9 +505,6 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, kno totalSize += resultSize capped = append(capped, r) } - for _, r := range results[len(capped):] { - s.unobserved = append(s.unobserved, r.ProjectPath) - } return capped, discovered } @@ -864,6 +857,3 @@ func isInsideNodeModules(projectDir string) bool { normalized := strings.ReplaceAll(projectDir, "\\", "/") return strings.Contains(normalized, "/node_modules/") } - -// UnobservedProjects includes subtrees this run could not inspect. -func (s *NodeScanner) UnobservedProjects() []string { return s.unobserved } diff --git a/internal/detector/protected_reads_darwin_test.go b/internal/detector/protected_reads_darwin_test.go index 530773ed..5a736bd7 100644 --- a/internal/detector/protected_reads_darwin_test.go +++ b/internal/detector/protected_reads_darwin_test.go @@ -265,7 +265,7 @@ func TestProtectedPythonVenvDiscoveryIsNotEmptySuccess(t *testing.T) { } } -func TestMCPNestedReaderRefusalIsIncomplete(t *testing.T) { +func TestMCPNestedReaderRefusalIsSkipped(t *testing.T) { home, err := filepath.EvalSymlinks(t.TempDir()) if err != nil { t.Fatal(err) @@ -285,7 +285,7 @@ func TestMCPNestedReaderRefusalIsIncomplete(t *testing.T) { } e := protectedFixtureExecutor{Executor: executor.NewReal(), home: home} d := NewMCPDetector(e).WithSkipper(tcc.New(home)) - if got := d.DetectEnterprise(context.Background(), nil); len(got) != 0 || !d.Incomplete() { - t.Fatalf("nested-reader results=%v incomplete=%v", got, d.Incomplete()) + if got := d.DetectEnterprise(context.Background(), nil); len(got) != 0 { + t.Fatalf("nested reader returned a refused configuration: %v", got) } } diff --git a/internal/detector/pythonproject.go b/internal/detector/pythonproject.go index dcff952f..5c00bce6 100644 --- a/internal/detector/pythonproject.go +++ b/internal/detector/pythonproject.go @@ -108,9 +108,6 @@ func (d *PythonProjectDetector) ListProjects(searchDirs []string, knownLastVerif if len(candidates) > maxPythonProjects { d.log.Warn("Python project scan truncated at %d venvs (total discovered: %d) — lowest-priority venvs were skipped", maxPythonProjects, len(candidates)) - for _, c := range candidates[maxPythonProjects:] { - d.unobserved = append(d.unobserved, c.path) - } candidates = candidates[:maxPythonProjects] } @@ -132,9 +129,6 @@ func (d *PythonProjectDetector) ListProjects(searchDirs []string, knownLastVerif // so delta retries instead of caching an unverified empty result. d.log.Debug("python venv has no pip — skipping package list: %s (%s)", c.path, c.pm) } - if pkgs == nil { - d.unobserved = append(d.unobserved, c.path) - } projects = append(projects, model.ProjectInfo{ Path: c.path, PackageManager: c.pm, @@ -319,6 +313,3 @@ func (d *PythonProjectDetector) detectPM(projectDir string) string { } return "pip" } - -// UnobservedProjects includes subtrees this run could not inspect. -func (d *PythonProjectDetector) UnobservedProjects() []string { return d.unobserved } diff --git a/internal/model/model.go b/internal/model/model.go index 932b51fa..f3cf0b8b 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -2,8 +2,6 @@ package model // ScanResult is the community-mode JSON output structure. type ScanResult struct { - InventoryCoverage *InventoryCoverage `json:"inventory_coverage,omitempty"` - AgentVersion string `json:"agent_version"` AgentURL string `json:"agent_url"` ScanTimestamp int64 `json:"scan_timestamp"` @@ -377,8 +375,8 @@ type BrewScanResult struct { // PythonScanResult holds raw Python scan output for enterprise telemetry. type PythonScanResult struct { - // Partial preserves readable packages while preventing removal of unseen packages. - Partial bool `json:"partial,omitempty"` + // Partial keeps an incomplete global scan eligible for a local retry. + Partial bool `json:"-"` PackageManager string `json:"package_manager"` PMVersion string `json:"package_manager_version"` BinaryPath string `json:"binary_path"` // Resolved path to the package manager binary @@ -1188,13 +1186,3 @@ type GoConfigFinding struct { Key string `json:"key"` Detail string `json:"detail"` } - -// InventoryCoverage reports observations that cannot establish removals. -// Absent coverage preserves the full-snapshot contract of older agents. -type InventoryCoverage struct { - PythonGlobalsIncomplete bool `json:"python_globals_incomplete,omitempty"` - IDEExtensionsIncomplete bool `json:"ide_extensions_incomplete,omitempty"` - MCPConfigsIncomplete bool `json:"mcp_configs_incomplete,omitempty"` - NodeProjectsUnobserved []string `json:"node_projects_unobserved,omitempty"` - PythonProjectsUnobserved []string `json:"python_projects_unobserved,omitempty"` -} diff --git a/internal/scan/scanner.go b/internal/scan/scanner.go index 1ccb6d78..d4470a83 100644 --- a/internal/scan/scanner.go +++ b/internal/scan/scanner.go @@ -113,7 +113,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { jbDetector := detector.NewJetBrainsPluginDetector(exec).WithSkipper(tccSkipper) jbPlugins := jbDetector.Detect(ctx, ides) extensions = append(extensions, jbPlugins...) - coverage := &model.InventoryCoverage{IDEExtensionsIncomplete: extDetector.Incomplete() || jbDetector.Incomplete(), MCPConfigsIncomplete: mcpDetector.Incomplete()} // On Windows, filter out bundled/platform plugins (e.g., Eclipse's 500+ OSGi // bundles) unless explicitly requested. macOS detection doesn't produce bundled @@ -242,9 +241,7 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { if config.UseLegacyPythonScan { pythonPackages = pyDetector.ListPackages(ctx) } else { - dist := detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log) - pythonPackages = dist.ScanGlobalPackages() - coverage.PythonGlobalsIncomplete = dist.Incomplete() + pythonPackages = detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log).ScanGlobalPackages() } log.StepDone(time.Since(start)) @@ -256,7 +253,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } pythonProjects, _ = pyProjectDetector.ListProjects(searchDirs, nil) - coverage.PythonProjectsUnobserved = pyProjectDetector.UnobservedProjects() log.StepDone(time.Since(start)) } else { log.StepStart("Python package scanning") @@ -408,7 +404,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) error { Device: dev, AIAgentsAndTools: aiTools, IDEInstallations: ides, - InventoryCoverage: coverage, IDEExtensions: extensions, MCPConfigs: mcpConfigsToCommunity(mcpConfigs), NodePkgManagers: pkgManagers, diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index e8c5f396..49862929 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -51,7 +51,6 @@ const CurrentPayloadSchemaVersion = 1 // Payload is the enterprise telemetry JSON structure. type Payload struct { - InventoryCoverage *model.InventoryCoverage `json:"inventory_coverage,omitempty"` // PayloadSchemaVersion gates the delta-protocol sibling fields below // (NodeProjectsUnchanged etc.). Zero/absent = legacy snapshot, every // scanned project ships its full body in NodeProjects/PythonProjects. @@ -635,7 +634,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err jbDetector := detector.NewJetBrainsPluginDetector(exec).WithSkipper(tccSkipper) jbPlugins := jbDetector.Detect(phaseCtx, ides) extensions = append(extensions, jbPlugins...) - coverage := &model.InventoryCoverage{IDEExtensionsIncomplete: extDetector.Incomplete() || jbDetector.Incomplete()} // On Windows, filter out bundled/platform plugins (e.g., Eclipse's 500+ OSGi // bundles) unless explicitly requested. macOS is unaffected. @@ -697,7 +695,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Collecting MCP configuration files...") mcpDetector := detector.NewMCPDetector(exec).WithSkipper(tccSkipper) mcpConfigs := mcpDetector.DetectEnterprise(phaseCtx, searchDirs) - coverage.MCPConfigsIncomplete = mcpDetector.Incomplete() for _, c := range mcpConfigs { log.Progress(" Found: %s config (%s)", c.ConfigSource, c.Vendor) } @@ -847,7 +844,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err } } pythonProjects, pythonDiscovered = pyProjectDetector.ListProjects(searchDirs, knownPython) - coverage.PythonProjectsUnobserved = pyProjectDetector.UnobservedProjects() log.Progress(" Found %d Python projects", len(pythonProjects)) fmt.Fprintln(os.Stderr) endPhase(phaseCtx, phaseCancel, tracker, log, "python_scan") @@ -980,7 +976,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err } } nodeProjects, nodeDiscovered = nodeScanner.ScanProjects(phaseCtx, searchDirs, knownNPM) - coverage.NodeProjectsUnobserved = nodeScanner.UnobservedProjects() nodeScanMs = time.Since(scanStart).Milliseconds() log.Progress(" Found %d Node.js projects", len(nodeProjects)) log.Progress(" Scan duration: %dms", nodeScanMs) @@ -1217,7 +1212,6 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err InvocationMethod: invocationMethod, StatusInfo: &finalStatusInfo, - InventoryCoverage: coverage, IDEExtensions: extensions, IDEInstallations: ides, NodePkgManagers: pkgManagers, From dc1fcfc3301793fa606a846aac83be504e02df68 Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Thu, 1 Oct 2026 18:21:22 +0530 Subject: [PATCH 4/5] fix(tcc): preserve Python recovery and readable AI discovery --- internal/detector/aicli.go | 7 +- .../detector/mixed_ai_roots_darwin_test.go | 130 ++++++++++++++++++ internal/model/model.go | 2 +- internal/telemetry/delta.go | 8 +- internal/telemetry/delta_globals_test.go | 57 ++++++++ 5 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 internal/detector/mixed_ai_roots_darwin_test.go diff --git a/internal/detector/aicli.go b/internal/detector/aicli.go index 4a8c6922..1ade0710 100644 --- a/internal/detector/aicli.go +++ b/internal/detector/aicli.go @@ -901,12 +901,9 @@ func aiCLIBinaryCandidateDirs(exec executor.Executor, homeDir string) []string { } // globDirs expands one glob pattern, newest-looking first (descending lexical, -// the rule nvmNodeBinDirs already uses) and empty on any error. +// the rule nvmNodeBinDirs already uses). Keep readable matches on partial errors. func globDirs(exec executor.Executor, pattern string) []string { - matches, err := exec.Glob(pattern) - if err != nil || len(matches) == 0 { - return nil - } + matches, _ := exec.Glob(pattern) sort.Sort(sort.Reverse(sort.StringSlice(matches))) return matches } diff --git a/internal/detector/mixed_ai_roots_darwin_test.go b/internal/detector/mixed_ai_roots_darwin_test.go new file mode 100644 index 00000000..5f76634f --- /dev/null +++ b/internal/detector/mixed_ai_roots_darwin_test.go @@ -0,0 +1,130 @@ +//go:build darwin + +package detector + +import ( + "context" + "io/fs" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/step-security/dev-machine-guard/internal/executor" + "github.com/step-security/dev-machine-guard/internal/progress" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +type mixedAIExecutor struct{ protectedFixtureExecutor } + +func (e mixedAIExecutor) LookPath(string) (string, error) { return "", fs.ErrNotExist } +func (e mixedAIExecutor) FileExists(p string) bool { + return strings.HasPrefix(p, e.home+string(filepath.Separator)) && e.Executor.FileExists(p) +} +func (e mixedAIExecutor) Glob(p string) ([]string, error) { + if !strings.HasPrefix(p, e.home+string(filepath.Separator)) { + return nil, nil + } + return e.Executor.Glob(p) +} +func (e mixedAIExecutor) GuardedFiles(roots []string, guard func(string) string, max int64) executor.Executor { + e.Executor = e.Executor.GuardedFiles(roots, guard, max) + return e +} + +func TestMixedFNMKeepsReadableAmpIdentity(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + versions := filepath.Join(home, ".local", "share", "fnm", "node-versions") + installation := filepath.Join(versions, "v22.0.0", "installation") + manifestRoot := filepath.Join(installation, "lib", "node_modules", "@ampcode", "cli") + target := filepath.Join(manifestRoot, "bin", "amp.js") + binary := filepath.Join(installation, "bin", "amp") + for _, dir := range []string{filepath.Dir(target), filepath.Dir(binary)} { + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + } + if err := os.WriteFile(filepath.Join(manifestRoot, "package.json"), []byte(`{"name":"@ampcode/cli","version":"1.2.3"}`), 0600); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(target, []byte("fixture, never executed"), 0600); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, binary); err != nil { + t.Fatal(err) + } + e := mixedAIExecutor{protectedFixtureExecutor{Executor: executor.NewReal(), home: home}} + skipper := tcc.New(home) + guarded := tcc.GuardedFiles(e, skipper, maxLockfileSize, "pnpm", "Application Support/fnm") + spec := cliToolSpec{Name: "amp", Binaries: []string{"amp"}} + resolve := func() (cliResolution, bool) { + return resolveAmp(context.Background(), guarded, progress.NewNoop(), skipper, spec, home) + } + control, ok := resolve() + if !ok || control.BinaryPath != binary || control.StaticVersion != "1.2.3" { + t.Fatalf("readable control identity missing: %+v found=%v", control, ok) + } + protected := filepath.Join(home, "Documents", "node") + if err := os.MkdirAll(filepath.Join(protected, "installation", "bin"), 0700); err != nil { + t.Fatal(err) + } + if err := os.Symlink(protected, filepath.Join(versions, "v23.0.0")); err != nil { + t.Fatal(err) + } + baseline, baselineFound := resolveAmp(context.Background(), e, progress.NewNoop(), skipper, spec, home) + if !baselineFound || baseline != control { + t.Fatalf("baseline-shaped mixed-root control lost identity: %+v found=%v", baseline, baselineFound) + } + t.Logf("baseline-shaped mixed-root control=%+v found=%v", baseline, baselineFound) + matches, globErr := guarded.Glob(filepath.Join(versions, "*", "installation", "bin")) + if len(matches) != 1 || matches[0] != filepath.Dir(binary) || globErr == nil { + t.Fatalf("fixture must retain safe match plus refusal: %v %v", matches, globErr) + } + got, found := resolve() + t.Logf("control=%+v found=%v; mixed=%+v found=%v; matches=%v globErr=%v", control, ok, got, found, matches, globErr) + if !found || got != control { + t.Fatalf("protected sibling removed readable Amp identity: %+v found=%v", got, found) + } +} + +func TestMixedFactoryConfigKeepsReadableIdentity(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + binary := filepath.Join(home, ".local", "bin", "droid") + config := filepath.Join(home, ".factory") + protected := filepath.Join(home, "Documents", "factory") + for _, dir := range []string{filepath.Dir(binary), config, protected} { + if err := os.MkdirAll(dir, 0700); err != nil { + t.Fatal(err) + } + } + for _, p := range []string{binary, filepath.Join(config, "settings.json")} { + if err := os.WriteFile(p, []byte("fixture, never executed"), 0600); err != nil { + t.Fatal(err) + } + } + e := mixedAIExecutor{protectedFixtureExecutor{Executor: executor.NewReal(), home: home}} + skipper := tcc.New(home) + guarded := tcc.GuardedFiles(e, skipper, maxLockfileSize) + spec := cliToolSpec{Name: "factory", Binaries: []string{"~/.local/bin/droid"}} + control, ok := resolveFactory(context.Background(), guarded, progress.NewNoop(), skipper, spec, home) + if !ok || control.BinaryPath != binary { + t.Fatalf("ordinary Factory control missing: %+v found=%v", control, ok) + } + if err := os.Symlink(protected, filepath.Join(config, "protected")); err != nil { + t.Fatal(err) + } + matches, globErr := guarded.Glob(filepath.Join(config, "*")) + if len(matches) != 2 || globErr != nil { + t.Fatalf("one-level existence check should not traverse the protected sibling: %v %v", matches, globErr) + } + got, found := resolveFactory(context.Background(), guarded, progress.NewNoop(), skipper, spec, home) + if !found || got != control { + t.Fatalf("protected config sibling removed readable Factory identity: %+v found=%v", got, found) + } +} diff --git a/internal/model/model.go b/internal/model/model.go index f3cf0b8b..72c75212 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -375,7 +375,7 @@ type BrewScanResult struct { // PythonScanResult holds raw Python scan output for enterprise telemetry. type PythonScanResult struct { - // Partial keeps an incomplete global scan eligible for a local retry. + // Partial marks limited disk collection without changing the wire result. Partial bool `json:"-"` PackageManager string `json:"package_manager"` PMVersion string `json:"package_manager_version"` diff --git a/internal/telemetry/delta.go b/internal/telemetry/delta.go index 32f43ca8..56301f96 100644 --- a/internal/telemetry/delta.go +++ b/internal/telemetry/delta.go @@ -181,11 +181,9 @@ func globalRecordsFromPython(results []model.PythonScanResult) []state.GlobalRec continue } hash, _ := state.CanonicalHashJSON(decodeBase64OrRaw(r.RawStdoutBase64)) - exitCode := r.ExitCode - if r.Partial { - exitCode = 1 - } - out = append(out, state.GlobalRecord{PM: r.PackageManager, Hash: hash, ExitCode: exitCode}) + // Cache the body actually uploaded, including readable partial results. + // Global collection runs every scan, so recovery will produce a new hash. + out = append(out, state.GlobalRecord{PM: r.PackageManager, Hash: hash, ExitCode: r.ExitCode}) } return out } diff --git a/internal/telemetry/delta_globals_test.go b/internal/telemetry/delta_globals_test.go index 92a2a60a..c0e57188 100644 --- a/internal/telemetry/delta_globals_test.go +++ b/internal/telemetry/delta_globals_test.go @@ -1,6 +1,9 @@ package telemetry import ( + "encoding/base64" + "encoding/json" + "strings" "testing" "time" @@ -104,3 +107,57 @@ func TestSplitNodeGlobals_OneUnchangedRefPerPM(t *testing.T) { t.Errorf("want no unchanged refs, got %d", len(unchanged)) } } + +func TestPythonGlobals_PartialUploadRecovery(t *testing.T) { + complete := []model.PythonScanResult{{PackageManager: "pip", RawStdoutBase64: base64.StdEncoding.EncodeToString([]byte(`[{"name":"widgets","version":"1"},{"name":"gizmo","version":"1"}]`))}} + partial := []model.PythonScanResult{{PackageManager: "pip", Partial: true, RawStdoutBase64: base64.StdEncoding.EncodeToString([]byte(`[{"name":"widgets","version":"1"}]`))}} + now := time.Now() + saved := state.New("test") + saved.CommitAfterUpload(now, "exec-complete", "test", nil, nil, nil, globalRecordsFromPython(complete), true) + snap := buildDeltaSnapshot(saved, false, false, true, nil, nil, nil, nil, nil, partial) + if len(snap.pyGlobalsChanged) != 1 || len(snap.pyGlobalsUnchanged) != 0 { + t.Fatalf("partial package body not sent: %+v", snap) + } + wire, err := json.Marshal(snap.pyGlobalsChanged) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(wire), "partial") || snap.pyGlobalsChanged[0].ExitCode != 0 { + t.Fatalf("unexpected partial wire result: %s", wire) + } + // Without a successful upload, the existing baseline still owns the inventory. + if saved.PythonGlobal["pip"].LastUploadedExecutionID != "exec-complete" { + t.Fatal("building a snapshot advanced the upload baseline") + } + saved.CommitAfterUpload(now.Add(time.Minute), "exec-partial", "test", snap.npmRecords, snap.pyRecords, snap.npmGlobalRecords, snap.pyGlobalRecords, false) + if got := saved.PythonGlobal["pip"].LastUploadedExecutionID; got != "exec-partial" { + t.Errorf("partial upload predecessor = %q, want exec-partial", got) + } + repeated := buildDeltaSnapshot(saved, false, false, true, nil, nil, nil, nil, nil, partial) + if len(repeated.pyGlobalsChanged) != 0 || len(repeated.pyGlobalsUnchanged) != 1 || repeated.pyGlobalsUnchanged[0].LastUploadedExecutionID != "exec-partial" { + t.Errorf("identical partial scan must reference its uploaded body: %+v", repeated) + } + recovered := buildDeltaSnapshot(saved, false, false, true, nil, nil, nil, nil, nil, complete) + if len(recovered.pyGlobalsChanged) != 1 || len(recovered.pyGlobalsUnchanged) != 0 { + t.Fatalf("readable recovery must upload A+B, not the old reference: %+v", recovered) + } + saved.CommitAfterUpload(now.Add(2*time.Minute), "exec-recovered", "test", recovered.npmRecords, recovered.pyRecords, recovered.npmGlobalRecords, recovered.pyGlobalRecords, false) + unchanged := buildDeltaSnapshot(saved, false, false, true, nil, nil, nil, nil, nil, complete) + if len(unchanged.pyGlobalsChanged) != 0 || len(unchanged.pyGlobalsUnchanged) != 1 || unchanged.pyGlobalsUnchanged[0].LastUploadedExecutionID != "exec-recovered" { + t.Fatalf("recovered baseline not reusable: %+v", unchanged) + } + // Actual failed scans still send an error and leave the successful cache intact. + failed := []model.PythonScanResult{{PackageManager: "pip", Partial: true, ExitCode: 1, Error: "protected roots"}} + failure := buildDeltaSnapshot(saved, false, false, true, nil, nil, nil, nil, nil, failed) + if len(failure.pyGlobalsChanged) != 1 || len(failure.pyGlobalsUnchanged) != 0 { + t.Fatalf("failure not reported: %+v", failure) + } + saved.CommitAfterUpload(now.Add(3*time.Minute), "exec-failed", "test", nil, nil, nil, failure.pyGlobalRecords, false) + if saved.PythonGlobal["pip"].LastUploadedExecutionID != "exec-recovered" { + t.Fatal("failed scan replaced the successful baseline") + } + full := buildDeltaSnapshot(saved, true, false, true, nil, nil, nil, nil, nil, complete) + if len(full.pyGlobalsChanged) != 1 || len(full.pyGlobalsUnchanged) != 0 { + t.Fatal("full sync must send the package body") + } +} From b2a02fd42945ff61ba692e444d8b6784893d033e Mon Sep 17 00:00:00 2001 From: Subham Ray Date: Thu, 1 Oct 2026 18:54:08 +0530 Subject: [PATCH 5/5] fix(tcc): resend Node globals after refused scans --- internal/telemetry/delta.go | 7 ++++ internal/telemetry/delta_globals_test.go | 42 ++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/internal/telemetry/delta.go b/internal/telemetry/delta.go index 56301f96..bbc28cb1 100644 --- a/internal/telemetry/delta.go +++ b/internal/telemetry/delta.go @@ -336,6 +336,13 @@ func removedRefsFor(s *state.State, ecosystem string, discovered []string) []mod // reported (drops them from pending_ack and from the main maps), and // atomically saves the file. func commitDeltaSnapshot(s *state.State, snap *deltaSnapshot, path, executionID, agentVersion string) error { + // Failed Node globals can still upload readable roots or an unknown source. + // The previous manager-wide ref no longer certifies those source executions. + for _, r := range snap.npmGlobalRecords { + if r.ExitCode != 0 { + delete(s.NPMGlobal, r.PM) + } + } now := time.Now() s.CommitAfterUpload(now, executionID, agentVersion, snap.npmRecords, snap.pyRecords, diff --git a/internal/telemetry/delta_globals_test.go b/internal/telemetry/delta_globals_test.go index c0e57188..ed186914 100644 --- a/internal/telemetry/delta_globals_test.go +++ b/internal/telemetry/delta_globals_test.go @@ -3,6 +3,7 @@ package telemetry import ( "encoding/base64" "encoding/json" + "path/filepath" "strings" "testing" "time" @@ -161,3 +162,44 @@ func TestPythonGlobals_PartialUploadRecovery(t *testing.T) { t.Fatal("full sync must send the package body") } } + +func TestNodeGlobals_FailedScanRecovery(t *testing.T) { + for _, mixed := range []bool{false, true} { + t.Run(map[bool]string{false: "all-refused", true: "mixed-roots"}[mixed], func(t *testing.T) { + complete := []model.NodeScanResult{ + npmGlobalRoot("/n/v20/lib/node_modules", model.NodePackage{Name: "widgets", Version: "1"}), + npmGlobalRoot("/n/v22/lib/node_modules", model.NodePackage{Name: "gizmo", Version: "1"}), + } + partial := []model.NodeScanResult{{PackageManager: "npm", ExitCode: 1, Error: "global package roots include protected paths"}} + if mixed { + partial = append(partial, complete[0]) + } + saved := state.New("test") + now := time.Now() + saved.CommitAfterUpload(now, "exec-complete", "test", nil, nil, globalRecordsFromNode(complete), nil, true) + snapshot := buildDeltaSnapshot(saved, false, true, false, nil, nil, nil, nil, partial, nil) + if len(snapshot.npmGlobalsChanged) != len(partial) || len(snapshot.npmGlobalsUnchanged) != 0 { + t.Fatalf("failed global bodies not sent: %+v", snapshot) + } + statePath := filepath.Join(t.TempDir(), "scan-state.json") + if err := commitDeltaSnapshot(saved, snapshot, statePath, "exec-partial", "test"); err != nil { + t.Fatal(err) + } + restored, err := state.Load(statePath, "test") + if err != nil { + t.Fatal(err) + } + recovered := buildDeltaSnapshot(restored, false, true, false, nil, nil, nil, nil, complete, nil) + if len(recovered.npmGlobalsChanged) != len(complete) || len(recovered.npmGlobalsUnchanged) != 0 { + t.Fatalf("recovery must upload each readable root after a failed global body, not reuse the old manager-wide reference: %+v", recovered) + } + if err := commitDeltaSnapshot(restored, recovered, statePath, "exec-recovered", "test"); err != nil { + t.Fatal(err) + } + unchanged := buildDeltaSnapshot(restored, false, true, false, nil, nil, nil, nil, complete, nil) + if len(unchanged.npmGlobalsChanged) != 0 || len(unchanged.npmGlobalsUnchanged) != 1 || unchanged.npmGlobalsUnchanged[0].LastUploadedExecutionID != "exec-recovered" { + t.Fatalf("complete recovered roots must become reusable: %+v", unchanged) + } + }) + } +}