Skip to content

Commit 651ae3c

Browse files
fix(pipeline): suppress weak short-name matches for Go selector calls
A Go selector call x.foo() whose receiver the Go LSP cannot type falls through to the generic registry resolver, which binds it by bare short name to an arbitrary same-named project symbol. Stdlib calls are the worst case: f.Close() on an *os.File gets a CALLS edge to whatever project Close wins candidate ranking (measured on a real Go repo: confidence 0.11, 15 candidates; suffix_match + unique_name were 36% of all CALLS edges, and one 14-line stdlib-only function got 3 out of 3 false outbound edges). Extend the TS/JS receiver-aware guard (#592/#606) to Go: - extract_calls.c: flag Go call_expression with a selector_expression callee as is_method, mirroring the TS/JS member_expression flag. - registry.c: add cbm_go_suppress_weak_method_match. Unlike the TS/JS drop-list, field_type_hint is KEPT (Go struct fields carry declared types, so the hint is receiver-aware — lrp_go_s8_field_type_hint), and unique_name is dropped only when its confidence carries the import-unreachability penalty (the stdlib-hijack shape); an unpenalized lone candidate inside the caller's import closure never enters the field-type-hint upgrade and must survive. - pass_calls.c / pass_parallel.c: feed the Go gate next to the TS/JS one; the drop still defers to the emit path so service/route/HTTP edges stay main-identical. Reproduce-first: pipeline_go_receiver_suppresses_weak_method_edge is RED without the extractor flag (the f.Close -> project Close edge exists) and GREEN with it; typed same-package calls, bare local calls and import-qualified cross-package calls still resolve. The old extraction contract test used Go as the flag-exempt language — Python takes that role, and extract_go_selector_call_flags_is_method pins the new behavior. Signed-off-by: Ilya Brykau <ilya.brykau@orca.security>
1 parent 5fbab7b commit 651ae3c

8 files changed

Lines changed: 242 additions & 7 deletions

File tree

internal/cbm/extract_calls.c

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3619,6 +3619,24 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML
36193619
}
36203620
}
36213621
}
3622+
// Go receiver-aware guard (same direction as the TS/JS flag above).
3623+
// Flag a selector call x.foo(). The Go AST cannot separate a method
3624+
// call on a value from a package-qualified call — but every selector
3625+
// call the Go LSP or the import/qualified registry strategies CAN
3626+
// place never reaches the weak short-name guards, so the flag only
3627+
// bites on unresolvable receivers (`f.Close()` on an os.File,
3628+
// `sha256.New()` behind an unindexed import), where a project-wide
3629+
// short-name match fabricates an edge to an unrelated project
3630+
// symbol sharing the name. Bare calls (helper()) keep
3631+
// is_method=false and resolve same-module/import paths as before.
3632+
if (ctx->language == CBM_LANG_GO &&
3633+
strcmp(ts_node_type(node), "call_expression") == 0) {
3634+
TSNode gofn = ts_node_child_by_field_name(node, TS_FIELD("function"));
3635+
if (!ts_node_is_null(gofn) &&
3636+
strcmp(ts_node_type(gofn), "selector_expression") == 0) {
3637+
call.is_method = true;
3638+
}
3639+
}
36223640

36233641
TSNode args = ts_node_child_by_field_name(node, TS_FIELD("arguments"));
36243642
// ObjectScript stores args under oref_method/method_args, not the

