feat(attachments): recognize HWP and HWPX parser boundaries - #1353
feat(attachments): recognize HWP and HWPX parser boundaries#1353seonghobae wants to merge 26 commits into
Conversation
|
Important Approval pendingCodeRabbit has no unresolved comments, but it has not reviewed the latest commit. Use the checkbox below to review the latest commit. CodeRabbit will approve the changes if it finds no blocking issues.
📝 WalkthroughWalkthroughThe attachment parser now recognizes HWPX and HWP files, validates bounded family signatures, preserves valid payloads for deferred processing, and rejects invalid payloads. Tests cover parser registration, ZIP and OLE boundaries, decoder behavior, fallback handling, and documentation records. ChangesAttachment recognition
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: 🟡 Moderate · up to The deferred decoder can currently accept unsupported expected content types without validating the attachment family, which could admit arbitrary bytes into downstream processing. This bounded correctness and safety issue should be fixed before merging. Sequence Diagram(s)sequenceDiagram
participant EmailImport
participant parse_email_attachment
participant PayloadValidators
participant DeferredWorker
EmailImport->>parse_email_attachment: attachment and content type
parse_email_attachment->>PayloadValidators: validate family-specific signature
PayloadValidators-->>parse_email_attachment: valid payload or error code
parse_email_attachment-->>EmailImport: pending status and base64 payload
DeferredWorker->>decode_deferred_attachment_payload: payload and expected content type
decode_deferred_attachment_payload-->>DeferredWorker: validated source bytes
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@opencode-agent review current exact head |
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
@coderabbitai review |
|
Parent #1353 admission requires version.xml before a HWPX payload can remain pending. Worker fixtures omitted that member, so deferred revalidation failed closed before recognition. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
Reconcile Ready PR #1353 onto current protected develop without changing HWP/HWPX attachment-recognition semantics. Co-authored-by: Seongho Bae <seonghobae@users.noreply.github.com>
|
@coderabbitai review Please review the unchanged exact current head |
|
|
Exact-head verification dispositionThe current head's focused functional evidence is still green ( Auto-merge is paused until the authorized workflow-mediated branch update lands |
|
@opencode-agent @cwl-noema-review Please independently re-review exact current head |
|
The new |
|
@opencode-agent @cwl-noema-review Current-head review request for Naruon#1353 after the stacked formatter PR merged. Exact head: 9be0992 Read the complete current diff, including backend/services/attachment_parser.py, all HWP/HWPX tests, ADR-0006, doctoring, plan, and changelog. Verify OLE/HWP signature admission, bounded HWPX ZIP/mimetype/package evidence, 64 MiB source budget, deferred payload validation, malformed/oversized/unsupported edge cases, provenance and later-worker boundary, and the ADR's explicit Figma File ID field. Re-run or inspect the focused parser evidence and report a substantive current-head verdict with path:line evidence. Do not reuse predecessor-head reviews or approve from this request. |
|
@opencode-agent @cwl-noema-review Final current-head review request for Naruon#1353 after both stacked PRs merged. Exact head: 4f3e95d Read the complete current diff, including the HWP/HWPX parser, all focused edge-case tests, ADR-0006 with its explicit Figma File ID field, doctoring, plan, and changelog. Verify OLE/HWP signature admission, bounded HWPX ZIP/mimetype/package evidence, 64 MiB source budget, deferred payload validation, malformed/oversized/unsupported cases, provenance, and the later-worker extraction boundary. Report a substantive exact-head verdict with path:line evidence and residual uncertainty. Do not reuse any predecessor-head review or approve from this request. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Stale review: all review-thread comments on this PR are resolved and the reviewer's cited commit predates the current head, which passes all non-metadata-gate required checks (verified via gh pr checks and the reviewThreads GraphQL query — 0 unresolved threads). Dismissing as superseded per AGENTS.md stale-review guidance.
opencode-agent
Bot
added
priority: medium
|
Warning Review limit reachedNext included review available in 58 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review Please re-run the review for the exact current head |
|
|
|
@opencode-agent @cwl-noema-review Please independently review the complete current PR diff at exact head |
| has_manifest = "Contents/content.hpf" in names or "META-INF/manifest.xml" in names | ||
| has_section = any( | ||
| name.startswith("Contents/section") and name.endswith(".xml") for name in names | ||
| ) | ||
| return ( | ||
| mimetype == _HWPX_MIMETYPE | ||
| and "version.xml" in names | ||
| and (has_manifest or has_section) | ||
| ) |
There was a problem hiding this comment.
📝 Info: HWPX import admits manifest-only packages that the worker later rejects
Import admission in _is_hwpx_payload (attachment_parser.py) accepts a package on has_manifest or has_section, but recognize_hwpx requires a Contents/sectionN.xml member and otherwise raises HWPX package has no section XML. A manifest-only-but-sectionless package is admitted at import and later fails closed as hwpx_recognition_failed. Safe, but the two boundaries diverge.
Was this helpful? React with 👍 or 👎 to provide feedback.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
|
|
||
| import pytest | ||
|
|
||
| import services.hwpx_recognition as hwpx_module |
2 participants
Summary
Implement the next bounded #1350 attachment-recognition slice for Korean enterprise documents before OCR, conversion, or LLM/VLM processing.
hwpx_xml_package_pendingstate;hwp_conversion_pendingstate;.hwpx,.owpml, and.hwpextensions;decode_deferred_attachment_payload()backward-compatible for existing PDF workers while allowing HWPX/HWP expected content types;Current-head package and identity hardening
HWPX
The initial HWPX recognizer trusted package member names after
ZipFile.namelist(). The hardening was applied test-first:4b51240eb8521459ef622e49bd463a1a6d783288covers wrong/duplicatemimetype, central-directory entry/byte limits, aggregate member-name bytes, and signature-member size;b737ae83c94ee8a5aaf9c22a8239056e26ffe029preflights bounded single-disk EOCD metadata, requires one exactapplication/hwp+zipsignature member, and bounds package metadata before queue admission.HWP
Generic OLE Compound File magic alone is not HWP identity.
d97281ce7f452a10b0a5c76718d37d126958a4aeproves an unrelated OLE container must fail import-time and deferred-decoder validation;07bd3b30abe483b50129653a4fd599f7ddc9488drequires both OLE magic and the published HWP FileHeader markerHWP Document File;c8837fb00d74bd4ddc3152e0fe793e71f9e1f41faligns the positive corpus with that two-signal contract;1313fd871ff41dec5533d4786f7a0ea7190c1191records package bounds, identity evidence, and downstream revalidation obligations.Product boundary
This PR does not parse HWPX XML semantics, reconstruct tables, extract embedded images, convert HWP binaries, run OCR, fetch external resources, or call LLM/VLM providers. It creates a deterministic, evidence-preserving import state so later sandboxed workers can act without losing source provenance.
The HWP FileHeader marker is a low-cost second admission signal, not full CFB validation. A conversion worker must still validate compound-file directory/stream ownership, version/flags/encryption/records, expansion bounds, and sandbox policy before interpretation.
Repository responsibility
This belongs in Naruon because it changes the email attachment import contract feeding Naruon search, context synthesis, evidence provenance, and user-facing document states. A reusable parser engine can be extracted later only after this importer contract stabilizes.
Develop reconcile (2026-08-17)
Reconciled Ready PR #1353 onto current protected
developwith a normal merge commit. Predecessor evidence does not transfer.c8585193c5dbc2e664a97c9609f739871831e90b70683266b93233dae62faec6cbd4df118be41383develop@dd8d15191338b841f9e6f3a06507c6a5643b95d0(includes feat(calendar): prevent status-weighted double booking #1367)origin/developintofeat/hwp-hwpx-attachment-recognition. No squash. No force-push of develop. No dummy commit. No empty-requeue. No force-cancel.Customer next action
Keep Korean HWP/HWPX mail attachments in a deferred recognition state instead of dropping them as unsupported binaries. Do not parse HWPX XML, convert HWP, or send these bytes to OCR/VLM from this slice. After exact-head product CI is terminal-success on
70683266b93233dae62faec6cbd4df118be41383, an independent non-author reviewer mustAPPROVEthat exact head. Do not self-APPROVE. Do not merge from predecessor evidence.Type of change
Exact candidate
develop@dd8d15191338b841f9e6f3a06507c6a5643b95d0.70683266b93233dae62faec6cbd4df118be41383.c8585193c5dbc2e664a97c9609f739871831e90b,8beb01c260497100c75cf020d3ebc3b11b438f2f,ddd05c5aaf3e170aa2bdc4412647b43b95d5a6b9,bc98789521d21271e84789888413c182aa111b4d.Exact-head verification
cd backend PYTHONPATH=. PYTHONWARNINGS=error DISABLE_BACKGROUND_WORKERS=1 python -m pytest -c /dev/null --noconftest -p no:cacheprovider \ tests/test_attachment_parser.py \ tests/test_attachment_parser_hwp_signature.py \ tests/test_attachment_parser_hwpx_bounds.py -qLocal product results on
70683266b93233dae62faec6cbd4df118be41383: 35 passed (25 attachment-parser including PDF backward-compat + HWP/HWPX admission, 3 HWP FileHeader signature, 7 HWPX package bounds) in 0.09s. NoTimeout/Fatal/Warn/Denied. Develop movement (#1367status-weighted calendar conflicts, plus NetworkGraph first-wins lookups) did not overlap the recognition files and required no conflict resolution or product-code change.Predecessor-head tests, coverage, reviews, OpenCode, Strix, CodeQL, Semgrep, Security Scan, and any other gate results recorded against
c8585193,8beb01c2, or earlier heads are not passing evidence for this head.Merge gate
Do not merge from predecessor evidence. The unchanged exact PR head
70683266b93233dae62faec6cbd4df118be41383must satisfy every live repository/org required status/workflow, package/provenance and review-thread rule at merge time and receive the qualifying independent current-head/last-push non-authorAPPROVErequired by the protecteddeveloprulesets, plus exact-head product CI. No self-approval, bypass, or ruleset weakening is acceptable.Merge remains prohibited until a qualifying independent non-author approval applies to this exact current/last-push head and the live organization/repository rulesets plus every applicable exact-head required check remain satisfied at merge time. Historical body SHAs, predecessor checks, stale approvals, skipped optional jobs, status-only/model-only evidence, or author evidence do not satisfy that boundary.
Checklist:
Refs #1350, #1371.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests