Skip to content

Commit f76971f

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 3de05cd commit f76971f

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
@@ -3551,6 +3551,24 @@ CBMInvocationDescriptor handle_calls(CBMExtractCtx *ctx, TSNode node, const CBML
35513551
}
35523552
}
35533553
}
3554+
// Go receiver-aware guard (same direction as the TS/JS flag above).
3555+
// Flag a selector call x.foo(). The Go AST cannot separate a method
3556+
// call on a value from a package-qualified call — but every selector
3557+
// call the Go LSP or the import/qualified registry strategies CAN
3558+
// place never reaches the weak short-name guards, so the flag only
3559+
// bites on unresolvable receivers (`f.Close()` on an os.File,
3560+
// `sha256.New()` behind an unindexed import), where a project-wide
3561+
// short-name match fabricates an edge to an unrelated project
3562+
// symbol sharing the name. Bare calls (helper()) keep
3563+
// is_method=false and resolve same-module/import paths as before.
3564+
if (ctx->language == CBM_LANG_GO &&
3565+
strcmp(ts_node_type(node), "call_expression") == 0) {
3566+
TSNode gofn = ts_node_child_by_field_name(node, TS_FIELD("function"));
3567+
if (!ts_node_is_null(gofn) &&
3568+
strcmp(ts_node_type(gofn), "selector_expression") == 0) {
3569+
call.is_method = true;
3570+
}
3571+
}
35543572

