Skip to content
Merged
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
6 changes: 3 additions & 3 deletions internal/detectors/gradle/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ func (d Detector) ResolveGraph(ctx context.Context, req sdk.DetectionRequest) (s

rootManifest := detectors.InferManifestMetadata(req, evidencePatterns)
if len(parsed.modules) == 0 {
AttachGradlePositions(parsed.rootGraph, workingDir)
AttachGradlePositions(parsed.rootGraph, workingDir, "")
return sdk.DetectionResult{
Graphs: sdk.SingleGraphContainer(parsed.rootGraph, rootManifest),
}, nil
Expand All @@ -131,11 +131,11 @@ func (d Detector) ResolveGraph(ctx context.Context, req sdk.DetectionRequest) (s
// project-local node instances, so attaching positions here cannot leak file
// locations (or scopes) between entries.
func subprojectGraphEntries(parsed gradleParseResult, rootManifest sdk.ManifestMetadata, workingDir string) []sdk.GraphEntry {
AttachGradlePositions(parsed.rootGraph, workingDir)
AttachGradlePositions(parsed.rootGraph, workingDir, "")
entries := []sdk.GraphEntry{{Graph: parsed.rootGraph, Manifest: rootManifest}}

for _, moduleEntry := range parsed.modules {
AttachGradlePositions(moduleEntry.graph, filepath.Join(workingDir, filepath.FromSlash(moduleEntry.module.Dir)))
AttachGradlePositions(moduleEntry.graph, filepath.Join(workingDir, filepath.FromSlash(moduleEntry.module.Dir)), moduleEntry.module.Dir)
entries = append(entries, sdk.GraphEntry{
Graph: moduleEntry.graph,
Manifest: sdk.ManifestMetadata{Path: moduleEntry.module.Dir + "/" + moduleEntry.module.ManifestFile, Kind: sdk.ManifestKind(moduleEntry.module.ManifestFile)},
Expand Down
31 changes: 29 additions & 2 deletions internal/detectors/gradle/detector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,8 +287,8 @@ func TestResolveGraphMultiProjectEmitsPerModuleEntries(t *testing.T) {
projectDir := t.TempDir()
writeGradleFile(t, projectDir, "settings.gradle", "rootProject.name = 'demo'\ninclude(\":app\", \":lib\")\n")
writeGradleFile(t, projectDir, "build.gradle", "group = \"com.acme\"\n")
writeGradleFile(t, projectDir, "app/build.gradle", "dependencies {}\n")
writeGradleFile(t, projectDir, "lib/build.gradle", "dependencies {}\n")
writeGradleFile(t, projectDir, "app/build.gradle", "dependencies {\n implementation project(':lib')\n implementation 'com.google.guava:guava:33.0.0-jre'\n}\n")
writeGradleFile(t, projectDir, "lib/build.gradle", "dependencies {\n api 'org.slf4j:slf4j-api:2.0.12'\n}\n")

fixturePath, err := filepath.Abs(filepath.Join("testdata", "dependencies-multiproject.txt"))
if err != nil {
Expand Down Expand Up @@ -351,6 +351,33 @@ func TestResolveGraphMultiProjectEmitsPerModuleEntries(t *testing.T) {
if len(libRoots) != 1 || libRoots[0].Type != sdk.PackageTypeApplication || libRoots[0].Name != "lib" {
t.Fatalf("unexpected lib entry root: %#v", libRoots)
}

// Regression: subproject positions must keep the module directory prefix
// so SARIF/diff annotations point at the child build file, not the root.
guava, _ := appGraph.Node("com.google.guava:guava@33.0.0-jre")
if guava == nil || len(guava.Locations) == 0 {
t.Fatalf("guava location missing: %+v", guava)
}
if loc := guava.Locations[0]; loc.RealPath != "app/build.gradle" || loc.Position == nil || loc.Position.File != "app/build.gradle" || loc.Position.Line != 3 {
t.Fatalf("guava location = %+v, want app/build.gradle line 3", loc)
}
libSlf4j, _ := libGraph.Node("org.slf4j:slf4j-api@2.0.12")
if libSlf4j == nil || len(libSlf4j.Locations) == 0 {
t.Fatalf("lib slf4j-api location missing: %+v", libSlf4j)
}
if loc := libSlf4j.Locations[0]; loc.RealPath != "lib/build.gradle" || loc.Position == nil || loc.Position.File != "lib/build.gradle" || loc.Position.Line != 2 {
t.Fatalf("lib slf4j-api location = %+v, want lib/build.gradle line 2", loc)
}
// The consuming subproject's copy of the api dependency is a distinct
// node instance with no declaration in app/build.gradle, so it carries no
// location; SARIF unions locations across entry graphs to compensate.
appSlf4j, _ := appGraph.Node("org.slf4j:slf4j-api@2.0.12")
if appSlf4j == nil {
t.Fatal("app entry must expose lib's api dependency through the project edge")
}
if len(appSlf4j.Locations) != 0 {
t.Fatalf("app slf4j-api unexpectedly located: %+v", appSlf4j.Locations)
}
}

// TestResolveGraphSingleProjectStillSingleEntry pins the regression contract:
Expand Down
25 changes: 17 additions & 8 deletions internal/detectors/gradle/positions.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ var gradleLockfileLine = regexp.MustCompile(`^([a-zA-Z][a-zA-Z0-9._-]+):([a-zA-Z
// gradlePositions scans the build files in projectDir and returns
// artifactId -> SourcePosition. build.gradle, build.gradle.kts, and
// gradle.lockfile are all walked; the first match per artifactId
// wins.
func gradlePositions(projectDir string) map[string]*sdk.SourcePosition {
// wins. relDir is the scan-root-relative subproject directory (slash
// form, empty for the root project) prefixed onto every recorded file
// so multi-project locations stay repo-relative.
func gradlePositions(projectDir, relDir string) map[string]*sdk.SourcePosition {
out := make(map[string]*sdk.SourcePosition)
files := []string{
"gradle.lockfile",
Expand All @@ -44,20 +46,24 @@ func gradlePositions(projectDir string) map[string]*sdk.SourcePosition {
}
for _, name := range files {
full := filepath.Join(projectDir, name)
relFile := name
if relDir != "" && relDir != "." {
relFile = relDir + "/" + name
}
_ = detectors.ScanLines(full, func(line int, text string) {
// Lockfile lines (com.foo:bar:1.0.0=...).
if m := gradleLockfileLine.FindStringSubmatch(text); m != nil {
record(out, name, m[2], line)
record(out, relFile, m[2], line)
return
}
// Source-coordinate string form.
if m := gradleDependencyCoord.FindStringSubmatch(text); m != nil {
record(out, name, m[2], line)
record(out, relFile, m[2], line)
return
}
// keyword-arg map form (only the name= portion).
if m := gradleNameKwArg.FindStringSubmatch(text); m != nil {
record(out, name, m[1], line)
record(out, relFile, m[1], line)
return
}
})
Expand All @@ -77,12 +83,15 @@ func record(out map[string]*sdk.SourcePosition, file, name string, line int) {
}

// AttachGradlePositions wires gradle build/lock file line numbers
// into a gradle-resolved graph.
func AttachGradlePositions(g *sdk.Graph, projectDir string) {
// into a gradle-resolved graph. Build files are read from projectDir;
// relDir is the scan-root-relative subproject directory (slash form,
// empty for the root project) used to keep recorded file paths
// repo-relative.
func AttachGradlePositions(g *sdk.Graph, projectDir, relDir string) {
if g == nil || projectDir == "" {
return
}
positions := gradlePositions(projectDir)
positions := gradlePositions(projectDir, relDir)
if len(positions) == 0 {
return
}
Expand Down
4 changes: 2 additions & 2 deletions internal/detectors/maven/detector.go
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ func (d Detector) ResolveGraph(ctx context.Context, req sdk.DetectionRequest) (s
if workingDir == "" {
workingDir = req.ProjectPath
}
AttachPomPositions(depsGraph, workingDir)
AttachPomPositions(depsGraph, workingDir, "pom.xml")

rootManifest := detectors.InferManifestMetadata(req, evidencePatterns)
modules, err := walkPomModules(workingDir)
Expand Down Expand Up @@ -208,7 +208,7 @@ func (d Detector) reactorGraphEntries(depsGraph *sdk.Graph, modules []mavenModul
if err != nil {
continue
}
AttachPomPositions(moduleGraph, filepath.Join(workingDir, filepath.FromSlash(matched.module.Dir)))
AttachPomPositions(moduleGraph, filepath.Join(workingDir, filepath.FromSlash(matched.module.Dir)), matched.module.Dir+"/pom.xml")
entries = append(entries, sdk.GraphEntry{
Graph: moduleGraph,
Manifest: sdk.ManifestMetadata{Path: matched.module.Dir + "/pom.xml", Kind: sdk.ManifestKind("pom.xml")},
Expand Down
76 changes: 76 additions & 0 deletions internal/detectors/maven/modules_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,82 @@ func TestMavenPerModuleEntriesFromTGF(t *testing.T) {
}
}

// TestMavenPerModuleEntriesAttachModuleRelativePositions pins the regression
// where reactor-module dependency locations lost the module directory prefix:
// a dep declared in module-a/pom.xml must surface as "module-a/pom.xml", not
// the module-relative "pom.xml" that points SARIF consumers at the parent pom.
func TestMavenPerModuleEntriesAttachModuleRelativePositions(t *testing.T) {
raw, err := os.ReadFile(filepath.Join("testdata", "dependency-tree-multimodule.tgf"))
if err != nil {
t.Fatalf("read fixture: %v", err)
}
depsGraph, err := depGraphFromMavenTGF(raw)
if err != nil {
t.Fatalf("depGraphFromMavenTGF() error = %v", err)
}

root := t.TempDir()
writePom(t, root, "pom.xml", `<project>
<groupId>com.bomly</groupId>
<artifactId>reactor</artifactId>
<modules>
<module>module-a</module>
<module>module-b</module>
<module>module-c</module>
</modules>
</project>`)
writePom(t, root, "module-a/pom.xml", `<project>
<parent><groupId>com.bomly</groupId></parent>
<artifactId>module-a</artifactId>
<dependencies>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.12.0</version>
</dependency>
</dependencies>
</project>`)
for _, name := range []string{"module-b", "module-c"} {
writePom(t, root, name+"/pom.xml", `<project>
<parent><groupId>com.bomly</groupId></parent>
<artifactId>`+name+`</artifactId>
</project>`)
}

modules, err := walkPomModules(root)
if err != nil {
t.Fatalf("walkPomModules() error = %v", err)
}
entries, matched := Detector{}.reactorGraphEntries(depsGraph, modules, sdk.ManifestMetadata{Path: "pom.xml", Kind: "pom.xml"}, root)
if matched != 3 {
t.Fatalf("expected 3 matched modules, got %d", matched)
}
var moduleA sdk.GraphEntry
for _, entry := range entries {
if entry.Manifest.Path == "module-a/pom.xml" {
moduleA = entry
}
}
if moduleA.Graph == nil {
t.Fatalf("module-a entry missing from %d entries", len(entries))
}
lang3, ok := moduleA.Graph.Node("org.apache.commons:commons-lang3@3.12.0")
if !ok || lang3 == nil || len(lang3.Locations) == 0 {
t.Fatalf("commons-lang3 location missing: %+v", lang3)
}
loc := lang3.Locations[0]
if loc.RealPath != "module-a/pom.xml" || loc.AccessPath != "module-a/pom.xml" {
t.Errorf("location paths = %q / %q, want module-a/pom.xml", loc.RealPath, loc.AccessPath)
}
if loc.Position == nil || loc.Position.File != "module-a/pom.xml" {
t.Fatalf("position = %+v, want file module-a/pom.xml", loc.Position)
}
// Exact version match anchors at the <version> line of the dependency block.
if loc.Position.Line != 8 {
t.Errorf("position line = %d, want 8", loc.Position.Line)
}
}

func TestMavenUnmatchedTGFRootsFallBackToRootEntry(t *testing.T) {
raw, err := os.ReadFile(filepath.Join("testdata", "dependency-tree-multimodule.tgf"))
if err != nil {
Expand Down
13 changes: 10 additions & 3 deletions internal/detectors/maven/positions.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,12 +158,19 @@ func pomArtifactPropertyVersion(artifact string, properties map[string]pomProper
return prop.value, prop.line, true
}

// AttachPomPositions wires pom.xml line numbers into a maven graph.
func AttachPomPositions(g *sdk.Graph, projectDir string) {
// AttachPomPositions wires pom.xml line numbers into a maven graph. The pom
// is read from projectDir; relPomPath is the scan-root-relative pom path
// (e.g. "pom.xml" for the root, "core/pom.xml" for a reactor module) stamped
// into every recorded position, so multi-module locations stay repo-relative
// in SARIF and diff annotations.
func AttachPomPositions(g *sdk.Graph, projectDir, relPomPath string) {
if g == nil || projectDir == "" {
return
}
positions := pomPositions(filepath.Join(projectDir, "pom.xml"), "pom.xml")
if relPomPath == "" {
relPomPath = "pom.xml"
}
positions := pomPositions(filepath.Join(projectDir, "pom.xml"), relPomPath)
if len(positions) == 0 {
return
}
Expand Down
60 changes: 56 additions & 4 deletions internal/detectors/positions_extractors_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ func TestMavenPomPositions(t *testing.T) {
mustPkg(t, g, "jackson-databind", "2.17.0", func(p *sdk.Dependency) { p.Org = "com.fasterxml.jackson.core" })
mustPkg(t, g, "junit", "4.13.2", func(p *sdk.Dependency) { p.Org = "junit" })
mustPkg(t, g, "commons-lang3", "3.17.0", func(p *sdk.Dependency) { p.Org = "org.apache.commons" })
maven.AttachPomPositions(g, dir)
maven.AttachPomPositions(g, dir, "pom.xml")
jd, _ := g.Node("com.fasterxml.jackson.core:jackson-databind@2.17.0")
if jd == nil || len(jd.Locations) == 0 || jd.Locations[0].Position.Line != 9 {
t.Errorf("jackson-databind location wrong: %+v", jd)
Expand Down Expand Up @@ -321,7 +321,7 @@ func TestMavenPomPositionsResolvePropertiesAfterDependencies(t *testing.T) {
`)
g := sdk.New()
mustPkg(t, g, "commons-lang3", "3.17.0", func(p *sdk.Dependency) { p.Org = "org.apache.commons" })
maven.AttachPomPositions(g, dir)
maven.AttachPomPositions(g, dir, "pom.xml")

lang3, _ := g.Node("org.apache.commons:commons-lang3@3.17.0")
if lang3 == nil || len(lang3.Locations) == 0 || lang3.Locations[0].Position.Line != 10 {
Expand All @@ -345,14 +345,43 @@ func TestMavenPomPositionsUseArtifactPropertyForManagedDependency(t *testing.T)
`)
g := sdk.New()
mustPkg(t, g, "commons-lang3", "3.17.0", func(p *sdk.Dependency) { p.Org = "org.apache.commons" })
maven.AttachPomPositions(g, dir)
maven.AttachPomPositions(g, dir, "pom.xml")

lang3, _ := g.Node("org.apache.commons:commons-lang3@3.17.0")
if lang3 == nil || len(lang3.Locations) != 1 || lang3.Locations[0].Position.Line != 3 {
t.Fatalf("commons-lang3 location = %+v, want artifact property line 3", lang3)
}
}

func TestMavenPomPositionsModuleRelativePath(t *testing.T) {
dir := t.TempDir()
writeFile(t, dir, "pom.xml", `<project>
<dependencies>
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-text</artifactId>
<version>1.9</version>
</dependency>
</dependencies>
</project>
`)
g := sdk.New()
mustPkg(t, g, "commons-text", "1.9", func(p *sdk.Dependency) { p.Org = "org.apache.commons" })
maven.AttachPomPositions(g, dir, "core/pom.xml")

text, _ := g.Node("org.apache.commons:commons-text@1.9")
if text == nil || len(text.Locations) == 0 {
t.Fatalf("commons-text location missing: %+v", text)
}
loc := text.Locations[0]
if loc.RealPath != "core/pom.xml" || loc.AccessPath != "core/pom.xml" {
t.Errorf("location paths = %q / %q, want core/pom.xml", loc.RealPath, loc.AccessPath)
}
if loc.Position == nil || loc.Position.File != "core/pom.xml" || loc.Position.Line != 6 {
t.Errorf("position = %+v, want core/pom.xml line 6", loc.Position)
}
}

func TestGradlePositions(t *testing.T) {
dir := t.TempDir()
writeFile(t, dir, "build.gradle", `dependencies {
Expand All @@ -365,7 +394,7 @@ func TestGradlePositions(t *testing.T) {
mustPkg(t, g, "jackson-databind", "2.17.0")
mustPkg(t, g, "spring-core", "6.0.0")
mustPkg(t, g, "junit", "4.13.2")
gradle.AttachGradlePositions(g, dir)
gradle.AttachGradlePositions(g, dir, "")

cases := map[string]int{"jackson-databind": 2, "spring-core": 3, "junit": 4}
for name, wantLine := range cases {
Expand All @@ -379,6 +408,29 @@ func TestGradlePositions(t *testing.T) {
}
}

func TestGradlePositionsSubprojectRelDirPrefix(t *testing.T) {
dir := t.TempDir()
writeFile(t, dir, "build.gradle", `dependencies {
api 'org.slf4j:slf4j-api:2.0.12'
}
`)
g := sdk.New()
mustPkg(t, g, "slf4j-api", "2.0.12")
gradle.AttachGradlePositions(g, dir, "lib")

p, _ := g.Node("slf4j-api@2.0.12")
if p == nil || len(p.Locations) == 0 {
t.Fatalf("slf4j-api location missing: %+v", p)
}
loc := p.Locations[0]
if loc.RealPath != "lib/build.gradle" || loc.AccessPath != "lib/build.gradle" {
t.Errorf("location paths = %q / %q, want lib/build.gradle", loc.RealPath, loc.AccessPath)
}
if loc.Position == nil || loc.Position.File != "lib/build.gradle" || loc.Position.Line != 2 {
t.Errorf("position = %+v, want lib/build.gradle line 2", loc.Position)
}
}

func TestSBTPositions(t *testing.T) {
dir := t.TempDir()
writeFile(t, dir, "build.sbt", `name := "demo"
Expand Down
19 changes: 16 additions & 3 deletions internal/output/sarif.go
Original file line number Diff line number Diff line change
Expand Up @@ -502,20 +502,33 @@ func sarifLocationDiffScore(uri string, region *sarifRegion, changedLines map[st
return 1
}

// dependenciesForFinding resolves the finding's dependency refs against every
// location graph, not only the first graph that knows the ref: entry graphs
// can hold distinct node instances for the same ref (e.g. a gradle library
// subproject's `api` dependency also appears in each consuming subproject's
// graph), and only the declaring module's instance carries manifest
// locations. Pointer-identical instances shared across graphs are collected
// once.
func dependenciesForFinding(f sdk.Finding, options []SARIFOptions) []*sdk.Dependency {
if len(options) == 0 || len(options[0].LocationGraphs) == 0 || len(f.DependencyRefs) == 0 {
return nil
}
out := make([]*sdk.Dependency, 0, len(f.DependencyRefs))
seen := make(map[*sdk.Dependency]struct{}, len(f.DependencyRefs))
for _, ref := range f.DependencyRefs {
for _, graph := range options[0].LocationGraphs {
if graph == nil {
continue
}
if dep, ok := graph.Node(ref); ok && dep != nil {
out = append(out, dep)
break
dep, ok := graph.Node(ref)
if !ok || dep == nil {
continue
}
if _, dup := seen[dep]; dup {
continue
}
seen[dep] = struct{}{}
out = append(out, dep)
}
}
return out
Expand Down
Loading