Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions internal/detector/configaudit/bunfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,14 @@ func (d *BunDetector) collectFile(ctx context.Context, path, scope string) model
if target, err := os.Readlink(path); err == nil {
f.SymlinkTo = target
}
// See npmrc.go's collectFile: project-scope files are found by
// walking arbitrary (possibly attacker-controlled) directories, so a
// symlink there is not followed.
if scope == "project" {
f.Readable = false
f.ParseError = "refused: project-scoped symlink not followed"
return f
}
}

info, err := os.Stat(path)
Expand Down
42 changes: 42 additions & 0 deletions internal/detector/configaudit/bunfig_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package configaudit

import (
"context"
"os"
"os/user"
"path/filepath"
"strings"
Expand Down Expand Up @@ -249,3 +250,44 @@ registry = "https://registry.npmjs.org/"
}
}
}

// TestBunDetector_ProjectSymlinkNotFollowed: see npmrc_test.go's identical
// test — bunfig.toml discovery shares the same collectFile pattern and the
// same project-scope symlink-following bug and fix.
func TestBunDetector_ProjectSymlinkNotFollowed(t *testing.T) {
tmp := t.TempDir()

secretPath := filepath.Join(tmp, "outside", "secret-credential")
mustWriteFile(t, secretPath, "EXAMPLE_SENSITIVE_DATA\n")

projectPath := filepath.Join(tmp, "repo", "bunfig.toml")
if err := os.MkdirAll(filepath.Dir(projectPath), 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.Symlink(secretPath, projectPath); err != nil {
t.Fatalf("symlink: %v", err)
}
Comment on lines +267 to +269

d := NewBunDetector(executor.NewMock())
d.ownerLookup = fixedOwner()
d.gitTracked = func(_ context.Context, _ string) bool { return false }
d.inGitRepo = func(_ string) bool { return false }

f := d.collectFile(context.Background(), projectPath, "project")

if !f.Exists {
t.Fatalf("expected the symlink itself to be recorded as existing")
}
if f.SymlinkTo != secretPath {
t.Errorf("SymlinkTo = %q, want %q", f.SymlinkTo, secretPath)
}
if f.Readable {
t.Errorf("project-scoped symlink should not be marked readable")
}
if len(f.Sections) != 0 {
t.Errorf("target file contents must not be parsed, got sections: %+v", f.Sections)
}
if f.SHA256 != "" {
t.Errorf("target file must not be hashed, got %q", f.SHA256)
}
}
12 changes: 12 additions & 0 deletions internal/detector/configaudit/npmrc.go
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,18 @@ func (d *NPMRCDetector) collectFile(ctx context.Context, path, scope string) mod
if target, err := os.Readlink(path); err == nil {
f.SymlinkTo = target
}
// Project-scope files are discovered by walking arbitrary directories,
// including cloned git repos. A malicious repo can commit a .npmrc
// symlink pointing anywhere on disk; following it would read and
// upload whatever it points to. Record the link but refuse to read
// through it. user/global/builtin paths are resolved by the detector
// itself (npm's own config resolution, or the user's home), so a
// symlink there is the operator's own dotfile setup, not attacker input.
if scope == "project" {
f.Readable = false
f.ParseError = "refused: project-scoped symlink not followed"
return f
}
}

// Stat (follows symlinks) for size/mtime/mode.
Expand Down
65 changes: 65 additions & 0 deletions internal/detector/configaudit/npmrc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,71 @@ func TestNPMRCDetector_RespectsEnvOverridesForUserAndGlobal(t *testing.T) {
}
}

