diff --git a/.kiro/specs/contributor-scan/tasks.md b/.kiro/specs/contributor-scan/tasks.md index 7464035..70d8df7 100644 --- a/.kiro/specs/contributor-scan/tasks.md +++ b/.kiro/specs/contributor-scan/tasks.md @@ -135,7 +135,7 @@ - _Requirements: 4.4, 8.1_ - _Depends: 2.2_ -- [ ] 2.5 Module-level regions the engine can actually ask about +- [x] 2.5 Module-level regions the engine can actually ask about - A setting in `if __name__ == '__main__':` belongs to a region the mapper calls `app.py:__main__`, but Joern names that method ``. The function filter matches nothing, so module-level configuration is invisible. Before 2.2's file scoping it was worse, not better: the literal was reported eight times diff --git a/benchmarks/measurements/2026-09-09-repo-vampi-module-scope.json b/benchmarks/measurements/2026-09-09-repo-vampi-module-scope.json new file mode 100644 index 0000000..180a236 --- /dev/null +++ b/benchmarks/measurements/2026-09-09-repo-vampi-module-scope.json @@ -0,0 +1,140 @@ +{ + "against": "benchmarks/measurements/2026-09-09-repo-vampi-crossfunction.json (task 2.4, same checkout and budget)", + "caveat": "Still one 520-line repository, and still the one every change this session was developed against. is what Joern's Python frontend calls a module body; other frontends name it differently (JavaScript uses :program), so this is verified for Python only.", + "completed": true, + "config": "shipped defaults", + "cost": "3 more regions (22 -> 25) for about 4s more query time and no more model calls, because the budget is a total and was already spent. regions_unasked rose 17 -> 20 accordingly: those regions got the free pass and are reported as unasked rather than as clean.", + "degradations": [ + { + "detail": "no path records; obligations evaluated within entry-point handlers only", + "files": [ + "api_views/books.py", + "api_views/users.py", + "models/books_model.py", + "models/user_model.py" + ], + "language": "python", + "reason": "obligations_function_local", + "stage": "obligations" + }, + { + "reason": "budget_exhausted", + "regions_unasked": 20, + "stage": "model" + }, + { + "reason": "hunter_model_unavailable", + "stage": "map" + } + ], + "delta": { + "cost_usd": "0.0067 -> 0.0068", + "entailed_findings": "8 -> 9", + "peak_rss_mb": "827.3 -> 822.9", + "query_seconds": "37.44 -> 41.32", + "regions": "22 -> 25", + "scan_seconds": "182.2 -> 185.77", + "vampi_bind_address": "8 findings in the wrong files (pre-2.2) -> 0 (2.2) -> 1, at app.py:17:" + }, + "exit_code": 0, + "findings_by_rung": { + "execution_confirmed": 0, + "model_corroborated": 0, + "model_entailed": 9, + "suspicion": 54 + }, + "findings_total": 63, + "generated": "2026-09-09", + "how": { + "a_module_body_is_not_a_handler": "Caught by this run: the marker-derived region inherited the handler families and entailed a missing authorization check on the main guard -- a finding with no possible remediation. Module regions are offered the families a file gets, not the families a handler gets.", + "a_module_is_its_own_body_not_its_children": "A module method spans its whole file, so the taint query's line-range containment made the parent of every function in it, and a module region would have reported every handler's sinks a second time. Under module scope, membership is direct.", + "every_function_scoped_file_gets_its_module_body": "A file whose regions are all function-scoped has code in no function -- settings, route registration, globals -- that no region can see. A file-level region beside them would report every handler's findings twice, which is why the earlier rule dropped it. A module-scope region instead PARTITIONS the file. Adding it was only cheap after batching: before that, a region was a JVM.", + "the_main_guard_marker_is_not_a_function": "__main__ is what the mapper emits for the `if __name__` main guard. Sent to the engine as a function name it matched nothing, because a CPG calls a file's top-level code . The region model translates the marker -- the one place that turns what the mapper meant into what the scanner asks." + }, + "known_outcomes": [ + { + "family": "injection", + "finding_id": "model:injection:models/user_model.py:73:get_user", + "id": "VAmPI-SQLI", + "line": 73, + "location": "models/user_model.py:get_user:73", + "matches": 3, + "result": "found", + "rung": "model_entailed" + }, + { + "family": "access_control", + "finding_id": "model:access_control:api_views/users.py:?:update_password", + "id": "VAmPI-BOLA", + "line": null, + "location": "api_views/users.py:update_password:187", + "matches": 5, + "result": "found", + "rung": "model_entailed" + } + ], + "mode": "standard", + "model": { + "arbitrate_seconds": 121.96, + "build_seconds": 5.43, + "by_rung": { + "execution_confirmed": 0, + "model_corroborated": 0, + "model_entailed": 9, + "suspicion": 38 + }, + "cost_usd": 0.0068, + "model_calls": 105, + "query_seconds": 41.32, + "regions_scanned": 25, + "regions_unasked": 20, + "regions_unjudged": 0, + "seconds": 168.71, + "unjudged_sample": [] + }, + "peak_rss_mb": 822.9, + "reading": "Module-level code is now askable. VAmPI's host='0.0.0.0' is reported exactly once, at app.py:17:, where before task 2.2 it was reported eight times in eight files that do not contain it, and after 2.2 it was reported nowhere. All three findings the model layer can reach at a known location are now correct: the SQL injection, the BOLA, and the permissive bind address.", + "repo": { + "commit": "f16052dce83f05847133ec98f01c5193a41de7d8", + "known": [ + { + "family": "injection", + "id": "VAmPI-SQLI", + "in_scope": true, + "location": "models/user_model.py:get_user:73", + "weakness": "SQL injection: the username is interpolated into the query string at line 72 and executed at 73" + }, + { + "family": "access_control", + "id": "VAmPI-BOLA", + "in_scope": true, + "location": "api_views/users.py:update_password:187", + "weakness": "unauthorized password change: the path username is used instead of the authenticated token subject" + } + ], + "language": "python", + "license": "MIT", + "measures": [ + "detection" + ], + "name": "vampi", + "url": "https://github.com/erev0s/VAmPI.git" + }, + "scan_seconds": 185.77, + "spec": "contributor-scan 2.5 (Req 4.4)", + "stages": { + "completed": [ + "static", + "map" + ], + "requested": [ + "static", + "map" + ], + "skipped": [] + }, + "still_open": [ + "2.6 access control entails 7 of 9 handlers, including register_user and get_all_users, which are meant to be public. Unchanged, and still the largest precision problem.", + "2.7 dominance witnesses carry no line, so access-control sites still read api_views/users.py:?:update_password." + ] +} diff --git a/src/openultrasast/cpg/queries/taint.sc b/src/openultrasast/cpg/queries/taint.sc index c186ca8..800e1c3 100644 --- a/src/openultrasast/cpg/queries/taint.sc +++ b/src/openultrasast/cpg/queries/taint.sc @@ -113,14 +113,21 @@ seen } + // A module body spans its whole file, so line-range containment would make `` the parent of every + // function in it -- and a module-scope region would then report every handler's sinks a second time. The + // module region exists to cover the code that is in NO function, so its membership is direct. + val moduleScope = function == "" + def nestedInLabeled(m: io.shiftleft.codepropertygraph.generated.nodes.Method): Boolean = - labeledMethods.exists { lm => - m.name == function || ( - m.filename == lm.filename && - m.lineNumber.getOrElse(-1) >= lm.lineNumber.getOrElse(0) && - m.lineNumberEnd.getOrElse(-1) <= lm.lineNumberEnd.getOrElse(Int.MaxValue) - ) - } + if (moduleScope) m.name == function + else + labeledMethods.exists { lm => + m.name == function || ( + m.filename == lm.filename && + m.lineNumber.getOrElse(-1) >= lm.lineNumber.getOrElse(0) && + m.lineNumberEnd.getOrElse(-1) <= lm.lineNumberEnd.getOrElse(Int.MaxValue) + ) + } // Parameter sources: the labeled function's OWN parameters, never a nested closure's. In a function-level // pair the function boundary IS the trust boundary -- the corpus is built so the labeled function's inputs diff --git a/src/openultrasast/model/regions.py b/src/openultrasast/model/regions.py index 844c739..4ec2fff 100644 --- a/src/openultrasast/model/regions.py +++ b/src/openultrasast/model/regions.py @@ -39,6 +39,15 @@ _ACCESS_RANK: dict[str, float] = { _LANGUAGES = {"python", "javascript", "typescript", "java", "c", "c_cpp"} _NORMALISE = {"typescript": "javascript", "c_cpp": "c"} +# What a CPG calls a file's top-level code. A module body is a method like any other to the engine, and +# naming it here is what lets a region ask about the code that is not inside any function. +MODULE_SCOPE = "" + +# What the mapper emits for `if __name__ == '__main__':` -- a marker for a module-level entry point, not +# the name of a function. Sent to the engine as a function name it matched nothing, so a repository's +# module-level settings were unaskable: VAmPI's `host='0.0.0.0'` went unreported. +_MODULE_MARKERS = frozenset({"__main__"}) + @dataclass(frozen=True) class ScanRegion: @@ -49,7 +58,7 @@ class ScanRegion: language: str families: tuple[str, ...] rank: float - source: str # "entry_point" | "file_fallback" + source: str # "entry_point" | "module_scope" | "file_fallback" def regions_for(entries: Sequence[object], targets: Sequence[object]) -> tuple[ScanRegion, ...]: @@ -78,16 +87,50 @@ def regions_for(entries: Sequence[object], targets: Sequence[object]) -> tuple[S if not families: continue function = getattr(entry, "function_name", None) or None + module_body = function in _MODULE_MARKERS + if module_body: + # A module body is not a handler, whatever the mapper called it, so it is offered the families a + # file gets rather than the families a handler gets. Carrying `access_control` here entailed a + # missing authorization check on `if __name__ == '__main__':`. + function, families = MODULE_SCOPE, _families(language, has_handler=False) + if not families: + continue rank = _ACCESS_RANK.get(str(getattr(entry, "access_level", "")), 0.5) key = (path, function) current = best.get(key) if current is None or rank > current.rank: - best[key] = ScanRegion(path=path, function=function, language=language, families=families, rank=rank, source="entry_point") + best[key] = ScanRegion( + path=path, + function=function, + language=language, + families=families, + rank=rank, + source="module_scope" if module_body else "entry_point", + ) # A nameless entry point IS the file. Where a named region already covers that file it adds nothing but # a second full-file pass, so it is dropped -- unless the file has no named region at all. named_paths = {path for path, function in best if function is not None} regions = [region for (path, function), region in best.items() if function is not None or path not in named_paths] + + # Every file whose regions are all function-scoped still has code that is in no function: the settings, the + # route registrations, the globals. Those regions cannot see it -- their queries filter by function name -- + # and there is no file-level region beside them, because one would duplicate every function's findings. + # So the module body gets its own region, which partitions the file rather than overlapping it. + # + # This was cheap to add only after batching: before it, a region was a JVM. + scoped = {region.path for region in regions if region.function is not None} + have_module = {region.path for region in regions if region.function == MODULE_SCOPE} + for path in sorted(scoped - have_module): + language = by_path.get(path) + if language is None: + continue + families = _families(language, has_handler=False) + if families: + regions.append( + ScanRegion(path=path, function=MODULE_SCOPE, language=language, families=families, rank=0.15, source="module_scope") + ) + covered = {region.path for region in regions} for path, language in by_path.items(): diff --git a/src/openultrasast/model/scan.py b/src/openultrasast/model/scan.py index e8b3ed8..6ab888f 100644 --- a/src/openultrasast/model/scan.py +++ b/src/openultrasast/model/scan.py @@ -31,7 +31,7 @@ from .config_value import request_params as config_params from .dominance import request_params as dominance_params from .ladder import Rung from .pipeline import ModelFinding, scan_region -from .regions import ScanRegion +from .regions import MODULE_SCOPE, ScanRegion from .specs import ConfigSpec, DominanceSpec, TaintSpec, config_specs, dominance_specs, taint_specs from .taint import request_params as taint_params from .taxonomy import load_families @@ -261,8 +261,11 @@ def _is_entry_point(region: ScanRegion) -> bool: A named function the entry-point mapper found, not a file the walker fell back to. The distinction is what makes treating parameters as untrusted sound: a handler's parameters carry request data, an arbitrary helper's carry whatever its caller had. + + A module body is excluded even when the mapper marked it an entry point: a module has no parameters, so + there is nothing for the parameter-source rule to mean there. """ - return region.source == "entry_point" and bool(region.function) + return region.source == "entry_point" and bool(region.function) and region.function != MODULE_SCOPE def _spec_for(family: str, language: str) -> ArbiterSpec | None: diff --git a/tests/test_model_regions.py b/tests/test_model_regions.py index f7be218..fae554b 100644 --- a/tests/test_model_regions.py +++ b/tests/test_model_regions.py @@ -47,9 +47,9 @@ def test_an_entry_point_becomes_a_region() -> None: from openultrasast.model.regions import regions_for regions = regions_for([_entry()], [_target()]) - assert len(regions) == 1 - assert regions[0].path == "api/users.py" and regions[0].function == "update_password" - assert regions[0].source == "entry_point" + handlers = [r for r in regions if r.source == "entry_point"] + assert len(handlers) == 1 + assert handlers[0].path == "api/users.py" and handlers[0].function == "update_password" def test_a_file_with_no_entry_point_still_gets_a_region_that_says_so() -> None: @@ -91,7 +91,7 @@ def test_public_entry_points_outrank_internal_ones() -> None: entries = [_entry(path="a.py", function="internal", access="local-only"), _entry(path="b.py", function="exposed", access="public")] targets = [_target(path="a.py"), _target(path="b.py")] - ranked = regions_for(entries, targets) + ranked = [r for r in regions_for(entries, targets) if r.source == "entry_point"] assert [r.function for r in ranked] == ["exposed", "internal"], "highest risk first" assert ranked[0].rank > ranked[1].rank @@ -109,7 +109,50 @@ def test_a_file_with_an_entry_point_is_not_also_a_fallback_region() -> None: from openultrasast.model.regions import regions_for regions = regions_for([_entry()], [_target()]) - assert len(regions) == 1 + assert not [r for r in regions if r.source == "file_fallback"] + + +def test_a_function_scoped_file_still_gets_its_module_body_asked_about() -> None: + """contributor-scan 2.5. Settings, route registration and globals are in no function. + + A file whose regions are all function-scoped cannot see them -- every query filters by function name -- + and a file-level region beside them would report every handler's findings a second time. So the module + body gets a region of its own, which partitions the file rather than overlapping it. VAmPI's + `host='0.0.0.0'` sits in `if __name__ == '__main__':` and was reported nowhere without this. + """ + from openultrasast.model.regions import MODULE_SCOPE, regions_for + + regions = regions_for([_entry()], [_target()]) + + module = [r for r in regions if r.function == MODULE_SCOPE] + assert len(module) == 1, "exactly one module region per function-scoped file" + assert module[0].path == "api/users.py" + assert module[0].source == "module_scope" + assert module[0].rank < min(r.rank for r in regions if r.source == "entry_point") + + +def test_a_file_level_region_is_not_given_a_second_module_region() -> None: + """A file-level region already covers the module body; a second one would duplicate it.""" + from openultrasast.model.regions import MODULE_SCOPE, regions_for + + regions = regions_for([], [_target(path="config.py")]) + + assert [r.function for r in regions] == [None] + assert not [r for r in regions if r.function == MODULE_SCOPE] + + +def test_the_main_guard_marker_is_translated_to_module_scope() -> None: + """`__main__` is what the mapper emits for `if __name__ == '__main__':`, not a function that exists. + + Sent to the engine as a function name it matched nothing at all, which is why a module-level setting + could be reported zero times while looking like a scanned file. + """ + from openultrasast.model.regions import MODULE_SCOPE, regions_for + + regions = regions_for([_entry(path="app.py", function="__main__", access="local-only")], [_target(path="app.py")]) + + assert MODULE_SCOPE in {r.function for r in regions} + assert "__main__" not in {r.function for r in regions} def test_an_unsupported_language_yields_no_region() -> None: @@ -133,15 +176,16 @@ def test_nameless_entry_points_do_not_duplicate_a_file_already_covered() -> None _entry(function=None, access="public"), ] regions = regions_for(entries, [_target()]) - assert len(regions) == 1 - assert regions[0].function == "ping" + assert [r.function for r in regions if r.source == "entry_point"] == ["ping"] + # The module region partitions the file rather than duplicating the handler, so it is not a second pass. + assert not [r for r in regions if r.function is None] def test_a_file_with_only_nameless_entry_points_still_gets_one_region() -> None: from openultrasast.model.regions import regions_for regions = regions_for([_entry(function=None), _entry(function=None)], [_target()]) - assert len(regions) == 1 and regions[0].function is None + assert [r.function for r in regions] == [None], "two nameless entries are one file-level region" def test_two_named_handlers_in_one_file_are_two_regions() -> None: @@ -149,4 +193,20 @@ def test_two_named_handlers_in_one_file_are_two_regions() -> None: from openultrasast.model.regions import regions_for regions = regions_for([_entry(function="a"), _entry(function="b")], [_target()]) - assert sorted(r.function for r in regions) == ["a", "b"] + assert sorted(r.function for r in regions if r.source == "entry_point") == ["a", "b"] + + +def test_a_module_body_is_not_offered_the_handler_families() -> None: + """`if __name__ == '__main__':` is not an endpoint, whatever the mapper labelled it. + + Carrying `access_control` into a module region entailed a missing authorization check on a main guard -- + a finding with no possible remediation, on the first repository the region existed. + """ + from openultrasast.model.regions import MODULE_SCOPE, regions_for + + regions = regions_for([_entry(path="app.py", function="__main__", access="local-only")], [_target(path="app.py")]) + module = next(r for r in regions if r.function == MODULE_SCOPE) + + assert "access_control" not in module.families + assert "config_secrets" in module.families, "settings are exactly what module bodies carry" + assert module.source == "module_scope"