Release v2.3.0: encrypted workbooks, parallel CSV, ExcelReader.Arrow package - #98
Merged
Merged
Conversation
Extracts the OLE/CFB parse prefix that used to live inline in XlsCompoundFile.BuildWorkbook (header/DIFAT/FAT/directory parsing, mini-stream reads, FAT-chain reads, bounds checks) into a standalone CfbContainer type that can look up any named directory stream, not just "Workbook"/"Book". Every bounds/limit check (sector size, mini-stream cutoff, maxSectors, name-length, FAT-chain cycle detection, etc.) was moved verbatim, including its explanatory comment. CfbContainer exposes ContainsStream, ReadStream (materializing, with an ExcelLimitExceededException guard), OpenStreamView (a seekable read-only Stream over the FAT or mini-FAT chain), and StreamLength. XlsCompoundFile.BuildWorkbook now parses via CfbContainer.Parse and keeps only the Workbook-specific mini-cutoff / Chained / Streamed selection. Pure code motion: no crypto code, no behavior change on the XLS path. This is the foundation an encrypted-workbook reader needs, since an encrypted OOXML file is a CFB container holding "EncryptionInfo" and "EncryptedPackage" streams instead of "Workbook".
Implements AgileKeyDerivation per [MS-OFFCRYPTO] 2.3.4.7 (Encryption Key
Generation), 2.3.4.10 (PasswordKeyEncryptor Generation), 2.3.4.12
(Initialization Vector Generation), 2.3.4.13 (Data Encryption), and
2.3.4.14 (DataIntegrity Generation), cross-checked against the live
spec pages rather than trusting the brief's sketch verbatim.
One deviation from the brief, spec-verified: BlockKey pads a
short hash with 0x36 bytes, not zero bytes, per 2.3.4.7's final
paragraph and the general IV rule in 2.3.4.12 ("pad ... by appending
0x36"). This path isn't exercised by the current AES-256/SHA-512
fixtures (hash always longer than the target key), so it had no
oracle to catch a wrong constant.
Verified against the real EncryptedFixtures (password "hunter2"):
Should_Derive_Key_When_Password_Correct and
Should_Report_PasswordIncorrect_When_Password_Wrong both pass for
agile-aes256-sha512.xlsx and .xlsb on net10.0 and net8.0.
DecryptNoPadding fed a block-size-misaligned ciphertext straight into Aes.TransformFinalBlock with Padding = None, which throws a raw CryptographicException. EncryptionInfo is untrusted, attacker-controlled input parsed before any password check, and every other malformed value in this codebase is converted to InvalidDataException/ ExcelEncryptionException/ExcelLimitExceededException before reaching a caller (see EncryptionDescriptor.ReadCryptoParameters). This also matters for Task 10's fuzz target: FuzzMutation.AcceptableExceptionTypes doesn't include CryptographicException, so a mutated EncryptedVerifierHashInput/Value, EncryptedKeyValue, or EncryptedHmacKey/Value would have failed that fuzz test. Adds a bounds check (ciphertext.Length % aes.BlockSize/8) before the AES call, and a regression test that truncates a real fixture's EncryptedVerifierHashInput by one byte and asserts InvalidDataException, not CryptographicException.
Implements the oracle-tested read-only Stream that decrypts the EncryptedPackage CFB stream of an agile-encrypted OOXML workbook on demand, in 4096-byte segments (AES-CBC, one segment cached at a time). - Create() opens the EncryptedPackage view, validates the 8-byte declared plaintext length against both the ciphertext length and MaxTotalDecompressedBytes before allocating anything, and derives the intermediate key via AgileKeyDerivation. - Read/ReadAsync follow the three-tier sync/async convention from ARCHITECTURE.md: a blocking Read, a ReadAsync whose cached-segment fast path returns a completed ValueTask, and a split-out ReadSlowAsync holding the awaiting refill loop. - The final segment's ciphertext is block-padded to 16 bytes while its plaintext is truncated to the declared Length; the copy bound min(..., Length - position) in Read/ReadSlowAsync handles this without any special-casing. - Dispose zeroes the derived key and segment cache with CryptographicOperations.ZeroMemory, returns the pooled cache buffer, and disposes the ciphertext view. Should_Match_Oracle_When_Read_Sequentially/_Async are byte-exact against msoffcrypto-tool's independently produced plaintext for both real fixtures (agile-aes256-sha512.xlsx/.xlsb), passing on the first run with no debugging needed.
…ataException Review of commit 29eb0e1 found that SegmentCiphertextRange's comment claimed the container's total ciphertext length was validated (in Create) to be block-aligned, but no such validation existed. Both the CFB entry size and the 8-byte declared-length prefix inside EncryptedPackage are attacker-controlled; a crafted file can make the final segment's remaining ciphertext land mid-block. That reached ICryptoTransform.TransformBlock (PaddingMode.None) and threw a raw ArgumentOutOfRangeException instead of a graceful, typed rejection - the same class of bug Task 5 already fixed in AgileKeyDerivation.DecryptNoPadding (commit f5ce19c). - Create now validates view.Length - PrefixSize is a multiple of the 16-byte AES block size, right after the existing declared-length bounds check, and throws InvalidDataException (matching DecryptNoPadding's convention) before any segment is ever touched. - Added a CipherBlockSize constant alongside the existing SegmentSize/PrefixSize constants. - Fixed SegmentCiphertextRange's comment to state the validation truthfully instead of asserting an invariant nothing enforced. - Added Should_Reject_NonBlockAligned_Ciphertext, which patches a real fixture's on-disk EncryptedPackage entry (CFB directory Size field and on-disk declared-length prefix) 5 bytes shorter than its real, valid ciphertext length - always landing on a non-block-aligned total regardless of the fixture's exact size - and asserts InvalidDataException, not ArgumentOutOfRangeException. Both TFMs green: 2413/2413, including all byte-exact oracle tests against both real fixtures.
Bumps Meziantou.Analyzer from 3.0.150 to 3.0.182 Bumps Microsoft.CodeAnalysis.Analyzers from 5.9.0-1.26328.17 to 5.9.0 Bumps Roslynator.Analyzers from 4.16.0 to 5.0.0 Bumps Roslynator.CodeAnalysis.Analyzers from 4.16.0 to 5.0.0 Bumps Roslynator.Formatting.Analyzers from 4.16.0 to 5.0.0 Bumps SonarAnalyzer.CSharp from 10.32.0.713 to 10.33.0.1635 --- updated-dependencies: - dependency-name: Meziantou.Analyzer dependency-version: 3.0.182 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: analyzers - dependency-name: Microsoft.CodeAnalysis.Analyzers dependency-version: 5.9.0 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: analyzers - dependency-name: Roslynator.Analyzers dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: analyzers - dependency-name: Roslynator.CodeAnalysis.Analyzers dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: analyzers - dependency-name: Roslynator.Formatting.Analyzers dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major dependency-group: analyzers - dependency-name: SonarAnalyzer.CSharp dependency-version: 10.33.0.1635 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: analyzers ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps MiniExcel from 1.45.0 to 1.46.0 --- updated-dependencies: - dependency-name: MiniExcel dependency-version: 1.46.0 dependency-type: direct:production update-type: version-update:semver-minor dependency-group: benchmarks ... Signed-off-by: dependabot[bot] <support@github.com>
Task 8 of the encrypted-workbook-reading plan: wires the CFB/CFB decryption pipeline from Tasks 2-7 into Excel.Open/OpenAsync's detection and the FromXlsx/FromXlsb format-specific entry points. - ExcelFileFormat gains EncryptedOoxml = 4 (explicit values for all members); NativeFormat's doc comment now explains the 5th managed value has no native counterpart. - New EncryptedPackageOpener: IsEncryptedContainer probes a CFB container's directory for an EncryptedPackage stream (detection only); Decrypt turns one into a seekable plaintext-ZIP stream, bundling the CfbContainer with DecryptedPackageStream via a small owning Stream wrapper since the latter never takes ownership of the former. - Excel.cs: TryClassifyHeader no longer finalizes on a CFB signature alone (only the OLE directory can tell legacy .xls apart from encrypted OOXML); DetectSeekable/DetectSeekableAsync resolve it via EncryptedPackageOpener.IsEncryptedContainer. ClassifyMemory keeps its pre-existing CFB-is-always-Xls behavior since the in-memory Open overload has no CFB directory probe. Open/OpenAsync gain an EncryptedOoxml arm that decrypts then classifies the resulting ZIP via the existing ClassifyZipStream. From/FromXlsb(+Async) gain a guard: a CFB stream with a password supplied is decrypted before construction, replacing a confusing raw ZipArchive error. - XlsReader.cs: the FilePass rejection message now names the real boundary (OOXML encryption is supported via ExcelReaderOptions.Password; legacy .xls encryption is not) instead of reading as a plain bug report. - XlsWorkbookBuilder gains WithFilePassRecord(), a byte[] twin of the existing BuildEncrypted() for tests using Excel.Open(ReadOnlyMemory<byte>, ...). New tests/ExcelReader.Tests/EncryptedOpenTests.cs covers: same rows from encrypted vs. plaintext fixtures (sync+async), PasswordRequired/ PasswordIncorrect, EncryptedOoxml detection, a real .xls still detecting as Xls through the new two-stage CFB probe, password support on FromXlsx, and the improved encrypted-.xls message.
Closes a review-flagged gap in Task 8's encrypted-workbook detection wiring: every existing corrupt-OLE regression test (CorruptOleSignatureThrows, UnsupportedOleSectorSizeThrows, the crafted-FAT/mini-FAT-sector-count tests) calls Excel.FromXls directly, bypassing TryClassifyHeader/DetectSeekable entirely — so EncryptedPackageOpener.IsEncryptedContainer's catch (InvalidDataException) fallback (the thing that lets a malformed file fall through to "not encrypted, let XLS diagnose it") had never been exercised end-to-end through the path a real caller uses. Adds Should_Preserve_Corrupt_Ole_Diagnosis_Through_Excel_Open to EncryptedOpenTests.cs, reusing XlsWorkbookBuilder.BuildPatched with the same sector-size corruption UnsupportedOleSectorSizeThrows already uses (signature intact, so detection actually reaches the CFB probe). Asserts Excel.DetectFileFormat still reports Xls without throwing, and Excel.Open still throws the same InvalidDataException it always has via Excel.FromXls — proving the new two-stage CFB probe doesn't change behavior for a genuinely corrupt (not encrypted, not valid .xls) file. No src/ changes: test-only, confirming behavior already matched the brief's contract.
- EncryptedPackageOpener.DecryptToMemory/IsEncryptedMemory: eager decrypt path for Excel.From/Excel.Open's ReadOnlyMemory<byte> overloads, which are documented to never suspend, so lazy decrypt-on-demand isn't an option here. Always verifies the dataIntegrity HMAC since everything is already decrypted. - PackageIntegrity.Verify: HMAC verification over the whole EncryptedPackage stream (8-byte prefix included). Wired into DecryptedPackageStream.Create as opt-in via ExcelReaderOptions.VerifyEncryptedIntegrity (default false, since it costs a full extra pass before the first row on the streaming path). - Fixes a real bug in AgileKeyDerivation.UnwrapHmac: the decrypted HMAC key was truncated to KeyData.SaltSize instead of the hash's native output length. Per [MS-OFFCRYPTO] 2.3.4.14 (and confirmed against msoffcrypto-tool's own writer, which allocates the key salt as hashSize random bytes) the HMAC key is hashSize bytes, not saltSize. With the wrong length, HMAC verification failed even for legitimate, untampered files. Found and fixed by cross-checking against an independent from-scratch Python re-implementation over the real fixture.
Addresses 6 findings from the final whole-branch review of the encrypted workbook reading plan: - AgileKeyDerivation: reject an encryptedKeyValue that decrypts shorter than the declared key size with InvalidDataException, instead of letting the keyLen slice throw ArgumentOutOfRangeException on attacker-controlled input. - EncryptionDescriptor: restrict blockSize to exactly 16 (AES-CBC's fixed block size) instead of the old [1,64] range, rejecting anything else as ExcelEncryptionException/UnsupportedScheme instead of letting aes.IV throw CryptographicException. - Fuzz: remove the 1_000 MaxPasswordSpinCount override that made every input to the `encrypted` target dead-end on ExcelLimitExceededException before reaching any real derivation/decryption code; add a self-check (OpenEncryptedSeedForSelfCheck / VerifyEncryptedSeedReachesRealCode) that hard-fails if this regresses, and raise the mutation-fuzz regression test's round count from 200 to 1000. - Excel.DetectFileFormat(ReadOnlyMemory<byte>): probe for encrypted OOXML via EncryptedPackageOpener.IsEncryptedMemory before falling back to Xls, mirroring the stream-based overload. - Python docs: remove the stale claim that an explicit format bypasses CFB sniffing for encrypted files (fixed by an earlier merged change). - Add `encrypted` to the nightly fuzz workflow's target matrix, and document the C++ password API in cpp/README.md.
…api-v2.2.0 chore: promote public API for v2.2.0
Covers Excel.ParseCsvParallelAsync's architecture (chunk partitioning, speculative boundary resolution, ordered merge) in ARCHITECTURE.md and usage in README.md, plus CsvParallelParseBenchmark with generated conversion-heavy and narrow-int corpora to measure it against the sequential parser.
… path Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…et crash, Core reader reuse, README, polish)
Benchmark ResultsMeasured on ExcelReader.Benchmarks.ColdStartBenchmark
ExcelReader.Benchmarks.CsvParseBenchmark
ExcelReader.Benchmarks.CsvReadBenchmark
ExcelReader.Benchmarks.CsvWriteBenchmark
ExcelReader.Benchmarks.ParseBenchmark
ExcelReader.Benchmarks.ReadBenchmark
ExcelReader.Benchmarks.RealDataReadBenchmark
ExcelReader.Benchmarks.RecordWriteBenchmark
ExcelReader.Benchmarks.WriteBenchmark
ExcelReader.Benchmarks.XlsReadBenchmark
ExcelReader.Benchmarks.XlsWriteBenchmark
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #98 +/- ##
==========================================
+ Coverage 85.12% 85.45% +0.33%
==========================================
Files 136 154 +18
Lines 10041 11296 +1255
Branches 1868 2077 +209
==========================================
+ Hits 8547 9653 +1106
- Misses 1162 1255 +93
- Partials 332 388 +56 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
added 3 commits
August 30, 2026 14:48
rust.yml, python.yml, and cpp.yml each called build_native.py independently, publishing ExcelReader.Native from scratch 3x per OS (9 NativeAOT publishes per PR, on top of ci.yml's aot-sanity build). Replaced them with native-bindings.yml: a single build-native job per OS uploads the binary as an artifact, and test-rust/test-python/test-cpp jobs download it instead of rebuilding. build-native has no `needs`, so its failure fails a required check directly rather than only cascading into skipped (and thus GitHub-treated-as-passed) downstream jobs. Also added missing caches: NuGet in build-native, cargo via Swatinem/rust-cache in test-rust, pip in test-python. Job names kept identical to the old workflows so the branch protection required status checks (already updated on GitHub, plus the 3 new Build native library contexts) still match.
Adds a changes job (dorny/paths-filter) at the top of native-bindings.yml. build-native runs when any of native/rust/python/cpp paths changed; test-rust/test-python/test-cpp each additionally require native OR their own language's paths. A PR touching only C# core code or docs no longer pays for 3 OS x 3 binding matrices. Filtering is inside the workflow (per-job `if:`), not at `on.pull_request.paths` - that form would skip the whole workflow run for a non-matching PR, leaving its required status checks stuck at "expected" and blocking merge forever. A job skipped via `if:` still posts a real skipped conclusion, which GitHub treats as passing a required check. test-* jobs check needs.changes.outputs.*, not build-native's result, so an explicit `if:` doesn't accidentally bypass a genuine build-native failure - if build-native really fails, the artifact never exists and the test job's download-artifact step fails loudly instead of being silently skipped. `changes` itself is now a required check too: if paths-filter errors, that must fail loud rather than making every downstream `if:` evaluate false and skip silently.
actions/upload-artifact strips the executable bit. dlopen() itself tolerates that, but the NativeAOT runtime's PAL init - triggered by the first real P/Invoke, not by CDLL()/dlopen() - remaps the file with PROT_EXEC and hits SIGABRT (uncatchable from Python/Rust/C++, not a graceful load error) when that fails. Missed this in the artifact consolidation since none of the old rust.yml/python.yml/cpp.yml built the binary in a separate job from the one that used it. Observed as `Fatal Python error: Aborted` in test_native.py::test_library_loads, crashing inside lib.xl_abi_version() right after a successful CDLL() call - confirms the file loaded but the runtime's own remap failed. chmod +x after download in test-rust/test-python/test-cpp, Linux/macOS only (no such bit on Windows).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Highlights
DecryptedPackageStreamwith sequential/random access, password support threaded through.NET/Rust/Python/C++/native (ABI bump to 4).
RangedFileStream), lock-free per-chunkpull queue, quote-parity boundary reconciliation, automatic sequential fallback for sources
that can't be partitioned (non-seekable streams, non-UTF-8 encodings, small files).
Apache.Arrow.RecordBatchwith schemainference, string/int64/float64/bool/date/timestamp column support, shared-string caching.
Ready for NuGet publishing (packaged README, snupkg symbols, SourceLink, release notes).
optimizations; CSV read improvements.
Publishing
ExcelReader.Arrowships to NuGet for the first time alongsideExcelReader.NETandExcelReader.NET.Cli. Trusted Publishing on nuget.org is account-scoped, so the existingpolicy already covers it with no extra setup.
Native ABI
xl_open_optionsgained a password field; native ABI bumps to version 4. Affects onlydirect native/Rust/Python/C++ binding consumers, not the .NET public API.