From db07c9c68a4505f5ed579285f87055928ee84171 Mon Sep 17 00:00:00 2001 From: Dave Mihalcik Date: Tue, 8 Sep 2026 10:14:17 -0400 Subject: [PATCH] feat(xtest): 2.1 GiB ZIP64 boundary coverage (DSPX-4592) ZIP central-directory offsets and sizes are 32-bit unsigned on the wire. A reader that widens one with a signed read sees anything >= 2**31 as negative; at or above 2**32 the format mandates the ZIP64 sentinel, so the 32-bit field never holds a real value. That leaves exactly one broken window, [2**31, 2**32), and nothing in this suite reached it before now. Adds sizes.MEDIUM_BYTES (2.1 GiB, calibrated to land a TDFs manifest offset inside the window) and the window predicates, zipinspect.py (a raw ZIP central-directory reader independent of zipfile, which normalises ZIP64 away), test_zip64.py (the roundtrip cell, deselected unless the session reaches the window), a strict xfail scoped to pre-fix java readers, and a nightly-only zip64 CI job matrixed over the encrypting SDK. --- .github/workflows/check.yml | 13 +- .github/workflows/xtest.yml | 368 +++++++++++++++++++++++++++++- spec/DSPX-4592.md | 352 +++++++++++++++++++++++++++++ xtest/AGENTS.md | 4 +- xtest/conftest.py | 56 +++-- xtest/pyproject.toml | 2 + xtest/sizes.py | 65 +++++- xtest/tdfs.py | 43 +++- xtest/test_sizes_units.py | 40 ++++ xtest/test_tdfs_units.py | 43 +++- xtest/test_zip64.py | 142 ++++++++++++ xtest/test_zip64_units.py | 434 ++++++++++++++++++++++++++++++++++++ xtest/zipinspect.py | 350 +++++++++++++++++++++++++++++ 13 files changed, 1879 insertions(+), 33 deletions(-) create mode 100644 spec/DSPX-4592.md create mode 100644 xtest/test_zip64.py create mode 100644 xtest/test_zip64_units.py create mode 100644 xtest/zipinspect.py diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 0c6fc424..a77faeb0 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -36,9 +36,14 @@ jobs: working-directory: xtest # Offline tests for the harnesses whose own correctness gates a nightly # job: the benchmark statistics, measurement and CLI command builders, - # and the encryption fixture cache. No platform and no SDK builds - # required, so the part that has to be *correct* is checked on every PR - # rather than only when the nightly runs. + # the encryption fixture cache, and the ZIP64 central-directory parser. + # No platform and no SDK builds required, so the part that has to be + # *correct* is checked on every PR rather than only when the nightly runs. + # + # test_zip64_units.py matters disproportionately here: the nightly zip64 + # job's verdict is only as good as this parser, and a parser bug would + # report a conformant container as broken (or the reverse) after an hour + # of multi-GiB IO that nobody wants to repeat to debug it. # # --frozen --no-build: resolve nothing and build nothing, so a # dependency cannot slip in an unlocked version or a setup script on a @@ -48,7 +53,7 @@ jobs: uv run --frozen --no-build pytest --no-header -q test_bench_stats.py test_bench_measure.py test_bench_runner.py test_bench_arms.py test_sdk_commands.py test_tdfs_units.py - test_encryption_units.py test_sizes_units.py + test_encryption_units.py test_sizes_units.py test_zip64_units.py working-directory: xtest - name: Lint and test otdf-local run: | diff --git a/.github/workflows/xtest.yml b/.github/workflows/xtest.yml index db38c76d..ebbda99f 100644 --- a/.github/workflows/xtest.yml +++ b/.github/workflows/xtest.yml @@ -38,11 +38,16 @@ on: type: boolean default: false description: "Run the SDK performance regression benchmarks (adds ~45m per SDK). Needs two builds per SDK: set the *-ref inputs to 'main latest', since a bare 'main' installs no release to use as a baseline and every cell will skip." + run-zip64: + required: false + type: boolean + default: false + description: "Run the 2.1 GiB ZIP64 boundary tests (DSPX-4592; adds ~60m per encrypting SDK). Set java-ref to 'main latest' to also exercise the pre-fix java reader, which is the defect this covers." force-supports: required: false type: string default: "" - description: "Comma-separated feature names to treat as supported regardless of what each SDK's `cli.sh supports` reports. Use when evaluating a fix that has not been released yet: the version gates live in this repo and answer 'no' for exactly those unreleased builds, so the cells would otherwise skip. An unknown name fails the run rather than being ignored." + description: "Comma-separated feature names to treat as supported regardless of what each SDK's `cli.sh supports` reports (e.g. 'chunky'). Use when evaluating a fix that has not been released yet: the version gates live in this repo and answer 'no' for exactly those unreleased builds, so the cells would otherwise skip. An unknown name fails the run rather than being ignored." workflow_call: inputs: platform-ref: @@ -73,6 +78,10 @@ on: required: false type: boolean default: false + run-zip64: + required: false + type: boolean + default: false force-supports: required: false type: string @@ -101,6 +110,7 @@ jobs: contents: read outputs: platform-tag-to-sha: ${{ steps.version-info.outputs.platform-tag-to-sha }} + platform-main-sha: ${{ steps.version-info.outputs.platform-main-sha }} platform-tag-list: ${{ steps.version-info.outputs.platform-tag-list }} heads: ${{ steps.version-info.outputs.platform-heads }} default-tags: ${{ steps.version-info.outputs.default-tags }} @@ -201,6 +211,27 @@ jobs: } } + // Bench and ZIP64 hold the server on platform main independently + // of the platform lanes under test. Resolve that moving ref once + // here so every matrix job uses the same commit. Reuse the normal + // resolution when main was already requested; otherwise look up + // the branch without adding it to platform-tag-list. + let platformMainSha = versionData.platform + ?.find(({ tag, sha, err }) => tag === 'main' && sha && !err) + ?.sha; + if (!platformMainSha) { + const { data: platformMain } = await github.rest.repos.getBranch({ + owner: 'opentdf', + repo: 'platform', + branch: 'main' + }); + platformMainSha = platformMain.commit.sha; + } + if (!platformMainSha) { + throw new Error('Unable to resolve opentdf/platform main'); + } + core.setOutput('platform-main-sha', platformMainSha); + core.setOutput('all', JSON.stringify(versionData)); const sdkVersionList = []; @@ -824,7 +855,7 @@ jobs: id: run-platform uses: opentdf/platform/test/start-up-with-containers@18b8070f7ae1e3547234342f42d0d686dc77788f # keycloak-26.4 (opentdf/platform#3792) with: - platform-ref: ${{ fromJSON(needs.resolve-versions.outputs.platform-tag-to-sha)['main'] }} + platform-ref: ${{ needs.resolve-versions.outputs.platform-main-sha }} bootstrap-ref: main ec-tdf-enabled: true extra-keys: ${{ steps.load-extra-keys.outputs.EXTRA_KEYS }} @@ -946,7 +977,7 @@ jobs: done env: java_version_info: ${{ needs.resolve-versions.outputs.java }} - platform_ref: ${{ fromJSON(needs.resolve-versions.outputs.platform-tag-to-sha)['main'] }} + platform_ref: ${{ needs.resolve-versions.outputs.platform-main-sha }} - name: Build the ${{ matrix.sdk }} cli if: fromJson(steps.configure-sdk.outputs.heads)[0] != null @@ -1023,6 +1054,337 @@ jobs: path: ${{ steps.run-platform.outputs.platform-log-file }} if-no-files-found: ignore + # ZIP64 boundary conformance at 2.1 GiB (DSPX-4592). + # + # ZIP central-directory offsets and sizes are 32-bit *unsigned* on the wire. + # A reader that widens one with a signed read sees anything at or above 2**31 + # as negative; at or above 2**32 the format mandates the ZIP64 sentinel, so + # the 32-bit field never holds a real value. That leaves exactly one broken + # window, [2**31, 2**32), and nothing in this suite reached it: `--large` is + # 5 GiB, which steps straight over. + # + # So this job runs one size, `medium` (2.1 GiB), and only that size. Anything + # smaller does not make it cheaper, it makes it vacuous -- + # test_zip64.py asserts that an offset actually landed in the window rather + # than skipping, precisely so a mis-sized payload fails instead of passing. + # + # Never on pull requests: an hour of multi-GiB IO per SDK is not a PR gate. + zip64: + timeout-minutes: 90 + runs-on: ubuntu-latest + needs: resolve-versions + # Nightly cron only, matching bench. The Mon/Wed and weekly crons would + # re-run an identical comparison. + if: >- + github.event.schedule == '30 6 * * *' || + ((github.event_name == 'workflow_dispatch' || github.event_name == 'workflow_call') + && inputs.run-zip64) + permissions: + contents: read + packages: read + strategy: + # One runner per *encrypting* SDK; each decrypts with all three. Every + # runner therefore installs every SDK, and the split is about disk and + # wall clock rather than about what is installed: one encryptor per + # runner means at most two cached 2.1 GiB ciphertexts (one per + # negotiated target_mode), not three. + fail-fast: false + matrix: + sdk: [go, java, js] + steps: + - uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7.0.0 + with: + repository: opentdf/tests + path: otdftests + persist-credentials: false + + # Plaintext 2.1 GiB + one cached ciphertext 2.1 GiB + one decrypt output + # at a time 2.1 GiB is ~6.5 GiB, which the workspace volume cannot be + # relied on to hold alongside three SDK toolchains. /mnt is the runner's + # large ephemeral disk; XT_TMP_DIR moves the fixtures there. + - name: Reclaim disk and stage a scratch volume + id: scratch + run: |- + # Toolchains this job does not use. Removing them buys ~25 GiB on the + # workspace volume, which the platform containers and three SDK + # builds still have to share. + sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc \ + /usr/local/share/boost || true + sudo mkdir -p /mnt/xtest-tmp + sudo chown "$(id -u):$(id -g)" /mnt/xtest-tmp + df -h / /mnt + + - name: load extra keys from file + id: load-extra-keys + run: |- + echo "EXTRA_KEYS=$(jq -c > "${GITHUB_OUTPUT}" + + ######## SPIN UP PLATFORM BACKEND ############# + # Pinned to main with the default KAS only. This job is about the ZIP + # container the SDKs write and read; the six extra KAS instances the ABAC + # tests need would only consume runner memory and disk. + - name: Check out and start up platform with deps/containers + id: run-platform + uses: opentdf/platform/test/start-up-with-containers@18b8070f7ae1e3547234342f42d0d686dc77788f # keycloak-26.4 (opentdf/platform#3792) + with: + platform-ref: ${{ needs.resolve-versions.outputs.platform-main-sha }} + bootstrap-ref: main + ec-tdf-enabled: true + extra-keys: ${{ steps.load-extra-keys.outputs.EXTRA_KEYS }} + log-type: json + pqc-enabled: true + + - name: Install uv + uses: astral-sh/setup-uv@fac544c07dec837d0ccb6301d7b5580bf5edae39 # v8.2.0 + - uses: bufbuild/buf-action@fd21066df7214747548607aaa45548ba2b9bc1ff # v1.4.0 + with: + setup_only: true + token: ${{ secrets.BUF_TOKEN }} + version: "1.56.0" + + # All three toolchains unconditionally: every runner decrypts with every + # SDK, so every runner builds all three. + - name: Set up JDK + uses: actions/setup-java@ad2b38190b15e4d6bdf0c97fb4fca8412226d287 + with: + java-version: "11" + distribution: "adopt" + server-id: github + + - name: Set up Node 22 + uses: actions/setup-node@48b55a011bda9f5d6aeb4c2d9c7362e8dae4041e + with: + node-version: "22.x" + + - name: Capture platform otdfctl location + id: platform-otdfctl + run: |- + if [ -d "$PLATFORM_DIR/otdfctl" ] && [ -f "$PLATFORM_DIR/otdfctl/go.mod" ]; then + echo "dir=$(pwd)/$PLATFORM_DIR/otdfctl" >> "$GITHUB_OUTPUT" + sha=$(git -C "$PLATFORM_DIR" rev-parse HEAD) || { + echo "::error::Failed to get SHA from platform checkout at $PLATFORM_DIR" + exit 1 + } + echo "sha=$sha" >> "$GITHUB_OUTPUT" + else + echo "dir=" >> "$GITHUB_OUTPUT" + echo "sha=" >> "$GITHUB_OUTPUT" + fi + env: + PLATFORM_DIR: ${{ steps.run-platform.outputs.platform-working-dir }} + + ######## INSTALL EVERY SDK ############# + # The go install doubles as otdfctl, which conftest.py loads at import + # time to provision attributes and the KAS registry. + - name: Configure go sdk + id: configure-go + uses: ./otdftests/xtest/setup-cli-tool + with: + path: otdftests/xtest/sdk + sdk: go + version-info: "${{ needs.resolve-versions.outputs.go }}" + platform-otdfctl-dir: ${{ steps.platform-otdfctl.outputs.dir }} + platform-otdfctl-sha: ${{ steps.platform-otdfctl.outputs.sha }} + + - name: Configure java sdk + id: configure-java + uses: ./otdftests/xtest/setup-cli-tool + with: + path: otdftests/xtest/sdk + sdk: java + version-info: "${{ needs.resolve-versions.outputs.java }}" + platform-otdfctl-dir: ${{ steps.platform-otdfctl.outputs.dir }} + platform-otdfctl-sha: ${{ steps.platform-otdfctl.outputs.sha }} + + - name: Configure js sdk + id: configure-js + uses: ./otdftests/xtest/setup-cli-tool + with: + path: otdftests/xtest/sdk + sdk: js + version-info: "${{ needs.resolve-versions.outputs.js }}" + platform-otdfctl-dir: ${{ steps.platform-otdfctl.outputs.dir }} + platform-otdfctl-sha: ${{ steps.platform-otdfctl.outputs.sha }} + + - name: Cache Go modules + uses: actions/cache@2c8a9bd7457de244a408f35966fab2fb45fda9c8 # v6.0.0 + with: + path: | + ~/.cache/go-build + ~/go/pkg/mod + key: go-${{ runner.os }}-${{ hashFiles('otdftests/xtest/sdk/go/src/*/go.sum') }} + restore-keys: | + go-${{ runner.os }}- + + - name: Cache npm + uses: actions/cache@2c8a9bd7457de244a408f35966fab2fb45fda9c8 # v6.0.0 + with: + path: ~/.npm + key: npm-${{ runner.os }}-${{ hashFiles('otdftests/xtest/sdk/js/src/**/package-lock.json') }} + restore-keys: | + npm-${{ runner.os }}- + + - name: Cache Maven repository + uses: actions/cache@2c8a9bd7457de244a408f35966fab2fb45fda9c8 # v6.0.0 + with: + path: ~/.m2/repository + key: maven-${{ runner.os }}-${{ hashFiles('otdftests/xtest/sdk/java/src/**/pom.xml') }} + restore-keys: | + maven-${{ runner.os }}- + + - name: point java heads at the platform under test + if: fromJson(steps.configure-java.outputs.heads)[0] != null + run: |- + for row in $(echo "$java_version_info" | jq -c '.[]'); do + TAG=$(echo "$row" | jq -r '.tag') + HEAD=$(echo "$row" | jq -r '.head') + if [[ "$HEAD" == "true" ]]; then + echo "PLATFORM_BRANCH=$platform_ref" > "otdftests/xtest/sdk/java/${TAG}.env" + fi + done + env: + java_version_info: ${{ needs.resolve-versions.outputs.java }} + platform_ref: ${{ needs.resolve-versions.outputs.platform-main-sha }} + + - name: Build the go cli + if: fromJson(steps.configure-go.outputs.heads)[0] != null + run: make + working-directory: otdftests/xtest/sdk/go + env: + BUF_INPUT_HTTPS_USERNAME: opentdf-bot + BUF_INPUT_HTTPS_PASSWORD: ${{ secrets.PERSONAL_ACCESS_TOKEN_OPENTDF }} + + - name: Build the java cli + if: fromJson(steps.configure-java.outputs.heads)[0] != null + run: make + working-directory: otdftests/xtest/sdk/java + env: + BUF_INPUT_HTTPS_USERNAME: opentdf-bot + BUF_INPUT_HTTPS_PASSWORD: ${{ secrets.PERSONAL_ACCESS_TOKEN_OPENTDF }} + + - name: Build the js cli + if: fromJson(steps.configure-js.outputs.heads)[0] != null + run: make + working-directory: otdftests/xtest/sdk/js + env: + BUF_INPUT_HTTPS_USERNAME: opentdf-bot + BUF_INPUT_HTTPS_PASSWORD: ${{ secrets.PERSONAL_ACCESS_TOKEN_OPENTDF }} + + ######## RUN ############# + - name: Install test dependencies + run: uv sync --locked --no-build + working-directory: otdftests/xtest + + # Deliberately serial: no -n / --dist. Three concurrent workers each + # holding a 2.1 GiB plaintext, ciphertext and decrypt output would + # exhaust the scratch volume long before the last cell. + # + # Note what is *not* here: --skip-released-pairs. The xct job derives + # that flag from SKIP_RELEASED_PAIRS to avoid re-testing release-against- + # release combinations, but a released java decryptor reading a 2.1 GiB + # container is the exact defect under test. Skipping it would leave this + # job green and blind. + - name: Run ZIP64 boundary tests + id: zip64 + run: |- + uv run --frozen --no-build pytest -ra -v \ + --sizes medium \ + --sdks-encrypt "$ZIP64_SDK" \ + --sdks-decrypt "go java js" \ + --junitxml "test-results/zip64-${ZIP64_SDK}.xml" \ + --html "test-results/zip64-${ZIP64_SDK}.html" \ + --self-contained-html \ + test_zip64.py + working-directory: otdftests/xtest + env: + ZIP64_SDK: ${{ matrix.sdk }} + PLATFORM_DIR: "../../${{ steps.run-platform.outputs.platform-working-dir }}" + SCHEMA_FILE: "manifest.schema.json" + PLATFORM_TAG: main + # go's heads: conftest reads this to locate otdfctl under + # sdk/go/dist//, regardless of which SDK is encrypting. + OTDFCTL_HEADS: ${{ steps.configure-go.outputs.heads }} + # Nothing here requests the audit-log fixture, and a rewrap audit + # event says nothing about the container's ZIP encoding. + DISABLE_AUDIT_ASSERTIONS: "1" + # Multi-GiB fixtures go on the runner's large ephemeral volume, not + # the workspace disk. + XT_TMP_DIR: /mnt/xtest-tmp + # The java shim pipes CLI stdout to a file with no -Xmx, and the js + # shim is a bare npx. Both runtimes honour these automatically, so + # the headroom is set here rather than by editing the shims. If a + # shim turns out to buffer the whole payload rather than stream it, + # that is a finding for the SDK, not a reason to shrink the payload. + JAVA_TOOL_OPTIONS: -Xmx6g + NODE_OPTIONS: --max-old-space-size=8192 + + # A green job whose tests were all skipped is the exact failure mode this + # ticket exists to close, and it is invisible in the job status. Parse + # the junit XML rather than grepping the log: a skipped cell and a cell + # that printed the word "skipped" are different things. + - name: Confirm the ZIP64 cells actually ran + if: success() || failure() + working-directory: otdftests/xtest + env: + ZIP64_SDK: ${{ matrix.sdk }} + run: |- + python3 - <<'PY' + import os + import sys + import xml.etree.ElementTree as ET + + sdk = os.environ["ZIP64_SDK"] + path = f"test-results/zip64-{sdk}.xml" + try: + cases = ET.parse(path).getroot().iter("testcase") + except (OSError, ET.ParseError) as e: + sys.exit(f"::error::cannot read {path}: {e}") + + ran, xfailed, skipped = [], [], [] + for c in cases: + name = f"{c.get('classname')}::{c.get('name')}" + s = c.find("skipped") + if s is None: + ran.append(name) + # pytest files xfail under , but an xfailed cell did + # encrypt 2.1 GiB and did attempt the decrypt -- it exercised the + # defect and predicted the outcome. That is coverage, not a gap. + elif s.get("type") == "pytest.xfail": + xfailed.append(name) + else: + skipped.append(name) + + print(f"{len(ran)} ran, {len(xfailed)} xfailed, {len(skipped)} skipped") + for n in xfailed: + print(f" xfail: {n}") + for n in skipped: + print(f" skipped: {n}") + + if not ran and not xfailed: + sys.exit( + "::error::no ZIP64 cell executed. The job is green because " + "nothing ran, not because the 2-4 GiB band is conformant." + ) + PY + + - name: Upload ZIP64 results + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: success() || failure() + with: + name: ${{ job.status == 'success' && '✅' || '❌' }} zip64-${{ matrix.sdk }} + path: | + otdftests/xtest/test-results/*.xml + otdftests/xtest/test-results/*.html + if-no-files-found: warn + + - name: Upload server logs on failure + uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1 + if: failure() + with: + name: zip64-server-logs-${{ matrix.sdk }} + path: ${{ steps.run-platform.outputs.platform-log-file }} + if-no-files-found: ignore + publish-results: runs-on: ubuntu-latest needs: xct diff --git a/spec/DSPX-4592.md b/spec/DSPX-4592.md new file mode 100644 index 00000000..9db9efdf --- /dev/null +++ b/spec/DSPX-4592.md @@ -0,0 +1,352 @@ +--- +ticket: DSPX-4592 +title: zip64 conformance issues: xtest 2.1 GiB e2e coverage +status: draft +authors: [dmihalcik@virtru.com] +branches: [opentdf/tests:DSPX-4592-java-underflow] +prs: [] +created: 2026-09-02 +updated: 2026-09-02 +--- + +# zip64 conformance issues: xtest 2.1 GiB e2e coverage + +## Summary +BackgroundSibling tickets track ZIP64/APPNOTE conformance defects in the three SDKs' hand-rolled ZIP readers and writers, found while auditing java-sdk PR #393. This ticket owns the shared cross-SDK test coverage those three all reference. +The defect class only manifests for payloads in the 2-4 GiB band, and no test in any repo currently covers that band. +Why 2.1 GiB specificallyThe ZIP central directory stores offsets and sizes in 32-bit fields that are unsigned on the wire. A reader that widens them with a signed read sees anything at or above 2^31 (2147483648) as negative. Above 2^32 the format requires the ZIP64 sentinel plus an extra field, so the 32-bit field is never populated with a real value and the bug cannot fire. +That leaves exactly one broken window: 2^31 <= value < 2^32. 2.1 GiB (2254857830 bytes) sits ~107 MB inside the low edge of it, which is enough margin that segment padding and manifest size cannot push the interesting offsets back below the threshold. +What a 2.1 GiB payload actually exercisesjava-sdk writer, pre-#393. ZipWriter.writeByteArray hardcoded fileInfo.isZip64 = false. The manifest is written after the payload, so at a 2.1 GiB payload its local-header offset is ~2.1 GiB and gets written as a raw 32-bit value with no ZIP64 extra field. PR #393 fixes this via isZip64 = needsZip64(startPosition, data.length). Note the payload entry itself was never affected - stream() always set isZip64 = true - which is precisely why this went unnoticed. +java-sdk reader, pre-#393. readInt() sign-extends, so that ~2.1 GiB manifest offset comes back negative and the read fails or seeks to nonsense. Fixed by readUnsignedInt() in #393. +go-sdk writer, today. Switches to ZIP64 only at 4 GiB, so it writes both a 32-bit manifest offset and a 32-bit 2.1 GiB payload size. Any pre-#393 Java reader chokes on both. This is the live cross-SDK break and the reason the sibling go ticket moves the threshold to MaxInt32. +web-sdk. Always ZIP64, so it should pass every cell unchanged - a useful control. +The gap in current coveragextest/conftest.py already has a large-file path, but it steps straight over the broken band: +parser.addoption( + "--large", + action="store_true", + help="generate a large (greater than 4 GiB) file for testing", +) +... +length = (5 * 2**30) if size == "large" else 128size is parametrized binary - 128 bytes or 5 GiB (pytest_generate_tests, pt_file). 5 GiB is above 2^32, so every SDK takes the full ZIP64 path and none of the 32-bit fields are populated with real values. The one size that would catch this class of bug is the one size not tested. +Separately: --large has no CI wiring at all. It does not appear anywhere in .github/workflows/xtest.yml, so even the 5 GiB path only ever runs when someone passes the flag locally. +Work1. Make size a real parametrizationReplace the boolean --large with something that can express the band - e.g. --sizes small,medium,large mapping to 128 B / 2.1 GiB / 5 GiB, keeping --large as a deprecated alias for small,large so existing invocations do not break. Update the pt_file fixture and its docstring accordingly (the current docstring says 'large' (>4 GiB) or 'small' (128 bytes)). +2. Fix plaintext generation for multi-GiB sizespt_file currently generates content with a Python loop, one formatted line per 16 bytes: +for i in range(0, length, 16): + f.write(f"{i:15,d}\n")At 2.1 GiB that is ~140 million iterations of string formatting, which will dominate the job's runtime. Generate large files in bulk (build one block, write it repeatedly) while keeping the content deterministic and non-compressible enough to stay realistic. This matters more than it sounds - if the fixture takes 20 minutes the test will get disabled. +3. Cover the pre-fix reader, not just current mainThe Java reader bug is in released artifacts; main after #393 will pass. The existing --sdks-decrypt version-qualified spec (e.g. java@v0.7.x) is the mechanism - make sure the matrix pins at least one released java-sdk as a decryptor against a 2.1 GiB TDF written by go and by java, so the test actually reproduces the failure today and turns green only once the sibling tickets land. Check the interaction with --skip-released-pairs / SKIP_RELEASED_PAIRS so these pairs are not silently skipped - per the repo's own guidance, confirm the test ran and was not SKIPPED rather than trusting a green check. +4. CI wiringDo not add this to the per-PR path. Budget: a 2.1 GiB round trip is ~2.1 GiB plaintext + ~2.1 GiB ciphertext + ~2.1 GiB decrypted output per cell, and ubuntu-latest has limited free disk. Recommended shape: +A separate job (or separate workflow) with its own timeout-minutes, not folded into the existing 60-minute matrix job. +Attach to one of the existing cron schedules (30 6 * * * nightly, or the Sunday 0 18 * * 0 weekly) plus workflow_dispatch so it can be run on demand against a branch. +Restrict the SDK matrix to the cells that matter rather than the full cross product; clean up artifacts between cells. +If disk turns out to be the binding constraint, note it here and consider a larger runner rather than shrinking the payload below 2^31 - a smaller payload silently stops testing anything. +Acceptance criteriaA 2.1 GiB size is expressible and runs end-to-end through encrypt/decrypt across the go, java, and web SDKs. +Large-file plaintext generation is fast enough not to dominate job runtime. +At least one released (pre-#393) java-sdk is exercised as a decryptor for a 2.1 GiB TDF and is confirmed to reproduce the failure before the sibling fixes land. +The new coverage runs on a schedule and via workflow_dispatch, not on every PR, with an explicit timeout. +Test is confirmed to actually execute (not SKIPPED) by grepping the job log. +Sibling tickets' "Shared cross-SDK work" sections are satisfied by this ticket. +SequencingThis should land before the go and java fixes, so it goes red first and demonstrates the fixes work. It is not blocked by them. +Filed as a Task rather than a Bug: this is test coverage for defects tracked in the three sibling tickets, not a defect in its own right. + +## Problem / Motivation + +A TDF is a ZIP container, and all three SDKs write and read that ZIP by hand. +The central directory stores each entry's local-header offset and sizes in +32-bit fields that are **unsigned on the wire**. Three regimes follow, and only +the middle one is dangerous: + +| payload | 32-bit field holds | signed read | ZIP64 required? | +|---|---|---|---| +| `< 2**31` | the real value | correct | no | +| `[2**31, 2**32)` | the real value, **or** the ZIP64 sentinel | **negative** | no — writer's choice | +| `>= 2**32` | always the `0xFFFFFFFF` sentinel | n/a | yes | + +Below `2**31` a signed widen is harmless. At or above `2**32` the format +mandates the sentinel, so the 32-bit field never carries a real value and a +sign-extending reader never sees one. The defect class fires in exactly one +window, and it is the window nobody tested. + +The suite already had a large-file path — `--large` — and it made the gap +worse rather than better: it generates 5 GiB, which is *above* `2**32`, so +every SDK takes the full ZIP64 path and every 32-bit field holds the sentinel. +The one size that would catch this is the one size the suite could not express. +`--large` also had no CI wiring anywhere in `.github/workflows/`, so even the +5 GiB path only ran when someone remembered the flag locally. + +Concretely, today: go-sdk switches to ZIP64 only at 4 GiB, so at 2.1 GiB it +writes a real 32-bit manifest offset; a pre-#393 java-sdk reads it with +`readInt()`, gets a negative number, and fails. That is a live cross-SDK +interop break with no test that can see it. + +## Proposed Solution + +Four pieces, all in `opentdf/tests`. + +**A size vocabulary (`xtest/sizes.py`).** `small` = 128 B, `medium` = +2 254 857 830 B (2.1 GiB), `large` = 5 GiB, plus the window constants and an +`in_zip64_window()` predicate. One module so the byte counts and the "is this +in the broken band" question have a single home, importable from both +`conftest.py` and the tests without a cycle. `medium` sits ~107 MB inside the +low edge, enough margin that segment padding and manifest size cannot push the +manifest's local-header offset back under `2**31`. + +**`--sizes` replaces `--large` (`xtest/conftest.py`).** A comma-separated list +validated against the table, defaulting to `small`; `--large` survives as a +deprecated alias for `small,large` so existing local invocations keep working. +`size` stays a session-scoped parametrization, so listing more than one size +fans out every test that takes `pt_file` — which is the expressibility this +ticket asks for, and the reason CI passes exactly one. + +Plaintext generation splits by size. `small` keeps the existing line generator +byte-for-byte, because existing tests compare against that content. Anything +larger builds one 1 MiB pseudorandom block and writes it repeatedly with an +8-byte big-endian block counter patched into each write, so the content stays +deterministic *and* position-dependent without ever materialising the payload +in memory. DEFLATE's window is 32 KiB, so the repetition does not make the +payload compressible. Measured: 2.1 GiB in 0.5 s, deflate ratio 1.000. + +**Structural conformance assertions (`xtest/zipinspect.py`).** A roundtrip +alone reports "decrypt failed" and leaves you guessing which side was wrong, an +hour and 6 GiB of IO after the fact. This is a raw central-directory reader +that keeps the 32-bit fields *and* the resolved values side by side, so a test +can say which SDK wrote a non-conformant container independently of whether any +reader coped. `zipfile` cannot be used: it normalises ZIP64 away, discarding +precisely the encoding under test. + +**A dedicated module and a nightly job (`xtest/test_zip64.py`, the `zip64` job +in `xtest.yml`).** Separate from `test_tdfs.py` because it needs three things +that test does not: explicit deletion of the decrypt output so artifacts do not +accumulate 2.1 GiB at a time, the structural assertions, and selectability in +CI without dragging the rest of the suite to multi-GiB payloads. + +Known-broken cells use `xfail(strict=True)` keyed on `SDK.semver()` rather than +a permanently red job or a skip. A pre-fix java decryptor reports XFAIL with a +reason; when the fix ships, `latest` stops matching the predicate and the cell +must pass; and if a cell believed broken starts passing, strict xfail fails the +job so somebody comes and deletes the predicate. Self-maintaining, no follow-up +PR. + +## Inputs / Outputs / Contracts + +``` +--sizes small,medium,large # default: small +--large # deprecated alias for --sizes small,large +XT_TMP_DIR= # relocate fixtures off the workspace volume +``` + +`--large` and `--sizes` together is a `UsageError`, not a silent precedence +rule. + +```python +# xtest/sizes.py +SIZES: dict[str, int] # name -> bytes +SIZE_ORDER: tuple[str, ...] # cheapest first +ZIP64_WINDOW_LOW = 2**31 +ZIP64_WINDOW_HIGH = 2**32 +def in_zip64_window(n: int) -> bool +def exercises_zip64_window(size: str) -> bool + +# xtest/zipinspect.py +@dataclass(frozen=True) +class CentralDirectoryEntry: + name: str + raw_compressed_size / raw_uncompressed_size / raw_local_header_offset: int + compressed_size / uncompressed_size / local_header_offset: int + has_zip64_extra: bool + uses_zip64_for_offset / uses_zip64_for_sizes: bool # properties + def signed_read_of_offset(self) -> int # reproduces the defect + +def central_directory(path: Path) -> list[CentralDirectoryEntry] +def entries_in_window(entries) -> list[CentralDirectoryEntry] +def assert_zip64_above_4gib(entries) -> None +def describe(entries) -> str + +# xtest/tdfs.py +JAVA_ZIP64_READER_FIX: tuple[int, int, int] +def zip64_reader_xfail(decrypt_sdk: SDK) -> pytest.MarkDecorator | None +``` + +Marker: `zip64`, deselected (not skipped) unless the session's resolved sizes +reach `ZIP64_WINDOW_LOW`. Deselected, because a multi-GiB roundtrip has no +business in the PR matrix and a skip would report it as a test that exists and +was declined. + +CI: `run-zip64` boolean on `workflow_dispatch` and `workflow_call`; the job +also fires on the nightly `30 6 * * *` cron. Never on `pull_request`. + +## Edge Cases & Constraints + +**A vacuous pass is the failure mode to design against.** If the fixture +generates the wrong size, every assertion here passes without touching a line +of the code under test — which is exactly the hole this ticket exists to close. +So `test_zip64.py` *asserts* that some entry's local-header offset landed at or +above `2**31` rather than skipping when it did not, and the nightly job parses +its own junit XML and fails if no cell executed. Shrinking `medium` below +`2**31` does not make the test cheaper, it makes it silently meaningless. + +**Both encodings are legal in the window.** A writer may emit a real unsigned +32-bit value or opt into the ZIP64 sentinel; APPNOTE permits both, and which +one go-sdk emits is what the sibling ticket changes. So the test records and +logs which was chosen and asserts only on the `>= 2**32` case, where there is +no latitude. + +**Writer checks run outside the reader's xfail.** A writer regression must not +hide behind a known reader bug: assertions under an xfail marker report XFAIL +and nobody looks. `add_marker` is therefore applied after the structural +assertions and immediately before the decrypt. + +**Disk, not CPU, is binding.** `fixtures/encryption.py` caches ciphertexts +session-wide and never deletes them, and `rt_file()` outputs were never deleted +either. One encryptor per runner plus `rt_file.unlink()` in a `finally` gives +~6.5 GiB peak; without both it is 15 GiB+ and the runner fails. `XT_TMP_DIR` +puts that on the runner's large ephemeral volume. If disk still binds, take a +larger runner. + +**JVM and node heap.** The java shim pipes CLI stdout to a file with no `-Xmx` +and the js shim is a bare `npx`. If either buffers the payload rather than +streaming it, 2.1 GiB will OOM. The job sets `JAVA_TOOL_OPTIONS=-Xmx6g` and +`NODE_OPTIONS=--max-old-space-size=8192`, which both runtimes honour without a +shim edit. A shim that buffers unconditionally is a finding for the sibling +tickets, not a reason to shrink the payload. + +**`--skip-released-pairs` is deliberately not passed.** The `xct` job derives +it from `SKIP_RELEASED_PAIRS` to avoid re-testing release-against-release +combinations; here a released java decryptor is the entire point. + +## Out of Scope + +- The fixes themselves. go-sdk's 4 GiB → `MaxInt32` threshold change and + java-sdk #393 are the sibling tickets; this ticket only has to make them + demonstrable. +- Sizes above `2**32`. `large` still exists and still works, but nothing new + is asserted about it beyond the existing mandatory-ZIP64 check. +- Non-ZTDF containers. `nano` has its own framing and none of this applies. +- Making the multi-GiB path fast enough for the PR gate. It is a nightly. +- Fixing the segment-size defaulting itself. The `chunky` cell added here + demonstrates it; DSPX-4589 finding 4 and DSPX-4590 finding 7 fix it. + +## Acceptance Criteria + +Verified offline (no platform required): + +- [x] A 2.1 GiB size is expressible: `--sizes medium` parses, parametrizes + `size` session-wide, and selects the `zip64` cells; the default session + still resolves to `small` and deselects them. +- [x] Multi-GiB plaintext generation does not dominate job runtime — + 2.1 GiB in 0.5 s (4.4 GiB/s), byte-identical across regenerations, + deflate ratio 1.000 over the first 64 MiB. +- [x] The container's ZIP encoding is asserted on directly, so a failure + names the SDK at fault instead of reporting "decrypt failed". +- [x] `zipinspect`'s own unit tests (18) run offline on every PR via + `check.yml`, so the nightly's verdict does not rest on an unverified + parser. +- [x] The coverage runs on the nightly cron and via `workflow_dispatch` + (`run-zip64`), never on a PR, with its own `timeout-minutes: 90`. +- [x] Execution is confirmed by parsing the junit XML, not by grepping the + log; the job fails if no cell executed. Verified against a sample + report, including that `xfail` is counted as executed and a plain + `skip` is not. +- [x] The multi-segment defect the 2.1 GiB run turned up is covered on the + **PR gate**, not only the nightly: `chunky` (5 MiB) is a `feature_type` + and a size, and `test_chunky_roundtrip` runs in the standard `xct` job + with no CI change. It costs one 5 MiB encrypt and decrypt per pair; the + fixture generates in 0.05 s. + +Verified by the first live run +([33771257397](https://github.com/opentdf/tests/actions/runs/33771257397), +2026-09-03, `run-zip64=true java-ref="main latest"`): + +- [x] The 2.1 GiB roundtrip runs across go, java and web as encryptors, and + the fixture is not the bottleneck: whole jobs took 9m37s–12m45s against + a 90-minute budget, of which pytest was 90s–207s. +- [x] Every writer put an offset in the window — 2254888013 (go), + 2254888021 (java), 2254918149 (js) — so `_assert_reaches_the_window` + held in practice, not just by construction. +- [x] A pre-#393 java decryptor reports `XFAIL` with the DSPX-4592 reason + against all three writers. The failure is the sign-extension path + (`FileChannel.position()` throwing `IllegalArgumentException` on a + negative offset), not an OOM. +- [x] No OOM in either shim with the heap headroom the job sets. +- [x] `xct (main, go@main)` passed on the same run, so the `--sizes` + refactor did not disturb the default path. +- [ ] Sibling tickets' "Shared cross-SDK work" sections are satisfied. + +## First live run: what it found + +The job is red, which is the intended sequencing — but for more reasons than +the ticket anticipated. + +**java main is still pre-fix**, as expected — java-sdk#393 is open, not +merged (checked 2026-09-03). So main is the baseline, and its writer emits the +identical container to v0.18.0: + +``` +entry offset raw usize zip64 signed +0.payload 0 4294967295 2254887958 True -1 +0.manifest.json 2254888021 2254888021 139174 False -2040079275 +``` + +That is exactly the defect the ticket describes: `stream()` sets +`isZip64 = true` for the payload, `writeByteArray` hardcodes `false` for the +manifest, so the manifest's local-header offset goes out as a raw 32-bit value +with no extra field. The reader half is missing too — java@main fails to read +its own container at `ZipReader$Entry.getData:167`, and go's at +`ZipReader.:294`, both `FileChannel.position()` rejecting a negative +argument. + +Because `zip64_reader_xfail` keys on a released semver, java@main is *not* +xfailed and fails hard. That is the honest report — main is broken — but it +means the nightly stays red until #393 actually lands. + +**web-sdk is conformant, and confirms the parser.** js emits the +`0xFFFFFFFF` sentinel plus an extra field for both entries, and `zipinspect` +resolved the real values (offset 2254918149, size 2254918058) out of it. The +control behaved as predicted. + +**A defect nobody had filed: neither go nor java can read web-sdk's 2.1 GiB +payload.** js→js passes; js→go fails with +`splitKey.GetSignature failed: fail to create gmac signature`, and js→java@main +gets *past* the ZIP layer and then fails with +`tried to calculate GMAC on too small a payload. payload is 0 bytes while GMAC +is 16 bytes` at `TDF.calculateSignature:385`. + +**Root cause found, and it is not ZIP64.** web-sdk omits `segmentSize` and +`encryptedSegmentSize` from a segment object whenever they equal the manifest +defaults (`web-sdk/lib/tdf3/src/tdf.ts:696-700` — the ternaries yield +`undefined`, which drops the key from the JSON). Readers are meant to fall back +to the mandatory `segmentSizeDefault` / `encryptedSegmentSizeDefault`. Neither +consumer does: + +- go — `sdk/manifest.go:3-7` declares `Size`/`EncryptedSize` as plain `int64` + with no fallback, so an absent key is `0`. `sdk/tdf.go:993` then reads an + empty buffer, the `len(readBuf) != seg.EncryptedSize` guard at `:997` passes + vacuously, and `calculateSignature` fails the `kGMACPayloadLength > len(data)` + check at `:1531`. +- java — `Manifest.java:101-102` are plain `long`s; `TDF.java:334` allocates + `new byte[0]`; `TDF.java:385` throws. + +One cause, both errors. `manifest.schema.json` settles which side is wrong: +`segmentSizeDefault` and `encryptedSegmentSizeDefault` are **required** on +`integrityInformation`, and `segments/items` has **no** `required` list — the +per-segment values are optional overrides by design. web-sdk is conformant; go +and java are not. + +The manifest sizes above are independent confirmation: web-sdk spends 36.7 +bytes per segment, exactly `{"hash":"<24 b64 chars>"},`, while go spends 90.3 +carrying the full triple. + +**The real threshold is 1 MiB, not 2.1 GiB** — web-sdk's `DEFAULT_SEGMENT_SIZE` +(`tdf.ts:76`), the point at which the first exactly-default-sized chunk appears. +Below it the lone chunk is partial, its size is written explicitly, and both +readers cope; that is the only reason the 128-byte nightly is green. So every +web-sdk TDF over 1 MiB is currently unreadable by go and java, and has been +since 2022-08-22 (web-sdk e991829). It went four years undetected for exactly +the reason this ticket exists: xtest tested 128 bytes, and the 5 GiB `--large` +fixture had no CI wiring. + +Tracked on the reader side, where the defect is, rather than as a fourth +ticket: **DSPX-4589 finding 4** (java) and **DSPX-4590 finding 7** (go), each +with an acceptance criterion that a web-sdk TDF over 1 MiB round-trips. web-sdk +is conformant and is not being asked to change, so DSPX-4591 is unaffected. +Both fixes want a ~2 MiB regression cell, not a multi-GiB one — see Out of +Scope. diff --git a/xtest/AGENTS.md b/xtest/AGENTS.md index 2dc875a6..5997f809 100644 --- a/xtest/AGENTS.md +++ b/xtest/AGENTS.md @@ -27,7 +27,7 @@ fixture system. | `--sdks-encrypt`, `--sdks-decrypt` | Asymmetric encrypt/decrypt SDK selection (use when reproducing cross-SDK interop bugs). | | `--containers ztdf ztdf-ecwrap` | Which TDF container types to exercise. | | `--no-audit-logs` | Skip audit-log assertions for this run. CLI equivalent of `DISABLE_AUDIT_ASSERTIONS=1`. | -| `--sizes small,chunky` | Which payload sizes to parametrize over (`small` 128 B, `chunky` 5 MiB, `large` 5 GiB). Defaults to `small`. Every extra size fans out every test taking `pt_file`. `--large` is a deprecated alias for `small,large`. | +| `--sizes small,chunky` | Which payload sizes to parametrize over (`small` 128 B, `chunky` 5 MiB, `medium` 2.1 GiB, `large` 5 GiB). Defaults to `small`. Every extra size fans out every test taking `pt_file`. `--large` is a deprecated alias for `small,large`. | ## Environment Variables @@ -35,7 +35,7 @@ Beyond the repo-wide ones in `../AGENTS.md`: | Variable | Purpose | |----------|---------| -| `XT_TMP_DIR` | Root for generated fixtures and ciphertexts (default `tmp/`). Point at a large volume for `large` runs. | +| `XT_TMP_DIR` | Root for generated fixtures and ciphertexts (default `tmp/`). Point at a large volume for `medium`/`large` runs. | | `XT_FORCE_SUPPORTS` | Comma-separated features to treat as supported, bypassing the `cli.sh supports` gate. For evaluating a fix before it releases — see `../AGENTS.md`. Unknown names raise. | ## Authoring a New Test diff --git a/xtest/conftest.py b/xtest/conftest.py index 4a19f132..3dfc865f 100644 --- a/xtest/conftest.py +++ b/xtest/conftest.py @@ -151,7 +151,8 @@ def resolve_sizes(config: pytest.Config) -> list[str]: "deprecated spelling of --sizes small,large" ) warnings.warn( - "--large is deprecated; use --sizes small,large", + "--large is deprecated; use --sizes small,large (or --sizes medium " + "for the 2-4 GiB ZIP64 band, which --large steps straight over)", DeprecationWarning, stacklevel=2, ) @@ -428,23 +429,51 @@ def pytest_configure(config: pytest.Config): ) +def _item_exercises_zip64_window(item: pytest.Item, session_sizes: list[str]) -> bool: + """Whether this item has a payload large enough for the ZIP64 tests. + + Size-aware items must be judged by their own parametrized value. Marked + items without a ``size`` parameter retain the session-level behaviour so + a future ZIP64 test with a purpose-built fixture is not dropped merely + because it does not use :func:`pt_file`. + """ + callspec = getattr(item, "callspec", None) + item_size = callspec.params.get("size") if callspec is not None else None + if isinstance(item_size, str): + return sizes.exercises_zip64_window(item_size) + return any(sizes.exercises_zip64_window(size) for size in session_sizes) + + def pytest_collection_modifyitems( config: pytest.Config, items: list[pytest.Item] ) -> None: - """Drop the benchmark cells entirely unless --bench asked for them. - - Deselected rather than skipped: a 20-minute cell has no business in the - regular integration matrix, and a skip would report it as a test that - exists and was declined rather than one that was never in scope. + """Drop cells the session did not ask for. + + Two groups, deselected rather than skipped for the same reason: neither a + 20-minute benchmark nor a 2.1 GiB roundtrip has any business in the + regular integration matrix, and a skip would report them as tests that + exist and were declined rather than ones that were never in scope. + + - ``benchmark``: needs --bench. + - ``zip64``: needs a payload size that can reach the 2**31 boundary. At + the default 128 bytes these tests cannot exercise anything, and the one + thing worse than not running them is running them green on a payload + that never touches the code path. """ - if config.getoption("--bench", default=False): - return - keep, drop = [], [] + drop: list[pytest.Item] = [] + want_bench = bool(config.getoption("--bench", default=False)) + session_sizes = resolve_sizes(config) for item in items: - (drop if item.get_closest_marker("benchmark") else keep).append(item) + if not want_bench and item.get_closest_marker("benchmark"): + drop.append(item) + elif item.get_closest_marker("zip64") and not _item_exercises_zip64_window( + item, session_sizes + ): + drop.append(item) if drop: + dropped = set(map(id, drop)) config.hook.pytest_deselected(items=drop) - items[:] = keep + items[:] = [i for i in items if id(i) not in dropped] def pytest_sessionfinish(session: pytest.Session, exitstatus: int): @@ -584,8 +613,9 @@ def pt_file(tmp_dir: Path, size: str) -> Path: Args: tmp_dir: Temporary directory for test files size: a key of :data:`sizes.SIZES` -- 'small' (128 bytes), - 'chunky' (5 MiB, several default-sized segments), or - 'large' (5 GiB) + 'chunky' (5 MiB, several default-sized segments), + 'medium' (2.1 GiB, inside the ZIP64 broken window), or + 'large' (5 GiB, above it) Returns: Path to the generated plaintext file diff --git a/xtest/pyproject.toml b/xtest/pyproject.toml index 5884dea1..d2f13502 100644 --- a/xtest/pyproject.toml +++ b/xtest/pyproject.toml @@ -89,6 +89,7 @@ known-first-party = [ "fixtures", "perf", "sizes", + "zipinspect", ] [tool.ruff.format] @@ -113,4 +114,5 @@ addopts = "-ra -v" markers = [ "benchmark: paired A/B performance cell; only collected under --bench", "no_audit_logs: opt this test out of the default audit-log assertions", + "zip64: multi-GiB ZIP64 boundary cell; only collected when --sizes reaches 2**31", ] diff --git a/xtest/sizes.py b/xtest/sizes.py index e9d5cf15..6f44551a 100644 --- a/xtest/sizes.py +++ b/xtest/sizes.py @@ -1,13 +1,53 @@ -"""Plaintext payload sizes for cross-SDK test fixtures. +"""Plaintext payload sizes, and the ZIP64 window they are chosen around. Kept free of pytest and of ``tdfs`` so that both ``conftest.py`` and the test modules can name a size without importing each other. + +The ZIP central directory stores local-header offsets and entry sizes in 32-bit +fields that are *unsigned on the wire*. Three regimes follow, and only one of +them can expose a signed-widening bug: + +=========================== ========================================== +value what a reader sees +=========================== ========================================== +``v < 2**31`` a signed read and an unsigned read agree +``2**31 <= v < 2**32`` a signed read comes back negative +``v >= 2**32`` ZIP64 sentinel; the 32-bit field is never + populated with a real value, so the bug + cannot fire +=========================== ========================================== + +That middle row is the only broken window, and it is exactly what +:data:`SIZES`'s ``medium`` entry exists to land a TDF's manifest offset in. """ from __future__ import annotations -#: 5 MiB. This is the smallest size at which *every* SDK's writer emits more -#: than one **default-sized** segment. +#: Smallest value a 32-bit field must be read as unsigned to survive. +ZIP64_WINDOW_LOW = 2**31 + +#: At and above this the format requires the ZIP64 sentinel plus an extra +#: field, so the 32-bit field holds 0xFFFFFFFF rather than a real value. +ZIP64_WINDOW_HIGH = 2**32 + +#: 2.1 GiB. Sits ~102 MiB inside the low edge of the broken window. +#: +#: The margin is the point. A TDF writes ``0.payload`` first and +#: ``0.manifest.json`` after it, so the manifest's local-header offset is +#: roughly the payload size -- and that offset is the value under test. The +#: gap to 2**31 has to be wider than anything that could shift it: segment +#: padding, manifest length, per-entry header overhead. 102 MiB is not a +#: round number because it does not need to be; it needs to be unarguably +#: larger than those. +#: +#: Shrinking this below 2**31 does not make the test cheaper, it makes it +#: vacuous -- every SDK takes the safe path and the test passes without +#: exercising anything. See the assertion in test_zip64.py that fails loudly +#: rather than letting that happen quietly. +MEDIUM_BYTES = 2_254_857_830 + +#: 5 MiB. Nothing to do with the ZIP64 window -- this is the smallest size at +#: which *every* SDK's writer emits more than one **default-sized** segment. #: #: A segment only exercises the ``chunky`` path if its size equals the #: manifest-level default, because that is precisely the case web-sdk omits @@ -23,8 +63,25 @@ SIZES: dict[str, int] = { "small": 128, "chunky": CHUNKY_BYTES, + "medium": MEDIUM_BYTES, "large": 5 * 2**30, } #: Order to emit parametrized sizes in, cheapest first. -SIZE_ORDER: tuple[str, ...] = ("small", "chunky", "large") +SIZE_ORDER: tuple[str, ...] = ("small", "chunky", "medium", "large") + + +def in_zip64_window(n: int) -> bool: + """True for values a signed 32-bit read would mangle.""" + return ZIP64_WINDOW_LOW <= n < ZIP64_WINDOW_HIGH + + +def exercises_zip64_window(size: str) -> bool: + """True if a payload of this size can put a real value in the broken window. + + Note this is ``>=`` the low edge rather than :func:`in_zip64_window`: a + 5 GiB payload does not itself land in the window, but the run that asked + for it is plainly a large-file run and the zip64 module has something to + say about its ZIP64 encoding too. + """ + return SIZES[size] >= ZIP64_WINDOW_LOW diff --git a/xtest/tdfs.py b/xtest/tdfs.py index 392bd31b..c16abc72 100644 --- a/xtest/tdfs.py +++ b/xtest/tdfs.py @@ -883,8 +883,9 @@ def skip_chunky_skew(ct_file: Path, decrypt_sdk: SDK): """Skip if ``ct_file`` needs segment-size defaulting and the reader lacks it. A skip and not an xfail: this cell runs on the PR gate, where a - permanently-red job trains people to ignore it, and it needs no dated - guess about which release carries the fix. + permanently-red job trains people to ignore it, and unlike + :func:`zip64_reader_xfail` it needs no dated guess about which release + carries the fix. The cost is that it stays skipped until somebody edits ``sdk/{go,java}/cli.sh`` to answer yes -- the ``supports`` case statement @@ -906,6 +907,44 @@ def skip_chunky_skew(ct_file: Path, decrypt_sdk: SDK): ) +#: First java-sdk release containing java-sdk#393. +#: +#: Before it, ``ZipReader.readInt()`` sign-extends, so a central-directory +#: offset in ``[2**31, 2**32)`` comes back negative and the read fails or +#: seeks to nonsense. A 2.1 GiB payload puts the manifest's offset exactly +#: there. See DSPX-4592. +#: +#: Keep this honest. Set too high, a fixed release keeps reporting XFAIL and +#: a genuine regression hides behind it; set too low, the strict xfail turns +#: every pre-fix cell into a hard failure. Update it when the release with +#: #393 actually ships, not when the PR merges. +JAVA_ZIP64_READER_FIX = (0, 19, 0) + + +def zip64_reader_xfail(decrypt_sdk: SDK) -> pytest.MarkDecorator | None: + """An xfail marker for decryptors known to mishandle the 2-4 GiB band. + + ``strict=True`` deliberately. The point of this test is to flip to green + when the sibling fixes land: an XPASS here means a build we believed + broken now reads the container correctly, and that should fail the run so + somebody comes and deletes this predicate rather than leaving a + permanently-XFAIL cell that nobody reads. + + Branch builds (``main``) have no semver and are never marked -- they are + the builds expected to carry the fix. + """ + sv = decrypt_sdk.semver() + if decrypt_sdk.sdk == "java" and sv is not None and sv < JAVA_ZIP64_READER_FIX: + return pytest.mark.xfail( + strict=True, + reason=( + f"DSPX-4592: {decrypt_sdk} predates java-sdk#393; readInt() " + "sign-extends the manifest's central-directory offset" + ), + ) + return None + + def _parse_semver(version: str) -> tuple[int, int, int] | None: """Parse a version string (with optional 'v' prefix) into (major, minor, patch).""" m = _version_re.match(version.lstrip("v")) diff --git a/xtest/test_sizes_units.py b/xtest/test_sizes_units.py index b91a8911..67db8363 100644 --- a/xtest/test_sizes_units.py +++ b/xtest/test_sizes_units.py @@ -12,6 +12,41 @@ class TestSizes: + def test_medium_is_inside_the_broken_window(self): + """The whole ticket rests on this one number being in the band.""" + assert sizes.in_zip64_window(sizes.MEDIUM_BYTES) + + def test_medium_has_margin_below_the_low_edge(self): + """Manifest size and segment padding must not push the offset back under 2**31. + + The manifest is written after the payload, so its local-header offset + is the payload size plus header overhead -- but the assertion that + matters is the reverse: the payload alone must already clear the + boundary by more than any plausible overhead. + """ + margin = sizes.MEDIUM_BYTES - sizes.ZIP64_WINDOW_LOW + assert margin > 100 * 2**20, ( + f"only {margin} bytes of margin above 2**31; segment padding and " + "manifest size could push the interesting offset back below it" + ) + + def test_small_and_large_sit_outside_the_window(self): + """The two pre-existing sizes are exactly why this ticket exists.""" + assert sizes.SIZES["small"] < sizes.ZIP64_WINDOW_LOW + assert sizes.SIZES["large"] >= sizes.ZIP64_WINDOW_HIGH + assert not sizes.in_zip64_window(sizes.SIZES["small"]) + assert not sizes.in_zip64_window(sizes.SIZES["large"]) + + # Named size_name, not size: `size` is parametrized session-wide by + # conftest's pytest_generate_tests, and reusing it here is a collection + # error rather than a shadow. + @pytest.mark.parametrize( + ("size_name", "expected"), + [("small", False), ("chunky", False), ("medium", True), ("large", True)], + ) + def test_which_sizes_select_the_zip64_tests(self, size_name: str, expected: bool): + assert sizes.exercises_zip64_window(size_name) is expected + def test_chunky_clears_every_sdk_default_segment(self): """5 MiB has to buy more than one *default-sized* segment, everywhere. @@ -25,6 +60,11 @@ def test_chunky_clears_every_sdk_default_segment(self): largest_known_default = 2 * 2**20 assert sizes.CHUNKY_BYTES > 2 * largest_known_default + def test_chunky_stays_cheap(self): + """It runs on the PR gate, so it must not creep toward the nightly's cost.""" + assert sizes.CHUNKY_BYTES < 64 * 2**20 + assert not sizes.in_zip64_window(sizes.CHUNKY_BYTES) + class TestSizesOptionParsing: def test_dedups_and_orders_cheapest_first(self): diff --git a/xtest/test_tdfs_units.py b/xtest/test_tdfs_units.py index b6b9b5f2..8bb22ad0 100644 --- a/xtest/test_tdfs_units.py +++ b/xtest/test_tdfs_units.py @@ -1,20 +1,24 @@ -"""Offline tests for tdfs.py's XT_FORCE_SUPPORTS override and chunky gating (DSPX-4638, DSPX-4589). +"""Offline tests for tdfs.py's anti-vacuous-green machinery (DSPX-4592, DSPX-4638). -No platform, no SDK, no subprocess. ``_parse_forced_supports`` and -``skip_chunky_skew`` are both safeguards built specifically to stop a real -regression from hiding behind a skip -- so they are worth testing on their -own. +No platform, no SDK, no subprocess. ``_parse_forced_supports``, +``zip64_reader_xfail``, and ``skip_chunky_skew`` are all safeguards built +specifically to stop a real regression from hiding behind a skip or a stale +xfail -- so they are worth testing on their own, the same way the ZIP64 +parser they sit next to is tested in ``test_zip64_units.py``. """ import json import zipfile from pathlib import Path +from types import SimpleNamespace from typing import cast import pytest import tdfs +# --- tdfs._parse_forced_supports --------------------------------------------- + class TestParseForcedSupports: def test_parses_comma_and_whitespace_separated_names(self): @@ -32,6 +36,35 @@ def test_unknown_name_raises(self): tdfs._parse_forced_supports("hexles") +# --- tdfs.zip64_reader_xfail -------------------------------------------------- + + +def _stub_sdk(sdk: str, semver: tuple[int, int, int] | None) -> tdfs.SDK: + """A duck-typed stand-in exposing only what zip64_reader_xfail reads.""" + return cast(tdfs.SDK, SimpleNamespace(sdk=sdk, semver=lambda: semver)) + + +class TestZip64ReaderXfail: + def test_pre_fix_java_gets_a_strict_xfail(self): + stub = _stub_sdk("java", (0, 18, 0)) + marker = tdfs.zip64_reader_xfail(stub) + assert marker is not None + assert marker.mark.kwargs["strict"] is True + + def test_post_fix_java_is_not_marked(self): + stub = _stub_sdk("java", tdfs.JAVA_ZIP64_READER_FIX) + assert tdfs.zip64_reader_xfail(stub) is None + + def test_non_java_sdk_is_never_marked(self): + stub = _stub_sdk("go", (0, 1, 0)) + assert tdfs.zip64_reader_xfail(stub) is None + + def test_branch_build_is_never_marked(self): + """A branch build (e.g. 'main') has no semver and is expected to carry the fix.""" + stub = _stub_sdk("java", None) + assert tdfs.zip64_reader_xfail(stub) is None + + # --- tdfs.elides_segment_sizes / tdfs.skip_chunky_skew ------------------------ diff --git a/xtest/test_zip64.py b/xtest/test_zip64.py new file mode 100644 index 00000000..01eabefe --- /dev/null +++ b/xtest/test_zip64.py @@ -0,0 +1,142 @@ +"""Cross-SDK coverage for the ZIP64 boundary at 2 GiB (DSPX-4592). + +Every test here is marked ``zip64`` and is deselected unless the session asks +for a payload size that can reach ``2**31`` -- see the size table in +``sizes.py`` for why 2.1 GiB and not something rounder, and +``pytest_collection_modifyitems`` in ``conftest.py`` for the deselection. + +Run it with:: + + uv run pytest test_zip64.py --sizes medium --sdks "go java js" -v + +These are separate from ``test_tdfs.py`` for three reasons that all come down +to the payload size: the decrypted output is deleted as soon as it has been +compared rather than accumulating at 2.1 GiB a time, the container's ZIP +encoding is asserted on directly, and CI can select them without dragging the +rest of the suite up to multi-GiB payloads. +""" + +import filecmp +import logging +from pathlib import Path + +import pytest + +import tdfs +import zipinspect +from abac import Attribute +from fixtures.encryption import EncryptFactory +from sizes import SIZES, ZIP64_WINDOW_LOW + +logger = logging.getLogger(__name__) + +# ``no_audit_logs`` is inert today -- nothing here requests the ``audit_logs`` +# fixture, and it is not autouse -- but it states the intent: this module tests +# the container encoding, and a rewrap audit event says nothing about that. +pytestmark = [pytest.mark.zip64, pytest.mark.no_audit_logs] + + +def _assert_reaches_the_window( + entries: list[zipinspect.CentralDirectoryEntry], + pt_file: Path, + encrypt_sdk: tdfs.SDK, +) -> None: + """Fail unless some entry actually landed at or above 2**31. + + This is the load-bearing assertion in the module. Everything else here + tests how an SDK handles a value in the broken window; if no value got + there, the rest of the test passes without exercising a single line of + the code under test, and reports success for it. + + That is the exact failure this ticket exists to close -- the suite already + had a large-file path that stepped over the window -- so it is an + assertion rather than a skip. + """ + biggest = max((e.local_header_offset for e in entries), default=0) + assert biggest >= ZIP64_WINDOW_LOW, ( + f"{encrypt_sdk} wrote a container whose largest local-header offset is " + f"{biggest}, below 2**31 ({ZIP64_WINDOW_LOW}), from a " + f"{pt_file.stat().st_size}-byte payload. Nothing in this test is " + f"exercising the 2-4 GiB band.\n" + zipinspect.describe(entries) + ) + + +def test_zip64_band_roundtrip( + request: pytest.FixtureRequest, + encrypt_sdk: tdfs.SDK, + decrypt_sdk: tdfs.SDK, + pt_file: Path, + size: str, + in_focus: set[tdfs.SDK], + attribute_default_rsa: Attribute, + encrypted_tdf: EncryptFactory, +): + """Encrypt and decrypt a payload whose manifest offset is in the broken window. + + The whole cross-SDK matrix runs against one payload: writer defects and + reader defects both surface as a failure to round-trip, and which SDK is + at fault is what the structural assertions below disambiguate. + """ + if not in_focus & {encrypt_sdk, decrypt_sdk}: + pytest.skip("Not in focus") + tdfs.skip_hexless_skew(encrypt_sdk, decrypt_sdk) + + ct_file = encrypted_tdf( + encrypt_sdk, + target_mode=tdfs.select_target_version(encrypt_sdk, decrypt_sdk), + attr_values=attribute_default_rsa.value_fqns, + ) + + entries = zipinspect.central_directory(ct_file) + logger.info( + "%s wrote %s at size=%s (%d bytes):\n%s", + encrypt_sdk, + ct_file.name, + size, + SIZES[size], + zipinspect.describe(entries), + ) + + # Writer conformance first, and outside the reader's xfail below. A + # writer regression must not hide behind a known reader bug: if these + # fail under an xfail marker the cell reports XFAIL and nobody looks. + _assert_reaches_the_window(entries, pt_file, encrypt_sdk) + zipinspect.assert_zip64_above_4gib(entries) + + in_window = zipinspect.entries_in_window(entries) + logger.info( + "%s: %d entr%s in [2**31, 2**32); zip64 extra field used for %s", + encrypt_sdk, + len(in_window), + "y" if len(in_window) == 1 else "ies", + [e.name for e in in_window if e.has_zip64_extra] or "none", + ) + + # Keep the independent segment-defaulting incompatibility out of the + # ZIP64 result. In particular, web-sdk uses ZIP64 sentinels in this band, + # so those containers do not exercise Java's signed 32-bit read defect. + tdfs.skip_chunky_skew(ct_file, decrypt_sdk) + + # Apply the reader xfail only when a real 32-bit value (not the sentinel) + # exercises the signed-risk window. Writer conformance has already been + # checked above, so a failure from this point belongs to the reader. + if zipinspect.entries_with_raw_values_in_window(entries): + if mark := tdfs.zip64_reader_xfail(decrypt_sdk): + request.node.add_marker(mark) + + rt_file = encrypted_tdf.rt_file(ct_file, decrypt_sdk) + try: + decrypt_sdk.decrypt(ct_file, rt_file, "ztdf") + # shallow=False explicitly: the default compares a stat signature + # first and only falls through to a byte compare because the mtimes + # happen to differ. At this size the difference between checking the + # bytes and checking the size is worth not leaving to chance. + assert filecmp.cmp(pt_file, rt_file, shallow=False), ( + f"{decrypt_sdk} decrypted {ct_file.name} without error but the " + f"output does not match the {pt_file.stat().st_size}-byte input" + ) + finally: + # 2.1 GiB per pair. The ciphertext is session-cached and shared, but + # these are not, and a full matrix would fill the runner's disk long + # before the last cell. + rt_file.unlink(missing_ok=True) diff --git a/xtest/test_zip64_units.py b/xtest/test_zip64_units.py new file mode 100644 index 00000000..bebc679f --- /dev/null +++ b/xtest/test_zip64_units.py @@ -0,0 +1,434 @@ +"""Offline tests for the ZIP64 boundary machinery (DSPX-4592). + +No platform, no SDK, no subprocess. These run in ``check.yml`` on every PR, +because the multi-GiB test they support runs only on a nightly cron -- a +parser bug found six weeks later, in a job nobody watches, on a fixture that +takes twenty minutes to reproduce, is a bad trade against a few seconds here. + +The central directories are synthesized byte by byte rather than produced by +``zipfile``. A real 2.1 GiB container is exactly what cannot be built in a +unit test, and ``zipfile`` will not emit a 32-bit field holding a value in +``[2**31, 2**32)`` on request -- which is the encoding under test. +""" + +import struct +import zipfile +from pathlib import Path +from types import SimpleNamespace +from typing import cast + +import pytest + +import conftest +import zipinspect +from sizes import MEDIUM_BYTES, ZIP64_WINDOW_HIGH, ZIP64_WINDOW_LOW +from zipinspect import ZIP64_SENTINEL_32, MalformedZipError + +# --- Synthetic container construction --------------------------------------- + + +def cen_record( + name: str, + *, + raw_offset: int, + raw_usize: int = 0, + raw_csize: int = 0, + zip64_offset: int | None = None, + zip64_usize: int | None = None, + zip64_csize: int | None = None, +) -> bytes: + """One central-directory header, with an optional ZIP64 extra field. + + ``zip64_*`` values are written into the extra field in APPNOTE 4.5.3's + fixed order (uncompressed, compressed, offset); pass them only for the + fields whose 32-bit slot holds the sentinel, which is the same contract + the parser relies on. + """ + extra = b"" + body = b"" + if zip64_usize is not None: + body += struct.pack(" Path: + """Write a container that is nothing but a central directory and an EOCD. + + The parser never reads entry data, so leaving it out keeps these tests + instant while exercising every field it does read. + """ + cd = b"".join(records) + cd_offset = 0 + eocd = ( + b"PK\x05\x06" + + struct.pack(" Path: + """Same, but located through a ZIP64 EOCD record and its locator. + + The 32-bit EOCD carries sentinels, so a reader that stops there sees + 0xFFFF entries at offset 0xFFFFFFFF. This is how a container whose + central directory sits past 4 GiB has to be read. + """ + cd = b"".join(records) + cd_offset = 0 + eocd64 = ( + b"PK\x06\x06" + + struct.pack("4 GiB offset with no ZIP64 encoding, + # which is the state a non-conformant writer would leave behind. + broken = [ + zipinspect.CentralDirectoryEntry( + name="0.manifest.json", + raw_compressed_size=0, + raw_uncompressed_size=0, + raw_local_header_offset=12345, + compressed_size=0, + uncompressed_size=0, + local_header_offset=5 * 2**30, + has_zip64_extra=False, + ) + ] + zipinspect.assert_zip64_above_4gib(entries) # the conformant one passes + with pytest.raises(AssertionError, match="ZIP64 sentinel"): + zipinspect.assert_zip64_above_4gib(broken) + + def test_above_4gib_uncompressed_size_without_the_sentinel_fails(self): + """The size branches had no test of their own; the offset test above doesn't touch them.""" + broken = [ + zipinspect.CentralDirectoryEntry( + name="0.payload", + raw_compressed_size=0, + raw_uncompressed_size=12345, + raw_local_header_offset=0, + compressed_size=0, + uncompressed_size=5 * 2**30, + local_header_offset=0, + has_zip64_extra=False, + ) + ] + with pytest.raises(AssertionError, match="ZIP64 sentinel"): + zipinspect.assert_zip64_above_4gib(broken) + + def test_above_4gib_compressed_size_without_the_sentinel_fails(self): + """Compressed size must be checked against its own raw field, not the uncompressed one. + + A TDF is STORED, not DEFLATEd, so the compressed field is at least as + likely to cross 2**32 as the uncompressed one -- but a check that only + looks at ``uses_zip64_for_sizes`` (an OR over both raw fields) would + let a correctly-sentineled uncompressed field paper over a broken + compressed one. This entry has exactly that shape. + """ + broken = [ + zipinspect.CentralDirectoryEntry( + name="0.payload", + raw_compressed_size=12345, + raw_uncompressed_size=ZIP64_SENTINEL_32, + raw_local_header_offset=0, + compressed_size=5 * 2**30, + uncompressed_size=5 * 2**30, + local_header_offset=0, + has_zip64_extra=True, + ) + ] + with pytest.raises(AssertionError, match="compressed-size field"): + zipinspect.assert_zip64_above_4gib(broken) + + def test_above_4gib_sizes_with_the_sentinel_pass(self, tmp_path: Path): + """The positive counterpart: both size fields correctly ZIP64-encoded.""" + p = synth_zip( + tmp_path / "big-sizes.zip", + [ + cen_record( + "0.payload", + raw_offset=0, + raw_usize=ZIP64_SENTINEL_32, + raw_csize=ZIP64_SENTINEL_32, + zip64_usize=5 * 2**30, + zip64_csize=5 * 2**30 + 1, + ) + ], + ) + entries = zipinspect.central_directory(p) + zipinspect.assert_zip64_above_4gib(entries) + + def test_window_entries_are_reported_for_either_encoding(self, tmp_path: Path): + """Both a raw value and a sentinel in the band are legal and both are listed.""" + p = synth_zip( + tmp_path / "mixed.zip", + [ + cen_record("raw", raw_offset=MEDIUM_BYTES), + cen_record( + "sentinel", + raw_offset=ZIP64_SENTINEL_32, + zip64_offset=MEDIUM_BYTES + 1024, + ), + cen_record("small", raw_offset=1024), + ], + ) + entries = zipinspect.central_directory(p) + assert {e.name for e in zipinspect.entries_in_window(entries)} == { + "raw", + "sentinel", + } + + def test_only_raw_window_values_exercise_signed_read(self, tmp_path: Path): + """The sentinel redirects to ZIP64 data and is not a signed read risk.""" + p = synth_zip( + tmp_path / "mixed.zip", + [ + cen_record("raw", raw_offset=MEDIUM_BYTES), + cen_record( + "sentinel", + raw_offset=ZIP64_SENTINEL_32, + zip64_offset=MEDIUM_BYTES + 1024, + ), + ], + ) + entries = zipinspect.central_directory(p) + assert { + e.name for e in zipinspect.entries_with_raw_values_in_window(entries) + } == {"raw"} + + def test_describe_includes_the_numbers_needed_to_debug(self, tmp_path: Path): + p = synth_zip( + tmp_path / "d.zip", [cen_record("0.manifest.json", raw_offset=MEDIUM_BYTES)] + ) + text = zipinspect.describe(zipinspect.central_directory(p)) + assert "0.manifest.json" in text + assert str(MEDIUM_BYTES) in text diff --git a/xtest/zipinspect.py b/xtest/zipinspect.py new file mode 100644 index 00000000..87977222 --- /dev/null +++ b/xtest/zipinspect.py @@ -0,0 +1,350 @@ +"""Raw ZIP central-directory reader, for asserting on the *encoding*. + +``zipfile`` cannot be used for this. It normalises ZIP64 away -- ask it for an +entry's header offset and you get the resolved value, whether that came from +the 32-bit field or from a ZIP64 extra field. The distinction it discards is +precisely what these tests are about, so the bytes are parsed here instead. + +Only the tail of the file plus the central directory is read, so this stays +cheap on a multi-GiB container. + +Reference: APPNOTE.TXT 4.3.12 (central directory header), 4.3.16 (end of +central directory), 4.5.3 (the ZIP64 extended information extra field). +""" + +from __future__ import annotations + +import struct +from dataclasses import dataclass +from pathlib import Path + +from sizes import ZIP64_WINDOW_HIGH, in_zip64_window + +# Signatures, little-endian. +_CEN_SIG = b"PK\x01\x02" +_EOCD_SIG = b"PK\x05\x06" +_EOCD64_SIG = b"PK\x06\x06" +_EOCD64_LOCATOR_SIG = b"PK\x06\x07" + +#: Written into a 32-bit field to mean "the real value is in the ZIP64 extra +#: field". APPNOTE 4.4.1.4. +ZIP64_SENTINEL_32 = 0xFFFFFFFF +ZIP64_SENTINEL_16 = 0xFFFF + +#: Header ID of the ZIP64 extended information extra field. APPNOTE 4.5.3. +ZIP64_EXTRA_ID = 0x0001 + +_EOCD_SIZE = 22 +_EOCD64_LOCATOR_SIZE = 20 +#: A ZIP comment is a 16-bit length, so the EOCD cannot start further back +#: than this from the end of the file. +_MAX_EOCD_SEARCH = _EOCD_SIZE + 0xFFFF + + +class MalformedZipError(Exception): + """The container is not a ZIP we can parse at all.""" + + +@dataclass(frozen=True, slots=True) +class CentralDirectoryEntry: + """One central-directory record, with the raw fields kept alongside. + + ``raw_*`` are the 32-bit values exactly as they appear on the wire. + The unprefixed attributes are the resolved values, ZIP64 extra field + applied where present. Comparing the two is how a caller tells "this + writer emitted a real 2.1 GiB value in a 32-bit field" from "this writer + emitted the sentinel and put the value in the extra field". + """ + + name: str + raw_compressed_size: int + raw_uncompressed_size: int + raw_local_header_offset: int + compressed_size: int + uncompressed_size: int + local_header_offset: int + has_zip64_extra: bool + + @property + def uses_zip64_for_offset(self) -> bool: + return self.raw_local_header_offset == ZIP64_SENTINEL_32 + + @property + def uses_zip64_for_sizes(self) -> bool: + return ZIP64_SENTINEL_32 in ( + self.raw_compressed_size, + self.raw_uncompressed_size, + ) + + def signed_read_of_offset(self) -> int: + """What a reader that sign-extends a 32-bit read would compute. + + The defect this module exists to catch, expressed directly: for a raw + value at or above 2**31 this returns a negative number, and a seek to + it fails or lands on nonsense. + """ + return struct.unpack(" int: + """Offset of the EOCD record within the tail buffer. + + Searched backwards: the signature can legitimately appear inside a file + comment, and the last occurrence is the real one. + """ + idx = data.rfind(_EOCD_SIG) + if idx < 0: + raise MalformedZipError("no end-of-central-directory record found") + return idx + + +def _parse_zip64_extra( + extra: bytes, + *, + want_uncompressed: bool, + want_compressed: bool, + want_offset: bool, +) -> tuple[bool, int | None, int | None, int | None]: + """Pull the 64-bit values out of the ZIP64 extended information field. + + The field is positional, not tagged: values appear only for the 32-bit + fields that held the sentinel, in a fixed order (uncompressed size, + compressed size, local header offset, disk start). So which values are + present depends on the record that referenced it, which is what the + ``want_*`` flags carry in. + + Returns ``(present, uncompressed, compressed, offset)``; the values are + None when the corresponding 32-bit field did not hold the sentinel. + """ + pos = 0 + while pos + 4 <= len(extra): + header_id, size = struct.unpack_from(" len(extra): + break + if header_id != ZIP64_EXTRA_ID: + pos += size + continue + body = extra[pos : pos + size] + # Read the 64-bit values in APPNOTE order, consuming one only for each + # 32-bit field that actually held the sentinel. A truncated field + # yields None rather than raising: a malformed extra field is a + # finding for the caller's assertions, not a parse error. + values: list[int | None] = [] + at = 0 + for want in (want_uncompressed, want_compressed, want_offset): + if want and at + 8 <= len(body): + values.append(struct.unpack_from(" list[CentralDirectoryEntry]: + """Parse every central-directory record in ``path``. + + Reads the tail of the file to locate the directory, then the directory + itself. The payload is never touched, so cost is independent of container + size. + """ + size = path.stat().st_size + with path.open("rb") as f: + tail_len = min(size, _MAX_EOCD_SEARCH) + f.seek(size - tail_len) + tail = f.read(tail_len) + + eocd_at = _find_eocd(tail) + ( + cd_entries_this_disk, + cd_entries_total, + cd_size, + cd_offset, + ) = struct.unpack_from("= 0 and tail[locator_at : locator_at + 4] == _EOCD64_LOCATOR_SIG: + (eocd64_offset,) = struct.unpack_from(" str: + """One line per entry, for attaching to a failure message. + + A structural failure is nearly unreadable without the actual numbers, and + reproducing it costs a multi-GiB encrypt. + """ + header = ( + f"{'entry':<20} {'offset':>14} {'raw':>12} " + f"{'usize':>14} {'csize':>14} {'zip64':>6} {'signed':>14}" + ) + return "\n".join( + [header] + + [ + f"{e.name:<20} {e.local_header_offset:>14} " + f"{e.raw_local_header_offset:>12} {e.uncompressed_size:>14} " + f"{e.compressed_size:>14} " + f"{str(e.has_zip64_extra):>6} {e.signed_read_of_offset():>14}" + for e in entries + ] + ) + + +def assert_zip64_above_4gib(entries: list[CentralDirectoryEntry]) -> None: + """Every value at or above 2**32 must use the ZIP64 sentinel plus extra field. + + Unlike the 2-4 GiB band, there is no latitude here: a 32-bit field + physically cannot hold the value, so a writer that does not emit the + sentinel has produced a container whose stated offsets are wrong. + """ + for e in entries: + if e.local_header_offset >= ZIP64_WINDOW_HIGH: + assert e.uses_zip64_for_offset and e.has_zip64_extra, ( + f"entry {e.name!r} is at offset {e.local_header_offset}, at or " + f"above 2**32, but its 32-bit field holds " + f"{e.raw_local_header_offset} rather than the ZIP64 sentinel\n" + + describe(entries) + ) + if e.uncompressed_size >= ZIP64_WINDOW_HIGH: + assert e.raw_uncompressed_size == ZIP64_SENTINEL_32 and e.has_zip64_extra, ( + f"entry {e.name!r} is {e.uncompressed_size} bytes, at or above " + f"2**32, but its 32-bit size field holds " + f"{e.raw_uncompressed_size} rather than the ZIP64 sentinel\n" + + describe(entries) + ) + if e.compressed_size >= ZIP64_WINDOW_HIGH: + assert e.raw_compressed_size == ZIP64_SENTINEL_32 and e.has_zip64_extra, ( + f"entry {e.name!r} is {e.compressed_size} bytes compressed, at " + f"or above 2**32, but its 32-bit compressed-size field holds " + f"{e.raw_compressed_size} rather than the ZIP64 sentinel\n" + + describe(entries) + ) + + +def entries_in_window( + entries: list[CentralDirectoryEntry], +) -> list[CentralDirectoryEntry]: + """Entries with an offset or size in ``[2**31, 2**32)``. + + These are the records a sign-extending reader mishandles. Both encodings + -- a real unsigned 32-bit value, or the ZIP64 sentinel -- are legal here, + which is why this returns them for reporting rather than asserting on + which one the writer chose. + """ + return [ + e + for e in entries + if in_zip64_window(e.local_header_offset) + or in_zip64_window(e.uncompressed_size) + ] + + +def entries_with_raw_values_in_window( + entries: list[CentralDirectoryEntry], +) -> list[CentralDirectoryEntry]: + """Entries that exercise unsigned reads of real 32-bit values in the window. + + ``0xffffffff`` is numerically in the window, but it is a sentinel directing + the reader to the ZIP64 extra field. It therefore does not exercise the + signed 32-bit read defect this predicate identifies. + """ + return [ + e + for e in entries + if any( + value != ZIP64_SENTINEL_32 and in_zip64_window(value) + for value in ( + e.raw_compressed_size, + e.raw_uncompressed_size, + e.raw_local_header_offset, + ) + ) + ]