diff --git a/.github/workflows/check.yml b/.github/workflows/check.yml index 7e42f1435..37b6339e5 100644 --- a/.github/workflows/check.yml +++ b/.github/workflows/check.yml @@ -34,18 +34,26 @@ jobs: uv run ruff format --check . uv run pyright working-directory: xtest - # The benchmark harness's own tests: statistics, measurement, and the - # CLI command builders. No platform and no SDK builds required, so the - # part of the gate that has to be *correct* is checked on every PR - # rather than only when the nightly benchmark runs. + # Offline tests for the harnesses whose own correctness gates a nightly + # job: the benchmark statistics, measurement and CLI command builders, + # 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 # runner that already has everything installed from the step above. - - name: Test xtest benchmark harness + - name: Test xtest offline harnesses run: >- 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_bench_arms.py test_sdk_commands.py test_encryption_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 5488e9274..ad2ef5eaf 100644 --- a/.github/workflows/xtest.yml +++ b/.github/workflows/xtest.yml @@ -38,6 +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 (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: @@ -68,6 +78,14 @@ on: required: false type: boolean default: false + run-zip64: + required: false + type: boolean + default: false + force-supports: + required: false + type: string + default: "" schedule: - cron: "30 6 * * *" # 0630 UTC - cron: "0 5 * * 1,3" # 500 UTC (Monday, Wednesday) @@ -77,6 +95,13 @@ concurrency: group: ${{ github.workflow }}-pr-${{ github.event.pull_request.number || github.run_id }} cancel-in-progress: true +# Workflow-scoped so every pytest step inherits it, rather than repeating the +# same env line in each of them and having the next new job silently miss it. +# Empty on pull_request and schedule (no inputs), which xtest reads as "force +# nothing" -- so the PR gate and the nightlies are unaffected. +env: + XT_FORCE_SUPPORTS: ${{ inputs.force-supports }} + jobs: resolve-versions: timeout-minutes: 10 @@ -85,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 }} @@ -185,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 = []; @@ -808,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 }} @@ -930,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 @@ -1007,6 +1054,336 @@ 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 one cached 2.1 GiB ciphertext, 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/AGENTS.md b/AGENTS.md index cfca4cde6..ffa864447 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -56,6 +56,34 @@ See `xtest/AGENTS.md` for the full table of `--sdks`, `--containers`, - `OT_ROOT_KEY` — root key for key-management tests - `SCHEMA_FILE` — path to manifest schema file - `DISABLE_AUDIT_ASSERTIONS` — set to `1`/`true`/`yes` to skip audit-log assertions (CI equivalent of `--no-audit-logs`) +- `XT_TMP_DIR` — root for generated fixtures and ciphertexts (default `tmp/`). + Point it at a large volume for multi-GiB runs. +- `XT_FORCE_SUPPORTS` — comma-separated feature names to treat as supported + regardless of what each SDK's `cli.sh supports` reports. See below. + +### Evaluating an unreleased fix: `XT_FORCE_SUPPORTS` + +`SDK.supports(feature)` answers from the `supports` case statements in +`xtest/sdk/{go,java,js}/cli.sh` — **in this repo, not in the SDK repos**. Most +cases are version gates, so a build from an unmerged branch reports the last +*released* version and answers "no" for precisely the fix you are trying to +evaluate. The cell then skips and the run is green without having tested +anything. + +`XT_FORCE_SUPPORTS` short-circuits that: + +```bash +otdf-sdk-mgr install tip --ref pr:396 java # pr:N works on install +XT_FORCE_SUPPORTS=chunky uv run pytest test_tdfs.py --sdks "js java" -v +``` + +It applies to every SDK in the run — to force one side only, narrow with +`--sdks-encrypt` / `--sdks-decrypt`. An unrecognised feature name raises rather +than being ignored, since a silently-ignored typo is indistinguishable from a +clean run. In CI, pass `force-supports` to the `X-Test` workflow dispatch. + +Note `versions resolve` (which backs the workflow's `*-ref` inputs) does **not** +accept the `pr:N` shorthand — pass a branch name there instead. ### Audit Log Assertions diff --git a/spec/DSPX-4592.md b/spec/DSPX-4592.md new file mode 100644 index 000000000..9db9efdf3 --- /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 04f02dc12..5997f8096 100644 --- a/xtest/AGENTS.md +++ b/xtest/AGENTS.md @@ -27,6 +27,16 @@ 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, `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 + +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 `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 5604bf0c1..e20b473dd 100644 --- a/xtest/conftest.py +++ b/xtest/conftest.py @@ -13,15 +13,19 @@ - fixtures.keys: Key management fixtures """ +import argparse import json import logging import os +import random import typing +import warnings from pathlib import Path from typing import cast import pytest +import sizes import tdfs from otdfctl import OpentdfCommandLineTool from perf import report, stats @@ -66,7 +70,7 @@ def pytest_report_header() -> list[str]: ] -def englist(s: tuple[str]) -> str: +def englist(s: tuple[str, ...]) -> str: """Convert tuple of strings to English list format (e.g., 'a, b, or c').""" if len(s) > 1: return ", ".join(s[:-1]) + ", or " + s[-1] @@ -103,6 +107,63 @@ def sdk_spec_type(v: str) -> str: return v +def sizes_opt_type(v: str) -> list[str]: + """Validate and de-duplicate a comma-separated list of size names. + + ``ArgumentTypeError`` rather than ``ValueError``: argparse prints the + former's message verbatim and replaces the latter's with a generic + "invalid value", which would hide the list of names that would have + worked. + """ + names = [s.strip() for s in v.split(",") if s.strip()] + if not names: + raise argparse.ArgumentTypeError("at least one size is required") + for name in names: + if name not in sizes.SIZES: + raise argparse.ArgumentTypeError( + f"unknown size {name!r}; expected one or more of " + f"{', '.join(sizes.SIZE_ORDER)}" + ) + # Cheapest first, so a fan-out run reports its fast cells before spending + # minutes on a multi-GiB one. + return [n for n in sizes.SIZE_ORDER if n in set(names)] + + +_SIZES_KEY = pytest.StashKey[list[str]]() + + +def resolve_sizes(config: pytest.Config) -> list[str]: + """Size names this session runs, honouring the deprecated --large alias. + + Cached on the config: this is called from both the parametrizer and the + collection filter, and the deprecation warning below should be emitted + once per session rather than once per caller. + """ + cached = config.stash.get(_SIZES_KEY, None) + if cached is not None: + return cached + + selected = cast(list[str] | None, config.getoption("--sizes")) + if config.getoption("--large"): + if selected is not None: + raise pytest.UsageError( + "--large and --sizes are mutually exclusive; --large is the " + "deprecated spelling of --sizes small,large" + ) + warnings.warn( + "--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, + ) + resolved = ["small", "large"] + else: + resolved = selected if selected is not None else ["small"] + + config.stash[_SIZES_KEY] = resolved + return resolved + + def pytest_addoption(parser: pytest.Parser): """Add custom CLI options for pytest.""" parser.addoption( @@ -128,7 +189,16 @@ def pytest_addoption(parser: pytest.Parser): parser.addoption( "--large", action="store_true", - help="generate a large (greater than 4 GiB) file for testing", + help="deprecated alias for --sizes small,large", + ) + parser.addoption( + "--sizes", + type=sizes_opt_type, + help="comma-separated plaintext sizes to run against, from " + f"{englist(tuple(sizes.SIZE_ORDER))} " + f"({', '.join(f'{k}={sizes.SIZES[k]}B' for k in sizes.SIZE_ORDER)}); " + "default small. Listing more than one fans out every test that takes " + "a plaintext file, so CI passes exactly one.", ) parser.addoption( "--no-audit-logs", @@ -245,11 +315,7 @@ def pytest_generate_tests(metafunc: pytest.Metafunc): - container: which container formats to test (ztdf, ztdf-ecwrap) """ if "size" in metafunc.fixturenames: - metafunc.parametrize( - "size", - ["large" if metafunc.config.getoption("large") else "small"], - scope="session", - ) + metafunc.parametrize("size", resolve_sizes(metafunc.config), scope="session") def list_opt(name: str, t: typing.Any) -> list[str]: ttt = typing.get_args(t) @@ -363,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): @@ -447,23 +541,99 @@ def pytest_runtest_setup(item: pytest.Item): # Core fixtures + +#: Chunk written per iteration by :func:`_write_bulk_plaintext`. +_BULK_BLOCK = 1 << 20 + +#: Sizes at or above this are generated in bulk rather than line by line. +#: The line generator formats one string per 16 bytes, which is fine for 128 +#: bytes and is ~140 million iterations at 2.1 GiB. +_BULK_THRESHOLD = 1 << 24 + + +def _write_line_plaintext(path: Path, length: int) -> None: + """The original generator: one right-aligned offset per 16 bytes. + + Kept byte-for-byte for the small size. Existing tests compare decrypted + output against this content, and there is nothing to gain from churning + it. + """ + with path.open("w") as f: + for i in range(0, length, 16): + f.write(f"{i:15,d}\n") + + +def _write_bulk_plaintext(path: Path, length: int) -> None: + """Write ``length`` deterministic, poorly-compressible bytes, quickly. + + One pseudorandom block is built once and written repeatedly, with a + block counter patched into its first eight bytes so the content is + position-dependent rather than a flat repeat. + + Repetition at a 1 MiB period is not something DEFLATE can exploit -- its + window is 32 KiB -- so the payload stays realistically incompressible + while costing one ``randbytes`` call instead of one per megabyte. + + Deliberately not ``rng.randbytes(length)`` the way ``fixtures/bench.py`` + does it: that materialises the whole payload in memory, which is fine at + 32 MiB and fatal at 2.1 GiB. + """ + block = bytearray(random.Random("dspx-4592").randbytes(_BULK_BLOCK)) + view = memoryview(block) + with path.open("wb") as f: + written = 0 + while written < length: + n = min(_BULK_BLOCK, length - written) + block[:8] = (written // _BULK_BLOCK).to_bytes(8, "big") + f.write(view[:n]) + written += n + + +def _plaintext_of(tmp_dir: Path, size: str) -> Path: + """Return a plaintext file of the named size, generating it if needed.""" + length = sizes.SIZES[size] + pt_file = tmp_dir / f"test-plain-{size}.txt" + # tmp_dir persists between runs, so a multi-GiB payload that is already + # there and the right length is reused rather than rewritten. Checking + # the length matters: a run killed mid-generation leaves a short file, + # and silently encrypting that would test the wrong size. + if pt_file.is_file() and pt_file.stat().st_size == length: + return pt_file + if length >= _BULK_THRESHOLD: + _write_bulk_plaintext(pt_file, length) + else: + _write_line_plaintext(pt_file, length) + return pt_file + + @pytest.fixture(scope="session") def pt_file(tmp_dir: Path, size: str) -> Path: - """Generate a plaintext test file. + """Generate a plaintext test file of the named size. Args: tmp_dir: Temporary directory for test files - size: 'large' (>4 GiB) or 'small' (128 bytes) + size: a key of :data:`sizes.SIZES` -- 'small' (128 bytes), + '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 """ - pt_file = tmp_dir / f"test-plain-{size}.txt" - length = (5 * 2**30) if size == "large" else 128 - with pt_file.open("w") as f: - for i in range(0, length, 16): - f.write(f"{i:15,d}\n") - return pt_file + return _plaintext_of(tmp_dir, size) + + +@pytest.fixture(scope="session") +def chunky_pt_file(tmp_dir: Path) -> Path: + """A 5 MiB plaintext: several segments, every one of them default-sized. + + Independent of ``--sizes`` on purpose. Adding 'chunky' to the session's + sizes would fan out every test that takes :func:`pt_file` -- the whole of + test_tdfs.py and test_policytypes.py -- to pay for a property one test + needs. A separate fixture buys the coverage for one extra encrypt and + decrypt of 5 MiB, which is cheap enough for the PR gate. + """ + return _plaintext_of(tmp_dir, "chunky") @pytest.fixture(scope="session") @@ -472,9 +642,14 @@ def tmp_dir(request: pytest.FixtureRequest) -> Path: When running with pytest-xdist, each worker gets its own subdirectory to prevent file collisions between parallel test processes. + + ``XT_TMP_DIR`` relocates the root. Multi-GiB roundtrips need more space + than a CI runner's workspace volume has, and the alternative to an + override is hard-coding a runner-specific path here. """ worker_id = getattr(request.config, "workerinput", {}).get("workerid", "master") - dname = Path(f"tmp/{worker_id}/") + root = Path(os.environ.get("XT_TMP_DIR", "tmp")) + dname = root / worker_id dname.mkdir(parents=True, exist_ok=True) return dname diff --git a/xtest/fixtures/encryption.py b/xtest/fixtures/encryption.py index 2d4d12984..a4de995ed 100644 --- a/xtest/fixtures/encryption.py +++ b/xtest/fixtures/encryption.py @@ -42,13 +42,23 @@ def __call__( mime_type: str = "text/plain", ) -> Path: attr_key = tuple(attr_values) if attr_values is not None else None - key = (str(encrypt_sdk), container, target_mode, attr_key, az, mime_type) + plaintext = self._pt_file.resolve() + key = ( + plaintext, + str(encrypt_sdk), + container, + target_mode, + attr_key, + az, + mime_type, + ) cached = self._cache.get(key) if cached is not None: return cached digest = hashlib.sha1(repr(key).encode()).hexdigest()[:8] ct_file = ( - self._tmp_dir / f"ct-{self._label}-{encrypt_sdk}-{container}-{digest}.tdf" + self._tmp_dir + / f"ct-{self._label}-{self._pt_file.stem}-{encrypt_sdk}-{container}-{digest}.tdf" ) encrypt_sdk.encrypt( self._pt_file, @@ -95,3 +105,15 @@ def encrypted_tdf( """ label = request.node.originalname or request.node.name return EncryptFactory(label, pt_file, tmp_dir, _encryption_cache) + + +@pytest.fixture +def chunky_tdf( + request: pytest.FixtureRequest, + chunky_pt_file: Path, + tmp_dir: Path, + _encryption_cache: dict[tuple, Path], +) -> EncryptFactory: + """An :class:`EncryptFactory` bound to the 5 MiB multi-segment plaintext.""" + label = request.node.originalname or request.node.name + return EncryptFactory(label, chunky_pt_file, tmp_dir, _encryption_cache) diff --git a/xtest/pyproject.toml b/xtest/pyproject.toml index 182e5e55c..d2f135022 100644 --- a/xtest/pyproject.toml +++ b/xtest/pyproject.toml @@ -81,7 +81,16 @@ ignore = [ ] [tool.ruff.lint.isort] -known-first-party = ["abac", "tdfs", "otdfctl", "assertions", "fixtures", "perf"] +known-first-party = [ + "abac", + "tdfs", + "otdfctl", + "assertions", + "fixtures", + "perf", + "sizes", + "zipinspect", +] [tool.ruff.format] quote-style = "double" @@ -105,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/sdk/go/cli.sh b/xtest/sdk/go/cli.sh index 91195bf05..2b2a86381 100755 --- a/xtest/sdk/go/cli.sh +++ b/xtest/sdk/go/cli.sh @@ -121,6 +121,17 @@ if [ "$1" == "supports" ]; then index($f)' exit $? ;; + chunky) + # Default an absent per-segment segmentSize/encryptedSegmentSize from + # the manifest-level defaults. Broken in every go build to date: a + # web-sdk TDF over 1 MiB fails in the GMAC check. Fix tracked as + # DSPX-4590; turn this into a version gate when it releases. + # + # Explicit rather than falling through to "Unknown feature" so that a + # typo'd feature name in tdfs.py cannot pass for a known-missing one. + echo "chunky unsupported: see DSPX-4590" + exit 1 + ;; *) echo "Unknown feature: $2" exit 2 diff --git a/xtest/sdk/java/cli.sh b/xtest/sdk/java/cli.sh index 79cb17d8e..afdca2681 100755 --- a/xtest/sdk/java/cli.sh +++ b/xtest/sdk/java/cli.sh @@ -149,6 +149,17 @@ if [ "$1" == "supports" ]; then java -jar "$SCRIPT_DIR"/cmdline.jar supports dpop_nonce_challenge exit $? ;; + chunky) + # Default an absent per-segment segmentSize/encryptedSegmentSize from + # the manifest-level defaults. Broken in every java build to date: a + # web-sdk TDF over 1 MiB fails in the GMAC check. Fix tracked as + # DSPX-4589; turn this into a version gate when it releases. + # + # Explicit rather than falling through to "Unknown feature" so that a + # typo'd feature name in tdfs.py cannot pass for a known-missing one. + echo "chunky unsupported: see DSPX-4589" + exit 1 + ;; *) echo "Unknown feature: $2" exit 2 diff --git a/xtest/sdk/js/cli.sh b/xtest/sdk/js/cli.sh index 9c0727d7f..f805e9678 100755 --- a/xtest/sdk/js/cli.sh +++ b/xtest/sdk/js/cli.sh @@ -120,6 +120,12 @@ if [[ "$1" == "supports" ]]; then npx $CTL help | grep -iE -- '--dpop-key|--dpopKey' exit $? ;; + chunky) + # web-sdk omits a segment's sizes when they equal the manifest defaults + # and defaults them back on read. Both halves predate any version we + # test. See DSPX-4591. + exit 0 + ;; *) echo "Unknown feature: $2" exit 2 diff --git a/xtest/sizes.py b/xtest/sizes.py new file mode 100644 index 000000000..6f44551a7 --- /dev/null +++ b/xtest/sizes.py @@ -0,0 +1,87 @@ +"""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 + +#: 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 +#: ``segmentSize``/``encryptedSegmentSize`` for. A payload smaller than one +#: default segment produces a single *partial* segment, whose size is written +#: out explicitly, and every reader copes -- which is the only reason a +#: 128-byte suite stayed green through four years of this bug. +#: +#: Defaults differ: web-sdk 1 MiB, go and java ~2 MiB. 5 MiB clears twice the +#: largest of them with room to spare. 2 MiB would only do it for web-sdk. +CHUNKY_BYTES = 5 * 2**20 + +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", "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 ee0994656..c16abc721 100644 --- a/xtest/tdfs.py +++ b/xtest/tdfs.py @@ -122,6 +122,20 @@ def is_sdk_type(val: str) -> TypeIs[sdk_type]: "autoconfigure", "better-messages-2024", "bulk_rewrap", + # Reader-side: default a segment's absent ``segmentSize`` / + # ``encryptedSegmentSize`` from the manifest-level + # ``segmentSizeDefault`` / ``encryptedSegmentSizeDefault``. + # + # Those per-segment fields are optional overrides -- ``segments/items`` in + # manifest.schema.json has no ``required`` list, while the two defaults are + # required on ``integrityInformation`` -- and web-sdk omits them whenever a + # segment is exactly default-sized. go and java deserialize them into + # primitive integers, so absent reads as 0, and the reader then hashes an + # empty buffer and dies inside the GMAC check. See DSPX-4589 finding 4 and + # DSPX-4590 finding 7. + # + # Only observable above one default segment; hence sizes.CHUNKY_BYTES. + "chunky", "connectrpc", # DPoP (RFC 9449): sender-constrained access tokens. SDK signs a DPoP proof # JWT per request; KAS validates the proof and binds the access token to @@ -157,6 +171,48 @@ def is_sdk_type(val: str) -> TypeIs[sdk_type]: "obligations", ] + +def _parse_forced_supports(raw: str) -> frozenset[str]: + """Parse ``XT_FORCE_SUPPORTS`` into a set of feature names. + + An unrecognised name is a hard error rather than a no-op. The override + exists to turn a skip into a real result, so a typo that quietly left the + skip in place would be indistinguishable from a clean run -- which is the + exact failure mode the override is meant to escape. + """ + names = {n.strip() for n in raw.split(",") if n.strip()} + known = set(get_args(feature_type)) + unknown = names - known + if unknown: + raise ValueError( + f"XT_FORCE_SUPPORTS names unknown feature(s) {sorted(unknown)}; " + f"valid features are {sorted(known)}" + ) + return frozenset(names) + + +#: Features to treat as supported no matter what the SDK reports. +#: +#: The ``supports`` case statements live in this repo (``sdk/*/cli.sh``) and +#: answer from a *released* version number, so they say "no" for precisely the +#: unreleased builds a fix needs to be evaluated against. Setting +#: ``XT_FORCE_SUPPORTS=chunky`` alongside ``otdf-sdk-mgr install tip --ref ...`` +#: makes those cells run for real and report pass or fail. +#: +#: Applies to every SDK in the run. To force a feature for one side only, narrow +#: the run with ``--sdks-encrypt`` / ``--sdks-decrypt`` rather than adding +#: per-SDK syntax here. +FORCED_SUPPORTS = _parse_forced_supports(os.environ.get("XT_FORCE_SUPPORTS", "")) + +if FORCED_SUPPORTS: + logger.warning( + "XT_FORCE_SUPPORTS is set: treating %s as supported by every SDK. " + "Results for those features reflect the build under test, not the " + "shim's version gate.", + ", ".join(sorted(FORCED_SUPPORTS)), + ) + + container_version = Literal["4.2.2", "4.3.0"] policy_type = Literal["plaintext", "encrypted"] @@ -693,6 +749,8 @@ def decrypt( ) def supports(self, feature: feature_type) -> bool: + if feature in FORCED_SUPPORTS: + return True if feature in self._supports: return self._supports[feature] self._supports[feature] = self._uncached_supports(feature) @@ -714,6 +772,22 @@ def _uncached_supports(self, feature: feature_type) -> bool: # go/java reconstruct by skipping already-satisfied splits, so a # duplicate KAS on the same split has always decrypted. return True + case ("chunky", "js"): + # web-sdk is the SDK that omits the sizes, and it defaults them + # on the way back in (lib/tdf3/src/tdf.ts destructures + # `segmentSize = segmentSizeDefault`). Both halves have been + # there since it started omitting them in 2022, so js->js has + # always round-tripped at multi-segment sizes. + # + # go and java answer this from their own `cli.sh supports` + # case statement, which is in this repo, not theirs -- see + # sdk/{go,java}/cli.sh. Both say no today. Flipping one is a + # deliberate edit *here* once the fix releases; it does not + # happen by itself when the SDK merges a patch. + # + # To evaluate a fix before it releases, set + # XT_FORCE_SUPPORTS=chunky -- see FORCED_SUPPORTS above. + return True case ("better-messages-2024", ("js" | "java")): return True case ("ns_grants", ("go" | "java")): @@ -789,6 +863,88 @@ def skip_connectrpc_skew(encrypt_sdk: SDK, decrypt_sdk: SDK, pfs: PlatformFeatur return False +def elides_segment_sizes(ct_file: Path) -> bool: + """True if any segment leaves its size to the manifest-level default. + + Asked of the container rather than of the writer that produced it. Only + web-sdk omits these today, but "which SDK omits them" is a fact with a + version axis and a fix pending on two others, and reading the manifest + costs one central-directory seek. If go or java ever start eliding, the + gate below picks it up with no edit here. + """ + integrity = manifest(ct_file).encryptionInformation.integrityInformation + return any( + s.segmentSize is None or s.encryptedSegmentSize is None + for s in integrity.segments + ) + + +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 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 + lives in this repo, so a fix merging in java-sdk or platform does not flip + it. The acceptance criteria on DSPX-4589 and DSPX-4590 both name this cell + for that reason. + + To evaluate an unreleased fix, set ``XT_FORCE_SUPPORTS=chunky`` (or pass + ``force-supports: chunky`` to the workflow dispatch) so this returns early + and the cell reports a real pass or fail. See :data:`FORCED_SUPPORTS`. + """ + if decrypt_sdk.supports("chunky"): + return + if not elides_segment_sizes(ct_file): + return + pytest.skip( + f"{decrypt_sdk} sdk doesn't yet support [chunky]: {ct_file.name} omits " + "per-segment sizes, which this reader cannot default from the manifest" + ) + + +#: 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_encryption_units.py b/xtest/test_encryption_units.py new file mode 100644 index 000000000..1edf722a0 --- /dev/null +++ b/xtest/test_encryption_units.py @@ -0,0 +1,55 @@ +"""Offline tests for the memoized encryption fixture.""" + +from pathlib import Path +from typing import Any, cast + +import tdfs +from fixtures.encryption import EncryptFactory + + +class RecordingSDK: + """Small duck-typed SDK that records which plaintexts were encrypted.""" + + def __init__(self) -> None: + self.inputs: list[Path] = [] + + def __str__(self) -> str: + return "fake@main" + + def encrypt(self, pt_file: Path, ct_file: Path, **_: Any) -> None: + self.inputs.append(pt_file) + ct_file.write_bytes(pt_file.read_bytes()) + + +def test_cache_distinguishes_plaintexts(tmp_path: Path): + small = tmp_path / "test-plain-small.txt" + medium = tmp_path / "test-plain-medium.txt" + small.write_bytes(b"small") + medium.write_bytes(b"medium") + cache: dict[tuple, Path] = {} + sdk_impl = RecordingSDK() + sdk = cast(tdfs.SDK, sdk_impl) + + small_ct = EncryptFactory("roundtrip", small, tmp_path, cache)(sdk) + medium_ct = EncryptFactory("roundtrip", medium, tmp_path, cache)(sdk) + + assert sdk_impl.inputs == [small, medium] + assert small_ct != medium_ct + assert "test-plain-small" in small_ct.name + assert "test-plain-medium" in medium_ct.name + assert small_ct.read_bytes() == b"small" + assert medium_ct.read_bytes() == b"medium" + + +def test_cache_still_shares_identical_plaintext_and_parameters(tmp_path: Path): + plaintext = tmp_path / "test-plain-small.txt" + plaintext.write_bytes(b"same input") + cache: dict[tuple, Path] = {} + sdk_impl = RecordingSDK() + sdk = cast(tdfs.SDK, sdk_impl) + + first = EncryptFactory("first-test", plaintext, tmp_path, cache)(sdk) + second = EncryptFactory("second-test", plaintext, tmp_path, cache)(sdk) + + assert first == second + assert sdk_impl.inputs == [plaintext] diff --git a/xtest/test_tdfs.py b/xtest/test_tdfs.py index 32d545425..d06c0557c 100644 --- a/xtest/test_tdfs.py +++ b/xtest/test_tdfs.py @@ -108,6 +108,55 @@ def test_tdf_roundtrip( audit_logs.assert_rewrap_success(min_count=1, since_mark=ec_mark) +def test_chunky_roundtrip( + encrypt_sdk: tdfs.SDK, + decrypt_sdk: tdfs.SDK, + chunky_pt_file: Path, + in_focus: set[tdfs.SDK], + attribute_default_rsa: Attribute, + chunky_tdf: EncryptFactory, +): + """Round-trip a payload spanning several default-sized segments. + + Distinct from :func:`test_tdf_roundtrip` in exactly one respect: 5 MiB + instead of 128 bytes, which is enough for a writer to emit a segment whose + size equals the manifest default. web-sdk omits ``segmentSize`` and + ``encryptedSegmentSize`` for such a segment -- legally, they are optional + overrides -- and go and java cannot currently default them back, so + js->go and js->java fail here while every other pair passes. + + Four years of a 128-byte suite never produced a full-sized segment, which + is why the bug survived. See DSPX-4589 finding 4 and DSPX-4590 finding 7. + """ + if not in_focus & {encrypt_sdk, decrypt_sdk}: + pytest.skip("Not in focus") + tdfs.skip_hexless_skew(encrypt_sdk, decrypt_sdk) + + ct_file = chunky_tdf( + encrypt_sdk, + target_mode=tdfs.select_target_version(encrypt_sdk, decrypt_sdk), + attr_values=attribute_default_rsa.value_fqns, + ) + + integrity = tdfs.manifest(ct_file).encryptionInformation.integrityInformation + # The load-bearing precondition. If the writer emitted one segment the + # payload was not chunky, and the rest of this passes without exercising + # anything -- the same vacuous green that hid the defect in the first + # place, so it is an assertion rather than a skip. + assert len(integrity.segments) > 1, ( + f"{encrypt_sdk} wrote {len(integrity.segments)} segment(s) from a " + f"{chunky_pt_file.stat().st_size}-byte payload " + f"(segmentSizeDefault={integrity.segmentSizeDefault}); sizes.CHUNKY_BYTES " + "is no longer larger than this SDK's default segment" + ) + + tdfs.skip_chunky_skew(ct_file, decrypt_sdk) + + rt_file = chunky_tdf.rt_file(ct_file, decrypt_sdk) + decrypt_sdk.decrypt(ct_file, rt_file, "ztdf") + assert filecmp.cmp(chunky_pt_file, rt_file, shallow=False) + + def test_tdf_spec_target_422( encrypt_sdk: tdfs.SDK, decrypt_sdk: tdfs.SDK, diff --git a/xtest/test_zip64.py b/xtest/test_zip64.py new file mode 100644 index 000000000..2dd51c37b --- /dev/null +++ b/xtest/test_zip64.py @@ -0,0 +1,141 @@ +"""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, + 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 000000000..8453b16db --- /dev/null +++ b/xtest/test_zip64_units.py @@ -0,0 +1,424 @@ +"""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 ( + CHUNKY_BYTES, + MEDIUM_BYTES, + SIZES, + ZIP64_WINDOW_HIGH, + ZIP64_WINDOW_LOW, + exercises_zip64_window, + in_zip64_window, +) +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, +) -> 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(" 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["small"] < ZIP64_WINDOW_LOW + assert SIZES["large"] >= ZIP64_WINDOW_HIGH + assert not in_zip64_window(SIZES["small"]) + assert not in_zip64_window(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 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. + + Segment defaults observed in the live 2.1 GiB run: web-sdk 1 MiB, go + and java ~2 MiB. Two full default segments from the largest of those + is 4 MiB, so anything at or below that tests nothing for go and java. + The runtime counterpart is the ``len(segments) > 1`` assertion in + test_tdfs.py::test_chunky_roundtrip, which catches a default this + constant has not been told about. + """ + largest_known_default = 2 * 2**20 + assert 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 CHUNKY_BYTES < 64 * 2**20 + assert not in_zip64_window(CHUNKY_BYTES) + + +class TestZip64Selection: + @pytest.mark.parametrize( + ("item_size", "expected"), [("small", False), ("medium", True)] + ) + def test_mixed_session_uses_each_items_size(self, item_size: str, expected: bool): + item = cast( + pytest.Item, + SimpleNamespace(callspec=SimpleNamespace(params={"size": item_size})), + ) + + assert ( + conftest._item_exercises_zip64_window(item, ["small", "medium"]) is expected + ) + + def test_item_without_size_uses_session_selection(self): + item = cast( + pytest.Item, + SimpleNamespace(callspec=SimpleNamespace(params={"container": "ztdf"})), + ) + + assert conftest._item_exercises_zip64_window(item, ["small", "medium"]) + assert not conftest._item_exercises_zip64_window(item, ["small"]) + + +# --- zipinspect.py ----------------------------------------------------------- + + +class TestCentralDirectory: + def test_reads_a_real_zip(self, tmp_path: Path): + """Agreement with zipfile on an ordinary container, as a sanity floor.""" + p = tmp_path / "ordinary.zip" + with zipfile.ZipFile(p, "w") as z: + z.writestr("0.payload", b"a" * 4096) + z.writestr("0.manifest.json", b"{}") + + entries = zipinspect.central_directory(p) + assert [e.name for e in entries] == ["0.payload", "0.manifest.json"] + with zipfile.ZipFile(p) as z: + expected = {i.filename: i.header_offset for i in z.infolist()} + assert {e.name: e.local_header_offset for e in entries} == expected + + def test_local_header_zip64_is_not_mistaken_for_central_directory_zip64( + self, tmp_path: Path + ): + """``force_zip64`` is a local-header decision and must not be read as a CD one. + + The two are independent: a writer can emit the ZIP64 extra field in + the local header while the central directory's values still fit in 32 + bits, which is exactly what this produces. Reporting + ``has_zip64_extra`` for it would make the conformance assertions think + a writer had opted into ZIP64 for a field it had not. + """ + p = tmp_path / "z64-local.zip" + with zipfile.ZipFile(p, "w") as z: + with z.open("0.payload", "w", force_zip64=True) as f: + f.write(b"b" * 8192) + + (entry,) = zipinspect.central_directory(p) + assert entry.uncompressed_size == 8192 + assert not entry.has_zip64_extra + assert not entry.uses_zip64_for_sizes + + def test_reads_a_zip64_end_of_central_directory(self, tmp_path: Path): + """When the EOCD holds sentinels, the real values come from the ZIP64 EOCD. + + A container whose central directory starts past 4 GiB -- which the + 'large' size produces -- can only be located this way, so the branch + is on the path for the very sizes this module exists to cover. + """ + records = [ + cen_record("0.payload", raw_offset=0), + cen_record("0.manifest.json", raw_offset=MEDIUM_BYTES), + ] + p = synth_zip64_eocd(tmp_path / "z64-eocd.zip", records) + entries = zipinspect.central_directory(p) + assert [e.name for e in entries] == ["0.payload", "0.manifest.json"] + assert entries[1].raw_local_header_offset == MEDIUM_BYTES + + def test_rejects_a_locator_pointing_at_nothing(self, tmp_path: Path): + p = synth_zip64_eocd( + tmp_path / "bad-locator.zip", + [cen_record("0.payload", raw_offset=0)], + eocd64_offset_override=1, + ) + with pytest.raises(MalformedZipError, match="zip64 locator"): + zipinspect.central_directory(p) + + def test_raw_value_in_the_window_is_preserved(self, tmp_path: Path): + """A 32-bit field holding a real 2.1 GiB value must not be normalised away. + + This is the go-writer shape: legal APPNOTE, and the input that a + sign-extending reader mishandles. If the parser resolved it through + the ZIP64 path the test would lose the ability to tell the two + encodings apart. + """ + p = synth_zip( + tmp_path / "window.zip", + [ + cen_record("0.payload", raw_offset=0, raw_usize=MEDIUM_BYTES), + cen_record("0.manifest.json", raw_offset=MEDIUM_BYTES + 64), + ], + ) + entries = zipinspect.central_directory(p) + manifest = entries[1] + assert manifest.raw_local_header_offset == MEDIUM_BYTES + 64 + assert manifest.local_header_offset == MEDIUM_BYTES + 64 + assert not manifest.has_zip64_extra + assert not manifest.uses_zip64_for_offset + + def test_signed_read_of_a_windowed_offset_goes_negative(self, tmp_path: Path): + """The defect itself, reproduced arithmetically. + + java-sdk's pre-#393 ``readInt()`` widens this field with a signed + read. Anything at or above 2**31 comes back negative and the + subsequent seek fails or lands on nonsense. + """ + p = synth_zip( + tmp_path / "signed.zip", + [cen_record("0.manifest.json", raw_offset=MEDIUM_BYTES)], + ) + (entry,) = zipinspect.central_directory(p) + assert entry.signed_read_of_offset() < 0 + assert entry.signed_read_of_offset() == MEDIUM_BYTES - ZIP64_WINDOW_HIGH + + def test_signed_read_is_harmless_below_the_window(self, tmp_path: Path): + """Below 2**31 the two reads agree, which is why smaller payloads miss this.""" + offset = ZIP64_WINDOW_LOW - 1 + p = synth_zip( + tmp_path / "safe.zip", [cen_record("0.manifest.json", raw_offset=offset)] + ) + (entry,) = zipinspect.central_directory(p) + assert entry.signed_read_of_offset() == offset + + def test_sentinel_resolves_through_the_extra_field(self, tmp_path: Path): + """The web-sdk shape: always ZIP64, so the 32-bit field is 0xFFFFFFFF.""" + true_offset = 6 * 2**30 + p = synth_zip( + tmp_path / "sentinel.zip", + [ + cen_record( + "0.manifest.json", + raw_offset=ZIP64_SENTINEL_32, + zip64_offset=true_offset, + ) + ], + ) + (entry,) = zipinspect.central_directory(p) + assert entry.local_header_offset == true_offset + assert entry.uses_zip64_for_offset + assert entry.has_zip64_extra + + def test_rejects_a_file_with_no_eocd(self, tmp_path: Path): + p = tmp_path / "junk.bin" + p.write_bytes(b"not a zip at all") + with pytest.raises(MalformedZipError): + zipinspect.central_directory(p) + + +class TestConformanceAssertions: + def test_above_4gib_without_the_sentinel_fails(self, tmp_path: Path): + """A 32-bit field cannot hold this value, so omitting the sentinel is a defect.""" + p = synth_zip( + tmp_path / "bad.zip", + [ + cen_record( + "0.manifest.json", + raw_offset=ZIP64_SENTINEL_32, + zip64_offset=5 * 2**30, + ) + ], + ) + entries = zipinspect.central_directory(p) + # Rewrite the entry to claim a >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_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 000000000..03811db80 --- /dev/null +++ b/xtest/zipinspect.py @@ -0,0 +1,320 @@ +"""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} {'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"{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.uses_zip64_for_sizes 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) + ) + + +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, + ) + ) + ]