mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
Merge push-correspondence-run-aware: run-aware base/head line-correspondence
Co-Authored-By: Claude Opus 4.8 <[email protected]>
This commit is contained in:
@@ -460,6 +460,47 @@ where reality differed.
|
|||||||
presign-capable endpoint; how that and the cluster's store access are provided is operator-owned infra, not
|
presign-capable endpoint; how that and the cluster's store access are provided is operator-owned infra, not
|
||||||
assumed here.
|
assumed here.
|
||||||
|
|
||||||
|
- **AX lane blocker (2026-10-11) — kube is the target lane, so this is tracked, not dismissed.** The replay
|
||||||
|
*dispatches* correctly but tasks do not *complete*: `pins_done` stayed 0 for both the 5-change proof and the
|
||||||
|
1,300-change slice 1. Sub-issues resolved along the way: (a) the replay image is `ousast-replay-task` (public);
|
||||||
|
(b) the ax CLI must be `~/go/bin/ax` — the `~/.local/ax-v0.3.1/ax` build calls an `UpdateTask` RPC the deployed
|
||||||
|
ax-server answers `Unimplemented` (client/server version mismatch); (c) the store endpoint
|
||||||
|
`files.because-security.com` / `sast-memory` passes the egress check. **Remaining blocker (AX/cluster-internal,
|
||||||
|
operator-owned):** dispatched tasks reach the `ousast-pool` workers and the replay actors execute and checkpoint,
|
||||||
|
but the result never returns to the coordinator and no completion is recorded. `ousast-pool` logs show a gVisor
|
||||||
|
cleanup failure: `runsc delete ... connecting to control server at PID NNN: connection refused` and `failed to
|
||||||
|
terminate workload after checkpoint ... runsc delete: exit status 128`. Until the completion / checkpoint-cleanup
|
||||||
|
path is fixed, the ax lane cannot produce results; the G0 baseline (7.2) was taken on the **docker lane**
|
||||||
|
instead. The ax lane must be restored before it is the baseline/G1 lane (the stated target).
|
||||||
|
|
||||||
|
- **Engine coverage gap is comparison, not deadline (2026-10-11, preliminary n=1).** The cold-cache default arm
|
||||||
|
(30s) times out the Joern CPG build → `quick_only`. But a long-deadline sample (`--arm long_deadline`, 300s) on a
|
||||||
|
vulnerability change ran **206s with no timeout and still did not cover** (`coverage_status: incomplete`,
|
||||||
|
`quick_only`): the reasons were `ambiguous_line_correspondence`, `unmatched_added_deleted_path_correspondence`,
|
||||||
|
`eligibility_provenance_mismatch`, `witness_unresolved`, `comparison_unknown` — i.e. the engine could not
|
||||||
|
correspond the replayed diff's lines between base and head. So **calibration / a longer deadline alone will not
|
||||||
|
give engine coverage**; the blocker is the engine's base↔head comparison on real replayed diffs (consistent with
|
||||||
|
the earlier "engine near-blind to fixes" slice-leak audit). This is G2 engine-fix territory (comparison /
|
||||||
|
line-correspondence), not a G0 deadline knob. To confirm beyond n=1, the long-deadline arm must run at scale
|
||||||
|
(needs the kube lane). Implication: the record-07 baseline is measured on the **quick-rule tier** (the default
|
||||||
|
arm), and the engine arm's value is gated on fixing the comparison path, not on calibration.
|
||||||
|
|
||||||
|
- **Line-correspondence fix made, but it is necessary-not-sufficient (2026-10-11).** Root cause confirmed in
|
||||||
|
`src/openultrasast/push/snapshot.py`: equal-region base↔head line pairs were mapped only when the line content
|
||||||
|
was *globally unique in the file*, so common lines (`}`, blanks, imports) tripped `ambiguous_line_correspondence`
|
||||||
|
and the finding's anchor was discarded → `comparison_unknown` → finding suppressed. Fixed to **run-aware
|
||||||
|
positional anchoring**: a pair maps when its content occupies exactly one contiguous equal run per side (intra-run
|
||||||
|
duplicates are forced/safe; cross-run duplicates stay withheld — soundness preserved, `test_push_snapshot.py`
|
||||||
|
moved-operation guard still passes). 163 push tests pass. BUT live-testing the fix (mounted over the installed
|
||||||
|
package, 300s) on a real messy change (a 14-file Java diff) did **not** flip coverage: it stayed `incomplete`
|
||||||
|
because the binding constraints are elsewhere — `capability_unavailable` (empty capability registry always blocks
|
||||||
|
`coverage_status=complete` until Task 8.3), the engine produced **no actionable defect at all**
|
||||||
|
(`actionable_defect_ids: []`, a detection gap, not correspondence), plus `eligibility_provenance_mismatch` and
|
||||||
|
`contributor-scan ... projection_unavailable`. So "near-blind to fixes" is **multi-causal**; the correspondence
|
||||||
|
fix removes one real cause but engine coverage on unseen diffs also needs the capability registry (Task 8.3), the
|
||||||
|
engine's actual vuln detection, and the eligibility/projection paths. Validating the correspondence fix's benefit
|
||||||
|
needs a change where correspondence is the sole blocker (a small single-file diff), or the engine arm at scale.
|
||||||
|
|
||||||
## Open Questions
|
## Open Questions
|
||||||
|
|
||||||
Maintainer decisions of 2026-10-04 resolve questions 1, 2, 4 and 5; question 3 stays open for G3.
|
Maintainer decisions of 2026-10-04 resolve questions 1, 2, 4 and 5; question 3 stays open for G3.
|
||||||
|
|||||||
@@ -404,12 +404,31 @@ class SnapshotAdapter:
|
|||||||
spans.append(ChangedSpan(path_head.hex(), new_start, new_start + new_count - 1, "head"))
|
spans.append(ChangedSpan(path_head.hex(), new_start, new_start + new_count - 1, "head"))
|
||||||
old_cursor, new_cursor = old_index + old_count, new_index + new_count
|
old_cursor, new_cursor = old_index + old_count, new_index + new_count
|
||||||
pairs.extend(_equal_line_pairs(old_lines, new_lines, old_cursor, len(old_lines), new_cursor, len(new_lines)))
|
pairs.extend(_equal_line_pairs(old_lines, new_lines, old_cursor, len(old_lines), new_cursor, len(new_lines)))
|
||||||
old_counts, new_counts = Counter(old_lines), Counter(new_lines)
|
# Equal-region pairs are positionally 1:1 within a contiguous run
|
||||||
|
# (strictly +1/+1 from _equal_line_pairs). A line's content is a
|
||||||
|
# trustworthy positional anchor when it occupies exactly one old and
|
||||||
|
# one new run; content repeated across runs split by an edit is aligned
|
||||||
|
# arbitrarily by git and is withheld as ambiguous. Duplicates within one
|
||||||
|
# run stay forced and safe.
|
||||||
|
anchor_pairs = () if ambiguous_rename else pairs
|
||||||
|
anchor_runs: list[list[tuple[int, int]]] = []
|
||||||
|
for pair in anchor_pairs:
|
||||||
|
last = anchor_runs[-1][-1] if anchor_runs else None
|
||||||
|
if last is not None and pair[0] == last[0] + 1 and pair[1] == last[1] + 1:
|
||||||
|
anchor_runs[-1].append(pair)
|
||||||
|
else:
|
||||||
|
anchor_runs.append([pair])
|
||||||
|
old_runs: dict[bytes, set[int]] = {}
|
||||||
|
new_runs: dict[bytes, set[int]] = {}
|
||||||
|
for run_index, run in enumerate(anchor_runs):
|
||||||
|
for old_index, new_index in run:
|
||||||
|
old_runs.setdefault(old_lines[old_index], set()).add(run_index)
|
||||||
|
new_runs.setdefault(new_lines[new_index], set()).add(run_index)
|
||||||
ambiguous = False
|
ambiguous = False
|
||||||
for old_index, new_index in () if ambiguous_rename else pairs:
|
for old_index, new_index in anchor_pairs:
|
||||||
_check_budget(budget, cancelled)
|
_check_budget(budget, cancelled)
|
||||||
content = old_lines[old_index]
|
content = old_lines[old_index]
|
||||||
if old_counts[content] != 1 or new_counts[content] != 1:
|
if len(old_runs.get(content, ())) != 1 or len(new_runs.get(content, ())) != 1:
|
||||||
ambiguous = True
|
ambiguous = True
|
||||||
continue
|
continue
|
||||||
mappings.append(LineCorrespondence(path_base.hex(), path_head.hex(), old_index + 1, old_index + 1, new_index + 1))
|
mappings.append(LineCorrespondence(path_base.hex(), path_head.hex(), old_index + 1, old_index + 1, new_index + 1))
|
||||||
|
|||||||
@@ -553,9 +553,12 @@ def test_change_context_unknown_base_binary_and_ambiguous_lines_are_unresolved(r
|
|||||||
assert "comparison_base_unavailable" in missing.unresolved_boundaries
|
assert "comparison_base_unavailable" in missing.unresolved_boundaries
|
||||||
result = SnapshotAdapter(repo).compare(comparison(base, head))
|
result = SnapshotAdapter(repo).compare(comparison(base, head))
|
||||||
assert "declaration_paths_unavailable" in result.unresolved_boundaries
|
assert "declaration_paths_unavailable" in result.unresolved_boundaries
|
||||||
assert any("ambiguous_line_correspondence" in b for b in result.unresolved_boundaries)
|
# The two unchanged `sink();` lines form one contiguous equal run, so they anchor
|
||||||
|
# positionally (intra-run duplicates are forced) and are no longer ambiguous.
|
||||||
|
assert not any("ambiguous_line_correspondence" in b for b in result.unresolved_boundaries)
|
||||||
|
assert {(m.base_start_line, m.head_start_line) for m in result.line_correspondences} >= {(2, 2), (3, 3)}
|
||||||
|
# The binary file still cannot be line-corresponded.
|
||||||
assert any("binary_line_correspondence_unavailable" in b for b in result.unresolved_boundaries)
|
assert any("binary_line_correspondence_unavailable" in b for b in result.unresolved_boundaries)
|
||||||
assert not result.line_correspondences
|
|
||||||
|
|
||||||
|
|
||||||
def test_change_context_is_bounded_and_never_executes_diff_helpers(repo: Path) -> None:
|
def test_change_context_is_bounded_and_never_executes_diff_helpers(repo: Path) -> None:
|
||||||
@@ -584,6 +587,36 @@ def test_change_context_moved_repeated_operations_do_not_claim_identity(repo: Pa
|
|||||||
assert result.decode_path(result.changed_paths[0]) == "api.js"
|
assert result.decode_path(result.changed_paths[0]) == "api.js"
|
||||||
|
|
||||||
|
|
||||||
|
def test_change_context_repeated_lines_in_one_run_map_positionally(repo: Path) -> None:
|
||||||
|
# Three blank lines and a sink form one contiguous equal run (only line 1 changed);
|
||||||
|
# intra-run duplicates are forced, so they all anchor without ambiguity.
|
||||||
|
base = binary_commit(repo, [(b"100644", b"a.py", b"old()\n\n\n\nsink()\n")])
|
||||||
|
head = binary_commit(repo, [(b"100644", b"a.py", b"new()\n\n\n\nsink()\n")])
|
||||||
|
result = SnapshotAdapter(repo).compare(comparison(base, head), declaration_paths=())
|
||||||
|
assert not any("ambiguous_line_correspondence" in b for b in result.unresolved_boundaries)
|
||||||
|
assert {m.base_start_line for m in result.line_correspondences} >= {2, 3, 4, 5}
|
||||||
|
|
||||||
|
|
||||||
|
def test_change_context_repeated_lines_split_across_hunks_are_withheld(repo: Path) -> None:
|
||||||
|
# 'dup()' appears in two equal runs split by the changed a/b/c lines; git's alignment
|
||||||
|
# of cross-run repeats is arbitrary, so those pairs are withheld and flagged.
|
||||||
|
base = binary_commit(repo, [(b"100644", b"a.py", b"a()\ndup()\nb()\ndup()\nc()\n")])
|
||||||
|
head = binary_commit(repo, [(b"100644", b"a.py", b"A()\ndup()\nB()\ndup()\nC()\n")])
|
||||||
|
result = SnapshotAdapter(repo).compare(comparison(base, head), declaration_paths=())
|
||||||
|
assert any("ambiguous_line_correspondence" in b for b in result.unresolved_boundaries)
|
||||||
|
assert not any(m.base_start_line in (2, 4) for m in result.line_correspondences)
|
||||||
|
|
||||||
|
|
||||||
|
def test_change_context_distinctive_line_anchors_despite_recurring_braces(repo: Path) -> None:
|
||||||
|
# '}' recurs across two runs (ambiguous), but the distinctive sink line is a single-run
|
||||||
|
# anchor and must still map -- this is the real "fix touches surrounding code" case.
|
||||||
|
base = binary_commit(repo, [(b"100644", b"a.js", b"f() {\n old();\n}\ng() {\n distinctive(x);\n}\n")])
|
||||||
|
head = binary_commit(repo, [(b"100644", b"a.js", b"f() {\n new();\n}\nh() {\n distinctive(x);\n}\n")])
|
||||||
|
result = SnapshotAdapter(repo).compare(comparison(base, head), declaration_paths=())
|
||||||
|
assert any("ambiguous_line_correspondence" in b for b in result.unresolved_boundaries)
|
||||||
|
assert any(m.base_start_line == 5 and m.head_start_line == 5 for m in result.line_correspondences)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.parametrize("limit", ["max_total_bytes", "max_diff_bytes", "max_mapping_lines", "max_files"])
|
@pytest.mark.parametrize("limit", ["max_total_bytes", "max_diff_bytes", "max_mapping_lines", "max_files"])
|
||||||
def test_change_context_total_limits_are_explicit(repo: Path, limit: str) -> None:
|
def test_change_context_total_limits_are_explicit(repo: Path, limit: str) -> None:
|
||||||
from openultrasast.push.snapshot import SnapshotLimits
|
from openultrasast.push.snapshot import SnapshotLimits
|
||||||
|
|||||||
Reference in New Issue
Block a user