Repository navigation
feat(xtest): decrypt TDFs whose ZIP entries are deflate-compressed - #671
pflynn-virtru wants to merge 2 commits into
Conversation
Add test_tdf_roundtrip_deflated_entries. It encrypts with each SDK, rewrites the TDF with every ZIP entry DEFLATE-compressed (same names, order and bytes), and requires each decrypting SDK to recover the original plaintext. It asserts the rewritten archive really is deflated, so it cannot pass on STORED data. Gate the reader side behind a new zip-deflate feature. Every SDK shim answers no until a release carries opentdf/platform#4247 (go), opentdf/java-sdk#424 (java) or opentdf/web-sdk#1099 (js). Set XT_FORCE_SUPPORTS=zip-deflate, or the force-supports workflow input, to run it against builds of those branches. tdfs.deflate_entries does the rewrite, streaming each entry so a large payload is never held in memory; offline unit tests cover it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 22 minutes. View limit details
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 57.14% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 14 functions across 6 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly and concisely describes the main change: adding xtest coverage for decrypting TDFs with DEFLATE-compressed ZIP entries. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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.
A rabbit zipped files with a soft little thump,
Then checked every entry from tail to its rump.
The headers said “DEFLATE,” the contents stayed clear,
The tests made the archive’s good order appear.
The payload came back, and the audit drew near.
Comment @coderabbitai help to get the list of available commands.
The zip-deflate gate skipped every cell, so CI never decrypted a deflated TDF with a real SDK. Answer yes for builds of main without asking the shim, so CI runs the cells against main and they fail until each SDK's reader PR merges. Released builds still defer to their shim and skip. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Signed-off-by: Paul Flynn <pflynn-virtru@users.noreply.github.com>
|
What this adds
A cross-SDK check that every SDK can decrypt a TDF whose ZIP entries are DEFLATE-compressed (method 8). All OpenTDF writers emit STORED entries, so until now no xtest TDF has gone through a reader's inflate path. A TDF written by another implementation may compress its manifest and payload.
test_tdf_roundtrip_deflated_entries(inxtest/test_tdfs.py, next to the roundtrip tests) runs over encrypt_sdk × decrypt_sdk ×pt_filesize, the same matrix astest_tdf_roundtrip. For each cell it:encrypt_sdk(ztdf);ZIP_DEFLATED, so the test can't pass on STORED data;decrypt_sdk, checks the plaintext matches the original, and checks the KAS rewrap audit event, as the neighbouring roundtrip tests do.tdfs.deflate_entries(scenario_name, tdf_file)does the rewrite. Its output is named likeupdate_manifest/update_payloadname theirs, so it lands in the sametmp_dir/XT_TMP_DIR. It copies entry by entry frominfolist()rather than extracting and re-walking a directory, becauseos.walkorder is not archive order. Entries are streamed, so a multi-GiB payload is never held in memory.The
zip-deflategatezip-deflateis a new reader-side feature infeature_type. It is not platform-only, soXT_FORCE_SUPPORTSaccepts it. Every SDK'ssupportsanswer is no for now, so the cells skip with<sdk> sdk doesn't yet support [zip-deflate]. Eachsupportscase should become a version gate once a release carries its SDK's PR:supportscasextest/sdk/go/cli.shxtest/sdk/java/cli.shxtest/sdk/js/cli.shEncrypt needs no new feature.
Running it against the PR builds
Install the branch builds and force the feature:
In CI, dispatch X-Test with
force-supports: zip-deflateand pointplatform-ref/otdfctl-ref/java-ref/js-refatfeat/zip-inbound-deflate.versions resolvetakes branch names, notpr:N. Without the override, the pull_request run skips these cells.Testing
From
xtest/:uv run ruff check .: All checks passed!uv run ruff format .: 56 files left unchangeduv run pyright: 0 errors, 0 warnings, 0 informationsuv run pytest test_tdfs_units.py test_zip64_units.py -q: 81 passed (7 new: names/order/bytes preserved for0.manifest.jsonandmanifest.json; DEFLATE in both the central directory and the local headers; the manifest actually shrinks; round-trips throughzipfilewith CRCs intact; written beside the source;zip-deflateis forceable fromXT_FORCE_SUPPORTS). A temporary mutation that wrote STORED entries made 2 of them fail.test_bench_*,test_sdk_commands.py,test_tdfs_units.py,test_encryption_units.py,test_sizes_units.py,test_zip64_units.py): 351 passedshellcheckandshfmt -don the threecli.shfiles: cleancli.sh supports zip-deflateexits 1 for go, java and js, run through throwaway dist copies.SDK.skip_if_unsupported("zip-deflate")skips for all three, andXT_FORCE_SUPPORTS=zip-deflatemakessupports()return true for all three.Not run: the integration test itself. It needs the platform stack and SDK builds, which I don't have locally. With no SDK dists it collects one cell, which skips with
got empty parameter set. Against fake dists it collects all 9 encrypt × decrypt cells, but fixture setup needs the platform before the skip can run.Cross-SDK verification
xtest run opentdf/tests actions/runs/39625561654 (2026-10-21) built the platform and otdfctl from opentdf/platform
feat/zip-inbound-deflate(553e9623), java-sdkfeat/zip-inbound-deflate(b02a4121) and web-sdkfeat/zip-inbound-deflate(94e09e4d), and rantest_tdf_roundtrip_deflated_entriesfrom this PR withforce-supports=zip-deflate. The test encrypts with one SDK, rewrites every ZIP entry with DEFLATE, checks the entries are compressed, and decrypts with another SDK.All 9 cells ran (none skipped) and passed:
These are the cells this PR adds. On this PR's own CI, which builds released SDKs and
main, they skip for released SDKs and fail formainbuilds until the three reader PRs merge: opentdf/platform#4247, opentdf/java-sdk#424 and opentdf/web-sdk#1099.For comparison, the same test fails wherever an SDK built from
maindecrypts (this PR's CI):json.Unmarshal failed: invalid character 'u' looking for beginning of valueInvalidZipException: Archive ended 1110 bytes into the 1438 byte entry [0.manifest.json]InvalidFileError: entry [0.payload] uses unsupported compression method [8]🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
zip-deflateis unavailable and provide guidance to use a release with support.Tests