src/pipeline/pass_calls.c

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -623,12 +623,20 @@ static int resolve_single_call(cbm_pipeline_ctx_t *ctx, CBMCall *call,
623623
* language gated on only one resolver produces an edge on the sequential
624624
* path and not the parallel one (or vice versa), breaking MT determinism.
625625
* ArkTS belongs to the JS/TS family here (#1842); dropping it would
626-
* reintroduce the #592/#606 false-edge class for .ets files. */
626+
* reintroduce the #592/#606 false-edge class for .ets files.
627+
*
628+
* Go (#1906) rides the same deferred-drop plumbing through its OWN
629+
* predicate: its drop-list differs (field_type_hint is receiver-aware for
630+
* Go, and unique_name drops only when import-unreachability-penalized), so
631+
* it composes via cbm_go_suppress_weak_method_match instead of widening
632+
* the shared gate. Same lockstep rule: mirror pass_parallel.c. */
627633
bool suppress_weak_member = lang == CBM_LANG_PYTHON || lang == CBM_LANG_JAVASCRIPT ||
628634
lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
629635
lang == CBM_LANG_ARKTS;
630636
bool drop_plain_call =
631-
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy);
637+
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy) ||
638+
cbm_go_suppress_weak_method_match(lang == CBM_LANG_GO, call->is_method, res.strategy,
639+
res.confidence);
632640

633641
/* Service-pattern HTTP/ASYNC calls to an EXTERNAL client library (e.g.
634642
* `requests.get("/api/orders/{id}")`) resolve to a QN containing the library

src/pipeline/pass_parallel.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2488,12 +2488,16 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB
24882488
* #606 direction.
24892489
*
24902490
* This language set MUST match the one in pass_calls.c exactly — see the
2491-
* note there. ArkTS belongs to the JS/TS family (#1842). */
2491+
* note there. ArkTS belongs to the JS/TS family (#1842). Go (#1906)
2492+
* composes via its own predicate (different drop-list — see
2493+
* cbm_go_suppress_weak_method_match), mirrored in pass_calls.c. */
24922494
bool suppress_weak_member = lang == CBM_LANG_PYTHON || lang == CBM_LANG_JAVASCRIPT ||
24932495
lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
24942496
lang == CBM_LANG_ARKTS;
24952497
bool drop_plain_call =
2496-
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy);
2498+
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy) ||
2499+
cbm_go_suppress_weak_method_match(lang == CBM_LANG_GO, call->is_method, res.strategy,
2500+
res.confidence);
24972501

24982502
/* Service-pattern HTTP/ASYNC client call (`requests.get(url)`): the
24992503
* service signal lives in the callee_name. The registry can mis-resolve

src/pipeline/pipeline.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,15 @@ bool cbm_perl_suppress_generic_match(bool is_perl, bool is_method, const char *c
280280
* Pure; unit-tested in test_registry.c. */
281281
bool cbm_suppress_weak_member_match(bool enabled, bool is_method, const char *strategy);
282282

