fix(sdk): zip64/APPNOTE conformance in the TDF3 zip reader and writer (DSPX-4591)

[dmihalcik-virtru]

https://virtru.atlassian.net/browse/DSPX-4591

Audit of java-sdk #393 against the Go
and Web zip implementations turned up four ZIP64/APPNOTE conformance divergences in
web-sdk. All four are addressed here.

Note on the writer's ZIP64 threshold: there isn't one, and that's deliberate.
ZipWriter sets this.zip64 = true in its constructor and never clears it, so
web-sdk unconditionally emits ZIP64 structures. That is already the safest posture
and is unchanged by this PR. The siblings (DSPX-4589 java-sdk, DSPX-4590 go-sdk) are
the ones moving thresholds.

Finding 1 — the reader never parsed the end of central directory record

getCDBuffers() scanned the last 1000 bytes of the file backwards for anything
matching the central directory signature 0x02014b50 and treated every hit as an
entry, inferring record boundaries from the position of the next hit.

Replaced with real end-of-central-directory parsing:

• ZipReader.getEndOfCentralDirectory() locates the EOCD record by scanning the tail
  for 0x06054b50 and accepting a candidate only when its declared comment length
  puts the end of the comment exactly at the end of the archive. That rules out
  payload bytes that happen to spell the signature, and it handles trailing archive
  comments. The tail read starts at 1 KiB (enough for the comment-less containers we
  write) and widens once to 22 + 0xffff + 20 bytes if the record isn't in there.
• When any of the three EOCD fields carries its sentinel (0xffff entry count,
  0xffffffff CD size, 0xffffffff CD offset) the reader follows the ZIP64 end of
  central directory locator that sits immediately before the EOCD and reads the ZIP64
  EOCD record it points at, taking the entry count, CD size and CD offset from there.
• getCDBuffers() now takes the central directory chunk plus the declared entry
  count, and walks exactly that many records from the declared offset, advancing by
  46 + fileNameLength + extraFieldLength + fileCommentLength and validating the
  signature and the remaining length at each step.
• Cross-checks added: the CD chunk must be the declared length, the declared CD size
  must be at least 46 * entryCount, and it is bounded at 16 MiB so a hostile or
  corrupt EOCD can't ask us to buffer the sentinel 4 GiB.

getCDBuffers changed signature. It's a public method on ZipReader but ZipReader
is not re-exported from any package entry point (lib/tdf3/src/utils/index.ts only),
so this isn't a published API break.

Finding 2 — ZIP64 extra field ignored unless versionNeededToExtract >= 45

Dropped the version gate in parseCDBuffer. APPNOTE 4.5.3 does not condition the
validity of a ZIP64 extended information extra field on the version-needed-to-extract
byte; the sentinel values in the fixed-size fields are what select it. The gate meant
a conformant producer writing a ZIP64 extra with a lower declared version had it
silently dropped, leaving 0xffffffff to flow into byteStart/byteEnd arithmetic
as if it were a real number.

sliceExtraFields already did the right thing (iterates the whole extra-field area,
rejects conflicting duplicate header IDs, bounds-checks dataSize, reads the ZIP64
fields in APPNOTE order) and is unchanged. Also added a length guard to parseCDBuffer
so a record shorter than the 46-byte fixed prefix is rejected rather than parsed out
of a short buffer.

Finding 3 — non-ZIP64 data descriptor wrote uncompressedSize twice

Kept the branch and fixed it, rather than deleting it. Justification: ZipWriter.zip64
is a public mutable field and the existing unit tests exercise zip64 = false for all
four writer methods (getLocalFileHeader, writeDataDescriptor,
writeCentralDirectoryRecord, writeEndOfCentralDirectoryRecord). Deleting one of
those four branches would leave the writer unable to emit a coherent non-ZIP64 archive
while the other three remain, which is a worse state than the bug.

writeDataDescriptor now takes an explicit optional compressedSize (defaulting to
uncompressedSize, since we only ever STORE) and writes it into the compressed size
slot in both the ZIP64 and non-ZIP64 branches. Output for every existing call site is
byte-identical; the existing characteristic tests are unchanged. Two new tests pass
distinct compressed and uncompressed sizes and assert each lands in its own slot.

Finding 4 — 32-bit shift on a potentially large size in an error path

(cdObj.uncompressedSize >> 10) became Math.floor(cdObj.uncompressedSize / 1024).
>> coerces to signed 32 bits, so a 2 GiB manifest used to be reported as
-2,097,152 KiB. Error-message only.

Tests

lib/tests/mocha/unit/zip.spec.ts gains a buildZip helper that assembles complete
in-memory archives with ZipWriter, so the reader is exercised end to end rather than
against hand-rolled record fragments. New coverage:

• zip64 and non-zip64 round trips (central directory, manifest, payload segment)
• an archive with a trailing comment
• an archive whose comment contains the EOCD signature bytes
• an archive with a maximum-length (64 KiB) comment, which forces the widened tail read
• a payload containing the bytes 0x02014b50 — the false-positive case
• an entry whose ZIP64 extra field is not the first extra field (both as a bare
  central directory record and inside a full archive)
• a ZIP64 extra field honoured with versionNeededToExtract forced to 20
• rejection cases: missing EOCD, missing ZIP64 locator, overstated entry count
• the oversized-manifest error message reports a positive KiB figure

The 0x02014b50 test was verified against the pre-change reader: with the old backward
scanner restored, getCentralDirectory() returns four entries
(['', '', '0.manifest.json', '0.payload']) instead of two, the two empty-named ones
being payload bytes misread as central directory records. It passes with the new
reader.

Deliberately not changed: segment-size emission

lib/tdf3/src/tdf.ts omits segmentSize / encryptedSegmentSize from a segment
object when they equal the manifest-level defaults, and that stays as it is. The
emission is legal: manifest.schema.json gives segments/items no required list,
and web-sdk's own reader defaults them back. go-sdk and java-sdk fail to apply that
fallback, which is why web-sdk TDFs over 1 MiB are currently unreadable by them —
those are the bugs, tracked as DSPX-4590 finding 7 and DSPX-4589 finding 4. Making
web-sdk write the redundant fields would paper over two real reader bugs and silently
un-cover them.

Cross-SDK interop

The shared interop harness is DSPX-4592 and is already built in opentdf/tests on
branch DSPX-4592-java-underflow. It needs nothing from this PR.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
87.6% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[github-actions]

X-Test Results

✅ js@main-main
✅ js@pull-1017-main
✅ js@pull-1017-v0.9.0
✅ java@main-main
✅ js@main-v0.9.0
✅ go@main-main
✅ js@v0.4.0-main
✅ js@v0.4.0-v0.9.0
✅ java@main-v0.9.0
✅ go@main-v0.9.0
coverage-roundtrip-browser
coverage-web-app-browser
coverage-web-app
opentdf-ctl
coverage-lib
opentdf-sdk-lib