From 828eaaefb0cc97bba4da63eb9270447476d26710 Mon Sep 17 00:00:00 2001 From: Seongho Bae Date: Mon, 14 Sep 2026 02:57:18 +0900 Subject: [PATCH] fix(noema): close document reader review gaps (#2178) Follow-up to #2172. The required hosted workflows remained queued repository-wide with zero available Actions runners; local focused, fixture, security, and syntax validation passed for the exact head. Apply the reviewed document-reader fixes. --- .../agent-review-runtime-quality-ci.yml | 16 +++++++++-- scripts/ci/noema_hwp_mcp_reader.mjs | 5 +++- scripts/ci/noema_review_document.py | 3 +- tests/test_noema_document_review_context.py | 28 +++++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/.github/workflows/agent-review-runtime-quality-ci.yml b/.github/workflows/agent-review-runtime-quality-ci.yml index 6340b2e055..3976424bfb 100644 --- a/.github/workflows/agent-review-runtime-quality-ci.yml +++ b/.github/workflows/agent-review-runtime-quality-ci.yml @@ -11,6 +11,11 @@ on: - "tests/test_noema_two_phase_handoff.py" - "tests/test_noema_refreshed_app_identity.py" - "tests/test_noema_token_lifetime_stale_run_contract.py" + - "scripts/ci/noema_review_document.py" + - "scripts/ci/noema_hwp_mcp_reader.mjs" + - "scripts/ci/noema-document-reader/package.json" + - "scripts/ci/noema-document-reader/package-lock.json" + - "tests/test_noema_document_review_context.py" - "docs/doctoring/noema-review-token-lifetime.md" - "docs/product-technical-gap-baseline.md" - ".github/workflows/opencode-review-dispatch.yml" @@ -185,6 +190,11 @@ jobs: tests/test_noema_two_phase_handoff.py|\ tests/test_noema_refreshed_app_identity.py|\ tests/test_noema_token_lifetime_stale_run_contract.py|\ + scripts/ci/noema_review_document.py|\ + scripts/ci/noema_hwp_mcp_reader.mjs|\ + scripts/ci/noema-document-reader/package.json|\ + scripts/ci/noema-document-reader/package-lock.json|\ + tests/test_noema_document_review_context.py|\ docs/doctoring/noema-review-token-lifetime.md) noema_suite=true ;; @@ -349,13 +359,15 @@ jobs: tests/test_noema_reviewer_token_lifetime.py \ tests/test_noema_two_phase_handoff.py \ tests/test_noema_refreshed_app_identity.py \ - tests/test_noema_token_lifetime_stale_run_contract.py + tests/test_noema_token_lifetime_stale_run_contract.py \ + tests/test_noema_document_review_context.py python -m compileall -q \ .github/actions/noema-review/two_phase.py \ tests/test_noema_reviewer_token_lifetime.py \ tests/test_noema_two_phase_handoff.py \ tests/test_noema_refreshed_app_identity.py \ - tests/test_noema_token_lifetime_stale_run_contract.py + tests/test_noema_token_lifetime_stale_run_contract.py \ + tests/test_noema_document_review_context.py - name: Verify OpenCode Rust coverage toolchain contract if: steps.affected_suites.outputs.opencode == 'true' diff --git a/scripts/ci/noema_hwp_mcp_reader.mjs b/scripts/ci/noema_hwp_mcp_reader.mjs index 37373cbbf6..ccd32683b7 100644 --- a/scripts/ci/noema_hwp_mcp_reader.mjs +++ b/scripts/ci/noema_hwp_mcp_reader.mjs @@ -28,7 +28,10 @@ try { const document = await documentModule.openDocument(filePath); documentModule.closeDocument(document); const text = await toolsModule.readHwp({ file_path: filePath }); - if (!text || /(?:파일 읽기 오류|File not found|text extraction error)/i.test(text)) { + if ( + !text || + /^(?:파일 읽기 오류|File not found|텍스트 추출 오류|text extraction error)/i.test(text) + ) { throw new Error("hwp-mcp returned an extraction error"); } process.stdout.write(`${text}\n`); diff --git a/scripts/ci/noema_review_document.py b/scripts/ci/noema_review_document.py index f0211cf035..17d3ca603c 100644 --- a/scripts/ci/noema_review_document.py +++ b/scripts/ci/noema_review_document.py @@ -16,6 +16,7 @@ from pathlib import PurePosixPath from defusedxml import ElementTree as ET +from defusedxml.common import DefusedXmlException MAX_DOCUMENT_BYTES = 8 * 1024 * 1024 @@ -79,7 +80,7 @@ def _extract_docx(raw: bytes) -> str: try: root = ET.fromstring(document_xml) - except ET.ParseError as exc: + except (ET.ParseError, DefusedXmlException) as exc: raise DocumentReadError("DOCX document.xml is malformed") from exc body = root.find(f"{W}body") diff --git a/tests/test_noema_document_review_context.py b/tests/test_noema_document_review_context.py index 1cfa6c40b1..e6ec2e6d70 100644 --- a/tests/test_noema_document_review_context.py +++ b/tests/test_noema_document_review_context.py @@ -34,6 +34,19 @@ def _docx_bytes(*, malformed: bool = False) -> bytes: return output.getvalue() +def _docx_entity_bytes() -> bytes: + """Build a DOCX whose entity declaration must be rejected safely.""" + xml = """ +]> + + &expansion; +""" + output = io.BytesIO() + with zipfile.ZipFile(output, "w", zipfile.ZIP_DEFLATED) as archive: + archive.writestr("word/document.xml", xml) + return output.getvalue() + + def _pr() -> dict[str, object]: return { "headRefOid": "head", @@ -75,6 +88,15 @@ def test_hosted_reader_bundle_is_pinned_and_local(): assert "python3 -m pip install --quiet --require-hashes --no-deps" in workflow assert "requirements-noema-document-ci-hashes.txt" in quality_workflow assert "Install exact Noema document dependencies" in quality_workflow + for path in ( + "scripts/ci/noema_review_document.py", + "scripts/ci/noema_hwp_mcp_reader.mjs", + "scripts/ci/noema-document-reader/package.json", + "scripts/ci/noema-document-reader/package-lock.json", + "tests/test_noema_document_review_context.py", + ): + assert path in quality_workflow + assert "tests/test_noema_document_review_context.py" in quality_workflow def test_docx_text_reaches_the_actual_reviewer_payload(monkeypatch): @@ -148,6 +170,12 @@ def test_malformed_docx_is_explicit_in_review_context(monkeypatch): assert "not a zip archive" not in context +def test_forbidden_docx_entities_are_explicitly_rejected(): + """Defused XML entity failures become the same bounded reader error.""" + with pytest.raises(document.DocumentReadError, match="DOCX document.xml is malformed"): + document.extract_review_document("docs/entity.docx", _docx_entity_bytes()) + + def test_hwp_reader_contract_is_local_and_fail_closed(monkeypatch): """HWP/HWPX use the configured local adapter and reject failed readers.""" monkeypatch.setenv(document.HWP_READER_ENV, "/trusted/hwp-mcp-source")