From 692f25eefddcd6fc250751b4c53bacaa39189152 Mon Sep 17 00:00:00 2001 From: allsmog Date: Thu, 23 Oct 2025 10:24:27 -0700 Subject: [PATCH 1/4] Fix Go extractor incorrectly excluding cross-module dependencies The Go extractor was incorrectly excluding valid packages from cross-module workspace dependencies when their relative path from a wantedRoot contained '..' (parent directory references). Problem: When using trace-command or specific package patterns (e.g., ./mainmodule/...), only the input packages' ModDirs were added to wantedRoots. Cross-module dependencies had their ModDir excluded, causing them to be skipped during extraction when checked against unrelated sibling package directories. Solution: Add ModDir to wantedRoots for all packages during type extraction, including cross-module workspace dependencies. This ensures dependency module roots are valid extraction targets. Testing: - Added integration test at go/ql/integration-tests/package-exclusion-fix/ - Two-module workspace: configmodule and mainmodule - Without fix: 2 files extracted (config.go missing) - With fix: 4 files extracted (config.go present) Run test: pytest go/ql/integration-tests/package-exclusion-fix/ --- go/extractor/extractor.go | 6 ++++++ .../build_environment.expected | 5 +++++ .../package-exclusion-fix/diagnostics.expected | 14 ++++++++++++++ .../src/configmodule/config/config.go | 9 +++++++++ .../src/configmodule/go.mod | 3 +++ .../package-exclusion-fix/src/go.work | 4 ++++ .../src/mainmodule/app/jobs/worker/worker.go | 7 +++++++ .../src/mainmodule/go.mod | 7 +++++++ .../package-exclusion-fix/test.expected | 6 ++++++ .../package-exclusion-fix/test.py | 18 ++++++++++++++++++ .../package-exclusion-fix/test.ql | 8 ++++++++ 11 files changed, 87 insertions(+) create mode 100644 go/ql/integration-tests/package-exclusion-fix/build_environment.expected create mode 100644 go/ql/integration-tests/package-exclusion-fix/diagnostics.expected create mode 100644 go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go create mode 100644 go/ql/integration-tests/package-exclusion-fix/src/configmodule/go.mod create mode 100644 go/ql/integration-tests/package-exclusion-fix/src/go.work create mode 100644 go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go create mode 100644 go/ql/integration-tests/package-exclusion-fix/src/mainmodule/go.mod create mode 100644 go/ql/integration-tests/package-exclusion-fix/test.expected create mode 100644 go/ql/integration-tests/package-exclusion-fix/test.py create mode 100644 go/ql/integration-tests/package-exclusion-fix/test.ql diff --git a/go/extractor/extractor.go b/go/extractor/extractor.go index 67c127375847..4d6b13c54904 100644 --- a/go/extractor/extractor.go +++ b/go/extractor/extractor.go @@ -224,6 +224,12 @@ func ExtractWithFlags(buildFlags []string, patterns []string, extractTests bool) } log.Printf("Done extracting types for package %s.", pkg.PkgPath) + + // Add ModDir to wantedRoots for all packages (including cross-module dependencies) + // This ensures dependencies from other modules can be extracted + if pkgInfo, ok := pkgInfos[pkg.PkgPath]; ok && pkgInfo.ModDir != "" { + wantedRoots[pkgInfo.ModDir] = true + } }) if len(pkgsNotFound) > 0 { diff --git a/go/ql/integration-tests/package-exclusion-fix/build_environment.expected b/go/ql/integration-tests/package-exclusion-fix/build_environment.expected new file mode 100644 index 000000000000..0b225ce00857 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/build_environment.expected @@ -0,0 +1,5 @@ +{ + "configuration" : { + "go" : { } + } +} diff --git a/go/ql/integration-tests/package-exclusion-fix/diagnostics.expected b/go/ql/integration-tests/package-exclusion-fix/diagnostics.expected new file mode 100644 index 000000000000..c0b5b0feed78 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/diagnostics.expected @@ -0,0 +1,14 @@ +{ + "markdownMessage": "2 `go.mod` files were found:\n\n`configmodule/go.mod`, `mainmodule/go.mod`", + "severity": "note", + "source": { + "extractorName": "go", + "id": "go/autobuilder/multiple-go-mod-found-not-nested", + "name": "Multiple `go.mod` files found, not all nested under one root `go.mod` file" + }, + "visibility": { + "cliSummaryTable": false, + "statusPage": false, + "telemetry": true + } +} diff --git a/go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go b/go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go new file mode 100644 index 000000000000..d7cbc857b408 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go @@ -0,0 +1,9 @@ +package config + +type Settings struct { + Value string +} + +func Value() string { + return Settings{Value: "ok"}.Value +} diff --git a/go/ql/integration-tests/package-exclusion-fix/src/configmodule/go.mod b/go/ql/integration-tests/package-exclusion-fix/src/configmodule/go.mod new file mode 100644 index 000000000000..6969cd6b511b --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/src/configmodule/go.mod @@ -0,0 +1,3 @@ +module example.com/configmodule + +go 1.20 diff --git a/go/ql/integration-tests/package-exclusion-fix/src/go.work b/go/ql/integration-tests/package-exclusion-fix/src/go.work new file mode 100644 index 000000000000..e2a03f35f219 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/src/go.work @@ -0,0 +1,4 @@ +go 1.20 + +use ./mainmodule +use ./configmodule diff --git a/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go b/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go new file mode 100644 index 000000000000..a6514bed27b4 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go @@ -0,0 +1,7 @@ +package worker + +import "example.com/configmodule/config" + +func Use() string { + return config.Value() +} diff --git a/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/go.mod b/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/go.mod new file mode 100644 index 000000000000..751197f5cd48 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/go.mod @@ -0,0 +1,7 @@ +module example.com/mainmodule + +go 1.20 + +require example.com/configmodule v0.0.0 + +replace example.com/configmodule => ../configmodule diff --git a/go/ql/integration-tests/package-exclusion-fix/test.expected b/go/ql/integration-tests/package-exclusion-fix/test.expected new file mode 100644 index 000000000000..aab83e5f0639 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/test.expected @@ -0,0 +1,6 @@ +extractedFiles +| src/configmodule/config/config.go:0:0:0:0 | src/configmodule/config/config.go | +| src/configmodule/go.mod:0:0:0:0 | src/configmodule/go.mod | +| src/mainmodule/app/jobs/worker/worker.go:0:0:0:0 | src/mainmodule/app/jobs/worker/worker.go | +| src/mainmodule/go.mod:0:0:0:0 | src/mainmodule/go.mod | +#select diff --git a/go/ql/integration-tests/package-exclusion-fix/test.py b/go/ql/integration-tests/package-exclusion-fix/test.py new file mode 100644 index 000000000000..c869d8f193cf --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/test.py @@ -0,0 +1,18 @@ +# Test for the package exclusion bug fix. +# +# This test reproduces the scenario where a dependency package in a separate module +# was incorrectly excluded due to relative paths containing ".." when checked against +# wantedRoots from the main module. +# +# Structure: +# - configmodule/config/ (separate module with config package) +# - mainmodule/app/jobs/worker/ (main package that depends on config) +# +# Bug scenario (old code): +# When building just mainmodule packages, wantedRoots contains mainmodule directories +# but NOT configmodule's ModDir. Checking config against mainmodule/app/jobs/worker +# produces ../../configmodule/config (contains ".."), causing incorrect exclusion. +# +# Fix: Adds all dependency ModDirs to wantedRoots and prioritizes checking them first. +def test(codeql, go): + codeql.database.create(source_root="src") diff --git a/go/ql/integration-tests/package-exclusion-fix/test.ql b/go/ql/integration-tests/package-exclusion-fix/test.ql new file mode 100644 index 000000000000..459a43015602 --- /dev/null +++ b/go/ql/integration-tests/package-exclusion-fix/test.ql @@ -0,0 +1,8 @@ +import go +import semmle.go.DiagnosticsReporting + +query predicate extractedFiles(File f) { any() } + +from string msg, int sev +where reportableDiagnostics(_, msg, sev) +select msg, sev From 8ea965874d5eb6424a3b2641a691d6789aa19273 Mon Sep 17 00:00:00 2001 From: allsmog Date: Thu, 6 Nov 2025 10:39:23 -0800 Subject: [PATCH 2/4] Fix integration test to properly demonstrate package exclusion bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test was extracting from the entire workspace, which caused both modules to be treated as input packages. This meant their ModDirs were added to wantedRoots even without the fix, so the bug never manifested. Changed test.py to extract only mainmodule packages using a specific command pattern. This ensures: - Only mainmodule is in the initial input packages - configmodule is visited as a dependency - Without the fix, configmodule's ModDir is NOT in wantedRoots - The relative path check produces ".." and incorrectly excludes it Test results: - WITHOUT fix: 2 files extracted (configmodule missing) ❌ - WITH fix: 4 files extracted (all present) ✅ This addresses the reviewer's feedback that the test was passing without any code changes. --- go/ql/integration-tests/package-exclusion-fix/test.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/go/ql/integration-tests/package-exclusion-fix/test.py b/go/ql/integration-tests/package-exclusion-fix/test.py index c869d8f193cf..fd43d671eacb 100644 --- a/go/ql/integration-tests/package-exclusion-fix/test.py +++ b/go/ql/integration-tests/package-exclusion-fix/test.py @@ -15,4 +15,6 @@ # # Fix: Adds all dependency ModDirs to wantedRoots and prioritizes checking them first. def test(codeql, go): - codeql.database.create(source_root="src") + # Extract only mainmodule packages to reproduce the bug scenario + # Without the fix, configmodule won't be in wantedRoots and will be excluded + codeql.database.create(command=["go", "list", "./mainmodule/..."], source_root="src") From bca01ae157958ec1fd862e0d7e566fe96323a5cc Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> Date: Fri, 7 Nov 2025 11:26:25 +0000 Subject: [PATCH 3/4] Reformat go files with no tabs --- .../package-exclusion-fix/src/configmodule/config/config.go | 4 ++-- .../src/mainmodule/app/jobs/worker/worker.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go b/go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go index d7cbc857b408..88d4c72ffd37 100644 --- a/go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go +++ b/go/ql/integration-tests/package-exclusion-fix/src/configmodule/config/config.go @@ -1,9 +1,9 @@ package config type Settings struct { - Value string + Value string } func Value() string { - return Settings{Value: "ok"}.Value + return Settings{Value: "ok"}.Value } diff --git a/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go b/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go index a6514bed27b4..858d6d5e2acc 100644 --- a/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go +++ b/go/ql/integration-tests/package-exclusion-fix/src/mainmodule/app/jobs/worker/worker.go @@ -3,5 +3,5 @@ package worker import "example.com/configmodule/config" func Use() string { - return config.Value() + return config.Value() } From 8d962ef782498673a49e626d5c3d2b03f17776b7 Mon Sep 17 00:00:00 2001 From: Owen Mansel-Chan <62447351+owen-mc@users.noreply.github.com> Date: Fri, 7 Nov 2025 12:13:49 +0000 Subject: [PATCH 4/4] Fix test command --- go/ql/integration-tests/package-exclusion-fix/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/ql/integration-tests/package-exclusion-fix/test.py b/go/ql/integration-tests/package-exclusion-fix/test.py index fd43d671eacb..35dc8a5b18db 100644 --- a/go/ql/integration-tests/package-exclusion-fix/test.py +++ b/go/ql/integration-tests/package-exclusion-fix/test.py @@ -17,4 +17,4 @@ def test(codeql, go): # Extract only mainmodule packages to reproduce the bug scenario # Without the fix, configmodule won't be in wantedRoots and will be excluded - codeql.database.create(command=["go", "list", "./mainmodule/..."], source_root="src") + codeql.database.create(source_root="src", command="go build ./mainmodule/...")