diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b174520..fd8f1afa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 See [VERSIONING.md](VERSIONING.md) for why the version starts at 1.8.1. +## [Unreleased] + +### Fixed + +- Guard targeted scanner reads and symlink targets before accessing excluded protected directories. +- Preserve readable inventory and prior project references when protected locations cannot be scanned. + ## [1.17.0] - 2026-09-24 ### Added 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 63d59131..1ade0710 100644 --- a/internal/detector/aicli.go +++ b/internal/detector/aicli.go @@ -341,6 +341,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 } @@ -900,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/aicli_agents_test.go b/internal/detector/aicli_agents_test.go index 96e50ed1..6ca1413b 100644 --- a/internal/detector/aicli_agents_test.go +++ b/internal/detector/aicli_agents_test.go @@ -3294,3 +3294,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..ff82363a 100644 --- a/internal/detector/configaudit/bunfig.go +++ b/internal/detector/configaudit/bunfig.go @@ -71,6 +71,11 @@ 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) + } return d } @@ -144,7 +149,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 +172,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 +202,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 +214,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 +246,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() diff --git a/internal/detector/configaudit/files.go b/internal/detector/configaudit/files.go new file mode 100644 index 00000000..2cf21cee --- /dev/null +++ b/internal/detector/configaudit/files.go @@ -0,0 +1,56 @@ +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 + +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..1c57831a 100644 --- a/internal/detector/configaudit/npmrc.go +++ b/internal/detector/configaudit/npmrc.go @@ -82,6 +82,11 @@ 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) + } return d } @@ -170,7 +175,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 +233,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 +246,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 +280,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() 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..cabfb416 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. @@ -574,7 +576,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 +592,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 +617,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() @@ -747,7 +749,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 +771,13 @@ 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) + } + return d +} diff --git a/internal/detector/configaudit/pnpm.go b/internal/detector/configaudit/pnpm.go index f7b0fd88..ddc2d611 100644 --- a/internal/detector/configaudit/pnpm.go +++ b/internal/detector/configaudit/pnpm.go @@ -63,6 +63,11 @@ 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) + } return d } @@ -131,7 +136,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 +166,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 +178,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 +210,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() 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..efc8790c --- /dev/null +++ b/internal/detector/configaudit/protected_reads_darwin_test.go @@ -0,0 +1,78 @@ +//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 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/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 36ddd118..4e9db69b 100644 --- a/internal/detector/configaudit/yarn.go +++ b/internal/detector/configaudit/yarn.go @@ -77,6 +77,11 @@ 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) + } return d } @@ -145,7 +150,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 +173,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 +203,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 +215,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 +246,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() 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 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 a311d8c4..4a943469 100644 --- a/internal/detector/mcp.go +++ b/internal/detector/mcp.go @@ -65,6 +65,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 fbba9b0b..e5996833 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() { @@ -165,7 +165,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 { if codexConfig && source == "discovered_mcp" { source, vendor = "codex", "OpenAI" } diff --git a/internal/detector/mcp_discovery_test.go b/internal/detector/mcp_discovery_test.go index 83932ae5..757c35f6 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] { @@ -120,7 +121,7 @@ func TestDiscoverWalkedMCPConfigs_Codex(t *testing.T) { want := writeFile(t, root, "widgets/.codex/config.toml") writeFile(t, root, "widgets/config.toml") writeFile(t, root, "widgets/node_modules/tool/.codex/config.toml") - d := &MCPDetector{} + d := NewMCPDetector(executor.NewReal()) specs := d.discoverWalkedMCPConfigs([]string{root}, "") if len(specs) != 1 || specs[0].ConfigPath != want || specs[0].SourceName != "codex" || specs[0].Vendor != "OpenAI" { t.Fatalf("configs = %+v", specs) @@ -134,7 +135,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) @@ -168,7 +169,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 0ae0613f..c381d6b5 100644 --- a/internal/detector/mcp_test.go +++ b/internal/detector/mcp_test.go @@ -233,7 +233,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(`{ @@ -306,7 +306,7 @@ func TestExtractMCPServers_ClaudeCodeProjectScoped(t *testing.T) { } func TestExtractMCPServers_VSCodeFormat(t *testing.T) { - det := &MCPDetector{} + det := NewMCPDetector(executor.NewReal()) content := []byte(`{ "servers": { @@ -532,7 +532,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": { @@ -627,7 +627,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 @@ -664,7 +664,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) @@ -702,7 +702,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 { @@ -717,7 +717,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 { @@ -739,7 +739,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") @@ -758,7 +758,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) @@ -853,7 +853,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)) @@ -874,7 +874,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/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/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.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_global.go b/internal/detector/nodedist_global.go index 597d58ce..dd0c87b0 100644 --- a/internal/detector/nodedist_global.go +++ b/internal/detector/nodedist_global.go @@ -5,6 +5,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" ) // nodeGlobalRoot is a global node_modules directory paired with the package @@ -25,14 +26,28 @@ type nodeGlobalRoot struct { // global-roots scan. Where a manager (nvm/fnm/volta) keeps per-version trees, // every installed version's global dir is included. func NodeGlobalRoots(exec executor.Executor) []nodeGlobalRoot { + roots, _ := nodeGlobalRoots(exec) + return roots +} + +func nodeGlobalRoots(exec executor.Executor) ([]nodeGlobalRoot, map[string]bool) { var roots []nodeGlobalRoot + refused := make(map[string]bool) // The candidate lists overlap: npm_config_prefix=/usr/local resolves to the // same directory as the built-in /usr/local entry, and PREFIX can repeat // either. Callers emit one scan result per root, so a duplicate would scan // and upload the same directory twice. seen := make(map[string]struct{}) add := func(pm, dir string) { - if dir == "" || !exec.DirExists(dir) { + if dir == "" { + return + } + before := tcc.Refusals(exec) + exists := exec.DirExists(dir) + if tcc.Refusals(exec) != before { + refused[pm] = true + } + if !exists { return } dir = filepath.Clean(dir) @@ -44,10 +59,13 @@ func NodeGlobalRoots(exec executor.Executor) []nodeGlobalRoot { roots = append(roots, nodeGlobalRoot{pm: pm, dir: dir}) } addGlob := func(pm, pattern string) { - if matches, err := exec.Glob(pattern); err == nil { - for _, m := range matches { - add(pm, m) - } + before := tcc.Refusals(exec) + matches, _ := exec.Glob(pattern) + if tcc.Refusals(exec) != before { + refused[pm] = true + } + for _, m := range matches { + add(pm, m) } } home := nodeHomeDir(exec) @@ -105,7 +123,7 @@ func NodeGlobalRoots(exec executor.Executor) []nodeGlobalRoot { } } - return roots + return roots, refused } // pnpmGlobalHomes returns candidate pnpm home directories (PNPM_HOME plus the 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..bf441bc6 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" ) @@ -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..197fc209 100644 --- a/internal/detector/nodepm_fallback.go +++ b/internal/detector/nodepm_fallback.go @@ -98,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))) 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..9c8f078c 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 } @@ -433,14 +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 + var unobserved []string 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 { + if !os.IsNotExist(err) { + unobserved = append(unobserved, path) + } return nil } if entry.IsDir() { if s.skipper.ShouldSkip(path, dir) { + unobserved = append(unobserved, path) return filepath.SkipDir } name := entry.Name() @@ -473,6 +479,7 @@ func (s *NodeScanner) ScanProjects(ctx context.Context, searchDirs []string, kno discovered = append(discovered, p.dir) } + discovered = retainUnobservedProjects(discovered, knownLastVerified, unobserved) projects = orderScanProjects(projects, knownLastVerified) if len(projects) > maxNodeProjects { @@ -767,7 +774,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,21 +795,26 @@ 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 { - roots := NodeGlobalRoots(s.exec) - if len(roots) == 0 { - s.log.Debug("node global disk scan: no global node_modules roots found") - return nil + roots, refused := nodeGlobalRoots(s.exec) + results := make([]model.NodeScanResult, 0, len(roots)+len(refused)) + for _, pm := range []string{"npm", "pnpm", "yarn", "bun"} { + if refused[pm] { + results = append(results, model.NodeScanResult{PackageManager: pm, ExitCode: 1, Error: "global package roots include protected paths"}) + } } - results := make([]model.NodeScanResult, 0, len(roots)) 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/project_discovery.go b/internal/detector/project_discovery.go new file mode 100644 index 00000000..0cecb7b0 --- /dev/null +++ b/internal/detector/project_discovery.go @@ -0,0 +1,32 @@ +package detector + +import ( + "path/filepath" + "slices" + "strings" + "time" +) + +// Keep prior references under unreadable subtrees without claiming a fresh scan. +// Readable siblings remain eligible for real removal reconciliation. +func retainUnobservedProjects(discovered []string, known map[string]time.Time, unobserved []string) []string { + seen := make(map[string]bool, len(discovered)) + for _, path := range discovered { + seen[path] = true + } + var retained []string + for path := range known { + if seen[path] { + continue + } + for _, root := range unobserved { + rel, err := filepath.Rel(root, path) + if err == nil && rel != ".." && !strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + retained = append(retained, path) + break + } + } + } + slices.Sort(retained) + return append(discovered, retained...) +} diff --git a/internal/detector/protected_reads_darwin_test.go b/internal/detector/protected_reads_darwin_test.go new file mode 100644 index 00000000..5a736bd7 --- /dev/null +++ b/internal/detector/protected_reads_darwin_test.go @@ -0,0 +1,291 @@ +//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) + } + } +} + +func TestMCPNestedReaderRefusalIsSkipped(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 { + t.Fatalf("nested reader returned a refused configuration: %v", got) + } +} diff --git a/internal/detector/pythondist.go b/internal/detector/pythondist.go index 1270ba3f..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 @@ -52,6 +53,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 +71,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,15 +84,14 @@ 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 { - roots = append(roots, matches...) - } + matches, _ := exec.Glob(pattern) + roots = append(roots, matches...) } if len(roots) == 0 { return []string{venvPath} @@ -101,6 +107,15 @@ func venvSitePackages(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 } @@ -109,7 +124,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 +139,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 @@ -138,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 @@ -153,7 +170,11 @@ 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)) + roots := GlobalPythonRoots(d.exec, d.log) + 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 +184,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,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 := filepath.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: @@ -331,16 +351,12 @@ 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() -} +// 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/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/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 79ee1b84..5824d212 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" ) @@ -144,3 +145,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..5c00bce6 100644 --- a/internal/detector/pythonproject.go +++ b/internal/detector/pythonproject.go @@ -19,9 +19,10 @@ const maxPythonProjects = 1000 // PythonProjectDetector scans for Python projects with virtual environments. type PythonProjectDetector struct { - exec executor.Executor - log *progress.Logger - skipper *tcc.Skipper + exec executor.Executor + log *progress.Logger + skipper *tcc.Skipper + unobserved []string // dist, when non-nil, makes per-venv package listing read install // metadata from disk instead of running `pip list`. dist *PythonDistDetector @@ -35,6 +36,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 } @@ -87,6 +89,7 @@ type venvCandidate struct { // so callers can distinguish "missing from disk" from "dropped by the cap" // when comparing against prior state. func (d *PythonProjectDetector) ListProjects(searchDirs []string, knownLastVerified map[string]time.Time) (projects []model.ProjectInfo, discovered []string) { + d.unobserved = nil var candidates []venvCandidate for _, dir := range searchDirs { d.log.Progress(" Searching in: %s", dir) @@ -100,6 +103,7 @@ func (d *PythonProjectDetector) ListProjects(searchDirs []string, knownLastVerif discovered = append(discovered, c.path) } + discovered = retainUnobservedProjects(discovered, knownLastVerified, d.unobserved) candidates = orderVenvs(candidates, knownLastVerified) if len(candidates) > maxPythonProjects { @@ -246,14 +250,18 @@ 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 { + if !os.IsNotExist(err) { + d.unobserved = append(d.unobserved, path) + } return nil } if !entry.IsDir() { return nil } if d.skipper.ShouldSkip(path, dir) { + d.unobserved = append(d.unobserved, path) return filepath.SkipDir } name := entry.Name() @@ -263,7 +271,11 @@ func (d *PythonProjectDetector) discoverInDir(dir string) []venvCandidate { return filepath.SkipDir } + before := tcc.Refusals(d.exec) pipPath, isVenv := d.isVenvDir(path) + if tcc.Refusals(d.exec) != before { + d.unobserved = append(d.unobserved, path) + } if !isVenv { return nil } diff --git a/internal/detector/pythonscan.go b/internal/detector/pythonscan.go index 19caf19f..b155e851 100644 --- a/internal/detector/pythonscan.go +++ b/internal/detector/pythonscan.go @@ -100,7 +100,12 @@ 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) + 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 { s.log.Debug("python global disk scan: no site-packages roots found") return nil @@ -111,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{{ @@ -135,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/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 e932eaf1..54687c2d 100644 --- a/internal/detector/skills.go +++ b/internal/detector/skills.go @@ -100,6 +100,7 @@ func (d *SkillsDetector) WithAgentVersions(v map[string]string) *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/detector/tcc_regression_darwin_test.go b/internal/detector/tcc_regression_darwin_test.go new file mode 100644 index 00000000..0b31bdfa --- /dev/null +++ b/internal/detector/tcc_regression_darwin_test.go @@ -0,0 +1,173 @@ +//go:build darwin + +package detector + +import ( + "context" + "os" + "path/filepath" + "slices" + "strings" + "testing" + "time" + + "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/state" + "github.com/step-security/dev-machine-guard/internal/tcc" +) + +type isolatedGlobalExecutor struct{ protectedFixtureExecutor } + +func (e isolatedGlobalExecutor) Getenv(key string) string { + if key == "PNPM_HOME" { + return filepath.Join(e.home, "Documents", "pnpm") + } + return e.protectedFixtureExecutor.Getenv(key) +} +func (e isolatedGlobalExecutor) DirExists(path string) bool { + return strings.HasPrefix(path, e.home+string(filepath.Separator)) && e.Executor.DirExists(path) +} +func (e isolatedGlobalExecutor) Glob(pattern string) ([]string, error) { + if !strings.HasPrefix(pattern, e.home+string(filepath.Separator)) { + return nil, nil + } + return e.Executor.Glob(pattern) +} +func (e isolatedGlobalExecutor) GuardedFiles(roots []string, guard func(string) string, max int64) executor.Executor { + e.Executor = e.Executor.GuardedFiles(roots, guard, max) + return e +} + +func TestProtectedPnpmKeepsSafeGlobalPackages(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + for _, root := range []string{filepath.Join(home, ".npm-global", "lib", "node_modules"), filepath.Join(home, ".config", "yarn", "global", "node_modules")} { + path := filepath.Join(root, "widgets", "package.json") + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(`{"name":"widgets","version":"1.0.0"}`), 0600); err != nil { + t.Fatal(err) + } + } + exec := isolatedGlobalExecutor{protectedFixtureExecutor{Executor: executor.NewReal(), home: home}} + skipper := tcc.New(home) + scanner := NewNodeScanner(exec, progress.NewNoop(), "").WithSkipper(skipper).WithDiskScan(NewNodeDistDetector(exec).WithSkipper(skipper)) + got := scanner.scanGlobalPackagesFromDisk() + for _, pm := range []string{"npm", "yarn"} { + found := false + for _, result := range got { + if result.PackageManager == pm && result.ExitCode == 0 && len(result.Packages) == 1 && result.Packages[0].Name == "widgets" { + found = true + } + } + if !found { + t.Errorf("missing safe %s inventory: %+v", pm, got) + } + } + refused := false + for _, result := range got { + if result.PackageManager == "pnpm" && result.ExitCode != 0 { + refused = true + } + } + if !refused { + t.Error("missing pnpm incomplete result") + } +} + +func TestProtectedDiscoveryRetainsKnownProjects(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) + for _, family := range []string{"node", "python"} { + t.Run(family, func(t *testing.T) { + project := filepath.Join(home, "Documents", family) + if family == "python" { + project = filepath.Join(project, ".venv") + } + if err := os.MkdirAll(project, 0700); err != nil { + t.Fatal(err) + } + known := map[string]time.Time{project: time.Unix(123, 0)} + var discovered []string + if family == "node" { + _, discovered = NewNodeScanner(exec, progress.NewNoop(), "").WithSkipper(skipper).WithDiskScan(NewNodeDistDetector(exec).WithSkipper(skipper)).ScanProjects(context.Background(), []string{filepath.Join(home, "Documents")}, known) + } else { + _, discovered = NewPythonProjectDetector(exec).WithSkipper(skipper).WithDiskScan(NewPythonDistDetector(exec).WithSkipper(skipper)).ListProjects([]string{filepath.Join(home, "Documents")}, known) + } + if !slices.Contains(discovered, project) { + t.Errorf("unobserved project lost from reconciliation: %v", discovered) + } + }) + } +} + +func TestProtectedDiscoveryReconcilesReadableRemoval(t *testing.T) { + for _, family := range []string{"node", "python"} { + t.Run(family, func(t *testing.T) { + home, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + safe := filepath.Join(home, "work", "safe") + protected := filepath.Join(home, "Documents", "protected") + removed := filepath.Join(home, "work", "removed") + if family == "python" { + safe = filepath.Join(safe, ".venv") + protected = filepath.Join(protected, ".venv") + removed = filepath.Join(removed, ".venv") + } + for _, path := range []string{safe, protected} { + if family == "node" { + mustWrite(t, filepath.Join(path, "package.json"), `{"name":"widgets"}`) + mustWrite(t, filepath.Join(path, "node_modules", "widgets", "package.json"), `{"name":"widgets","version":"1.0.0"}`) + } else { + mustWrite(t, filepath.Join(path, "pyvenv.cfg"), "home = /example/python") + mustWrite(t, filepath.Join(path, "lib", "python3.12", "site-packages", "widgets-1.0.0.dist-info", "METADATA"), "Name: widgets\nVersion: 1.0.0\n") + } + } + exec := protectedFixtureExecutor{Executor: executor.NewReal(), home: home} + known := map[string]time.Time{safe: time.Unix(123, 0), protected: time.Unix(123, 0), removed: time.Unix(123, 0)} + scan := func(skipper *tcc.Skipper) []string { + if family == "node" { + _, found := NewNodeScanner(exec, progress.NewNoop(), "").WithSkipper(skipper).WithDiskScan(NewNodeDistDetector(exec).WithSkipper(skipper)).ScanProjects(context.Background(), []string{home}, known) + return found + } + _, found := NewPythonProjectDetector(exec).WithSkipper(skipper).WithDiskScan(NewPythonDistDetector(exec).WithSkipper(skipper)).ListProjects([]string{home}, known) + return found + } + ecosystem := state.EcosystemNPM + saved := state.New("test") + entries := saved.NPMProjects + if family == "python" { + ecosystem = state.EcosystemPython + entries = saved.PythonProjects + } + for path := range known { + entries[path] = state.ProjectEntry{LastVerifiedAt: known[path]} + } + _, _, deletions := saved.Reconcile(ecosystem, scan(tcc.New(home))) + if len(deletions) != 1 || deletions[0] != removed { + t.Fatalf("deletions=%v, want only readable missing project %s", deletions, removed) + } + recovered := scan(nil) + if !slices.Contains(recovered, safe) || !slices.Contains(recovered, protected) { + t.Fatalf("recovery lost projects: %v", recovered) + } + if err := os.RemoveAll(protected); err != nil { + t.Fatal(err) + } + _, _, deletions = saved.Reconcile(ecosystem, scan(nil)) + if !slices.Contains(deletions, protected) { + t.Fatalf("real uninstall not removed: %v", deletions) + } + }) + } +} 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/executor/executor.go b/internal/executor/executor.go index c0b85efa..7b788644 100644 --- a/internal/executor/executor.go +++ b/internal/executor/executor.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "io" + "io/fs" "os" "os/exec" "os/user" @@ -23,7 +24,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. @@ -48,6 +49,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. @@ -68,6 +71,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 @@ -114,23 +119,64 @@ 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) { + path, err := guardedAbsolutePath(path) + if err != nil { + return "", err + } + if _, err := g.resolver.Resolve(path); err != nil { + return "", err + } + return g.Executor.Readlink(path) +} + func (g *guardedFiles) EvalSymlinks(path string) (string, error) { + path, err := guardedAbsolutePath(path) + if err != nil { + return "", err + } return g.resolver.Resolve(path) } func (g *guardedFiles) Stat(path string) (os.FileInfo, error) { + path, err := guardedAbsolutePath(path) + if err != nil { + return nil, err + } return g.resolver.Stat(path) } func (g *guardedFiles) ReadFile(path string) ([]byte, error) { + path, err := guardedAbsolutePath(path) + if err != nil { + return nil, err + } return g.resolver.ReadFile(path, g.maxReadBytes) } func (g *guardedFiles) ReadDir(path string) ([]os.DirEntry, error) { + path, err := guardedAbsolutePath(path) + if err != nil { + return nil, err + } return g.resolver.ReadDir(path) } func (g *guardedFiles) ReadDirLimit(path string, max int) ([]os.DirEntry, bool, error) { + path, err := guardedAbsolutePath(path) + if err != nil { + return nil, false, err + } return g.resolver.ReadDirLimit(path, max) } @@ -406,3 +452,17 @@ 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) { + path, err := guardedAbsolutePath(path) + if err != nil { + return nil, err + } + 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..e8c88785 --- /dev/null +++ b/internal/executor/guarded_files_test.go @@ -0,0 +1,116 @@ +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) + } +} + +func TestGuardedFilesRelativeWalkRead(t *testing.T) { + dir, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + t.Chdir(dir) + if err := os.Mkdir("projects", 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile("projects/package.json", []byte("ordinary"), 0600); err != nil { + t.Fatal(err) + } + root := filepath.VolumeName(dir) + string(filepath.Separator) + guarded := NewReal().GuardedFiles([]string{root}, nil, 1024) + for _, search := range []string{".", "projects", filepath.Join(dir, "projects")} { + t.Run(search, func(t *testing.T) { + if entries, more, err := guarded.ReadDirLimit(search, 10); err != nil || more || len(entries) != 1 { + t.Fatalf("ReadDirLimit(%q): count=%d more=%v err=%v", search, len(entries), more, err) + } + count := 0 + err := guarded.WalkDir(search, func(path string, entry os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if entry.Name() == "package.json" { + data, err := guarded.ReadFile(path) + if err != nil { + t.Errorf("ReadFile(%q): %v", path, err) + } + if string(data) == "ordinary" { + count++ + } + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if count != 1 { + t.Errorf("read %d manifests, want 1", count) + } + }) + } +} diff --git a/internal/executor/guarded_glob.go b/internal/executor/guarded_glob.go new file mode 100644 index 00000000..5be5242a --- /dev/null +++ b/internal/executor/guarded_glob.go @@ -0,0 +1,94 @@ +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) + }) +} + +// Preserve dot-dot until the reader has checked preceding symlink targets. +func guardedAbsolutePath(path string) (string, error) { + if path == "" { + return "", fs.ErrInvalid + } + if filepath.IsAbs(path) { + return path, nil + } + cwd, err := os.Getwd() + if err != nil { + return "", err + } + if filepath.VolumeName(path) != "" { + return filepath.Abs(path) + } + return cwd + string(filepath.Separator) + path, nil +} diff --git a/internal/executor/mock.go b/internal/executor/mock.go index 7a3cf998..69c1466c 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" @@ -518,3 +519,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 fea827e7..b9900c4f 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" @@ -259,3 +260,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/model/model.go b/internal/model/model.go index 31fd43d4..72c75212 100644 --- a/internal/model/model.go +++ b/internal/model/model.go @@ -375,6 +375,8 @@ type BrewScanResult struct { // PythonScanResult holds raw Python scan output for enterprise telemetry. type PythonScanResult struct { + // Partial marks limited disk collection without changing the wire result. + 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 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 543e6889..171fcaae 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 { @@ -237,3 +244,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 47ade9a0..d4470a83 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,7 +140,7 @@ 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)) @@ -230,7 +232,7 @@ 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)) @@ -317,7 +319,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..806ece12 --- /dev/null +++ b/internal/tcc/reader.go @@ -0,0 +1,86 @@ +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 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) +} + +// 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 identifies an executor with protected direct reads. +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/delta.go b/internal/telemetry/delta.go index 5de1a963..bbc28cb1 100644 --- a/internal/telemetry/delta.go +++ b/internal/telemetry/delta.go @@ -181,6 +181,8 @@ func globalRecordsFromPython(results []model.PythonScanResult) []state.GlobalRec continue } hash, _ := state.CanonicalHashJSON(decodeBase64OrRaw(r.RawStdoutBase64)) + // 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 @@ -334,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 92a2a60a..ed186914 100644 --- a/internal/telemetry/delta_globals_test.go +++ b/internal/telemetry/delta_globals_test.go @@ -1,6 +1,10 @@ package telemetry import ( + "encoding/base64" + "encoding/json" + "path/filepath" + "strings" "testing" "time" @@ -104,3 +108,98 @@ 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") + } +} + +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) + } + }) + } +} diff --git a/internal/telemetry/telemetry.go b/internal/telemetry/telemetry.go index 404fcc15..49862929 100644 --- a/internal/telemetry/telemetry.go +++ b/internal/telemetry/telemetry.go @@ -612,7 +612,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) @@ -627,11 +627,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...) @@ -663,7 +663,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) } @@ -673,7 +673,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 { @@ -807,7 +807,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) @@ -836,7 +836,8 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err detector.NewPythonDistDetector(exec).WithSkipper(tccSkipper).WithLogger(log)) } var knownPython map[string]time.Time - if scanState != nil && !scanStateFullSync { + // Full sync still needs prior paths to retain unobserved protected projects. + if scanState != nil { knownPython = make(map[string]time.Time, len(scanState.PythonProjects)) for path, entry := range scanState.PythonProjects { knownPython[path] = entry.LastVerifiedAt @@ -933,7 +934,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) @@ -967,7 +968,8 @@ func Run(exec executor.Executor, log *progress.Logger, cfg *cli.Config) (err err log.Progress("Searching for Node.js projects...") scanStart := time.Now() var knownNPM map[string]time.Time - if scanState != nil && !scanStateFullSync { + // Full sync still needs prior paths to retain unobserved protected projects. + if scanState != nil { knownNPM = make(map[string]time.Time, len(scanState.NPMProjects)) for path, entry := range scanState.NPMProjects { knownNPM[path] = entry.LastVerifiedAt @@ -1095,7 +1097,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) + } +}