mirror of
https://github.com/norandom/OpenUltraSAST.git
synced 2026-10-16 03:49:26 +00:00
fix(hunt): diff producer records where a push removed lines; worktree checkout is proven complete
Two instrument findings from the first access-control feature table (952 rows, 74 positive rows): 1. A fix that adds a guard is, replayed base-to-head, a push that removes it. The diff producer only kept hunks with added lines at head, so 38 of 74 positive rows had no hunk in their file at all and 69 of 74 were "not on a changed line", while 60% of ordinary rows were. The diff annotation separated the labels by construction and was blind to exactly the access-control shape. The producer (version 2) now records per file the head lines after which the push removed code; distance and on_changed_line count them, the model's hunks input renders them as removed_after_line, and diff.deletion_count is an engineered feature. 2. 150 rows (16%, every positive of one repository) read empty files from the worktree although git holds the blobs; the build ran while another process checked out from the same clone. _full_checkout now proves the checkout: git status must be clean, a dirty tree is restored once, then InstrumentFailure checkout_incomplete. The table script cross-checks every unread row against the blob size and refuses to screen a partly unread tree, and records the diff-scoped recall ceiling (positive rows a label-blind enumeration would reach). Co-Authored-By: Claude Fable 5.1 <[email protected]>
This commit is contained in:
co-authored by
Claude Fable 5.1
parent
60cbefec9b
commit
537b61e8d9
@@ -20,6 +20,7 @@ ROW = {
|
||||
"label": 1,
|
||||
"family": "injection",
|
||||
"feature.diff.hunks": [[3, 3], [10, 12]],
|
||||
"feature.diff.deletions": [9],
|
||||
"feature.diff.on_changed_line": False,
|
||||
"feature.diff.distance": 2,
|
||||
"feature.source.function": "second",
|
||||
@@ -56,7 +57,7 @@ def test_feature_frame_engineers_counts_distances_and_flags():
|
||||
frame = features.feature_frame([ROW, absent])
|
||||
first = frame.iloc[0]
|
||||
assert first["diff.distance"] == 2 and first["diff.on_changed_line"] == 0
|
||||
assert first["diff.hunk_count"] == 2 and first["diff.hunk_lines"] == 4
|
||||
assert first["diff.hunk_count"] == 2 and first["diff.hunk_lines"] == 4 and first["diff.deletion_count"] == 1
|
||||
assert first["source.span_lines"] == 3 and first["source.is_module"] == 0
|
||||
assert first["source.caller_count"] == 3 and first["source.caller_files"] == 2 and first["source.entry_point_count"] == 1
|
||||
assert (first["semantic.sources"], first["semantic.sinks"], first["semantic.sanitizers"]) == (1, 2, 1)
|
||||
|
||||
@@ -20,8 +20,7 @@ def repository(tmp_path):
|
||||
git("init", "-q")
|
||||
git("config", "user.name", "Fixture")
|
||||
git("config", "user.email", "[email protected]")
|
||||
initial = ('def first():\n value = input()\n return value\n\n'
|
||||
'def second():\n value = first()\n return value\n')
|
||||
initial = "def first():\n value = input()\n return value\n\ndef second():\n value = first()\n return value\n"
|
||||
(tmp_path / "app.py").write_text(initial)
|
||||
(tmp_path / "notes.txt").write_text("GHSA-xxxx-xxxx-xxxx CVE-2024-0001\n")
|
||||
(tmp_path / "unknown.zzz").write_text("opaque\n")
|
||||
@@ -33,10 +32,22 @@ def repository(tmp_path):
|
||||
git("add", "app.py", "unknown.zzz")
|
||||
git("commit", "-qm", "fix XSS CVE-2024-0001")
|
||||
head = git("rev-parse", "HEAD")
|
||||
data = {"id": "change", "repo": "fixture/repo", "base": base, "head": head, "kind": "vulnerability", "fold": 1,
|
||||
"path": "app.py", "function": "second", "function_lines": [5, 7], "family": "injection", "case_id": "verified",
|
||||
"advisory": "GHSA-xxxx-xxxx-xxxx", "commit_message": "fix XSS CVE-2024-0001",
|
||||
"files": [{"path": "app.py", "head_lines": [[3, 3]]}]}
|
||||
data = {
|
||||
"id": "change",
|
||||
"repo": "fixture/repo",
|
||||
"base": base,
|
||||
"head": head,
|
||||
"kind": "vulnerability",
|
||||
"fold": 1,
|
||||
"path": "app.py",
|
||||
"function": "second",
|
||||
"function_lines": [5, 7],
|
||||
"family": "injection",
|
||||
"case_id": "verified",
|
||||
"advisory": "GHSA-xxxx-xxxx-xxxx",
|
||||
"commit_message": "fix XSS CVE-2024-0001",
|
||||
"files": [{"path": "app.py", "head_lines": [[3, 3]]}],
|
||||
}
|
||||
return tmp_path, data
|
||||
|
||||
|
||||
@@ -55,10 +66,11 @@ def test_every_column_candidates_and_cached_work(repository, monkeypatch):
|
||||
rows = compute_rows(root, change)
|
||||
assert len(calls) == 1
|
||||
table = FeatureTable(rows)
|
||||
assert len(table.feature_columns()) == 12
|
||||
assert len(table.feature_columns()) == 13
|
||||
assert [r["label"] for r in rows] == [None, 1]
|
||||
first, second = rows
|
||||
assert first["feature.diff.hunks"] == [[3, 3]]
|
||||
assert first["feature.diff.deletions"] == [] and first["provenance.diff.version"] == "2"
|
||||
assert first["feature.diff.distance"] == 2
|
||||
assert not first["feature.diff.on_changed_line"]
|
||||
assert first["feature.source.function"] == "first"
|
||||
@@ -149,10 +161,38 @@ def test_head_mismatch_is_not_a_clean_measurement(repository):
|
||||
|
||||
def test_obligation_result_uses_dominance_not_absence_of_findings(repository, monkeypatch):
|
||||
root, change = repository
|
||||
text = ('from flask import request\n@app.route("/item")\ndef first():\n'
|
||||
' return User.query.get(request.args["id"])\n')
|
||||
text = 'from flask import request\n@app.route("/item")\ndef first():\n return User.query.get(request.args["id"])\n'
|
||||
(root / "app.py").write_text(text)
|
||||
monkeypatch.setattr(module, "_git", lambda *args: change["head"])
|
||||
row = compute_rows(root, {**change, "kind": "ordinary", "files": [{"path": "app.py", "head_lines": [[4, 4]]}]})[0]
|
||||
assert row["feature.semantic.obligations"]
|
||||
assert row["feature.semantic.obligations"][0]["discharged"] is False
|
||||
|
||||
|
||||
def test_a_removed_guard_anchors_the_function_it_left(tmp_path):
|
||||
"""Replayed base-to-head, a fix that added a check is a push that removes it: no added line, one deletion anchor."""
|
||||
|
||||
def git(*args):
|
||||
return subprocess.run(["git", "-C", str(tmp_path), *args], check=True, capture_output=True, text=True).stdout.strip()
|
||||
|
||||
git("init", "-q")
|
||||
git("config", "user.name", "Fixture")
|
||||
git("config", "user.email", "[email protected]")
|
||||
check = " if not user.is_admin:\n raise PermissionError\n"
|
||||
guarded = "def first():\n return 1\n\n\ndef admin(user):\n" + check + " return delete_everything()\n"
|
||||
(tmp_path / "app.py").write_text(guarded)
|
||||
git("add", "app.py")
|
||||
git("commit", "-qm", "fix: require admin")
|
||||
base = git("rev-parse", "HEAD")
|
||||
(tmp_path / "app.py").write_text(guarded.replace(check, ""))
|
||||
git("add", "app.py")
|
||||
git("commit", "-qm", "unguarded")
|
||||
head = git("rev-parse", "HEAD")
|
||||
change = {"id": "c", "case_id": "c-1", "repo": "fixture/repo", "base": base, "head": head, "kind": "ordinary", "fold": 1,
|
||||
"files": [{"path": "app.py", "head_lines": [[1, 1], [5, 6]]}]} # fmt: skip
|
||||
rows = compute_rows(tmp_path, change)
|
||||
row = next(r for r in rows if r["feature.source.function"] == "admin")
|
||||
assert row["feature.diff.hunks"] == [] and row["feature.diff.deletions"] == [5]
|
||||
assert row["feature.diff.on_changed_line"] and row["feature.diff.distance"] == 0
|
||||
first = next(r for r in rows if r["feature.source.function"] == "first")
|
||||
assert first["feature.diff.distance"] == 4 and not first["feature.diff.on_changed_line"]
|
||||
|
||||
@@ -43,7 +43,7 @@ def test_signature_drops_masked_inputs_and_keeps_the_order():
|
||||
def test_render_rows_numbers_the_function_and_omits_masked_inputs():
|
||||
inputs = hp.render_rows(CHANGE, ROWS, TEXT)
|
||||
assert inputs["change_summary"] == "app.py::second"
|
||||
assert json.loads(inputs["hunks"]) == {"app.py": [[3, 3]]}
|
||||
assert json.loads(inputs["hunks"]) == {"app.py": {"changed": [[3, 3]], "removed_after_line": []}}
|
||||
assert " 5 def second():" in inputs["function_text"] and " 6 " in inputs["function_text"]
|
||||
assert "input()" not in inputs["function_text"] # only the span is shown
|
||||
assert inputs["families"].startswith("injection: ")
|
||||
|
||||
@@ -108,3 +108,22 @@ def test_instrument_failure_is_recorded_with_a_type_only(repository, tmp_path):
|
||||
assert record["status"] == "instrument_failure"
|
||||
assert record["instrument"]["failure"] == "CalledProcessError"
|
||||
assert record["push"]["admission"]["defects"] == []
|
||||
|
||||
|
||||
def test_full_checkout_restores_a_truncated_file_and_refuses_an_unread_tree(repository, tmp_path, monkeypatch):
|
||||
root, _manifest, changes = repository
|
||||
worktree = tmp_path / "head"
|
||||
subprocess.run(
|
||||
["git", "-C", str(root), "worktree", "add", "--detach", str(worktree), changes[0]["head"]], check=True, capture_output=True
|
||||
)
|
||||
(worktree / "app.py").write_bytes(b"")
|
||||
runner._full_checkout(worktree)
|
||||
assert (worktree / "app.py").read_bytes() # the truncated file was restored, not read as empty
|
||||
real = runner._git
|
||||
|
||||
def still_dirty(target, *args):
|
||||
return " M app.py\n" if args[:2] == ("status", "--porcelain") else real(target, *args)
|
||||
|
||||
monkeypatch.setattr(runner, "_git", still_dirty)
|
||||
with pytest.raises(runner.InstrumentFailure, match="checkout_incomplete"):
|
||||
runner._full_checkout(worktree)
|
||||
|
||||
Reference in New Issue
Block a user