// TestNPMRCDetector_ProjectSymlinkNotFollowed: a malicious repo can commit a
// .npmrc that is a symlink to a predictable file elsewhere on the developer's
// machine (e.g. a credential file). Project-scope discovery walks arbitrary,
// possibly attacker-controlled directories, so that symlink must not be
// followed — the link is recorded as metadata but its target is never read,
// hashed, or parsed into telemetry.
func TestNPMRCDetector_ProjectSymlinkNotFollowed(t *testing.T) {
tmp := t.TempDir()

secretPath := filepath.Join(tmp, "outside", "secret-credential")
mustWriteFile(t, secretPath, "EXAMPLE_SENSITIVE_DATA\n")

projectPath := filepath.Join(tmp, "repo", ".npmrc")
if err := os.MkdirAll(filepath.Dir(projectPath), 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.Symlink(secretPath, projectPath); err != nil {
t.Fatalf("symlink: %v", err)
}
Comment on lines +363 to +365

d := NewNPMRCDetector(executor.NewMock())
d.ownerLookup = fixedOwner()
d.gitTracked = func(_ context.Context, _ string) bool { return false }
d.inGitRepo = func(_ string) bool { return false }

f := d.collectFile(context.Background(), projectPath, "project")

if !f.Exists {
t.Fatalf("expected the symlink itself to be recorded as existing")
}
if f.SymlinkTo != secretPath {
t.Errorf("SymlinkTo = %q, want %q", f.SymlinkTo, secretPath)
}
if f.Readable {
t.Errorf("project-scoped symlink should not be marked readable")
}
if len(f.Entries) != 0 {
t.Errorf("target file contents must not be parsed, got entries: %+v", f.Entries)
}
if f.SHA256 != "" {
t.Errorf("target file must not be hashed, got %q", f.SHA256)
}

// A user-scope symlink (e.g. dotfiles managed via a symlink) is a setup
// the machine's own user controls, not attacker input — only project-scope
// (untrusted tree walk) discovery refuses to follow the link.
userPath := filepath.Join(tmp, "home", ".npmrc")
realConfig := filepath.Join(tmp, "dotfiles", ".npmrc")
mustWriteFile(t, realConfig, "registry=https://registry.npmjs.org/\n")
if err := os.MkdirAll(filepath.Dir(userPath), 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.Symlink(realConfig, userPath); err != nil {
t.Fatalf("symlink: %v", err)
}

uf := d.collectFile(context.Background(), userPath, "user")
if !uf.Readable {
t.Errorf("user-scope symlink should still be followed, got ParseError=%q", uf.ParseError)
}
if len(uf.Entries) == 0 {
t.Errorf("expected user-scope symlink target to be parsed")
}
}

// mustWriteFile creates parent dirs as needed and writes the content.
func mustWriteFile(t *testing.T, path, content string) {
t.Helper()
Expand Down
8 changes: 8 additions & 0 deletions internal/detector/configaudit/pnpm.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,14 @@ func (d *PnpmDetector) collectFile(ctx context.Context, path, scope string) mode
if target, err := os.Readlink(path); err == nil {
f.SymlinkTo = target
}
// See npmrc.go's collectFile: project-scope files are found by
// walking arbitrary (possibly attacker-controlled) directories, so a
// symlink there is not followed.
if scope == "project" {
f.Readable = false
f.ParseError = "refused: project-scoped symlink not followed"
return f
}
}

info, err := os.Stat(path)
Expand Down
42 changes: 42 additions & 0 deletions internal/detector/configaudit/pnpm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package configaudit

import (
"context"
"os"
"os/user"
"path/filepath"
"strings"
Expand All @@ -10,6 +11,47 @@ import (
"github.com/step-security/dev-machine-guard/internal/executor"
)

// TestPnpmDetector_ProjectSymlinkNotFollowed: see npmrc_test.go's identical
// test — pnpm reuses the .npmrc walker/collector, so it shares the same
// project-scope symlink-following bug and fix.
func TestPnpmDetector_ProjectSymlinkNotFollowed(t *testing.T) {
tmp := t.TempDir()

secretPath := filepath.Join(tmp, "outside", "secret-credential")
mustWriteFile(t, secretPath, "EXAMPLE_SENSITIVE_DATA\n")

projectPath := filepath.Join(tmp, "repo", ".npmrc")
if err := os.MkdirAll(filepath.Dir(projectPath), 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.Symlink(secretPath, projectPath); err != nil {
t.Fatalf("symlink: %v", err)
}
Comment on lines +27 to +29

d := NewPnpmDetector(executor.NewMock())
d.ownerLookup = fixedOwner()
d.gitTracked = func(_ context.Context, _ string) bool { return false }
d.inGitRepo = func(_ string) bool { return false }

f := d.collectFile(context.Background(), projectPath, "project")

if !f.Exists {
t.Fatalf("expected the symlink itself to be recorded as existing")
}
if f.SymlinkTo != secretPath {
t.Errorf("SymlinkTo = %q, want %q", f.SymlinkTo, secretPath)
}
if f.Readable {
t.Errorf("project-scoped symlink should not be marked readable")
}
if len(f.Entries) != 0 {
t.Errorf("target file contents must not be parsed, got entries: %+v", f.Entries)
}
if f.SHA256 != "" {
t.Errorf("target file must not be hashed, got %q", f.SHA256)
}
}

func TestPnpmDetector_Discovery_AllScopes(t *testing.T) {
tmp := t.TempDir()

Expand Down
8 changes: 8 additions & 0 deletions internal/detector/configaudit/yarn.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,14 @@ func (d *YarnDetector) collectFile(ctx context.Context, path, scope, flavor stri
if target, err := os.Readlink(path); err == nil {
f.SymlinkTo = target
}
// See npmrc.go's collectFile: project-scope files are found by
// walking arbitrary (possibly attacker-controlled) directories, so a
// symlink there is not followed.
if scope == "project" {
f.Readable = false
f.ParseError = "refused: project-scoped symlink not followed"
return f
}
}

info, err := os.Stat(path)
Expand Down
42 changes: 42 additions & 0 deletions internal/detector/configaudit/yarn_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package configaudit

import (
"context"
"os"
"os/user"
"path/filepath"
"strings"
Expand Down Expand Up @@ -218,6 +219,47 @@ func TestYarnDetector_MalformedBerryYAML(t *testing.T) {
}
}

// TestYarnDetector_ProjectSymlinkNotFollowed: see npmrc_test.go's identical
// test — a malicious repo's .yarnrc(.yml) can be a symlink to a predictable
// file elsewhere on disk; project-scope discovery must not follow it.
func TestYarnDetector_ProjectSymlinkNotFollowed(t *testing.T) {
tmp := t.TempDir()

secretPath := filepath.Join(tmp, "outside", "secret-credential")
mustWriteFile(t, secretPath, "EXAMPLE_SENSITIVE_DATA\n")

projectPath := filepath.Join(tmp, "repo", ".yarnrc.yml")
if err := os.MkdirAll(filepath.Dir(projectPath), 0o755); err != nil {
t.Fatalf("mkdir: %v", err)
}
if err := os.Symlink(secretPath, projectPath); err != nil {
t.Fatalf("symlink: %v", err)
}
Comment on lines +235 to +237

d := NewYarnDetector(executor.NewMock())
d.ownerLookup = fixedOwner()
d.gitTracked = func(_ context.Context, _ string) bool { return false }
d.inGitRepo = func(_ string) bool { return false }

f := d.collectFile(context.Background(), projectPath, "project", "berry")

if !f.Exists {
t.Fatalf("expected the symlink itself to be recorded as existing")
}
if f.SymlinkTo != secretPath {
t.Errorf("SymlinkTo = %q, want %q", f.SymlinkTo, secretPath)
}
if f.Readable {
t.Errorf("project-scoped symlink should not be marked readable")
}
if len(f.Entries) != 0 {
t.Errorf("target file contents must not be parsed, got entries: %+v", f.Entries)
}
if f.SHA256 != "" {
t.Errorf("target file must not be hashed, got %q", f.SHA256)
}
}

func TestYarnFlavorFromVersion(t *testing.T) {
cases := []struct {
in string
Expand Down
10 changes: 8 additions & 2 deletions internal/detector/mcp.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/tailscale/hujson"

"github.com/step-security/dev-machine-guard/internal/aiagents/redact"
"github.com/step-security/dev-machine-guard/internal/executor"
"github.com/step-security/dev-machine-guard/internal/model"
"github.com/step-security/dev-machine-guard/internal/tcc"
Expand Down Expand Up @@ -288,7 +289,12 @@ func filterProjectScopedMCPServers(projectsRaw json.RawMessage) map[string]any {
return filtered
}

// filterServerFields keeps only command, args, serverUrl, url from each server entry.
// filterServerFields keeps only command, args, serverUrl, url from each server
// entry. env and headers are dropped outright since those are the fields
// vendors document for credentials. But command/args/url/serverUrl are
// real-world credential locations too (a bearer token or API key in a query
// string, an --api-key flag baked into args), so the kept values still pass
// through redact.Value before being uploaded.
func filterServerFields(serversRaw json.RawMessage) map[string]any {
var servers map[string]map[string]any
if err := json.Unmarshal(serversRaw, &servers); err != nil {
Expand All @@ -302,7 +308,7 @@ func filterServerFields(serversRaw json.RawMessage) map[string]any {
filtered := make(map[string]any)
for k, v := range serverConfig {
if allowedKeys[k] {
filtered[k] = v
filtered[k] = redact.Value(v)
}
}
result[name] = filtered
Expand Down
26 changes: 26 additions & 0 deletions internal/detector/mcp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -818,6 +818,32 @@ func TestFilterMCPContent_NonOpenCodeUnchanged(t *testing.T) {
}
}

// TestFilterServerFields_RedactsSecretsInKeptFields: command/args/url/serverUrl
// are the fields we keep (env/headers are dropped outright), but real MCP
// configs sometimes carry a bearer token or API key inside those kept fields
// too — e.g. a query-string token on the server URL, or an --api-key flag in
// 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{}
content := `{"mcpServers":{"pipeboard":{
"url":"https://meta-ads.mcp.pipeboard.co/?token=abcdEFGH12345678opaqueTokenValue",
"command":"npx",
"args":["-y","server","--api-key=abcdEFGH12345678opaqueTokenValue"]
}}}`

filtered, ok := det.filterMCPContent("cursor", "/Users/testuser/.cursor/mcp.json", []byte(content))
if !ok {
t.Fatalf("expected filtering to succeed")
}
if strings.Contains(string(filtered), "abcdEFGH12345678opaqueTokenValue") {
t.Fatalf("secret leaked into filtered output: %s", filtered)
}
if !strings.Contains(string(filtered), "token=") || !strings.Contains(string(filtered), "REDACTED") {
t.Errorf("expected the url token to be replaced with a redaction placeholder, got: %s", filtered)
}
}

// TestMCPConfigDefinitions_OpenCodeIsPlatformAgnostic: the two OpenCode
// definitions leave the Windows and Linux fields empty on purpose, so every
// consumer that walks mcpConfigDefinitions — including the known-user-config
Expand Down
Loading