283+
/* Go analog of the TS/JS guard, same failure class: a selector call whose
284+
* receiver the Go LSP could not type must not be bound by a receiver-blind
285+
* short-name strategy. Drops suffix_match / fuzzy always, and unique_name only
286+
* when its confidence is import-unreachability-penalized (the stdlib/vendor
287+
* hijack shape). field_type_hint is deliberately NOT dropped for Go — struct
288+
* fields carry declared types, so the hint is receiver-aware there. */
289+
bool cbm_go_suppress_weak_method_match(bool is_go, bool is_method, const char *strategy,
290+
double confidence);
291+
283292
/* #725: drop a suffix_match CALLS edge when the caller language and the
284293
* target file's language disagree. unique_name (candidates == 1) is #1572
285294
* and is left alone; same_module / import_map / lsp_* are kept. JS/TS/TSX

src/pipeline/registry.c

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -464,6 +464,33 @@ bool cbm_suppress_weak_member_match(bool enabled, bool is_method, const char *st
464464
strcmp(strategy, "field_type_hint") == 0 || strcmp(strategy, "fuzzy") == 0;
465465
}
466466

467+
bool cbm_go_suppress_weak_method_match(bool is_go, bool is_method, const char *strategy,
468+
double confidence) {
469+
if (!is_go || !is_method || !strategy || !strategy[0]) {
470+
return false;
471+
}
472+
/* Go analog of the TS/JS guard above, same failure class: a selector call
473+
* whose receiver the Go LSP could not type reaches the registry and a bare
474+
* short-name strategy binds it to an arbitrary same-named project symbol
475+
* (`f.Close()` on an os.File -> a project `Close`, suffix_match over 15
476+
* candidates). Unlike the TS/JS list, field_type_hint is KEPT: a Go struct
477+
* field carries a declared type, so the parallel resolver's field-type
478+
* hint is receiver-aware for Go (lrp_go_s8_field_type_hint), not a
479+
* heuristic. */
480+
if (strcmp(strategy, "suffix_match") == 0 || strcmp(strategy, "fuzzy") == 0) {
481+
return true;
482+
}
483+
/* unique_name is dropped only when PENALIZED: resolve_name_lookup scales
484+
* CONF_UNIQUE_NAME by DEFAULT_CONFIDENCE exactly when the lone candidate
485+
* is not reachable through the caller's imports — the stdlib/vendor
486+
* hijack shape (`io.Copy` -> a project `Copy`). An unpenalized
487+
* unique_name target sits inside the caller's import closure (or the
488+
* file has no imports, e.g. a same-package call) and must be kept —
489+
* dropping it kills genuinely-typed lone-candidate calls that never
490+
* enter the field-type-hint upgrade (candidate_count == 1). */
491+
return strcmp(strategy, "unique_name") == 0 && confidence < CONF_UNIQUE_NAME;
492+
}
493+
467494
static bool js_ts_family(CBMLanguage lang) {
468495
return lang == CBM_LANG_JAVASCRIPT || lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
469496
lang == CBM_LANG_ARKTS;

tests/test_extraction.c

Lines changed: 36 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4743,9 +4743,14 @@ TEST(extract_perl_method_call_flags_is_method) {
47434743
/* Languages OUTSIDE the is_method flag set (only Perl and TS/JS/TSX set it) must
47444744
* be unaffected: a Go method call never sets is_method. */
47454745
TEST(extract_flag_exempt_method_call_not_flagged_is_method) {
4746-
CBMFileResult *r = extract("package m\n"
4747-
"func run(o Obj) { o.Commit(); helper() }\n",
4748-
CBM_LANG_GO, "t", "x.go");
4746+
/* Rust is flag-exempt: only Perl, Python, TS/JS and Go set is_method.
4747+
* Guards the blast radius of the receiver-aware flags for every other
4748+
* language. */
4749+
CBMFileResult *r = extract("fn run(o: Obj) {\n"
4750+
" o.commit();\n"
4751+
" helper();\n"
4752+
"}\n",
4753+
CBM_LANG_RUST, "t", "x.rs");
47494754
ASSERT_NOT_NULL(r);
47504755
ASSERT_FALSE(r->has_error);
47514756
for (int i = 0; i < r->calls.count; i++) {
@@ -4824,6 +4829,33 @@ TEST(extract_python_member_call_flags_is_method) {
48244829
PASS();
48254830
}
48264831

4832+
TEST(extract_go_selector_call_flags_is_method) {
4833+
/* Go selector calls are flagged so the weak-match guard can fire when the
4834+
* Go LSP cannot type the receiver; bare calls stay unflagged. */
4835+
CBMFileResult *r = extract("package m\n"
4836+
"func run(o Obj) { o.Commit(); helper() }\n",
4837+
CBM_LANG_GO, "t", "x.go");
4838+
ASSERT_NOT_NULL(r);
4839+
ASSERT_FALSE(r->has_error);
4840+
bool saw_selector = false;
4841+
bool saw_bare = false;
4842+
for (int i = 0; i < r->calls.count; i++) {
4843+
const CBMCall *c = &r->calls.items[i];
4844+
if (c->callee_name && strstr(c->callee_name, "Commit") != NULL) {
4845+
ASSERT_TRUE(c->is_method);
4846+
saw_selector = true;
4847+
}
4848+
if (c->callee_name && strcmp(c->callee_name, "helper") == 0) {
4849+
ASSERT_FALSE(c->is_method);
4850+
saw_bare = true;
4851+
}
4852+
}
4853+
ASSERT_TRUE(saw_selector);
4854+
ASSERT_TRUE(saw_bare);
4855+
cbm_free_result(r);
4856+
PASS();
4857+
}
4858+
48274859
/* TS/JS/TSX receiver-aware flag (#592/#606; same intent as the Perl flag above).
48284860
* A member call x.foo() with a non-this/super receiver is flagged is_method so
48294861
* the resolver can suppress a weak short-name match (`re.test()` must not bind a
@@ -6503,6 +6535,7 @@ SUITE(extraction) {
65036535
RUN_TEST(extract_perl_method_call_flags_is_method);
65046536
RUN_TEST(extract_flag_exempt_method_call_not_flagged_is_method);
65056537
RUN_TEST(extract_python_member_call_flags_is_method);
6538+
RUN_TEST(extract_go_selector_call_flags_is_method);
65066539
RUN_TEST(extract_ts_member_call_flags_is_method);
65076540
RUN_TEST(extract_ts_this_super_receiver_not_flagged);
65086541
RUN_TEST(extract_js_member_call_flags_is_method);

tests/test_pipeline.c

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4693,6 +4693,99 @@ TEST(pipeline_python_receiver_suppresses_weak_method_edge) {
46934693
PASS();
46944694
}
46954695

4696+
TEST(pipeline_go_receiver_suppresses_weak_method_edge) {
4697+
char tmp[256];
4698+
snprintf(tmp, sizeof(tmp), "/tmp/cbm_go_recv_XXXXXX");
4699+
if (!cbm_mkdtemp(tmp)) {
4700+
FAIL("tmpdir");
4701+
}
4702+
4703+
/* go.mod makes project imports resolvable — real Go repos always have one,
4704+
* and import reachability (the unique_name penalty) depends on it. */
4705+
write_temp_file(tmp, "go.mod", "module example.com/myapp\n\ngo 1.22\n");
4706+
/* The lone project symbol named "Close" — a real method. */
4707+
write_temp_file(tmp, "storage/storage.go",
4708+
"package storage\n"
4709+
"\n"
4710+
"type Storage struct{ open bool }\n"
4711+
"\n"
4712+
"func NewStorage() *Storage { return &Storage{open: true} }\n"
4713+
"\n"
4714+
"func (s *Storage) Close() {\n"
4715+
"\ts.open = false\n"
4716+
"}\n"
4717+
"\n"
4718+
"func Boot() {\n"
4719+
"\ts := NewStorage()\n"
4720+
"\ts.Close()\n"
4721+
"}\n");
4722+
/* Cross-package control target: imported by hash.go, so the caller file
4723+
* has a non-empty import map (like any real Go file) and unreachable
4724+
* unique_name candidates get the import penalty. */
4725+
write_temp_file(tmp, "util/util.go",
4726+
"package util\n"
4727+
"\n"
4728+
"func Tag() string { return \"t\" }\n");
4729+
/* Stdlib receiver: `f.Close()` closes an *os.File, NOT the project method.
4730+
* The Go LSP cannot bind it to a project symbol → the registry would guess
4731+
* Close by short name (weak). This is the false edge to suppress —
4732+
* the exact shape that attached every file/rows/gzip Close in a real Go
4733+
* repo to one unrelated project method. */
4734+
write_temp_file(tmp, "hash/hash.go",
4735+
"package hash\n"
4736+
"\n"
4737+
"import (\n"
4738+
"\t\"os\"\n"
4739+
"\n"
4740+
"\t\"example.com/myapp/util\"\n"
4741+
")\n"
4742+
"\n"
4743+
"func FileLen(path string) int64 {\n"
4744+
"\tf, err := os.Open(path)\n"
4745+
"\tif err != nil {\n"
4746+
"\t\treturn 0\n"
4747+
"\t}\n"
4748+
"\tdefer f.Close()\n"
4749+
"\tst, err := f.Stat()\n"
4750+
"\tif err != nil {\n"
4751+
"\t\treturn 0\n"
4752+
"\t}\n"
4753+
"\treturn st.Size()\n"
4754+
"}\n"
4755+
"\n"
4756+
"func localHelper() int { return 1 }\n"
4757+
"\n"
4758+
"func CallsLocal() int { return localHelper() }\n"
4759+
"\n"
4760+
"func UsesUtil() string { return util.Tag() }\n");
4761+
4762+
char db_path[512];
4763+
snprintf(db_path, sizeof(db_path), "%s/go_recv.db", tmp);
4764+
cbm_pipeline_t *p = cbm_pipeline_new(tmp, db_path, CBM_MODE_FULL);
4765+
ASSERT_NOT_NULL(p);
4766+
ASSERT_EQ(cbm_pipeline_run(p), 0);
4767+
const char *project = cbm_pipeline_project_name(p);
4768+
4769+
cbm_store_t *s = cbm_store_open_path(db_path);
4770+
ASSERT_NOT_NULL(s);
4771+
4772+
/* (1) The false edge is suppressed (reproduce-first: RED before the fix). */
4773+
ASSERT_FALSE(cross_file_call_exists(s, project, "FileLen", "Close"));
4774+
/* (2) The same-package typed-receiver call survives (LSP / same_module —
4775+
* both outside the weak drop-list). */
4776+
ASSERT_TRUE(cross_file_call_exists(s, project, "Boot", "Close"));
4777+
/* (3) The bare local call survives (is_method stays false for bare calls). */
4778+
ASSERT_TRUE(cross_file_call_exists(s, project, "CallsLocal", "localHelper"));
4779+
/* (4) The import-qualified cross-package call survives (import-aware
4780+
* strategies are outside the drop-list). */
4781+
ASSERT_TRUE(cross_file_call_exists(s, project, "UsesUtil", "Tag"));
4782+
4783+
cbm_store_close(s);
4784+
cbm_pipeline_free(p);
4785+
th_rmtree(tmp);
4786+
PASS();
4787+
}
4788+
46964789
/* Fixture for the #1928 cross-language reference-guard probes (sequential and
46974790
* parallel twins). pad_files > 0 adds filler files to push the index over the
46984791
* parallel-pipeline threshold, since USAGE/WRITES/READS have one resolver per
@@ -13115,6 +13208,7 @@ SUITE(pipeline) {
1311513208
#endif
1311613209
RUN_TEST(pipeline_tsjs_receiver_suppresses_weak_method_edge);
1311713210
RUN_TEST(pipeline_python_receiver_suppresses_weak_method_edge);
13211+
RUN_TEST(pipeline_go_receiver_suppresses_weak_method_edge);
1311813212
RUN_TEST(pipeline_go_rw_usage_never_cross_into_c);
1311913213
RUN_TEST(pipeline_go_rw_usage_never_cross_into_c_parallel);
1312013214
RUN_TEST(pipeline_go_bare_ref_never_binds_field);

tests/test_registry.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,46 @@ TEST(dynamic_suppress_keeps_high_confidence_and_non_methods) {
901901
PASS();
902902
}
903903

904+
TEST(go_suppress_drops_weak_selector_matches) {
905+
/* Go selector call with an untyped receiver, landed via a receiver-blind
906+
* short-name strategy → drop (same failure class as #592/#606).
907+
* suffix_match/fuzzy drop at any confidence; unique_name drops only when
908+
* import-unreachability-penalized (CONF_UNIQUE_NAME 0.75 * 0.5 = 0.375 —
909+
* the `io.Copy` -> project `Copy` stdlib-hijack shape). */
910+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "suffix_match", 0.9));
911+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "suffix_match", 0.11));
912+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "fuzzy", 0.9));
913+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "unique_name", 0.375));
914+
PASS();
915+
}
916+
917+
TEST(go_suppress_keeps_typed_and_import_aware_matches) {
918+
/* Unpenalized unique_name = lone candidate inside the caller's import
919+
* closure (or an import-free file, e.g. same-package) — a genuinely-typed
920+
* lone-candidate call never enters the field-type-hint upgrade, so it must
921+
* survive (lrp_go_s8_field_type_hint). */
922+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "unique_name", 0.75));
923+
/* field_type_hint is receiver-aware for Go — struct fields carry declared
924+
* types (lrp_go_s8_field_type_hint) — so it stays, unlike the TS/JS list. */
925+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "field_type_hint", 0.85));
926+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "same_module", 0.9));
927+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "import_map", 0.95));
928+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "import_map_suffix", 0.9));
929+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "qualified_suffix", 0.9));
930+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "callee_suffix", 0.5));
931+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "service_pattern", 0.5));
932+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "lsp_strategy_cross_file", 0.92));
933+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "lsp_direct", 0.95));
934+
/* A bare call (is_method=false) is a free-function call → never suppressed. */
935+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, false, "suffix_match", 0.11));
936+
/* Non-Go languages are never affected by this gate. */
937+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(false, true, "suffix_match", 0.11));
938+
/* No match (NULL/empty strategy) → nothing to suppress. */
939+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, NULL, 0.5));
940+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "", 0.5));
941+
PASS();
942+
}
943+
904944
/* ── Suite ─────────────────────────────────────────────────────── */
905945

906946
/* Method call THROUGH an imported symbol that is itself an indexed node
@@ -997,4 +1037,6 @@ SUITE(registry) {
9971037
RUN_TEST(go_bare_ref_never_binds_field);
9981038
RUN_TEST(dynamic_suppress_drops_weak_method_matches);
9991039
RUN_TEST(dynamic_suppress_keeps_high_confidence_and_non_methods);
1040+
RUN_TEST(go_suppress_drops_weak_selector_matches);
1041+
RUN_TEST(go_suppress_keeps_typed_and_import_aware_matches);
10001042
}

0 commit comments

Comments
 (0)