35553573
TSNode args = ts_node_child_by_field_name(node, TS_FIELD("arguments"));
35563574
// 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
@@ -2478,12 +2478,16 @@ static void resolve_file_calls(resolve_ctx_t *rc, resolve_worker_state_t *ws, CB
24782478
* #606 direction.
24792479
*
24802480
* This language set MUST match the one in pass_calls.c exactly — see the
2481-
* note there. ArkTS belongs to the JS/TS family (#1842). */
2481+
* note there. ArkTS belongs to the JS/TS family (#1842). Go (#1906)
2482+
* composes via its own predicate (different drop-list — see
2483+
* cbm_go_suppress_weak_method_match), mirrored in pass_calls.c. */
24822484
bool suppress_weak_member = lang == CBM_LANG_PYTHON || lang == CBM_LANG_JAVASCRIPT ||
24832485
lang == CBM_LANG_TYPESCRIPT || lang == CBM_LANG_TSX ||
24842486
lang == CBM_LANG_ARKTS;
24852487
bool drop_plain_call =
2486-
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy);
2488+
cbm_suppress_weak_member_match(suppress_weak_member, call->is_method, res.strategy) ||
2489+
cbm_go_suppress_weak_method_match(lang == CBM_LANG_GO, call->is_method, res.strategy,
2490+
res.confidence);
24872491

24882492
/* Service-pattern HTTP/ASYNC client call (`requests.get(url)`): the
24892493
* 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
@@ -4690,9 +4690,14 @@ TEST(extract_perl_method_call_flags_is_method) {
46904690
/* Languages OUTSIDE the is_method flag set (only Perl and TS/JS/TSX set it) must
46914691
* be unaffected: a Go method call never sets is_method. */
46924692
TEST(extract_flag_exempt_method_call_not_flagged_is_method) {
4693-
CBMFileResult *r = extract("package m\n"
4694-
"func run(o Obj) { o.Commit(); helper() }\n",
4695-
CBM_LANG_GO, "t", "x.go");
4693+
/* Rust is flag-exempt: only Perl, Python, TS/JS and Go set is_method.
4694+
* Guards the blast radius of the receiver-aware flags for every other
4695+
* language. */
4696+
CBMFileResult *r = extract("fn run(o: Obj) {\n"
4697+
" o.commit();\n"
4698+
" helper();\n"
4699+
"}\n",
4700+
CBM_LANG_RUST, "t", "x.rs");
46964701
ASSERT_NOT_NULL(r);
46974702
ASSERT_FALSE(r->has_error);
46984703
for (int i = 0; i < r->calls.count; i++) {
@@ -4771,6 +4776,33 @@ TEST(extract_python_member_call_flags_is_method) {
47714776
PASS();
47724777
}
47734778

4779+
TEST(extract_go_selector_call_flags_is_method) {
4780+
/* Go selector calls are flagged so the weak-match guard can fire when the
4781+
* Go LSP cannot type the receiver; bare calls stay unflagged. */
4782+
CBMFileResult *r = extract("package m\n"
4783+
"func run(o Obj) { o.Commit(); helper() }\n",
4784+
CBM_LANG_GO, "t", "x.go");
4785+
ASSERT_NOT_NULL(r);
4786+
ASSERT_FALSE(r->has_error);
4787+
bool saw_selector = false;
4788+
bool saw_bare = false;
4789+
for (int i = 0; i < r->calls.count; i++) {
4790+
const CBMCall *c = &r->calls.items[i];
4791+
if (c->callee_name && strstr(c->callee_name, "Commit") != NULL) {
4792+
ASSERT_TRUE(c->is_method);
4793+
saw_selector = true;
4794+
}
4795+
if (c->callee_name && strcmp(c->callee_name, "helper") == 0) {
4796+
ASSERT_FALSE(c->is_method);
4797+
saw_bare = true;
4798+
}
4799+
}
4800+
ASSERT_TRUE(saw_selector);
4801+
ASSERT_TRUE(saw_bare);
4802+
cbm_free_result(r);
4803+
PASS();
4804+
}
4805+
47744806
/* TS/JS/TSX receiver-aware flag (#592/#606; same intent as the Perl flag above).
47754807
* A member call x.foo() with a non-this/super receiver is flagged is_method so
47764808
* the resolver can suppress a weak short-name match (`re.test()` must not bind a
@@ -6450,6 +6482,7 @@ SUITE(extraction) {
64506482
RUN_TEST(extract_perl_method_call_flags_is_method);
64516483
RUN_TEST(extract_flag_exempt_method_call_not_flagged_is_method);
64526484
RUN_TEST(extract_python_member_call_flags_is_method);
6485+
RUN_TEST(extract_go_selector_call_flags_is_method);
64536486
RUN_TEST(extract_ts_member_call_flags_is_method);
64546487
RUN_TEST(extract_ts_this_super_receiver_not_flagged);
64556488
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
@@ -4688,6 +4688,99 @@ TEST(pipeline_python_receiver_suppresses_weak_method_edge) {
46884688
PASS();
46894689
}
46904690

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

tests/test_registry.c

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -853,6 +853,46 @@ TEST(dynamic_suppress_keeps_high_confidence_and_non_methods) {
853853
PASS();
854854
}
855855

856+
TEST(go_suppress_drops_weak_selector_matches) {
857+
/* Go selector call with an untyped receiver, landed via a receiver-blind
858+
* short-name strategy → drop (same failure class as #592/#606).
859+
* suffix_match/fuzzy drop at any confidence; unique_name drops only when
860+
* import-unreachability-penalized (CONF_UNIQUE_NAME 0.75 * 0.5 = 0.375 —
861+
* the `io.Copy` -> project `Copy` stdlib-hijack shape). */
862+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "suffix_match", 0.9));
863+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "suffix_match", 0.11));
864+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "fuzzy", 0.9));
865+
ASSERT_TRUE(cbm_go_suppress_weak_method_match(true, true, "unique_name", 0.375));
866+
PASS();
867+
}
868+
869+
TEST(go_suppress_keeps_typed_and_import_aware_matches) {
870+
/* Unpenalized unique_name = lone candidate inside the caller's import
871+
* closure (or an import-free file, e.g. same-package) — a genuinely-typed
872+
* lone-candidate call never enters the field-type-hint upgrade, so it must
873+
* survive (lrp_go_s8_field_type_hint). */
874+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "unique_name", 0.75));
875+
/* field_type_hint is receiver-aware for Go — struct fields carry declared
876+
* types (lrp_go_s8_field_type_hint) — so it stays, unlike the TS/JS list. */
877+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "field_type_hint", 0.85));
878+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "same_module", 0.9));
879+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "import_map", 0.95));
880+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "import_map_suffix", 0.9));
881+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "qualified_suffix", 0.9));
882+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "callee_suffix", 0.5));
883+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "service_pattern", 0.5));
884+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "lsp_strategy_cross_file", 0.92));
885+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "lsp_direct", 0.95));
886+
/* A bare call (is_method=false) is a free-function call → never suppressed. */
887+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, false, "suffix_match", 0.11));
888+
/* Non-Go languages are never affected by this gate. */
889+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(false, true, "suffix_match", 0.11));
890+
/* No match (NULL/empty strategy) → nothing to suppress. */
891+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, NULL, 0.5));
892+
ASSERT_FALSE(cbm_go_suppress_weak_method_match(true, true, "", 0.5));
893+
PASS();
894+
}
895+
856896
/* ── Suite ─────────────────────────────────────────────────────── */
857897

858898
/* Method call THROUGH an imported symbol that is itself an indexed node
@@ -947,4 +987,6 @@ SUITE(registry) {
947987
RUN_TEST(cross_language_suffix_match_drops_py_vs_js);
948988
RUN_TEST(dynamic_suppress_drops_weak_method_matches);
949989
RUN_TEST(dynamic_suppress_keeps_high_confidence_and_non_methods);
990+
RUN_TEST(go_suppress_drops_weak_selector_matches);
991+
RUN_TEST(go_suppress_keeps_typed_and_import_aware_matches);
950992
}

0 commit comments

Comments
 (0)