diff --git a/.github/actions/noema-review/two_phase.py b/.github/actions/noema-review/two_phase.py index c850da81b9..0a6d499392 100755 --- a/.github/actions/noema-review/two_phase.py +++ b/.github/actions/noema-review/two_phase.py @@ -164,6 +164,7 @@ def prepare_verdict(repo: str, number: int, expected_head: str, path: Path) -> i return 0 diff, truncated = gate.fetch_diff(repo, number) + diff, truncated, unobserved = gate.augment_binary_document_diff(repo, number, pull_request, diff, truncated) changed_files = gate.fetch_changed_files(repo, number) changed_paths = tuple(file_path for file_path, _status in changed_files) review_context = gate.build_review_context(repo, number, pull_request, changed_files) @@ -177,6 +178,7 @@ def prepare_verdict(repo: str, number: int, expected_head: str, path: Path) -> i expected, review_context, changed_paths, + unobserved, ) except gate.NoemaTransportError as exc: _emit_transport_capacity_outputs(exc, expected_head=expected) diff --git a/.github/workflows/noema-review.yml b/.github/workflows/noema-review.yml index 9be705a50c..663741cabf 100644 --- a/.github/workflows/noema-review.yml +++ b/.github/workflows/noema-review.yml @@ -764,6 +764,10 @@ jobs: NOEMA_REVIEW_ACTOR: ${{ steps.noema_github_app_token.outputs['app-slug'] && format('{0}[bot]', steps.noema_github_app_token.outputs['app-slug']) || '' }} NOEMA_REVIEW_INSTALLATION_ID: ${{ steps.noema_github_app_token.outputs['installation-id'] }} NOEMA_TRANSPORT_RETRY_ATTEMPT: ${{ github.event.client_payload.transport_retry_attempt || 0 }} + # Exact allowlist of declared corresponding-author emails that a + # manuscript's front matter may carry; they reach the model only as + # [CORRESPONDING_AUTHOR_EMAIL]. Empty means every email fails closed. + NOEMA_CORRESPONDING_AUTHOR_EMAILS: ${{ vars.NOEMA_CORRESPONDING_AUTHOR_EMAILS || '' }} run: | set -euo pipefail if [ -z "${PR_NUMBER:-}" ]; then diff --git a/CHANGELOG.md b/CHANGELOG.md index 34281625cb..f24d90a1ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,24 @@ +### Noema reviews binary-only document PRs through a base/head object diff + +- A DOCX/HWPX/PDF/image change reaches Noema as `Binary files … differ` with no hunk. `changed_diff_locations()` returned an empty set, so `validate_substantive_verdict()` raised "requires parseable changed-line evidence" for every approve or request_changes, and a binary-only document PR could never get a formal verdict. That empty textual patch is an unsupported textual diff, not "no change". Separately, `fetch_file_content_at_ref()` decoded PDF and image bytes with `errors="replace"` straight into the prompt. +- New `scripts/ci/document_blob_diff.py` (stdlib only) materializes the base blob at the merge base and the head blob (contents API for the blob SHA, then the Git blobs API for the bytes) and extracts ordered objects. DOCX yields body paragraphs, tables and relationship-resolved figures; HWPX yields manifest/section paragraphs, nested tables and `binaryItemIDRef` figures; PDF and images are one opaque hashed `page` object. Every object carries a sha256. The objects are aligned into added, removed, modified and neighbouring unchanged objects, and a byte change with no object change becomes a package-level `style` object, never "no change". The result is emitted as `document_diff_review.v1` (contextual-orchestrator#1220 contract; all five fixture envelopes pass that PR's validator). +- Fail-closed safety: blob size, member count, total expansion, per-member compression ratio, traversal, encrypted members, macros (`vbaProject.bin`, `Scripts/`, `macroEnabled`), external image/OLE/template/frame relationships, external HWPX manifest hrefs, DTD/entity XML, unresolved figures, participant-material directories, and email, resident-registration-number, mobile-number and secret patterns in extracted text all reject the review instead of redacting. Original bytes and media never leave the runner; figures and pages carry only hashes. +- `noema_review_gate.augment_binary_document_diff()`, called from both the direct and the two-phase paths, replaces each binary document stanza with synthetic hunks whose LEFT/RIGHT line numbers are base/head object ordinals. The existing citation validator therefore accepts findings on document objects unchanged. Opaque binaries are no longer decoded into the changed-file context. +- Review fixes on the same PR (contextual-orchestrator lead findings, reproduced with that lead's fixtures): + - Object text is now cut on UTF-8 byte boundaries (8 KiB per text) and the envelope total is counted in bytes (256 KiB). This matches contextual-orchestrator#1220, which had rejected a 5,000-character Korean paragraph (15,000 B) with 400 and a >256 KiB Korean envelope with 413. Changed objects claim the byte budget first, and unchanged context is dropped once bytes or object slots run out. XML-legal control characters (CR) are blanked. + - The participant/secret scan now covers every extracted base and head object, not only the diffed ones. The changed-file context path applies the same scan to the whole `extract_review_document` body of a DOCX/HWP/HWPX and withholds it on a match, so a phone number in an unchanged paragraph far from the edit no longer reaches the prompt. +- Second review round: + - Fail-open fix: c6f4b49b kept changed objects with `""` text once the 256 KiB budget ran out, so a late substantive change ("결론: 효과 없음") never reached the reviewer. Now if any changed object's text would be cut by the 8 KiB per-object bound or the envelope budget, the envelope fails closed and no provider is called. Only unchanged context is cut, ending with `…[truncated]`, or dropped. A regression test checks that no changed object ever carries empty or truncated text. + - Figures carry the text of an adjacent "Figure N"/"Fig."/"그림 N" caption, so contextual-orchestrator's rule finding (changed image, unchanged caption) fires. + - HWPX sections follow the manifest spine; a spine section missing from the package fails closed. + - Emails still fail closed by default, with one narrow exception for a declared corresponding author. An address is replaced by `[CORRESPONDING_AUTHOR_EMAIL]` before hashing, diffing, context or prompt only when it is on the workflow's exact `NOEMA_CORRESPONDING_AUTHOR_EMAILS` allowlist and sits in a front-matter author-contact paragraph (before the abstract or introduction, within the first 20 paragraphs, declaring "Corresponding author"/"Correspondence"/"교신저자"). Allowlist mismatches, the same address elsewhere, emails after the abstract, undeclared contacts, extra addresses, table cells and participant phone numbers are all still rejected, and no error message contains the address. +- Tests: `tests/test_document_blob_diff.py` (58). The budget fail-open, empty-changed-text invariant, caption, spine and corresponding-author cases fail on c6f4b49b. +- Third round (exact-head review of e6f0671f and CodeRabbit): + - A changed figure (captioned or not), a PDF/image `page`, or a textless package object proves bytes changed but not what changed. `document_blob_diff.unobserved_changed_objects()` derives their `path#locator` ids from the envelope itself, and `augment_binary_document_diff` returns them as structured review metadata. `call_llm` lists them in the prompt and passes them to `validate_substantive_verdict`, which refuses any `approve` while one is present (`NoemaModelOutputError`); comment and request_changes stay available. The flag no longer comes from scanning rendered or truncated diff text, so neither a caption (8da729e4 bypass 1) nor a `MAX_DIFF_CHARS` cut (bypass 2) can hide an unobserved change. + - An added or removed package with no extractable body still gets a citable package object (`added`/`removed`). + - When `fetch_diff` already cut the diff, `augment_binary_document_diff` re-reads the full diff before searching for binary stanzas, so a document after the cut is no longer skipped. +- Tests: 66. The captioned and uncaptioned image swap, diff-cut, PDF-only and prompt/leak cases fail on 8da729e4. A text-only DOCX edit is the negative control: no unobserved ids, and a valid approve passes. The binary-only request_changes integration test, the fail-closed augmentation test and the no-raw-decode test fail on the previous gate and pass now. The new module has 100% branch coverage. + ### Noema transport capacity schedules a bounded continuation re-dispatch - After gateway failover, HTTP 429/5xx no longer end only as a permanent required-check failure with `caller attempts=1`. ADR-0031 classifies that class as `provider_capacity_unavailable`, keeps the single gateway request per job, surfaces `provider_attempt_count` from the orchestrator error envelope, and authorizes at most two same-head `repository_dispatch` retries after a capped `Retry-After` or deterministic 60–180 s jitter. Review is never skipped. Refs #2165. diff --git a/scripts/ci/document_blob_diff.py b/scripts/ci/document_blob_diff.py new file mode 100644 index 0000000000..7de18330f0 --- /dev/null +++ b/scripts/ci/document_blob_diff.py @@ -0,0 +1,625 @@ +"""Object-level base/head diff for binary review documents, fail closed. + +GitHub renders a changed DOCX/HWPX/PDF/image as ``Binary files … differ`` with +no hunk. An empty textual patch is not "no change"; it is an unsupported +textual diff. This module turns the base and head blobs into ordered document +objects (paragraphs, tables, figures, or one opaque blob), diffs them, and +emits the ``document_diff_review.v1`` envelope. Only bounded extracted text and +sha256 hashes leave the runner. Original bytes, embedded media, macros, +external relationships, and participant or secret patterns never do: they +raise :class:`DocumentSafetyError` instead. +""" + +from __future__ import annotations + +import difflib +import hashlib +import io +import re +import zipfile +from collections.abc import Callable, Sequence +from dataclasses import dataclass +from pathlib import PurePosixPath +from typing import Any +from xml.etree import ElementTree + +EXTRACTOR_VERSION = "document_blob_diff/1" +CONTRACT_VERSION = "document_diff_review.v1" +PACKAGE_SUFFIXES = frozenset({".docx", ".hwpx"}) +OPAQUE_SUFFIXES = frozenset( + {".pdf", ".png", ".jpg", ".jpeg", ".gif", ".webp", ".tif", ".tiff", ".bmp"} +) +REVIEW_DOCUMENT_SUFFIXES = PACKAGE_SUFFIXES | OPAQUE_SUFFIXES +MAX_BLOB_BYTES = 25 * 1024 * 1024 +MAX_MEMBERS = 1000 +MAX_UNCOMPRESSED_BYTES = 200 * 1024 * 1024 +MAX_MEMBER_RATIO = 200 +RATIO_FLOOR_BYTES = 1024 * 1024 +MAX_OBJECTS = 200 +# UTF-8 byte bounds, matching contextual-orchestrator document_diff_review.v1. +MAX_OBJECT_TEXT_BYTES = 8 * 1024 +MAX_ENVELOPE_TEXT_BYTES = 256 * 1024 +_DISALLOWED_CONTROL = re.compile(r"[\x00-\x08\x0b-\x1f\x7f\ud800-\udfff]") +# Relationship types that make a reader fetch or execute remote content. A +# plain hyperlink is metadata and stays allowed. +_REMOTE_CONTENT_REL = re.compile(r"/(image|oleObject|attachedTemplate|frame|subDocument|package)$") +_PARTICIPANT_DIRECTORY = re.compile( + r"(^|/)(participants|participant_data|raw_data|subjects|interviews|consent|consents|pii)(/|$)", + re.IGNORECASE, +) +_PARTICIPANT_PATTERNS = ( + re.compile(r"[A-Za-z0-9._%+-]+@[A-Za-z0-9.-]+\.[A-Za-z]{2,}"), + re.compile(r"(? bool: + """Return whether ``path`` is a binary document this module reviews.""" + return PurePosixPath(path).suffix.lower() in REVIEW_DOCUMENT_SUFFIXES + + +def _sha256(data: bytes) -> str: + """Return the lowercase hex sha256 of ``data``.""" + return hashlib.sha256(data).hexdigest() + + +def _local(tag: str) -> str: + """Return an XML tag without its namespace.""" + return tag.rsplit("}", 1)[-1] + + +def open_package(raw: bytes) -> zipfile.ZipFile: + """Open a DOCX/HWPX container after size, bomb, traversal and macro checks.""" + if len(raw) > MAX_BLOB_BYTES: + raise DocumentSafetyError(f"blob exceeds {MAX_BLOB_BYTES} bytes") + try: + package = zipfile.ZipFile(io.BytesIO(raw)) + except zipfile.BadZipFile as exc: + raise DocumentSafetyError("document is not a readable ZIP package") from exc + members = package.infolist() + if len(members) > MAX_MEMBERS: + raise DocumentSafetyError(f"package has more than {MAX_MEMBERS} members") + total = 0 + for member in members: + name = member.filename + if name.startswith("/") or ".." in PurePosixPath(name).parts or "\\" in name: + raise DocumentSafetyError(f"package member path is unsafe: {name!r}") + if member.flag_bits & 0x1: + raise DocumentSafetyError("package member is encrypted") + lowered = name.lower() + if lowered.endswith("vbaproject.bin") or lowered.startswith("scripts/"): + raise DocumentSafetyError(f"package contains macro content: {name!r}") + total += member.file_size + if ( + member.file_size > RATIO_FLOOR_BYTES + and member.file_size > MAX_MEMBER_RATIO * max(member.compress_size, 1) + ): + raise DocumentSafetyError(f"package member compression ratio is unsafe: {name!r}") + if total > MAX_UNCOMPRESSED_BYTES: + raise DocumentSafetyError(f"package expands beyond {MAX_UNCOMPRESSED_BYTES} bytes") + content_types = _read_optional(package, "[Content_Types].xml") + if content_types is not None and b"macroEnabled" in content_types: + raise DocumentSafetyError("package declares macro-enabled content") + return package + + +def _read_optional(package: zipfile.ZipFile, name: str) -> bytes | None: + """Read one member or return None when it is absent.""" + try: + return package.read(name) + except KeyError: + return None + + +def _parse(data: bytes, name: str) -> ElementTree.Element: + """Parse package XML, failing closed on malformed or DTD-bearing input.""" + if b" str: + """Join ``text_tag`` descendants, skipping any subtree rooted in ``skip``.""" + parts: list[str] = [] + + def walk(node: ElementTree.Element) -> None: + """Depth-first text collection that honours ``skip``.""" + if skip and id(node) in skip: + return + if _local(node.tag) == text_tag and node.text: + parts.append(node.text) + for child in node: + walk(child) + + walk(element) + return "".join(parts) + + +def _table_text(table: ElementTree.Element, row_tag: str, cell_tag: str, text_tag: str) -> str: + """Render a table as ``cell | cell`` rows separated by newlines.""" + rows = [] + for row in table.iter(): + if _local(row.tag) != row_tag: + continue + cells = [_text(cell, text_tag) for cell in row if _local(cell.tag) == cell_tag] + rows.append(" | ".join(cells)) + return "\n".join(rows) + + +def _figure(package: zipfile.ZipFile, target: str, locator: str) -> DocumentObject: + """Hash one embedded media member; a missing target fails closed.""" + data = _read_optional(package, target) + if data is None: + raise DocumentSafetyError(f"figure target is missing: {target!r}") + return DocumentObject("figure", locator, _sha256(data), None) + + +def _text_object(kind: str, locator: str, text: str) -> DocumentObject: + """Hash a paragraph/table by its extracted text.""" + return DocumentObject(kind, locator, _sha256(text.encode("utf-8")), text) + + +def extract_docx(raw: bytes) -> list[DocumentObject]: + """Return DOCX body paragraphs, tables and figures in document order.""" + package = open_package(raw) + document = _read_optional(package, "word/document.xml") + if document is None: + raise DocumentSafetyError("DOCX has no word/document.xml") + relationships: dict[str, str] = {} + rels = _read_optional(package, "word/_rels/document.xml.rels") + if rels is not None: + for rel in _parse(rels, "document.xml.rels"): + if rel.get("TargetMode") == "External" and _REMOTE_CONTENT_REL.search(rel.get("Type", "")): + raise DocumentSafetyError(f"DOCX has an external {rel.get('Type', '').rsplit('/', 1)[-1]} relationship") + target = rel.get("Target", "") + relationships[rel.get("Id", "")] = target[1:] if target.startswith("/") else "word/" + target + body = next((node for node in _parse(document, "document.xml") if _local(node.tag) == "body"), None) + if body is None: + raise DocumentSafetyError("DOCX document has no body") + objects: list[DocumentObject] = [] + paragraphs = tables = 0 + for node in body: + kind = _local(node.tag) + if kind == "tbl": + tables += 1 + objects.append(_text_object("table", f"tbl{tables}", _table_text(node, "tr", "tc", "t"))) + elif kind == "p": + paragraphs += 1 + text = _text(node, "t") + if text: + objects.append(_text_object("paragraph", f"p{paragraphs}", text)) + for blip in (d for d in node.iter() if _local(d.tag) == "blip"): + rel_id = next((v for k, v in blip.attrib.items() if _local(k) == "embed"), "") + if rel_id not in relationships: + raise DocumentSafetyError(f"DOCX figure relationship is unresolved: {rel_id!r}") + objects.append(_figure(package, relationships[rel_id], f"p{paragraphs}/fig:{relationships[rel_id]}")) + return objects + + +def extract_hwpx(raw: bytes) -> list[DocumentObject]: + """Return HWPX section paragraphs, tables and figures in document order.""" + package = open_package(raw) + manifest = _read_optional(package, "Contents/content.hpf") + if manifest is None: + raise DocumentSafetyError("HWPX has no Contents/content.hpf") + items: dict[str, str] = {} + spine: list[str] = [] + for item in _parse(manifest, "content.hpf").iter(): + if _local(item.tag) == "item": + href = item.get("href", "") + if re.match(r"^[a-z][a-z0-9+.-]*:", href, re.IGNORECASE): + raise DocumentSafetyError(f"HWPX manifest references external content: {href!r}") + items[item.get("id", "")] = href + elif _local(item.tag) == "itemref": + spine.append(item.get("idref", "")) + if spine: + # Reading order is the manifest spine, as the HWPX reader uses it. + sections = [] + for idref in spine: + href = items.get(idref, "") + if not re.fullmatch(r"Contents/section\d+\.xml", href): + continue + if href not in package.namelist(): + raise DocumentSafetyError(f"HWPX spine section is missing: {href!r}") + sections.append(href) + else: + sections = sorted( + (n for n in package.namelist() if re.fullmatch(r"Contents/section\d+\.xml", n)), + key=lambda n: int(re.search(r"\d+", n).group()), + ) + if not sections: + raise DocumentSafetyError("HWPX has no Contents/section*.xml") + objects: list[DocumentObject] = [] + paragraphs = tables = 0 + for section in sections: + for node in _parse(package.read(section), section): + if _local(node.tag) != "p": + continue + paragraphs += 1 + nested = [d for d in node.iter() if _local(d.tag) == "tbl"] + text = _text(node, "t", skip={id(t) for t in nested}) + if text: + objects.append(_text_object("paragraph", f"p{paragraphs}", text)) + for table in nested: + tables += 1 + objects.append(_text_object("table", f"tbl{tables}", _table_text(table, "tr", "tc", "t"))) + for pic in (d for d in node.iter() if "binaryItemIDRef" in d.attrib): + ref = pic.get("binaryItemIDRef", "") + if ref not in items: + raise DocumentSafetyError(f"HWPX figure reference is unresolved: {ref!r}") + objects.append(_figure(package, items[ref], f"p{paragraphs}/fig:{items[ref]}")) + return objects + + +def extract_objects(path: str, raw: bytes) -> list[DocumentObject]: + """Extract ordered objects; PDF/images are one opaque hashed ``page`` object.""" + suffix = PurePosixPath(path).suffix.lower() + if suffix == ".docx": + return extract_docx(raw) + if suffix == ".hwpx": + return extract_hwpx(raw) + if suffix in OPAQUE_SUFFIXES: + if len(raw) > MAX_BLOB_BYTES: + raise DocumentSafetyError(f"blob exceeds {MAX_BLOB_BYTES} bytes") + return [DocumentObject("page", "blob", _sha256(raw), None)] + raise DocumentSafetyError(f"unsupported review document type: {suffix or path!r}") + + +CORRESPONDING_AUTHOR_MARKER = "[CORRESPONDING_AUTHOR_EMAIL]" +FRONT_MATTER_MAX_PARAGRAPHS = 20 +_EMAIL = _PARTICIPANT_PATTERNS[0] +_CONTACT_DECLARATION = re.compile(r"corresponding\s+author|correspondence|교신\s*저자", re.IGNORECASE) +_FRONT_MATTER_END = re.compile(r"^\s*(abstract|초록|요약|introduction|서론|1\.\s)", re.IGNORECASE) +_CAPTION = re.compile(r"^\s*(figure|fig\.|그림)\s*\d+", re.IGNORECASE) + + +def redact_corresponding_author(paragraphs: Sequence[str], allowlist: frozenset[str]) -> list[str]: + """Replace declared, allowlisted corresponding-author emails with a marker. + + An address is replaced only when it is on the workflow's exact allowlist + AND sits in a front-matter paragraph (before the abstract/introduction and + within the first ``FRONT_MATTER_MAX_PARAGRAPHS``) that declares author + contact. Every other email, including the same address anywhere else, + is left in place so the participant scan rejects the document. + """ + out = [] + front = True + for index, text in enumerate(paragraphs): + if front and (index >= FRONT_MATTER_MAX_PARAGRAPHS or _FRONT_MATTER_END.match(text)): + front = False + emails = _EMAIL.findall(text) + if ( + emails + and front + and _CONTACT_DECLARATION.search(text) + and all(email.lower() in allowlist for email in emails) + ): + text = _EMAIL.sub(CORRESPONDING_AUTHOR_MARKER, text) + out.append(text) + return out + + +def _apply_contact_policy(objects: Sequence[DocumentObject], allowlist: frozenset[str]) -> list[DocumentObject]: + """Apply :func:`redact_corresponding_author` to paragraph objects, rehashing them.""" + paragraphs = [o for o in objects if o.kind == "paragraph"] + redacted = iter(redact_corresponding_author([o.text or "" for o in paragraphs], allowlist)) + out = [] + for obj in objects: + if obj.kind == "paragraph": + text = next(redacted) + obj = obj if text == obj.text else _text_object("paragraph", obj.locator, text) + out.append(obj) + return out + + +def _attach_captions(objects: Sequence[DocumentObject]) -> list[DocumentObject]: + """Give each figure the text of an adjacent "Figure N"/"그림 N" caption paragraph. + + The figure keeps its media hash, so a changed image with an unchanged + caption is visible as a modified figure whose base and head text agree. + """ + out = list(objects) + for index, obj in enumerate(out): + if obj.kind != "figure": + continue + for near in (index + 1, index - 1): + if 0 <= near < len(out) and out[near].kind == "paragraph" and _CAPTION.match(out[near].text or ""): + out[index] = DocumentObject("figure", obj.locator, obj.sha256, out[near].text) + break + return out + + +def diff_objects( + base: Sequence[DocumentObject], head: Sequence[DocumentObject] +) -> list[tuple[str, int | None, DocumentObject | None, int | None, DocumentObject | None]]: + """Align base/head objects by (kind, hash) and return changes with context. + + Each entry is ``(change, base_ordinal, base_object, head_ordinal, + head_object)`` with 1-based ordinals; ``change`` is added, removed, + modified, or unchanged. Only the unchanged object on each side of a change + is kept, as body/table/caption consistency context. + """ + keys_a = [(o.kind, o.sha256) for o in base] + keys_b = [(o.kind, o.sha256) for o in head] + opcodes = difflib.SequenceMatcher(a=keys_a, b=keys_b, autojunk=False).get_opcodes() + changes = [] + for position, (tag, i1, i2, j1, j2) in enumerate(opcodes): + if tag == "equal": + keep = set() + if position > 0: + keep.add(0) + if position < len(opcodes) - 1: + keep.add(i2 - i1 - 1) + for offset in sorted(keep): + changes.append(("unchanged", i1 + offset + 1, base[i1 + offset], j1 + offset + 1, head[j1 + offset])) + continue + span = max(i2 - i1, j2 - j1) + for offset in range(span): + i, j = i1 + offset, j1 + offset + old = base[i] if i < i2 else None + new = head[j] if j < j2 else None + if old is not None and new is not None and old.kind == new.kind: + changes.append(("modified", i + 1, old, j + 1, new)) + continue + if old is not None: + changes.append(("removed", i + 1, old, None, None)) + if new is not None: + changes.append(("added", None, None, j + 1, new)) + return changes + + +def assert_no_participant_material(text: str, where: str, sensitive: Callable[[str], bool]) -> None: + """Reject text carrying participant identifiers or secrets; never redact.""" + if any(p.search(text) for p in _PARTICIPANT_PATTERNS) or sensitive(text): + raise DocumentSafetyError(f"participant or secret pattern in extracted text at {where}") + + +TRUNCATION_MARKER = "…[truncated]" + + +def _bounded(text: str | None) -> str | None: + """Return ``text`` within ``MAX_OBJECT_TEXT_BYTES`` UTF-8 bytes. + + Controls other than newline and tab are blanked. A cut text ends with + ``TRUNCATION_MARKER`` so a reviewer can never mistake it for the whole text. + """ + if text is None: + return None + clean = _DISALLOWED_CONTROL.sub(" ", text) + encoded = clean.encode("utf-8") + if len(encoded) <= MAX_OBJECT_TEXT_BYTES: + return clean + room = MAX_OBJECT_TEXT_BYTES - len(TRUNCATION_MARKER.encode("utf-8")) + return encoded[:room].decode("utf-8", "ignore") + TRUNCATION_MARKER + + +def _size(text: str | None) -> int: + """Return the UTF-8 byte length of optional text.""" + return len(text.encode("utf-8")) if text else 0 + + +@dataclass(frozen=True) +class DocumentReview: + """A CO-strict envelope plus the object ordinals used for citable hunks.""" + + envelope: dict[str, Any] + ordinals: tuple[tuple[int | None, int | None], ...] + + +def _hash(digest: str | None) -> str | None: + """Return the envelope's ``sha256:`` form of a digest.""" + return f"sha256:{digest}" if digest else None + + +def build_envelope( + repo: str, + path: str, + base_blob: str | None, + head_blob: str | None, + base_raw: bytes | None, + head_raw: bytes | None, + sensitive: Callable[[str], bool] = lambda _text: False, + corresponding_author_emails: frozenset[str] = frozenset(), +) -> DocumentReview: + """Build a bounded ``document_diff_review.v1`` envelope for one path.""" + if _PARTICIPANT_DIRECTORY.search(path): + raise DocumentSafetyError(f"path is under a participant-material directory: {path}") + if base_blob is None and head_blob is None: + raise DocumentSafetyError("both base and head blobs are missing") + if base_blob == head_blob: + raise DocumentSafetyError("base and head blobs are identical") + base = extract_objects(path, base_raw) if base_raw is not None else [] + head = extract_objects(path, head_raw) if head_raw is not None else [] + base = _attach_captions(_apply_contact_policy(base, corresponding_author_emails)) + head = _attach_captions(_apply_contact_policy(head, corresponding_author_emails)) + # Scan every extracted object, not only the diffed ones: the same document + # text also reaches the changed-file context, so a participant identifier + # anywhere in either blob fails the review closed. + for obj in (*base, *head): + if obj.text is not None: + assert_no_participant_material(obj.text, obj.locator, sensitive) + entries = diff_objects(base, head) + changed = [e for e in entries if e[0] != "unchanged"] + context = [e for e in entries if e[0] == "unchanged"] + if len(changed) > MAX_OBJECTS: + raise DocumentSafetyError(f"document diff has more than {MAX_OBJECTS} changed objects") + # Every changed object is sent in full: if any changed text would be cut by + # the per-object bound or the envelope budget, the review fails closed and + # no provider is called. Only unchanged context is cut (marked) or dropped. + budget = MAX_ENVELOPE_TEXT_BYTES + kept: dict[int, tuple[str | None, str | None]] = {} + for entry in changed: + _change, _bo, old, _ho, new = entry + texts = (_bounded(old.text if old else None), _bounded(new.text if new else None)) + if any(_size(text) > MAX_OBJECT_TEXT_BYTES for text in (old and old.text, new and new.text)): + raise DocumentSafetyError( + f"changed object {(new or old).locator} exceeds the {MAX_OBJECT_TEXT_BYTES}-byte text bound" + ) + budget -= _size(texts[0]) + _size(texts[1]) + kept[id(entry)] = texts + if budget < 0: + raise DocumentSafetyError( + f"changed document objects exceed the {MAX_ENVELOPE_TEXT_BYTES}-byte review budget" + ) + for entry in context[: MAX_OBJECTS - len(changed)]: + _change, _bo, old, _ho, new = entry + texts = (_bounded(old.text if old else None), _bounded(new.text if new else None)) + cost = _size(texts[0]) + _size(texts[1]) + if cost > budget: + continue + budget -= cost + kept[id(entry)] = texts + objects: list[dict[str, Any]] = [] + ordinals: list[tuple[int | None, int | None]] = [] + for entry in entries: + if id(entry) not in kept: + continue + change, base_ord, old, head_ord, new = entry + subject = new or old + base_text, head_text = kept[id(entry)] + objects.append( + { + "page": None, + "object_kind": subject.kind, + "locator": subject.locator[:256], + "change": change, + "object_hash_base": _hash(old.sha256 if old else None), + "object_hash_head": _hash(new.sha256 if new else None), + "base_text": base_text, + "head_text": head_text, + } + ) + ordinals.append((base_ord, head_ord)) + if not changed: + # The bytes changed but no body object did (styles, settings, metadata, + # or a package with no extractable body): still a reviewable, citable + # change, never "no change". + change = "modified" if base_raw is not None and head_raw is not None else ( + "added" if base_raw is None else "removed" + ) + objects.append( + { + "page": None, + "object_kind": "style", + "locator": "package", + "change": change, + "object_hash_base": _hash(_sha256(base_raw)) if base_raw is not None else None, + "object_hash_head": _hash(_sha256(head_raw)) if head_raw is not None else None, + "base_text": None, + "head_text": None, + } + ) + ordinals.append((1 if base_raw is not None else None, 1 if head_raw is not None else None)) + envelope = { + "contract_version": CONTRACT_VERSION, + "repo": repo, + "path": path, + "base_blob": base_blob, + "head_blob": head_blob, + "extractor_version": EXTRACTOR_VERSION, + "participant_material": False, + "objects": objects, + } + return DocumentReview(envelope, tuple(ordinals)) + + +UNOBSERVED_TEXT = "(no text extracted; hash only)" +def unobserved_changed_objects(review: DocumentReview) -> list[str]: + """Return ``path#locator`` ids of changed objects whose content was not observed. + + A figure or page is never observed in v1 (no pixels or page render reach + the reviewer, caption or not), and a textless package object only proves + the bytes changed. Computed from the envelope, not from rendered or + truncated diff text, so neither a caption nor a diff cut can hide it. + """ + envelope = review.envelope + return [ + f"{envelope['path']}#{obj['locator']}" + for obj in envelope["objects"] + if obj["change"] != "unchanged" + and (obj["object_kind"] in {"figure", "page"} or (obj["base_text"] is None and obj["head_text"] is None)) + ] + + +def _line(obj: dict[str, Any], side: str) -> str: + """Render one synthetic diff line for an object side.""" + digest = obj[f"object_hash_{side}"].removeprefix("sha256:")[:12] + text = obj[f"{side}_text"] + body = " ".join(text.split())[:500] if text is not None else UNOBSERVED_TEXT + return f"[{obj['object_kind']} {obj['locator']} sha256:{digest}] {body}" + + +def synthetic_hunks(review: DocumentReview, old_path: str | None = None) -> str: + """Render changed objects as unified-diff hunks citable as ``path:ordinal``. + + LEFT line numbers are base object ordinals and RIGHT line numbers are head + object ordinals, so existing changed-line validation accepts citations of + document objects without any new location type. Unchanged context objects + stay in the envelope only. + """ + envelope = review.envelope + path = envelope["path"] + old = old_path or path + lines = [ + f"diff --git a/{old} b/{path}", + f"--- a/{old}" if envelope["base_blob"] else "--- /dev/null", + f"+++ b/{path}" if envelope["head_blob"] else "+++ /dev/null", + ] + for obj, (left, new) in zip(envelope["objects"], review.ordinals): + if obj["change"] == "unchanged": + continue + lines.append(f"@@ -{left or 0},{1 if left else 0} +{new or 0},{1 if new else 0} @@ {obj['change']}") + if left: + lines.append("-" + _line(obj, "base")) + if new: + lines.append("+" + _line(obj, "head")) + return "\n".join(lines) + + +_BINARY_STANZA = re.compile(r"^Binary files (?:a/(?P.+)|/dev/null) and (?:b/(?P.+)|/dev/null) differ$") + + +def _stanza(block: str) -> tuple[str | None, str | None] | None: + """Return the ``(old, new)`` paths of a block's binary stanza, if any.""" + for line in block.splitlines(): + match = _BINARY_STANZA.match(line) + if match: + return match.group("old"), match.group("new") + return None + + +def binary_document_stanzas(diff: str) -> list[tuple[str | None, str | None]]: + """Return ``(old, new)`` review-document paths the diff reports only as binary.""" + stanzas = [] + for block in re.split(r"(?m)^(?=diff --git )", diff): + pair = _stanza(block) + if pair and is_review_document(pair[1] or pair[0]) and pair not in stanzas: + stanzas.append(pair) + return stanzas + + +def replace_binary_stanzas(diff: str, hunks: dict[tuple[str | None, str | None], str]) -> str: + """Replace each ``diff --git`` block of a reviewed binary pair with its hunks.""" + out = [] + for block in re.split(r"(?m)^(?=diff --git )", diff): + pair = _stanza(block) + out.append(hunks[pair] + "\n" if pair in hunks else block) + return "".join(out) diff --git a/scripts/ci/noema_review_gate.py b/scripts/ci/noema_review_gate.py index c8709304fc..64c0b371c4 100644 --- a/scripts/ci/noema_review_gate.py +++ b/scripts/ci/noema_review_gate.py @@ -25,6 +25,7 @@ from typing import Any from scripts.ci.opencode_review_normalize_output import changed_file_is_material +from scripts.ci import document_blob_diff from scripts.ci.noema_review_document import DocumentReadError, extract_review_document @@ -580,6 +581,11 @@ def current_actor() -> str: def fetch_diff(repo: str, number: int) -> tuple[str, bool]: """Fetch the PR diff and truncate it to the bounded LLM prompt size.""" diff = run(["gh", "api", f"repos/{repo}/pulls/{number}", "-H", "Accept: application/vnd.github.v3.diff"]) + return bound_diff(diff) + + +def bound_diff(diff: str) -> tuple[str, bool]: + """Truncate a unified diff to ``MAX_DIFF_CHARS`` without splitting a changed line.""" truncated = len(diff) > MAX_DIFF_CHARS if truncated: marker = "[overlong changed line content omitted]" @@ -698,12 +704,26 @@ def _nearby_changed_locations( def validate_substantive_verdict( - verdict: dict[str, Any], diff: str, changed_paths: Sequence[str] = () + verdict: dict[str, Any], + diff: str, + changed_paths: Sequence[str] = (), + unobserved_document_objects: Sequence[str] = (), ) -> None: - """Reject formal verdicts without exact changed-line/adversarial evidence.""" + """Reject formal verdicts without exact changed-line/adversarial evidence. + + ``unobserved_document_objects`` are ``path#locator`` ids of changed + document objects (figures, pages, textless packages) whose content no + reviewer saw; any of them makes a formal approval fail closed. + """ decision = str(verdict.get("decision") or "").lower() if decision == "comment": return + if decision == "approve" and unobserved_document_objects: + raise NoemaModelOutputError( + "Noema cannot formally approve changed document content it did not observe " + f"({len(unobserved_document_objects)} unobserved object(s): " + f"{', '.join(unobserved_document_objects[:5])}); use comment or request_changes" + ) locations = changed_diff_locations(diff) if not locations: raise RuntimeError("Noema formal verdict requires parseable changed-line evidence") @@ -862,14 +882,108 @@ def fetch_file_content_at_ref(repo: str, path: str, ref: str) -> str: except (binascii.Error, ValueError) as exc: raise RuntimeError("GitHub content response contained malformed base64") from exc suffix = PurePosixPath(path).suffix.lower() + if suffix in document_blob_diff.OPAQUE_SUFFIXES: + # Never decode PDF/image bytes into the prompt; the object diff covers them. + return "[binary document: reviewed through the object-level diff; raw bytes are not sent]" if suffix in {".docx", ".hwp", ".hwpx"}: try: - return extract_review_document(path, raw) + text = extract_review_document(path, raw) except DocumentReadError as exc: raise RuntimeError(f"document extraction failed: {exc}") from exc + # The whole extracted body would reach the prompt: apply the same + # corresponding-author redaction and participant/secret gate as the + # object diff, and never send on a match. + text = "\n".join( + document_blob_diff.redact_corresponding_author( + text.split("\n"), corresponding_author_allowlist() + ) + ) + try: + document_blob_diff.assert_no_participant_material( + text, path, lambda value: scrub_sensitive_data(value) != value + ) + except document_blob_diff.DocumentSafetyError as exc: + raise RuntimeError(f"document context withheld: {exc}") from exc + return text return raw.decode("utf-8", errors="replace") +def fetch_file_blob_at_ref(repo: str, path: str, ref: str) -> tuple[str, bytes]: + """Fetch one file's blob SHA and raw bytes at an exact ref. + + The contents API names the blob; the Git blobs API returns its bytes for + files beyond the contents API's 1 MB inline limit. The bytes stay on the + runner: callers extract bounded objects from them and never forward them. + """ + encoded_path = urllib.parse.quote(path, safe="/") + encoded_ref = urllib.parse.quote(ref, safe="") + blob_sha = run( + ["gh", "api", f"repos/{repo}/contents/{encoded_path}?ref={encoded_ref}", "--jq", ".sha // empty"] + ).strip() + if not re.fullmatch(r"[0-9a-f]{40}", blob_sha): + raise RuntimeError(f"GitHub did not return a blob SHA for {path} at {ref}") + content = run(["gh", "api", f"repos/{repo}/git/blobs/{blob_sha}", "--jq", ".content // empty"]) + try: + raw = base64.b64decode("".join(content.split()), validate=True) + except (binascii.Error, ValueError) as exc: + raise RuntimeError("GitHub blob response contained malformed base64") from exc + return blob_sha, raw + + +def corresponding_author_allowlist() -> frozenset[str]: + """Return the workflow's exact corresponding-author email allowlist, lowercased.""" + raw = os.environ.get("NOEMA_CORRESPONDING_AUTHOR_EMAILS", "") + return frozenset(item.strip().lower() for item in raw.split(",") if item.strip()) + + +def augment_binary_document_diff( + repo: str, number: int, pr: dict[str, Any], diff: str, truncated: bool +) -> tuple[str, bool, tuple[str, ...]]: + """Replace binary-only document stanzas with citable object-level hunks. + + A DOCX/HWPX/PDF/image change arrives as ``Binary files … differ`` with no + hunk, which left a formal verdict with no changed line to cite. The base + blob at the merge base and the head blob are materialized, diffed as + document objects, and rendered as synthetic hunks whose line numbers are + object ordinals. Any safety rejection fails the review closed; a changed + blob never becomes "no change". A diff already cut to ``MAX_DIFF_CHARS`` + is re-read in full first, so a binary stanza after the cut is not lost. + Also returns the ids of changed objects whose content was not observed, + derived from the envelopes so a caption or a diff cut cannot hide them. + """ + if truncated: + diff = run(["gh", "api", f"repos/{repo}/pulls/{number}", "-H", "Accept: application/vnd.github.v3.diff"]) + stanzas = document_blob_diff.binary_document_stanzas(diff) + if not stanzas: + bounded, more = bound_diff(diff) + return bounded, truncated or more, () + head_sha = str(pr.get("headRefOid") or "") + merge_base = fetch_merge_base_sha(repo, str(pr.get("baseRefOid") or ""), head_sha) + hunks: dict[tuple[str | None, str | None], str] = {} + unobserved: list[str] = [] + for old_path, new_path in stanzas: + base_blob, base_raw = fetch_file_blob_at_ref(repo, old_path, merge_base) if old_path else (None, None) + head_blob, head_raw = fetch_file_blob_at_ref(repo, new_path, head_sha) if new_path else (None, None) + path = new_path or old_path + try: + review = document_blob_diff.build_envelope( + repo, + path, + base_blob, + head_blob, + base_raw, + head_raw, + sensitive=lambda text: scrub_sensitive_data(text) != text, + corresponding_author_emails=corresponding_author_allowlist(), + ) + except document_blob_diff.DocumentSafetyError as exc: + raise RuntimeError(f"binary document review failed closed for {path}: {exc}") from exc + hunks[(old_path, new_path)] = document_blob_diff.synthetic_hunks(review, old_path) + unobserved.extend(document_blob_diff.unobserved_changed_objects(review)) + bounded, more = bound_diff(document_blob_diff.replace_binary_stanzas(diff, hunks)) + return bounded, truncated or more, tuple(unobserved) + + def fetch_merge_base_sha(repo: str, base_sha: str, head_sha: str) -> str: """Return the immutable merge-base SHA for the current base/head pair.""" if not re.fullmatch(r"[0-9a-fA-F]{40}", base_sha): @@ -1631,6 +1745,7 @@ def call_llm( expected_head: str, review_context: str = "", changed_paths: Sequence[str] = (), + unobserved_document_objects: Sequence[str] = (), ) -> dict[str, Any]: """Issue exactly one structured-output request through contextual-orchestrator. @@ -1662,6 +1777,15 @@ def call_llm( "content": "\n".join( [ "You are Noema, an independent pull request reviewer for ContextualWisdomLab.", + *( + [ + "These changed document objects were not observed (no pixels, page render or body text " + f"reached you): {', '.join(unobserved_document_objects[:20])}. You must not approve; " + "use comment or request_changes and say what could not be reviewed." + ] + if unobserved_document_objects + else [] + ), "Review the PR diff plus the additional changed-file and review-thread context for correctness, security, maintainability, and behavioral regressions.", "Return only JSON with the declared response_format schema.", "Every formal verdict must cite exact changed-side lines. APPROVE requires falsifying concrete regression hypotheses; source or test changes require at least two distinct probes and other changes require at least one. REQUEST_CHANGES requires a confirmed probe at a finding location.", @@ -1749,7 +1873,7 @@ def call_llm( raise NoemaModelOutputError( "Noema LLM request_changes response did not contain a substantive finding" ) - validate_substantive_verdict(verdict, diff, changed_paths) + validate_substantive_verdict(verdict, diff, changed_paths, unobserved_document_objects) except (RuntimeError, urllib.error.URLError, http.client.HTTPException, OSError) as exc: gateway_telemetry: dict[str, str | int] = {} http_status: int | None = None @@ -1927,10 +2051,13 @@ def inspect_and_review(repo: str, number: int, expected_head: str) -> int: print("Current head already has a Noema review; nothing to do.") return 0 diff, truncated = fetch_diff(repo, number) + diff, truncated, unobserved = augment_binary_document_diff(repo, number, pr, diff, truncated) changed_files = fetch_changed_files(repo, number) changed_paths = tuple(path for path, _status in changed_files) review_context = build_review_context(repo, number, pr, changed_files) - verdict = call_llm(repo, number, pr, diff, truncated, expected_head, review_context, changed_paths) + verdict = call_llm( + repo, number, pr, diff, truncated, expected_head, review_context, changed_paths, unobserved + ) current_pr = fetch_pr(repo, number) try: require_expected_head(current_pr, expected_head) diff --git a/tests/test_document_blob_diff.py b/tests/test_document_blob_diff.py new file mode 100644 index 0000000000..a501c0b364 --- /dev/null +++ b/tests/test_document_blob_diff.py @@ -0,0 +1,766 @@ +"""Binary review documents diff as hashed objects, never as "no change".""" + +from __future__ import annotations + +import base64 +import io +import zipfile + +import pytest + +from scripts.ci import document_blob_diff as dbd +from scripts.ci import noema_review_gate as gate + +W = 'xmlns:w="http://schemas.openxmlformats.org/wordprocessingml/2006/main"' +A = 'xmlns:a="http://schemas.openxmlformats.org/drawingml/2006/main"' +R = 'xmlns:r="http://schemas.openxmlformats.org/officeDocument/2006/relationships"' +IMAGE_REL = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/image" +LINK_REL = "http://schemas.openxmlformats.org/officeDocument/2006/relationships/hyperlink" + + +def _zip(members: dict[str, bytes | str], *, encrypt: str = "") -> bytes: + """Build an in-memory ZIP package; ``encrypt`` flags one member as encrypted.""" + buffer = io.BytesIO() + with zipfile.ZipFile(buffer, "w", zipfile.ZIP_DEFLATED) as package: + for name, data in members.items(): + package.writestr(name, data) + raw = bytearray(buffer.getvalue()) + if encrypt: + # zipfile clears the encryption bit on write; set it in the central + # directory, which is what the reader's infolist() reports. + start = raw.index(b"PK\x01\x02") + raw[start + 8] |= 0x1 + return bytes(raw) + + +def _docx(paragraphs: list[str], *, table: list[list[str]] | None = None, image: bytes | None = None, + rels: str | None = None, extra: dict[str, bytes | str] | None = None, body: bool = True) -> bytes: + """Build a synthetic DOCX with paragraphs, an optional table and figure.""" + parts = [f"{text}" for text in paragraphs] + if table is not None: + rows = "".join( + "" + "".join(f"{c}" for c in row) + "" + for row in table + ) + parts.append(f"{rows}") + if image is not None: + parts.append('') + inner = f"{''.join(parts)}" if body else "" + members: dict[str, bytes | str] = { + "[Content_Types].xml": "", + "word/document.xml": f"{inner}", + } + if rels is None and image is not None: + rels = f'' + if rels is not None: + members["word/_rels/document.xml.rels"] = rels + if image is not None: + members["word/media/image1.png"] = image + members.update(extra or {}) + return _zip(members) + + +HP = 'xmlns:hp="http://www.hancom.co.kr/hwpml/2011/paragraph"' + + +def _hwpx(sections: list[str], *, manifest: str | None = None, extra: dict[str, bytes | str] | None = None) -> bytes: + """Build a synthetic HWPX with the given section bodies.""" + members: dict[str, bytes | str] = { + "Contents/content.hpf": manifest + if manifest is not None + else '' + '', + "BinData/image1.png": b"\x89PNG-one", + } + for index, body in enumerate(sections): + members[f"Contents/section{index}.xml"] = f"{body}" + members.update(extra or {}) + return _zip(members) + + +def _hp(text: str) -> str: + """One HWPX paragraph.""" + return f"{text}" + + +# ---------------------------------------------------------------- extraction + + +def test_docx_objects_are_ordered_hashed_and_resolve_figures() -> None: + """Paragraphs, tables and figures come back in body order with sha256.""" + raw = _docx(["Intro", "Method"], table=[["N", "1020"], ["M", "3.1"]], image=b"PNG1") + objects = dbd.extract_objects("paper.DOCX", raw) + assert [(o.kind, o.locator) for o in objects] == [ + ("paragraph", "p1"), + ("paragraph", "p2"), + ("table", "tbl1"), + ("figure", "p3/fig:word/media/image1.png"), + ] + assert objects[2].text == "N | 1020\nM | 3.1" + assert objects[3].text is None and len(objects[3].sha256) == 64 + + +def test_docx_package_absolute_relationship_target_and_hyperlinks_are_allowed() -> None: + """``/word/media`` targets resolve and external hyperlinks are plain metadata.""" + rels = ( + f'' + f'' + "" + ) + objects = dbd.extract_objects("a.docx", _docx([], image=b"PNG", rels=rels)) + assert objects[-1].locator == "p1/fig:word/media/image1.png" + + +def test_hwpx_objects_split_tables_from_paragraph_text_across_sections() -> None: + """Section order is numeric; a nested table is its own object.""" + table = "ab" + raw = _hwpx( + [ + _hp("first") + f"lead{table}", + '', + ] + + [""] * 9 + + [_hp("eleventh")] + ) + objects = dbd.extract_objects("x.hwpx", raw) + assert [(o.kind, o.locator, o.text) for o in objects] == [ + ("paragraph", "p1", "first"), + ("paragraph", "p2", "lead"), + ("table", "tbl1", "a | b"), + ("figure", "p3/fig:BinData/image1.png", None), + ("paragraph", "p4", "eleventh"), + ] + + +def test_opaque_documents_are_one_hashed_blob_object() -> None: + """PDF and images are never decoded; they are one hashed object.""" + (obj,) = dbd.extract_objects("scan.pdf", b"%PDF-1.7 binary") + assert (obj.kind, obj.locator, obj.text) == ("page", "blob", None) + assert dbd.is_review_document("fig.PNG") and not dbd.is_review_document("notes.md") + + +@pytest.mark.parametrize( + ("path", "raw", "message"), + ( + ("a.docx", b"not a zip", "not a readable ZIP"), + ("a.docx", _zip({"../evil": "x"}), "member path is unsafe"), + ("a.docx", _zip({"word\\evil": "x"}), "member path is unsafe"), + ("a.docx", _zip({"/abs": "x"}), "member path is unsafe"), + ("a.docx", _zip({"word/vbaProject.bin": "x"}), "macro content"), + ("a.hwpx", _zip({"Scripts/main.js": "x"}), "macro content"), + ("a.docx", _zip({"word/document.xml": "x"}, encrypt="word/document.xml"), "encrypted"), + ("a.docx", _zip({"big": b"\0" * (2 * 1024 * 1024)}), "compression ratio"), + ("a.docx", _zip({"[Content_Types].xml": "application/vnd.ms-word.document.macroEnabled"}), "macro-enabled"), + ("a.docx", _zip({"[Content_Types].xml": ""}), "no word/document.xml"), + ("a.docx", _docx(["x"], body=False), "no body"), + ("a.docx", _zip({"word/document.xml": ""}), "DTD or entity"), + ("a.docx", _zip({"word/document.xml": ""}), "not well-formed"), + ( + "a.docx", + _docx([], rels=f''), + "external image relationship", + ), + ("a.docx", _docx([], image=b"P", rels=""), "relationship is unresolved"), + ( + "a.docx", + _docx([], image=b"P", rels=f''), + "figure target is missing", + ), + ("a.hwpx", _zip({"x": "y"}), "no Contents/content.hpf"), + ("a.hwpx", _hwpx([], manifest=''), "external content"), + ("a.hwpx", _hwpx([]), "no Contents/section"), + ("a.hwpx", _hwpx(['']), "reference is unresolved"), + ("a.txt", b"x", "unsupported review document type"), + ("noext", b"x", "unsupported review document type"), + ), +) +def test_unsafe_or_malformed_documents_fail_closed(path: str, raw: bytes, message: str) -> None: + """Every unsafe or unreadable package raises instead of being skipped.""" + with pytest.raises(dbd.DocumentSafetyError, match=message): + dbd.extract_objects(path, raw) + + +def test_size_member_and_expansion_bounds_fail_closed(monkeypatch: pytest.MonkeyPatch) -> None: + """Blob size, member count and total expansion are bounded.""" + monkeypatch.setattr(dbd, "MAX_BLOB_BYTES", 10) + with pytest.raises(dbd.DocumentSafetyError, match="blob exceeds"): + dbd.extract_objects("a.docx", _docx(["x"])) + with pytest.raises(dbd.DocumentSafetyError, match="blob exceeds"): + dbd.extract_objects("a.pdf", b"%PDF-" + b"x" * 20) + monkeypatch.setattr(dbd, "MAX_BLOB_BYTES", 10**9) + monkeypatch.setattr(dbd, "MAX_MEMBERS", 1) + with pytest.raises(dbd.DocumentSafetyError, match="more than 1 members"): + dbd.extract_objects("a.docx", _docx(["x"])) + monkeypatch.setattr(dbd, "MAX_MEMBERS", 1000) + monkeypatch.setattr(dbd, "MAX_UNCOMPRESSED_BYTES", 10) + with pytest.raises(dbd.DocumentSafetyError, match="expands beyond"): + dbd.extract_objects("a.docx", _docx(["x"])) + + +# ---------------------------------------------------------------- diff + + +def _o(kind: str, key: str) -> dbd.DocumentObject: + """A tiny object whose hash is its key.""" + return dbd.DocumentObject(kind, key, key * 4, key) + + +def test_diff_objects_reports_changes_with_unchanged_neighbours() -> None: + """Alignment pairs same-kind replacements, splits kind changes, keeps context.""" + base = [_o("paragraph", "a"), _o("paragraph", "k"), _o("paragraph", "b"), _o("table", "t"), _o("paragraph", "z")] + head = [_o("paragraph", "a"), _o("paragraph", "k"), _o("paragraph", "B"), _o("figure", "f"), _o("paragraph", "y"), _o("paragraph", "n")] + changes = [(c, bo, ho) for c, bo, _o1, ho, _o2 in dbd.diff_objects(base, head)] + assert changes == [ + ("unchanged", 2, 2), + ("modified", 3, 3), + ("removed", 4, None), + ("added", None, 4), + ("modified", 5, 5), + ("added", None, 6), + ] + middle = [_o("paragraph", "x"), _o("paragraph", "m1"), _o("paragraph", "m2"), _o("paragraph", "m3"), _o("paragraph", "y")] + edited = [_o("paragraph", "X"), *middle[1:4], _o("paragraph", "Y")] + assert [(c, bo) for c, bo, *_ in dbd.diff_objects(middle, edited)] == [ + ("modified", 1), ("unchanged", 2), ("unchanged", 4), ("modified", 5), + ] + assert [c[0] for c in dbd.diff_objects(base, base[:2])] == ["unchanged", "removed", "removed", "removed"] + + +# ---------------------------------------------------------------- envelope + + +def test_envelope_matches_the_co_contract_and_hunks_are_citable() -> None: + """Envelope fields follow document_diff_review.v1; hunks cite object ordinals.""" + base = _docx(["Intro", "N = 957"], image=b"PNG-A") + head = _docx(["Intro", "N = 1,020"], image=b"PNG-B") + review = dbd.build_envelope("o/r", "paper.docx", "1" * 40, "2" * 40, base, head) + env = review.envelope + assert set(env) == { + "contract_version", "repo", "path", "base_blob", "head_blob", + "extractor_version", "participant_material", "objects", + } + assert env["contract_version"] == "document_diff_review.v1" and env["participant_material"] is False + assert [(o["object_kind"], o["change"], o["base_text"], o["head_text"]) for o in env["objects"]] == [ + ("paragraph", "unchanged", "Intro", "Intro"), + ("paragraph", "modified", "N = 957", "N = 1,020"), + ("figure", "modified", None, None), + ] + for obj in env["objects"]: + assert set(obj) == {"page", "object_kind", "locator", "change", "object_hash_base", "object_hash_head", "base_text", "head_text"} + assert obj["object_hash_base"].startswith("sha256:") and len(obj["object_hash_head"]) == 71 + assert env["objects"][0]["object_hash_base"] == env["objects"][0]["object_hash_head"] + hunks = dbd.synthetic_hunks(review) + assert gate.changed_diff_locations(hunks) == { + ("paper.docx", 2, "LEFT"), ("paper.docx", 2, "RIGHT"), + ("paper.docx", 3, "LEFT"), ("paper.docx", 3, "RIGHT"), + } + assert "(no text extracted; hash only)" in hunks + assert "PNG-A" not in hunks and "PNG-B" not in hunks and "Intro" not in hunks + + +def test_added_removed_and_style_only_changes_are_never_no_change() -> None: + """Added/removed documents and metadata-only edits still yield objects.""" + added = dbd.build_envelope("o/r", "n.pdf", None, "2" * 40, None, b"%PDF-new") + assert added.envelope["objects"][0]["object_kind"] == "page" + assert dbd.synthetic_hunks(added).splitlines()[1:3] == ["--- /dev/null", "+++ b/n.pdf"] + removed = dbd.build_envelope("o/r", "n.pdf", "1" * 40, None, b"%PDF-old", None) + hunks = dbd.synthetic_hunks(removed, "old/n.pdf") + assert hunks.splitlines()[:3] == ["diff --git a/old/n.pdf b/n.pdf", "--- a/old/n.pdf", "+++ /dev/null"] + style = dbd.build_envelope( + "o/r", "s.docx", "1" * 40, "2" * 40, _docx(["same"]), _docx(["same"], extra={"word/styles.xml": ""}) + ) + assert [(o["object_kind"], o["locator"], o["change"]) for o in style.envelope["objects"]] == [("style", "package", "modified")] + + +def test_envelope_rejects_identical_or_missing_blobs_participant_material_secrets_and_overflow( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Participant material or secrets are rejected, not redacted, and bounds hold.""" + with pytest.raises(dbd.DocumentSafetyError, match="identical"): + dbd.build_envelope("o/r", "a.pdf", "1" * 40, "1" * 40, b"x", b"y") + with pytest.raises(dbd.DocumentSafetyError, match="both base and head blobs are missing"): + dbd.build_envelope("o/r", "a.pdf", None, None, None, None) + with pytest.raises(dbd.DocumentSafetyError, match="participant-material directory"): + dbd.build_envelope("o/r", "study/Interviews/s1.docx", "1" * 40, "2" * 40, _docx(["a"]), _docx(["b"])) + for text in ("contact kim@example.org", "RRN 900101-1234567", "call 010-1234-5678"): + with pytest.raises(dbd.DocumentSafetyError, match="participant or secret"): + dbd.build_envelope("o/r", "a.docx", "1" * 40, "2" * 40, _docx(["ok"]), _docx([text])) + with pytest.raises(dbd.DocumentSafetyError, match="participant or secret"): + dbd.build_envelope("o/r", "a.docx", "1" * 40, "2" * 40, _docx(["ok"]), _docx(["token"]), sensitive=lambda t: "token" in t) + monkeypatch.setattr(dbd, "MAX_OBJECTS", 0) + with pytest.raises(dbd.DocumentSafetyError, match="more than 0 changed objects"): + dbd.build_envelope("o/r", "a.docx", "1" * 40, "2" * 40, _docx(["a"]), _docx(["b"])) + + +# ---------------------------------------------------------------- stanzas + + +BINARY_ONLY_DIFF = ( + "diff --git a/paper.docx b/paper.docx\n" + "index 1111111..2222222 100644\n" + "Binary files a/paper.docx and b/paper.docx differ\n" + "diff --git a/logo.bin b/logo.bin\n" + "Binary files a/logo.bin and b/logo.bin differ\n" + "diff --git a/new.pdf b/new.pdf\n" + "new file mode 100644\n" + "Binary files /dev/null and b/new.pdf differ\n" +) + + +def test_binary_stanzas_are_found_and_replaced_per_document() -> None: + """Only review-document binaries are selected; others keep their stanza.""" + assert dbd.binary_document_stanzas(BINARY_ONLY_DIFF + BINARY_ONLY_DIFF) == [ + ("paper.docx", "paper.docx"), + (None, "new.pdf"), + ] + replaced = dbd.replace_binary_stanzas(BINARY_ONLY_DIFF, {("paper.docx", "paper.docx"): "HUNK"}) + assert replaced.startswith("HUNK\ndiff --git a/logo.bin") + assert "Binary files /dev/null and b/new.pdf differ" in replaced + + +# ---------------------------------------------------------------- gate + + +def test_binary_only_textual_diff_alone_has_no_citable_line() -> None: + """RED characterization: the raw binary-only diff cannot carry a formal verdict.""" + verdict = {"decision": "approve", "reviewed_lines": [], "findings": []} + with pytest.raises(RuntimeError, match="requires parseable changed-line evidence"): + gate.validate_substantive_verdict(verdict, BINARY_ONLY_DIFF) + + +def _fake_github(monkeypatch: pytest.MonkeyPatch, blobs: dict[tuple[str, str], bytes]) -> None: + """Serve contents/blobs/compare calls from ``blobs`` keyed by (path, ref).""" + shas = {key: f"{index:040x}" for index, key in enumerate(blobs, start=1)} + + def fake_run(args, *, stdin=None): + """Return canned GitHub API output for the materialization calls.""" + url = args[2] + if "/compare/" in url: + return "b" * 40 + if "/contents/" in url: + path, ref = url.split("/contents/", 1)[1].split("?ref=") + return shas[(path, ref)] + sha = url.rsplit("/", 1)[1] + key = next(k for k, v in shas.items() if v == sha) + return base64.b64encode(blobs[key]).decode() + + monkeypatch.setattr(gate, "run", fake_run) + + +def test_binary_only_docx_pr_yields_a_citable_request_changes_finding(monkeypatch: pytest.MonkeyPatch) -> None: + """GREEN: a binary-only PR produces object hunks that a real finding can cite.""" + head = "c" * 40 + _fake_github( + monkeypatch, + { + ("paper.docx", "b" * 40): _docx(["Intro", "N = 957"]), + ("paper.docx", head): _docx(["Intro", "N = 1,020"]), + ("new.pdf", head): b"%PDF-1.7", + }, + ) + pr = {"baseRefOid": "a" * 40, "headRefOid": head} + diff, truncated, _unobserved = gate.augment_binary_document_diff("o/r", 7, pr, BINARY_ONLY_DIFF, False) + assert not truncated + assert "Binary files a/paper.docx" not in diff and "Binary files a/logo.bin" in diff + verdict = { + "decision": "request_changes", + "reviewed_lines": [{"path": "paper.docx", "line": 2, "side": "RIGHT"}], + "findings": [{"path": "paper.docx", "line": 2, "side": "RIGHT", "message": "Sample size contradicts Table 1."}], + } + locations = gate.changed_diff_locations(diff) + assert ("paper.docx", 2, "RIGHT") in locations and ("new.pdf", 1, "RIGHT") in locations + for finding in verdict["findings"]: + assert (finding["path"], finding["line"], finding["side"]) in locations + + +def test_augmentation_passes_through_text_diffs_and_fails_closed_on_unsafe_blobs(monkeypatch: pytest.MonkeyPatch) -> None: + """No stanza means no API call; a safety rejection fails the review closed.""" + monkeypatch.setattr(gate, "run", lambda *_a, **_k: pytest.fail("no API call expected")) + assert gate.augment_binary_document_diff("o/r", 7, {}, "diff --git a/x b/x\n", False) == ("diff --git a/x b/x\n", False, ()) + head = "c" * 40 + _fake_github( + monkeypatch, + {("paper.docx", "b" * 40): _docx(["ok"]), ("paper.docx", head): _zip({"word/vbaProject.bin": "x"}), ("new.pdf", head): b"%PDF"}, + ) + with pytest.raises(RuntimeError, match="binary document review failed closed for paper.docx: package contains macro"): + gate.augment_binary_document_diff("o/r", 7, {"baseRefOid": "a" * 40, "headRefOid": head}, BINARY_ONLY_DIFF, False) + + +def test_blob_fetch_fails_closed_on_missing_sha_or_bad_base64(monkeypatch: pytest.MonkeyPatch) -> None: + """A missing blob SHA or malformed payload raises.""" + monkeypatch.setattr(gate, "run", lambda args, **_k: "" if "/contents/" in args[2] else "!") + with pytest.raises(RuntimeError, match="did not return a blob SHA"): + gate.fetch_file_blob_at_ref("o/r", "a b.pdf", "main") + monkeypatch.setattr(gate, "run", lambda args, **_k: "c" * 40 if "/contents/" in args[2] else "!!notbase64") + with pytest.raises(RuntimeError, match="malformed base64"): + gate.fetch_file_blob_at_ref("o/r", "a.pdf", "main") + + +def test_opaque_head_content_is_never_decoded_into_the_prompt(monkeypatch: pytest.MonkeyPatch) -> None: + """PDF/image bytes no longer reach the changed-file context as mojibake.""" + monkeypatch.setattr(gate, "run", lambda *_a, **_k: base64.b64encode(b"%PDF secret-bytes").decode()) + text = gate.fetch_file_content_at_ref("o/r", "scan.pdf", "h" * 40) + assert "secret-bytes" not in text and "object-level diff" in text + + +# ---------------------------------------------------------------- CO byte bounds and full-body privacy + + +def _texts(envelope: dict) -> list[str]: + """All non-null base/head texts in an envelope.""" + return [t for o in envelope["objects"] for t in (o["base_text"], o["head_text"]) if t is not None] + + +def test_changed_korean_text_over_the_byte_bound_fails_closed_and_context_is_marked() -> None: + """Changed text is never cut: a 5,000-character Korean change (15,000 B) fails closed. + + RED on bddeb901: text was cut at 8,192 characters (15,000 bytes kept, so + contextual-orchestrator#1220 answered 400). An unchanged neighbour of the + same size is cut on a UTF-8 boundary and ends with the truncation marker. + """ + long_ko = "가" * 5000 + with pytest.raises(dbd.DocumentSafetyError, match="changed object p1 exceeds the 8192-byte text bound"): + dbd.build_envelope("o/r", "k.docx", "1" * 40, "2" * 40, _docx(["짧음"]), _docx([long_ko])) + review = dbd.build_envelope("o/r", "k.docx", "1" * 40, "2" * 40, _docx([long_ko, "old"]), _docx([long_ko, "new"])) + context = review.envelope["objects"][0] + assert context["change"] == "unchanged" + assert context["head_text"].endswith(dbd.TRUNCATION_MARKER) + assert len(context["head_text"].encode("utf-8")) <= dbd.MAX_OBJECT_TEXT_BYTES == 8 * 1024 + + +def test_changed_objects_over_the_envelope_budget_fail_closed_without_a_provider_call( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """contextual-orchestrator lead fixture: 40 changed Korean paragraphs, the last substantive. + + RED on c6f4b49b: the budget ran out and 24 changed objects were kept with + "" text, so "결론: 효과 없음" never reached the reviewer and a verdict could + approve an unseen change. Now the envelope fails closed and the gate raises + before any model request. + """ + base = _docx(["다" * 2700 + f"{i:03d}" for i in range(39)] + ["결론: 효과 있음"]) + head = _docx(["라" * 2700 + f"{i:03d}" for i in range(39)] + ["결론: 효과 없음"]) + with pytest.raises(dbd.DocumentSafetyError, match="changed document objects exceed the 262144-byte review budget"): + dbd.build_envelope("o/r", "k.docx", "1" * 40, "2" * 40, base, head) + c = "c" * 40 + _fake_github(monkeypatch, {("paper.docx", "b" * 40): base, ("paper.docx", c): head, ("new.pdf", c): b"%PDF"}) + with pytest.raises(RuntimeError, match="binary document review failed closed for paper.docx: changed document objects exceed"): + gate.augment_binary_document_diff("o/r", 7, {"baseRefOid": "a" * 40, "headRefOid": c}, BINARY_ONLY_DIFF, False) + + +@pytest.mark.parametrize("count", (1, 5, 30)) +def test_no_changed_object_ever_carries_empty_text(count: int) -> None: + """Invariant: every accepted changed object carries its full source text.""" + base = _docx([f"base {i} " + "가" * 900 for i in range(count)]) + head = _docx([f"head {i} " + "나" * 900 for i in range(count)]) + review = dbd.build_envelope("o/r", "k.docx", "1" * 40, "2" * 40, base, head) + changed = [o for o in review.envelope["objects"] if o["change"] != "unchanged"] + assert len(changed) == count + for obj in changed: + for side in ("base", "head"): + text = obj[f"{side}_text"] + assert text and not text.endswith(dbd.TRUNCATION_MARKER) + assert obj[f"object_hash_{side}"] == "sha256:" + dbd._sha256(text.encode("utf-8")) + + +def test_unchanged_context_is_dropped_before_changed_objects_lose_budget(monkeypatch: pytest.MonkeyPatch) -> None: + """Changed objects claim the budget first; context goes when bytes or slots run out.""" + base = _docx(["ctx-a", "old", "ctx-b"]) + head = _docx(["ctx-a", "new", "ctx-b"]) + monkeypatch.setattr(dbd, "MAX_ENVELOPE_TEXT_BYTES", 6) + review = dbd.build_envelope("o/r", "c.docx", "1" * 40, "2" * 40, base, head) + assert [(o["change"], o["base_text"], o["head_text"]) for o in review.envelope["objects"]] == [ + ("modified", "old", "new"), + ] + monkeypatch.setattr(dbd, "MAX_ENVELOPE_TEXT_BYTES", 1024) + monkeypatch.setattr(dbd, "MAX_OBJECTS", 2) + review = dbd.build_envelope("o/r", "c.docx", "1" * 40, "2" * 40, base, head) + assert [o["change"] for o in review.envelope["objects"]] == ["unchanged", "modified"] + + +def test_control_characters_are_blanked_to_match_the_co_text_rule() -> None: + """contextual-orchestrator rejects control characters other than newline and tab. + + XML 1.0 already refuses most of them; a `` `` reference still yields a + carriage return, which is blanked instead of sent. + """ + review = dbd.build_envelope( + "o/r", "c.docx", "1" * 40, "2" * 40, _docx(["a"]), _docx(["x y\tz"]) + ) + assert review.envelope["objects"][0]["head_text"] == "x y\tz" + + +def test_participant_identifier_far_from_the_change_fails_the_review_closed() -> None: + """A phone number in an unchanged paragraph far from the edit is still rejected. + + RED on bddeb901: only diffed objects and their neighbours were scanned, so + the phone number was not in the envelope and passed, while the full body + still reached the changed-file context. + """ + body = ["title", "010-2345-6789 연락처", "filler 1", "filler 2", "filler 3", "result old"] + edited = body[:-1] + ["result new"] + with pytest.raises(dbd.DocumentSafetyError, match="participant or secret pattern in extracted text at p2"): + dbd.build_envelope("o/r", "far.docx", "1" * 40, "2" * 40, _docx(body), _docx(edited)) + + +def test_full_document_context_is_withheld_when_it_carries_participant_material(monkeypatch: pytest.MonkeyPatch) -> None: + """The changed-file context path applies the same gate to the whole extracted body.""" + raw = _docx(["intro", "call 010-2345-6789"]) + monkeypatch.setattr(gate, "run", lambda *_a, **_k: base64.b64encode(raw).decode()) + monkeypatch.setattr(gate, "extract_review_document", lambda _path, _raw: "intro\ncall 010-2345-6789") + with pytest.raises(RuntimeError, match="document context withheld: participant or secret pattern"): + gate.fetch_file_content_at_ref("o/r", "far.docx", "c" * 40) + monkeypatch.setattr(gate, "extract_review_document", lambda _path, _raw: "intro only") + assert gate.fetch_file_content_at_ref("o/r", "ok.docx", "c" * 40) == "intro only" + + +# ---------------------------------------------------------------- captions, spine, corresponding author + + +def test_figure_objects_carry_their_caption_so_a_silent_image_swap_is_visible() -> None: + """A changed image under an unchanged "Figure N" caption keeps equal caption text.""" + review = dbd.build_envelope( + "o/r", "f.docx", "1" * 40, "2" * 40, + _docx(["Figure 1. Flow of participants"], image=b"IMG-A"), + _docx(["Figure 1. Flow of participants"], image=b"IMG-B"), + ) + figure = next(o for o in review.envelope["objects"] if o["object_kind"] == "figure") + assert figure["change"] == "modified" + assert figure["base_text"] == figure["head_text"] == "Figure 1. Flow of participants" + objects = dbd._attach_captions( + [dbd.DocumentObject("figure", "f", "h", None), dbd.DocumentObject("paragraph", "p", "t", "그림 2 결과")] + ) + assert objects[0].text == "그림 2 결과" + lone = dbd._attach_captions([dbd.DocumentObject("paragraph", "p", "t", "not a caption"), dbd.DocumentObject("figure", "f", "h", None)]) + assert lone[1].text is None + + +def test_hwpx_sections_follow_the_manifest_spine() -> None: + """Reading order is the spine, not the file-name number; missing spine sections fail closed.""" + manifest = ( + '' + '' + '' + '' + ) + raw = _hwpx([_hp("zero"), _hp("one")], manifest=manifest) + assert [o.text for o in dbd.extract_objects("s.hwpx", raw)] == ["one", "zero"] + broken = manifest.replace('href="Contents/section1.xml"', 'href="Contents/section9.xml"') + with pytest.raises(dbd.DocumentSafetyError, match="spine section is missing"): + dbd.extract_objects("s.hwpx", _hwpx([_hp("zero"), _hp("one")], manifest=broken)) + + +AUTHOR = "lead.author@univ.example.ac.kr" +MANUSCRIPT = [ + "Late-life anxiety reanalysis", + f"Corresponding author: Kim, Dept. of Psychology ({AUTHOR})", + "Abstract", + "Results changed here.", +] + + +def test_declared_allowlisted_corresponding_author_email_is_masked_everywhere(monkeypatch: pytest.MonkeyPatch) -> None: + """A normal manuscript passes; the real address never reaches envelope, hunks or errors.""" + allow = frozenset({AUTHOR}) + edited = MANUSCRIPT[:-1] + ["Results changed there."] + review = dbd.build_envelope("o/r", "m.docx", "1" * 40, "2" * 40, _docx(MANUSCRIPT), _docx(edited), corresponding_author_emails=allow) + serialized = str(review.envelope) + dbd.synthetic_hunks(review) + assert AUTHOR not in serialized + title_change = MANUSCRIPT[:1] + [MANUSCRIPT[1] + " revised"] + MANUSCRIPT[2:] + review = dbd.build_envelope("o/r", "m.docx", "1" * 40, "2" * 40, _docx(MANUSCRIPT), _docx(title_change), corresponding_author_emails=allow) + masked = [o["head_text"] for o in review.envelope["objects"] if o["change"] == "modified"][0] + assert dbd.CORRESPONDING_AUTHOR_MARKER in masked and AUTHOR not in masked + # The same marker reaches the whole-body context path through the workflow allowlist. + monkeypatch.setenv("NOEMA_CORRESPONDING_AUTHOR_EMAILS", f" {AUTHOR.upper()} , other@x.org") + monkeypatch.setattr(gate, "run", lambda *_a, **_k: base64.b64encode(_docx(MANUSCRIPT)).decode()) + monkeypatch.setattr(gate, "extract_review_document", lambda _p, _r: "\n".join(MANUSCRIPT)) + context = gate.fetch_file_content_at_ref("o/r", "m.docx", "c" * 40) + assert AUTHOR not in context and dbd.CORRESPONDING_AUTHOR_MARKER in context + + +@pytest.mark.parametrize( + ("paragraphs", "allow"), + ( + (MANUSCRIPT, frozenset()), + (MANUSCRIPT, frozenset({"someone.else@univ.example.ac.kr"})), + (MANUSCRIPT + [f"Please write to {AUTHOR} for data."], frozenset({AUTHOR})), + (["Title", "Abstract", f"Corresponding author: {AUTHOR}"], frozenset({AUTHOR})), + (["Title", f"Contact the lab at {AUTHOR}"], frozenset({AUTHOR})), + (["Title", f"Corresponding author: {AUTHOR}; co-author b@univ.example.ac.kr"], frozenset({AUTHOR})), + ([f"p{i}" for i in range(20)] + [f"Corresponding author: {AUTHOR}"], frozenset({AUTHOR})), + (["Title", f"Corresponding author: {AUTHOR}, tel 010-2345-6789"], frozenset({AUTHOR})), + ), +) +def test_every_other_email_placement_still_fails_closed_without_leaking_it(paragraphs: list[str], allow: frozenset[str]) -> None: + """Allowlist mismatch, body reuse, post-abstract, undeclared, extra address, late position, phone.""" + with pytest.raises(dbd.DocumentSafetyError) as raised: + dbd.build_envelope("o/r", "m.docx", "1" * 40, "2" * 40, _docx(["x"]), _docx(paragraphs), corresponding_author_emails=allow) + assert AUTHOR not in str(raised.value) + + +def test_email_inside_a_table_is_never_excepted() -> None: + """Tables are not front-matter author-contact paragraphs.""" + with pytest.raises(dbd.DocumentSafetyError, match="participant or secret"): + dbd.build_envelope( + "o/r", "m.docx", "1" * 40, "2" * 40, _docx(["x"]), + _docx(["Title"], table=[["Corresponding author", AUTHOR]]), + corresponding_author_emails=frozenset({AUTHOR}), + ) + + +# ---------------------------------------------------------------- exact-head and CodeRabbit review fixes + + +def _approve(path: str, line: int) -> dict: + """A structurally valid approve verdict citing one changed line (as in the gate tests).""" + probe = { + "path": path, "line": line, "side": "RIGHT", "hypothesis": "h", "attack_or_counterexample": "a", + "evidence": "e", "outcome": "falsified", + } + return { + "decision": "approve", + "summary": "Reviewed.", + "findings": [], + "reviewed_lines": [{"path": path, "line": line, "side": "RIGHT", "analysis": "Checked."}], + "adversarial_validation": {"status": "passed", "residual_risk": "none", "probes": [probe, dict(probe, hypothesis="h2")]}, + } + + +def _augment(monkeypatch: pytest.MonkeyPatch, blobs: dict, diff: str, truncated: bool = False): + """Run augmentation against canned blobs for head ``c*40``.""" + _fake_github(monkeypatch, blobs) + return gate.augment_binary_document_diff("o/r", 7, {"baseRefOid": "a" * 40, "headRefOid": "c" * 40}, diff, truncated) + + +PAPER = "diff --git a/paper.docx b/paper.docx\nBinary files a/paper.docx and b/paper.docx differ\n" + + +def test_hash_only_pdf_change_cannot_be_formally_approved(monkeypatch: pytest.MonkeyPatch) -> None: + """A PDF-only PR whose base/head say opposite things: approve fails closed, comment stays open.""" + c = "c" * 40 + pdf_diff = "diff --git a/r.pdf b/r.pdf\nBinary files a/r.pdf and b/r.pdf differ\n" + diff, _, unobserved = _augment(monkeypatch, {("r.pdf", "b" * 40): b"%PDF effect found", ("r.pdf", c): b"%PDF no effect"}, pdf_diff) + assert unobserved == ("r.pdf#blob",) + with pytest.raises(gate.NoemaModelOutputError, match=r"did not observe \(1 unobserved object\(s\): r.pdf#blob\)"): + gate.validate_substantive_verdict(_approve("r.pdf", 1), diff, (), unobserved) + gate.validate_substantive_verdict({"decision": "comment"}, diff, (), unobserved) + + +@pytest.mark.parametrize("caption", (True, False)) +def test_captioned_or_uncaptioned_image_swap_is_unobserved(monkeypatch: pytest.MonkeyPatch, caption: bool) -> None: + """RED bypass 1 on 8da729e4: a captioned image swap rendered with text and passed the regex guard.""" + c = "c" * 40 + text = ["Figure 1. Recruitment flow"] if caption else ["Body"] + diff, _, unobserved = _augment( + monkeypatch, {("paper.docx", "b" * 40): _docx(text, image=b"IMG-A"), ("paper.docx", c): _docx(text, image=b"IMG-B")}, PAPER + ) + assert unobserved == ("paper.docx#p2/fig:word/media/image1.png",) + line = 2 + with pytest.raises(gate.NoemaModelOutputError, match="did not observe"): + gate.validate_substantive_verdict(_approve("paper.docx", line), diff, (), unobserved) + + +def test_unobserved_flag_survives_a_diff_cut(monkeypatch: pytest.MonkeyPatch) -> None: + """RED bypass 2 on 8da729e4: the hash-only line past MAX_DIFF_CHARS vanished from the guard's view.""" + c = "c" * 40 + filler = "diff --git a/big.txt b/big.txt\n--- a/big.txt\n+++ b/big.txt\n@@ -1,1 +1,1 @@\n-a\n+" + "x" * (gate.MAX_DIFF_CHARS + 10) + "\n" + diff, truncated, unobserved = _augment( + monkeypatch, {("r.pdf", "b" * 40): b"%PDF old", ("r.pdf", c): b"%PDF new"}, + filler + "diff --git a/r.pdf b/r.pdf\nBinary files a/r.pdf and b/r.pdf differ\n", + ) + assert truncated and "r.pdf" not in diff + assert unobserved == ("r.pdf#blob",) + with pytest.raises(gate.NoemaModelOutputError, match="r.pdf#blob"): + gate.validate_substantive_verdict(_approve("big.txt", 1), diff, (), unobserved) + + +def test_observable_text_document_change_is_a_negative_control(monkeypatch: pytest.MonkeyPatch) -> None: + """A text-only DOCX edit is fully observed: no unobserved ids, and a valid approve passes.""" + c = "c" * 40 + diff, _, unobserved = _augment(monkeypatch, {("paper.docx", "b" * 40): _docx(["Old claim"]), ("paper.docx", c): _docx(["New claim"])}, PAPER) + assert unobserved == () + gate.validate_substantive_verdict(_approve("paper.docx", 1), diff, (), unobserved) + + +def test_prompt_lists_unobserved_objects_and_a_refused_approve_leaks_no_secret(monkeypatch: pytest.MonkeyPatch) -> None: + """The model is told which objects it did not see; the refusal names ids only.""" + import json as _json + + seen: dict = {} + monkeypatch.setenv("NOEMA_LLM_API_URL", "https://llm.example.test/chat") + monkeypatch.setenv("NOEMA_LLM_API_KEY", "sk-live-secret-value") + + class _Response: + """Minimal HTTP response carrying one approve verdict.""" + + def __init__(self, payload: dict) -> None: + """Store the JSON payload.""" + self.body = _json.dumps(payload).encode() + self.headers = {} + self.status = 200 + + def read(self) -> bytes: + """Return the body.""" + return self.body + + def __enter__(self): + """Context-manager entry.""" + return self + + def __exit__(self, *_exc) -> None: + """Context-manager exit.""" + + class _Opener: + """Capture the request and return an approve verdict.""" + + def open(self, request, timeout=None): + """Record the prompt and answer approve.""" + seen["body"] = _json.loads(request.data.decode()) + verdict = _approve("r.pdf", 1) + return _Response({"choices": [{"message": {"content": _json.dumps(verdict)}}]}) + + monkeypatch.setattr(gate.urllib.request, "build_opener", lambda *_a: _Opener()) + diff = "diff --git a/r.pdf b/r.pdf\n--- a/r.pdf\n+++ b/r.pdf\n@@ -1,1 +1,1 @@ modified\n-[page blob sha256:000000000000] x\n+[page blob sha256:111111111111] y\n" + with pytest.raises(RuntimeError) as raised: + gate.call_llm("o/r", 7, {"headRefOid": "c" * 40}, diff, False, "c" * 40, "", ("r.pdf",), ("r.pdf#blob",)) + prompt = _json.dumps(seen["body"]) + assert "These changed document objects were not observed" in prompt and "r.pdf#blob" in prompt + assert isinstance(raised.value, gate.NoemaModelOutputError) + assert "sk-live-secret-value" not in str(raised.value) + # Control: the identical model answer is accepted when every changed object was observed, + # so the refusal above comes from the unobserved guard and nothing else. + verdict = gate.call_llm("o/r", 7, {"headRefOid": "c" * 40}, diff, False, "c" * 40, "", ("r.pdf",), ()) + assert verdict["decision"] == "approve" + assert "were not observed" not in _json.dumps(seen["body"]) + + +def test_added_or_removed_package_without_extractable_objects_is_still_citable() -> None: + """A DOCX with an empty body added or removed yields a package object, not an empty hunk.""" + added = dbd.build_envelope("o/r", "e.docx", None, "2" * 40, None, _docx([])) + assert [(o["object_kind"], o["change"], o["object_hash_base"]) for o in added.envelope["objects"]] == [("style", "added", None)] + assert ("e.docx", 1, "RIGHT") in gate.changed_diff_locations(dbd.synthetic_hunks(added)) + removed = dbd.build_envelope("o/r", "e.docx", "1" * 40, None, _docx([]), None) + assert [(o["change"], o["object_hash_head"]) for o in removed.envelope["objects"]] == [("removed", None)] + assert ("e.docx", 1, "LEFT") in gate.changed_diff_locations(dbd.synthetic_hunks(removed)) + + +def test_truncated_diff_is_reread_so_a_late_binary_stanza_is_not_lost(monkeypatch: pytest.MonkeyPatch) -> None: + """A binary stanza after the MAX_DIFF_CHARS cut is recovered from the full diff.""" + c = "c" * 40 + full = "diff --git a/big.txt b/big.txt\n+" + "x" * 50 + "\n" + BINARY_ONLY_DIFF + blobs = {("paper.docx", "b" * 40): _docx(["A"]), ("paper.docx", c): _docx(["B"]), ("new.pdf", c): b"%PDF"} + _fake_github(monkeypatch, blobs) + served = gate.run + + def run_with_full_diff(args, *, stdin=None): + """Serve the full diff for the re-read; delegate blob calls.""" + if any("v3.diff" in a for a in args): + return full + return served(args, stdin=stdin) + + monkeypatch.setattr(gate, "run", run_with_full_diff) + cut = full[:40] + diff, truncated, _unobserved = gate.augment_binary_document_diff("o/r", 7, {"baseRefOid": "a" * 40, "headRefOid": c}, cut, True) + assert truncated is True + assert ("paper.docx", 1, "RIGHT") in gate.changed_diff_locations(diff) + assert "Binary files a/paper.docx" not in diff