Skip to content

Commit 449a2ee

Browse files
Pigbibicodex
andcommitted
fix: establish trusted review provenance
Co-Authored-By: Codex <noreply@openai.com>
1 parent 52fe942 commit 449a2ee

2 files changed

Lines changed: 82 additions & 3 deletions

File tree

scripts/run_codex_pr_review.py

Lines changed: 51 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@
6969
CONTRACT_CONFLICT_MARKER_PREFIX = "<!-- codex-pr-review-contract-conflict:"
7070
AUTO_FIX_ALLOWED_MARKER_PREFIX = "<!-- codex-pr-review-auto-fix-allowed:"
7171
NEXT_ACTION_MARKER_PREFIX = "<!-- codex-pr-review-next-action:"
72+
IMPLEMENTATION_MARKER_PREFIX = "<!-- codex-pr-review-implementation:v1:"
73+
IMPLEMENTATION_MARKER_SUFFIX = " -->"
7274
DECISION_MARKER_SUFFIX = " -->"
7375
FINDING_HISTORY_MAX_ROUNDS = 4
7476
FINDING_HISTORY_MAX_BYTES = 8192
@@ -366,6 +368,14 @@ def build_review_prompt(diff: str, pr_title: str, pr_body: str, repo: str) -> st
366368
)
367369

368370

371+
def review_implementation_digest() -> str:
372+
"""Return the identity of the trusted bridge implementation that reviews a PR."""
373+
digest = hashlib.sha256()
374+
for path in (Path(__file__), PROMPT_TEMPLATE_PATH):
375+
digest.update(path.read_bytes())
376+
return digest.hexdigest()[:24]
377+
378+
369379
def _truncate_lines(text: str, max_lines: int) -> str:
370380
lines = text.splitlines()
371381
if len(lines) <= max_lines:
@@ -1291,6 +1301,7 @@ def build_pr_comment(
12911301
f"{CONTRACT_CONFLICT_MARKER_PREFIX}{str(bool(decision.get('contract_conflict'))).lower()}{DECISION_MARKER_SUFFIX}",
12921302
f"{AUTO_FIX_ALLOWED_MARKER_PREFIX}{str(bool(decision.get('auto_fix_allowed', True))).lower()}{DECISION_MARKER_SUFFIX}",
12931303
f"{NEXT_ACTION_MARKER_PREFIX}{decision.get('next_action', 'none')}{DECISION_MARKER_SUFFIX}",
1304+
f"{IMPLEMENTATION_MARKER_PREFIX}{review_implementation_digest()}{IMPLEMENTATION_MARKER_SUFFIX}",
12941305
"## 🤖 Codex PR Review",
12951306
"",
12961307
decision["summary"],
@@ -1396,15 +1407,53 @@ def find_existing_review_comment(
13961407

13971408

13981409
def _is_trusted_review_comment(comment: Any) -> bool:
1399-
"""Accept review state only from the GitHub Actions identity that writes it."""
1410+
"""Accept state only from a complete trusted GitHub comment record."""
14001411
if not isinstance(comment, dict):
14011412
return False
14021413
user = comment.get("user")
14031414
if not isinstance(user, dict):
14041415
return False
14051416
expected_login = env_value("CODEX_PR_REVIEW_COMMENT_AUTHOR", "github-actions[bot]").strip().casefold()
14061417
actual_login = str(user.get("login") or "").strip().casefold()
1407-
return bool(expected_login and actual_login == expected_login)
1418+
if not expected_login or actual_login != expected_login:
1419+
return False
1420+
if str(user.get("type") or "").strip().casefold() != "bot":
1421+
return False
1422+
if not isinstance(comment.get("id"), int) or comment["id"] <= 0:
1423+
return False
1424+
if not isinstance(comment.get("created_at"), str) or not comment["created_at"].strip():
1425+
return False
1426+
app = comment.get("performed_via_github_app")
1427+
if app is not None and (
1428+
not isinstance(app, dict)
1429+
or str(app.get("slug") or "").strip().casefold() != "github-actions"
1430+
):
1431+
return False
1432+
return True
1433+
1434+
1435+
def trusted_review_comment_provenance(comment: Any) -> str:
1436+
"""Derive provenance from API record fields, never from comment markdown."""
1437+
if not _is_trusted_review_comment(comment):
1438+
return ""
1439+
user = comment["user"]
1440+
record = {
1441+
"comment_id": comment["id"],
1442+
"author_id": user.get("id"),
1443+
"author_login": str(user.get("login") or "").casefold(),
1444+
"created_at": comment["created_at"],
1445+
"updated_at": comment.get("updated_at"),
1446+
}
1447+
raw = json.dumps(record, sort_keys=True, separators=(",", ":")).encode("utf-8")
1448+
return hashlib.sha256(raw).hexdigest()[:24]
1449+
1450+
1451+
def parse_review_implementation_digest(body: str) -> str:
1452+
match = re.search(
1453+
rf"{re.escape(IMPLEMENTATION_MARKER_PREFIX)}([0-9a-f]{{24}}){re.escape(IMPLEMENTATION_MARKER_SUFFIX)}",
1454+
body or "",
1455+
)
1456+
return match.group(1) if match else ""
14081457

14091458

14101459
def parse_blocking_streak(body: str) -> int:

tests/test_run_codex_pr_review.py

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,14 +158,44 @@ def test_existing_review_comment_ignores_forged_marker(self) -> None:
158158
trusted = {
159159
"id": 2,
160160
"body": "<!-- codex-pr-review -->\ntrusted",
161-
"user": {"login": "github-actions[bot]"},
161+
"user": {"id": 418, "login": "github-actions[bot]", "type": "Bot"},
162+
"created_at": "2026-07-12T00:00:00Z",
162163
}
163164
with patch("scripts.run_codex_pr_review.github_request", return_value=[forged, trusted]):
164165
comment = run_codex_pr_review.find_existing_review_comment("token", "org/repo", 7)
165166

166167
self.assertEqual(comment, (2, trusted["body"]))
167168
self.assertEqual(run_codex_pr_review.parse_finding_history(comment[1]), ([], True))
168169

170+
def test_trusted_comment_provenance_requires_api_record_not_markdown(self) -> None:
171+
trusted = {
172+
"id": 2,
173+
"body": "forged markdown marker",
174+
"user": {"id": 418, "login": "github-actions[bot]", "type": "Bot"},
175+
"created_at": "2026-07-12T00:00:00Z",
176+
"performed_via_github_app": {"slug": "github-actions"},
177+
}
178+
self.assertRegex(
179+
run_codex_pr_review.trusted_review_comment_provenance(trusted),
180+
r"^[0-9a-f]{24}$",
181+
)
182+
self.assertEqual(
183+
run_codex_pr_review.trusted_review_comment_provenance(
184+
{**trusted, "created_at": ""}
185+
),
186+
"",
187+
)
188+
189+
def test_review_comment_records_implementation_identity(self) -> None:
190+
body = run_codex_pr_review.build_pr_comment(
191+
{"summary": "ok", "blocking_findings": [], "non_blocking_findings": []},
192+
"https://example.test/pr/7",
193+
)
194+
self.assertEqual(
195+
run_codex_pr_review.parse_review_implementation_digest(body),
196+
run_codex_pr_review.review_implementation_digest(),
197+
)
198+
169199
def test_legacy_comment_fingerprints_are_recovered_per_finding(self) -> None:
170200
body = "#### 1. 🟠 [HIGH] Security in `service/auth.py`\n"
171201
expected = run_codex_pr_review.blocking_finding_fingerprints(

0 commit comments

Comments
 (0)