Skip to content

fix(pipeline): a bare Go reference never binds a struct Field - #1944

Open
ilyabrykau-orca wants to merge 5 commits into
DeusData:mainfrom
ilyabrykau-orca:fix/go-bare-ref-field-binding
Open

fix(pipeline): a bare Go reference never binds a struct Field#1944
ilyabrykau-orca wants to merge 5 commits into
DeusData:mainfrom
ilyabrykau-orca:fix/go-bare-ref-field-binding

Conversation

@ilyabrykau-orca

Copy link
Copy Markdown

Fixes #1942. Stacked on #1937#1940 — the reproduce-first probe needs Go Field nodes to exist (#1940) before a bare local can falsely bind one, and #1940 needs this before its node recovery is net-positive for reference edges. The tip commit is this PR's change; suggested merge order #1937#1940 → this (or the last two together).

What

The READS/WRITES resolvers and the USAGE registry fallback hand bare reference text to the short-name registry, which contains Field nodes — so once Go struct fields exist, every Go local err := … binds whichever struct field is named err, project-wide:

onto Go Field nodes (stack without this fix) count
USAGE 21308 (top target: a test struct's field T, 3013 edges)
WRITES 5191

In Go the binding is impossible by construction: a field is only reachable through a selector expression (x.f), and selector references resolve on the LSP path — 100% of the Field-targeted USAGE edges in the census carried dot-less reference text, so there is no legitimate population mixed in.

How

cbm_go_suppress_bare_field_ref(is_go, ref_name, target_label) — pure predicate next to #1937's, consulted at the same four sites (both READS/WRITES resolvers, both USAGE registry fallbacks): drop when the file is Go, the target label is Field, and the reference has no .. Go-gated: C#/Java/C++/Python method bodies legitimately reference their own members bare — cp_reads_writes_cs_static_field pins that shape as required, and it stays untouched.

Field-validated (fresh index, ~1150-file Go+C repo, full stack)

USAGE onto Go fields 21308 → 0, WRITES 5191 → 0. The only remaining field-targeted reference class is 2466 CALLS, which is #1906/#1907's selector-guard territory. Combined stack ends below main's reference totals (USAGE 29198 → 21035, WRITES 2681 → 774) while adding the 4533 field nodes.

Tests

  • Reproduce-first pipeline_go_bare_ref_never_binds_field + _parallel twin (both resolvers, per fix(pipeline): guard USAGE/WRITES/READS against cross-language binds #1937's lesson): RED on the fix(extract): descend into Go struct field_declaration_list #1940 stack (ASSERT(!(cross_file_edge_exists(s, project, "Run", "err", "WRITES"))) failed on both paths), GREEN with the predicate. Fixture asserts the field node exists, so the probe can't pass vacuously.
  • Unit test go_bare_ref_never_binds_field: selector-shaped refs may bind, non-Field targets untouched, non-Go languages untouched, degenerate inputs safe.
  • Full scripts/test.sh venue leg (ASan+UBSan, all suites + contract steps): green on the stack. git clang-format --diff: clean.

#1932 tracks the family. Same root class as #1927: a name-shape heuristic binding without checking what the name can denote.

USAGE, WRITES and READS edges resolve through the same short-name
registry as CALLS but never consulted the DeusData#725 cross-language guard.
On a Go tree with eBPF C probes every Go identifier spelled like a C
one produced a reference edge into the C file: 31.5% of all WRITES on
the originally measured repo crossed the Go->C boundary, led by dozens
of Go test locals named event writing a C probe's automatic variable.

Add cbm_suppress_cross_language_ref() - the reference-edge analog of
cbm_suppress_cross_language_suffix_match - and consult it on BOTH
resolvers of each edge type: the sequential pass (pass_usages.c
resolve_usage_edges registry-fallback branch, resolve_rw_edges) and
their parallel twins (pass_parallel.c resolve_file_usages,
resolve_file_rw). The sequential-only version of this change left 344
Go->C WRITES alive on a ~1150-file repo because large repos resolve
through pass_parallel.c - the field census caught it, and the
parallel-twin test now pins it.

Unlike the CALLS guard the predicate takes no strategy parameter: a
reference edge carries no import-closure evidence, so every registry
strategy is a bare-name guess across a boundary. LSP-backed semantic
references resolve before the fallback and are unaffected. JS/TS stay
one family, and C/C++ count as one family too (.h maps to
CBM_LANG_CPP, so a .c file referencing its own header is not a
boundary).

Field-validated on the ~1150-file Go+C repo: Go->C/C++ WRITES 835->0,
USAGE 1545->0; C->Go 15/90->0; 6432 reference edges dropped in total,
every one cross-language (Go->.json 3034, Go->.hpp/.h 1612, Go->.c
746, Go->.sh/.yaml/.yml 443, ...) and none same-language. CALLS and
IMPORTS totals are byte-identical to main.

Fixes DeusData#1928

Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
Reproduce-first probe for the two fixes on this branch: RED without the
descend fix (count_defs_with_label(r, "Field") == 0, expected 3), GREEN
with it. Asserts the three named fields with their declared types in
return_type, the absence of the blank identifier, and that interface
members keep extracting exactly as before.

Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
@github-actions

Copy link
Copy Markdown

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. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

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.

The READS/WRITES resolvers and the USAGE registry fallback hand bare
reference text to the short-name registry, which contains Field nodes -
so once Go struct fields exist (DeusData#1935), every Go local err := ... binds
whichever struct field is named err, project-wide: 21308 USAGE and 5191
WRITES onto Go fields on the measured repo, top target a test struct's
field T collecting 3013 edges. In Go that binding is impossible by
construction: a field is only reachable through a selector expression
(x.f), and selector references resolve on the LSP path - every
Field-targeted reference edge in the census carried dot-less text.

Add cbm_go_suppress_bare_field_ref() next to the DeusData#1928 predicate and
consult it at the same four sites (both READS/WRITES resolvers, both
USAGE registry fallbacks): drop the bind when the file is Go, the
target label is Field, and the reference text has no '.'. Go-gated
because C#/Java/C++/Python method bodies legitimately reference their
own members bare (cp_reads_writes_cs_static_field pins that shape).

Field-validated on the DeusData#1940 stack: USAGE onto Go fields 21308 -> 0,
WRITES 5191 -> 0; the only remaining field-targeted edges are 2466
CALLS, which are DeusData#1906/DeusData#1907's selector-guard territory. Reproduce-
first pipeline probes (sequential + parallel twins) were RED on the
stack without this commit.

Fixes DeusData#1942

Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
@ilyabrykau-orca

Copy link
Copy Markdown
Author

CI retrigger (no-op amend, identical tree — no rerun rights): test-windows-guards died in its own harness setup — SETUP FAIL: ASCII baseline did not index … 'nodes': None on the guard's 2-file TypeScript fixture, before any guard ran. Same environmental signature #1907's first run hit and its retriggered run passed with identical code; this PR's diff is Go-gated end to end. No semantic changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(pipeline): a bare Go reference can bind a struct Field — READS/WRITES/USAGE attach every local err to whichever Field is named err

1 participant