fix: make rework comments reference-safe and bound correction plans

This commit is contained in:
2026-10-13 15:01:32 +00:00
parent a74fb747a5
commit 4e9c46bba2
4 changed files with 280 additions and 6 deletions
+113
View File
@@ -67,4 +67,117 @@ class ReworkCommentTests(unittest.TestCase):
self.readback(); identity=self.read("identity.json"); identity["role"]="other"; self.write("identity.json",identity); self.assertNotEqual(self.verify().returncode,0)
self.readback(); comments=self.read("comments.json"); comments["provider_counts"]["aggregate_comments"]+=1; self.write("comments.json",comments); self.assertNotEqual(self.verify().returncode,0)
def test_render_visible_identifier_lines_do_not_emit_issue_references(self):
body=self.expected["marker_free_body"]
self.assertIn("- Source Issue: 7",body)
self.assertIn("- Pull Request: 12",body)
self.assertIn("- Source Review: 99",body)
self.assertNotIn("- Source Issue: #7",body)
self.assertNotIn("- Pull Request: #12",body)
self.assertNotIn("- Source Review: #99",body)
def test_render_rejects_reference_hazards_in_every_safe_human_field(self):
hazards=("#123", "otherrepo#123", r"\\#123", "\\#123", "review #99")
fields=("changes","verification","reason","local_checks","runtime_browser")
# Exercise finding fields and global fields independently; references
# must not be emitted even when they resemble another repo or are escaped.
for field in fields:
for snippet in hazards:
with self.subTest(field=field,snippet=snippet):
data=json.loads(json.dumps(self.data))
if field in ("changes","verification","reason"):
data["findings"][0][field]="unsafe payload "+snippet
else:
data[field]="unsafe payload "+snippet
self.write("hazard.json",data)
result=self.invoke("render","--no-delegated-evidence","--input",self.p("hazard.json"),"--comment",self.p("hazard.md"),"--expected",self.p("hazard-expected.json"))
self.assertNotEqual(result.returncode,0)
def test_offline_verifier_rejects_reference_in_expected_body(self):
self.expected["marker_free_body"] += "\n- unsafe: otherrepo#123"
before_hash=self.expected["result_sha256"]
self.expected["result_sha256"]=hashlib.sha256(self.expected["marker_free_body"].encode()).hexdigest()
self.expected["marker"]=self.expected["marker"].replace(before_hash,self.expected["result_sha256"])
self.expected["body"]=self.expected["marker_free_body"]+self.expected["marker"]
self.write("expected.json",self.expected); self.readback()
self.assertNotEqual(self.verify().returncode,0)
def _reseal(self, value, body):
value=json.loads(json.dumps(value)); old=value["result_sha256"]
value["marker_free_body"]=body
value["result_sha256"]=hashlib.sha256(body.encode()).hexdigest()
value["marker"]=value["marker"].replace(old,value["result_sha256"])
value["body"]=body+value["marker"]
return value
def _legacy_expected(self):
old=json.loads(json.dumps(self.expected))
body=old["marker_free_body"]
body=body.replace("- Source Issue: #7","- Source Issue: 7").replace("- Pull Request: #12","- Pull Request: 12").replace("- Source Review: #99","- Source Review: 99")
# Preserve an explicitly legacy migration fixture; no other old text differs.
body=body.replace("- Source Issue: 7","- Source Issue: #7").replace("- Pull Request: 12","- Pull Request: #12").replace("- Source Review: 99","- Source Review: #99")
old["marker_free_body"]=body
old["result_sha256"]=hashlib.sha256(body.encode()).hexdigest()
old["marker"]=old["marker"].replace(self.expected["result_sha256"],old["result_sha256"])
old["body"]=body+old["marker"]
return old
def test_prepare_correction_returns_bounded_verified_legacy_migration_plan(self):
import importlib.util
spec=importlib.util.spec_from_file_location("rework_comment_correction_test",SCRIPT)
module=importlib.util.module_from_spec(spec); spec.loader.exec_module(module)
self.assertTrue(callable(getattr(module,"prepare_correction",None)), "approved correction API missing")
plan=module.prepare_correction(self._legacy_expected(),self.expected,comment_id=501,actor_id=42)
self.assertEqual(plan["schema"],"gitea-rework-correction-plan/v1")
self.assertEqual(plan["target"],self.expected["target"])
self.assertEqual(plan["comment_id"],501); self.assertEqual(plan["actor_id"],42)
self.assertEqual(plan["old_body"],self._legacy_expected()["body"])
self.assertEqual(plan["new_body"],self.expected["body"])
self.assertEqual(plan["old_body_sha256"],hashlib.sha256(plan["old_body"].encode()).hexdigest())
self.assertEqual(plan["new_body_sha256"],hashlib.sha256(plan["new_body"].encode()).hexdigest())
self.assertIs(plan["prior_handoff_verified"],False)
self.assertTrue(plan["immutable_source_required"])
def test_prepare_correction_rejects_tampered_or_unbounded_changes(self):
import importlib.util
spec=importlib.util.spec_from_file_location("rework_comment_correction_reject_test",SCRIPT)
module=importlib.util.module_from_spec(spec); spec.loader.exec_module(module)
self.assertTrue(callable(getattr(module,"prepare_correction",None)), "approved correction API missing")
old=self._legacy_expected()
cases=[]
bad=json.loads(json.dumps(old)); bad["result_sha256"]="0"*64; cases.append((bad,self.expected))
bad=json.loads(json.dumps(old)); bad["body"]=bad["body"].replace("#7","#8",1); cases.append((bad,self.expected))
bad=json.loads(json.dumps(old)); bad["marker_free_body"]=bad["marker_free_body"].replace("bounded fix","unsafe old text #123"); cases.append((bad,self.expected))
bad=json.loads(json.dumps(self.expected)); bad["marker_free_body"]=bad["marker_free_body"].replace("bounded fix","different extra text"); cases.append((old,bad))
bad=self._reseal(self.expected,self.expected["marker_free_body"].replace("bounded fix","different extra text")); cases.append((old,bad))
unsafe_old=self._reseal(old,old["marker_free_body"].replace("bounded fix","same unsafe #123"))
unsafe_new=self._reseal(self.expected,self.expected["marker_free_body"].replace("bounded fix","same unsafe #123")); cases.append((unsafe_old,unsafe_new))
bad=json.loads(json.dumps(self.expected)); bad["head"]="d"*40; cases.append((old,bad))
bad=json.loads(json.dumps(self.expected)); bad["correlation"]="e"*64; cases.append((old,bad))
for previous,new in cases:
with self.subTest(previous=previous["result_sha256"],new=new["correlation"]):
with self.assertRaises(module.Bad): module.prepare_correction(previous,new,comment_id=501,actor_id=42)
def test_prepare_correction_cli_emits_plan_not_approval_or_receipt(self):
self.write("previous.json",self._legacy_expected())
r=self.invoke("prepare-correction","--previous-expected",self.p("previous.json"),"--expected",self.p("expected.json"),"--comment-id","501","--actor-id","42","--output",self.p("plan.json"))
self.assertEqual(r.returncode,0,r.stderr)
plan=self.read("plan.json")
self.assertEqual(plan["schema"],"gitea-rework-correction-plan/v1")
self.assertIs(plan["approval_required"],True)
self.assertIs(plan["prior_handoff_verified"],False)
self.assertNotEqual(plan.get("status"),"verified")
self.assertEqual(self.d.joinpath("plan.json").stat().st_mode & 0o777,0o600)
def test_prepare_correction_cli_refuses_input_alias_and_preserves_file(self):
self.write("previous.json",self._legacy_expected()); before=self.d.joinpath("expected.json").read_bytes()
r=self.invoke("prepare-correction","--previous-expected",self.p("previous.json"),"--expected",self.p("expected.json"),"--comment-id","501","--actor-id","42","--output",self.p("expected.json"))
self.assertNotEqual(r.returncode,0)
self.assertEqual(self.d.joinpath("expected.json").read_bytes(),before)
def test_different_body_same_correlation_is_not_default_reuse(self):
old=self._legacy_expected()
self.readback([{"id":501,"html_url":"https://gitea/x/501","created_at":"2026-01-01T00:00:00Z","updated_at":"2026-01-01T00:00:00Z","body":old["body"],"user":{"id":42,"login":"bot"}}])
self.assertNotEqual(self.verify("reuse",1).returncode,0)
if __name__ == "__main__": unittest.main()