Repository navigation
feat(ingest): each embedded docx image names its part, digest and size (#732, slice 2) - #978
Conversation
#732, slice 2) The attachment inventory from slice 1 (#893) named a docx image only by its relationship id. The id is now resolved through word/_rels/document.xml.rels (ECMA-376 Part 2 §9.3, Annex A.3) to the package part holding the bytes, and that part is streamed through SHA-256. Linked images, absent parts and Confluence images keep part/digest/size empty; state stays unsupported until a later slice stores the bytes. Malformed relationships fail before any write, like the main part. Refs #732 Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYr4VjkzmxX2THRjfx7
…and size (#732, slice 2)
…and size (#732, slice 2)
…and size (#732, slice 2) fixup! feat(ingest): each embedded docx image names its part, digest and size (#732, slice 2) unit (run 42750861159): test_every_storage_primitive_is_an_audited_site read archive.open(name) as open-write, the first positional argument of an open on an unresolved receiver being taken as its mode. The member is opened for reading: say so with mode="r".
…and size (#732, slice 2) unit (run 42755857850): the mutation gate (ADR-1122) left 12 survivors and found no test importing ingest_document_media. - docx_relationships: the all-'..' guard was redundant (popping word/ already yields ''), so it is now 'if not named'. Id and Target are read without a default and a relationship missing either is skipped; a missing-Target test pins it. - docx_media_reader: the tests assert that the docx path appears in read errors, and that every read asks for at most 64 KiB. mode="r" (the ZipFile.open default) and 'UTF-8' vs 'utf-8' are registered equivalents. - ingest_document_media: the lookup has no default; tests_py/handlers/test_ingest_document_media.py covers it directly.
Co-Authored-By: Claude Fable 5.2 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF Signed-off-by: Claude <[email protected]>
Conflict: CHANGELOG.md, both entries under Added are kept. Co-Authored-By: Claude Fable 5.2 <[email protected]> Claude-Session: https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF Signed-off-by: Claude <[email protected]>
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Claude finished @cdeust's task in 3m 2s —— View job Review 1 of 3: PR #978 is done
I found no Blockers and listed three Notes in the review comment:
I did not run pytest, ruff or the craftsmanship gate here because the tool permissions blocked them, so CI is the check. Verdict: approve |
Review 1 of 3: PR #978 (head
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 6dda9ad1fa
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
| part = parts.get(attachment.reference) | ||
| if part is not None and part in digests: | ||
| digest, size = digests[part] | ||
| attachment = replace(attachment, part=part, digest=digest, size=size) |
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
When a local user or agent ingests an attacker-authored DOCX, many image elements can reuse one relationship with a long, valid ZIP member name. This line places the full part name on every attachment, and _ingestion_result serializes every entry without a response budget. A 17,712-byte file produced 33,566,893 bytes of attachment JSON (4,000 images × 8,203 characters); scaling within both 1 MiB XML caps can exhaust MCP memory during SDK text/structured conversion. Cap attachment count and target length, and enforce a cumulative metadata/output budget.
Useful? React with 👍 / 👎.
Summary
Refs #732 (tranche 2 ; l'issue reste ouverte).
Cause racine.
La tranche 1 (#893) inventoriait chaque image embarquée avec son id de relation docx (
r:embed/r:id), sans résoudre cet id vers la part du paquet qui porte les octets. Rien ne permettait donc d'identifier les octets d'une image, prérequis de leur conservation (critère « bytes, digest, position and document version »).core/docx_relationships.py(pur) :word/_rels/document.xml.rels→{id: part}pour chaque relation interne, cible résolue selon ECMA-376 Part 2 Annex A.3 (relative àword/, absolue depuis la racine, segments./..). Les ciblesTargetMode="External"et vides sont absentes. XML malformé →DocumentParseError.infrastructure/docx_media_reader.py: lit la part de relations (plafondMAX_DOCUMENT_BYTESd'ADR-0516, UTF-8) et passe chaque part nommée en SHA-256 par blocs de 64 KiB (mémoire bornée).handlers/ingest_document_media.py: composition ; chaque attachment résolu reçoitpart,digest(sha256:<hex>),size. Image liée, part absente, image Confluence : champs vides ("","",null).stateresteunsupported: les octets sont identifiés, pas conservés.docs/module-inventory.mdet CHANGELOG mis à jour.Type of change
Test plan
pytest tests_py/core/test_docx_relationships.py tests_py/infrastructure/test_docx_media_reader.py tests_py/handlers/test_ingest_document*.py+ tous les tests qui référencentingest_document/DocumentAttachment/module-inventory: 178 passed, 3 skipped.pytest tests_py/core/+ tests document : 3961 passed, 162 skipped, 1 échec préexistant surmain(test_wiki_coverage_dashboard.py::test_unreadable_page_is_skipped_not_fatal, environnement root).ruff check/ruff format --check(0.17.3) sur les fichiers touchés : OK.python scripts/check_craftsmanship.py: OK.Mutations visées par les nouveaux tests : résolution
..au-dessus de la racine, cible vide ou faite de seuls segments de point,TargetModeexterne vs interne, relation sansId, élément hors espace de noms OPC, plafond exact (max_byteségal / dépassé), lecture par blocs au-delà de 64 KiB, part vide (taille 0), CRC invalide, part absente du zip.Audit notes
Non vérifié.
scripts/mutation_check.shn'a pas pu tourner en local (téléchargement detorchrefusé par le proxy de l'environnement).unitcomplet de la CI non rejoué.Tranches restantes de #732 (hors de cette PR).
preserved-only) avec une référence de source accessible, côté stockage.extractedetfailedexplicites.https://claude.ai/code/session_018kyZYr4VjkzmxX2THRjfx7
Continuation
Continues #961, which was approved on its head but is in conflict with the main branch and has spent its three reviews. Same commits, plus one: the merge of the main branch, with the CHANGELOG conflict resolved by keeping both entries.
https://claude.ai/code/session_018kyZYsWnKSjpX4oNV19PCF
Generated by Claude Code