fix(pipeline): never bind a Go import to a symbol - #1938
Conversation
A Go import path names a package - never a function, method, field or another language's build target. Strategy 1 resolves every correct Go import (module path -> the package's Folder node); when it misses the import is external and the correct result is NO edge. Two name-guess fallbacks violated that: - Strategy 3's language-agnostic symbol-name fallback matched the path's last segment against any project definition of the same simple name and returned the lexicographically smallest survivor: import "os/exec" bound a test harness's exec() method (52 edges on the measured repo), two imports bound a Function extracted from a Makefile. 89 of 2295 Go IMPORTS edges (3.9%) were false; 27 of those are the import "C" case (DeusData#1926/DeusData#1931), the remaining 62 this. - Strategy 1b's sibling-file resolution admits symbol labels through import_targetable_label, re-creating the same bug one directory closer: with Strategy 3 gated, the field census still bound one os/exec import to a same-package exec() method. Gate both on the importing file's language through a new pure predicate, cbm_import_symbol_fallback_allowed(): false for Go, true for everything else - member-importing languages (Python, Java, Rust use crate::ops::helper) legitimately need the symbol fallback, and build/markup grammars (SCSS partials, Meson subdir, Pony use) the sibling one. Strategies 1, 2 and 4 are untouched. Reproduce-first test ei_go_import_never_binds_symbol asserts the IMPORTS relation EXACTLY (an internal package import stays; neither the cross-package nor the same-package decoy may be bound): RED count=2 expected==1 before each gate, GREEN after. Adds ei_edge_count_is, the exact-count sibling of ei_edge_present, since a floor cannot catch a fabricated extra edge. Field-validated on a real ~1150-file Go+C repo: Go IMPORTS by target label went Folder 2206 / Method 85 / Function 4 on main to Folder 2206 / Method 0 / Function 0 with the fix; all 2206 correct package-Folder edges survive. Fixes DeusData#1934 Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
e8d0590 to
7d76b88
Compare
|
Amended (force-push): the field census caught a second path with the same disease — Strategy 1b's sibling-file resolution admits symbol labels and bound one |
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Fixes #1934. Generalises #1926/#1931 —
import "C"was the special case of the same mechanism.What
A Go import path names a package — never a function, method, field, or another language's build target. Strategy 1 resolves every correct Go import (module path → the package's
Foldernode). When it misses, the import is external (stdlib / a module outside the tree) and the correct result is no edge. Two name-guess fallbacks violated that:pass_pkgmap.c) matched the path's last segment against any project definition of the same simple name and returned the lexicographically smallest survivor:os/exec→ a test harness'sexecMethod (52 edges),.../wait→ a rate limiter's method (6), two imports → a Function extracted from a Makefile (4).import_targetable_label, re-creating the same bug one directory closer: with only Strategy 3 gated, the field census still bound oneos/execimport to a same-packageexec()method.89 of 2295 Go IMPORTS edges (3.9%) on the measured repo were false; 27 of those are the
import "C"case (#1926/#1931), the remaining 62 are this.How
New pure predicate
cbm_import_symbol_fallback_allowed(CBMLanguage)(declared with the testable helpers inpipeline_internal.h): false for Go, true for everything else. Both fallbacks are gated on it; Strategies 1, 2 and 4 are untouched. Member-importing languages (Pythonfrom m import f, Java, Kotlin, Rustuse crate::ops::helper— the case Strategy 3's own comment cites) keep the symbol fallback, and build/markup grammars (SCSS partials, Meson subdir, Ponyuse) keep the sibling one. This is deliberately the issue's "cheaper first cut": the measurement found no correct Go import that needed either fallback.Field-validated (fresh index of the ~1150-file Go+C repo, this branch vs main)
Folder(correct)MethodFunctionTests
ei_go_import_never_binds_symbol: exact-count probe — the internalutilpackage import must be the entire IMPORTS relation, with both a cross-package and a same-packageexecdecoy present. RED[IMPORTS] FAIL count=2 expected==1before each gate (Strategy 3, then Strategy 1b), GREEN with both.ei_edge_count_is, the exact-count sibling of the floor-onlyei_edge_present— a floor cannot catch a fabricated extra edge (Meta: Go / cgo / Go+C extraction — tracking issue #1932's negative-assertion gap).scripts/test.shvenue leg (ASan+UBSan, all suites + contract steps): green.git clang-format --diff: clean.Base: upstream
maindirectly. #1932 tracks the family.