Skip to content

[Bug] SingleFile (.scdb) mode ignores DatabaseOptions.EncryptionKey, writing data in plaintext - #342

Closed
saltus7 wants to merge 2 commits into
MPCoreDeveloper:masterfrom
saltus7:master
Closed

[Bug] SingleFile (.scdb) mode ignores DatabaseOptions.EncryptionKey, writing data in plaintext#342
saltus7 wants to merge 2 commits into
MPCoreDeveloper:masterfrom
saltus7:master

Conversation

@saltus7

@saltus7 saltus7 commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Fixes a critical bug where SingleFile (.scdb) storage mode ignored DatabaseOptions.EncryptionKey, writing data in plaintext even when encryption was configured.

Root Cause

The SingleFileStorageProvider accepted the encryption configuration but never instantiated AesGcmEncryption or intercepted the I/O paths. Consequently, WriteBlockAsync, ReadBlockAsync, GetReadSpan, and GetReadStream operated directly on the raw FileStream, bypassing encryption entirely. (Directory mode was unaffected as it correctly wired ICryptoService via Database.Core.cs).

Changes

  • src/SharpCoreDB/DatabaseExtensions.cs: Resolves ICryptoService from DI when encryption is enabled and passes it to the SingleFile provider.
  • src/SharpCoreDB/Storage/SingleFileStorageProvider.cs: Wires AesGcmEncryption into the provider and intercepts all block read/write/stream paths to encrypt/decrypt data at rest.
  • tests/SharpCoreDB.Tests/Security/SingleFileEncryptionTests.cs: Adds a comprehensive 12-test regression suite covering plaintext-at-rest, correct-key roundtrip, wrong-key rejection, and file integrity.
  • src/SharpCoreDB/DatabaseOptions.cs, src/SharpCoreDB/SingleFileTable.cs, src/SharpCoreDB/Storage/Scdb/SingleFileDatabase.Batch.cs: (Included from local build/test adjustments).

Also included

  • global.json (root): Added test.runner config for MTP-native dotnet test on .NET 10 SDK. Without this, dotnet test fails with error Testing with VSTest target is no longer supported.

Fixes #341

Verification

  • Build succeeds (warnings are non-fatal)
  • Full test suite passes: 2,416 passed, 16 skipped, 0 failed (Total: 2,432)

scdb-dev added 2 commits August 28, 2026 22:00
- Added test.runner config to global.json for MTP-native dotnet test
- Build succeeds (291 warnings, non-fatal)
- All 2,404 tests pass, 16 skipped, 0 failed
@sonarqubecloud

Copy link
Copy Markdown

MPCoreDeveloper pushed a commit that referenced this pull request Aug 29, 2026
…n fix (#341)

Credit @saltus7 (PR #342) for independently root-causing issue #341 and submitting a complete proposed fix (AES-256-GCM at rest on every block I/O path + 12-test regression suite) before v1.9.7 shipped. Adds CONTRIBUTORS.md, CHANGELOG/NuGet-README credit lines and a README pointer, plus a Co-authored-by trailer so the contribution is recorded in git history.

Co-authored-by: saltus7 <saltus7@users.noreply.github.com>
@MPCoreDeveloper

Copy link
Copy Markdown
Owner

Thank you so much for this contribution — and sincere apologies for the crossed wires on our side.

The short version: your diagnosis was 100% correct, and your PR arrived before we shipped our own fix. We resolved issue #341 with our own implementation in v1.9.7 (commit 16ed7e22) and only found your PR after the fact. That's on us — you deserved to be looped in from the start.

What you got right:

  • Root causeSingleFileStorageProvider accepted EnableEncryption/EncryptionKey but never instantiated a cipher; WriteBlockAsync, ReadBlockAsync, GetReadSpan and GetReadStream bypassed encryption entirely, and directory mode was correctly wired (your note was spot on).
  • The fix surface — wire AesGcmEncryption into the provider and intercept every block I/O path.
  • The tests — your 12-test suite (plaintext-at-rest, correct-key round-trip, wrong-key rejection, file integrity) covers exactly the right invariants; our shipped SingleFileEncryptionTests validate the same ground.

Credit — we've made it permanent:

Since v1.9.7 already ships the fix and your branch is based on an earlier master, merging the PR as-is would conflict heavily with the released code — so I'm closing it as superseded by v1.9.7, with genuine thanks and full credit preserved. If any part of your implementation (e.g., the global.json test-runner tweak for the .NET 10 SDK) would be valuable on its own, we'd be happy to review it as a fresh PR.

We'd love to see more contributions from you — thank you for the excellent work. 🙏

@saltus7

saltus7 commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

Thank you for the incredibly thoughtful response and for ensuring the credit was preserved in the git history and release notes. I really appreciate the professional handling of the crossed wires!

It's great to see the fix shipped in v1.9.7. I'm evaluating SharpCoreDB for a local-first edge-node architecture (which requires strict AES-256-GCM at-rest encryption and single-file deployment), so finding and resolving this was a critical step for my own adoption of the library.

I'll hold off on the global.json PR for now to keep the noise down, but I'll definitely be keeping an eye on the project and look forward to future contributions as I integrate v1.9.7 into my stack. Thanks again for the excellent maintainership!

@saltus7

saltus7 commented Aug 29, 2026

Copy link
Copy Markdown
Collaborator Author

Quick follow-up: I've now reviewed the v1.9.7 implementation in detail, and I want to give credit where it's due.

My PR handled the happy path — encrypt on write, decrypt on read — but it left several serious gaps that I didn't catch. Your fix addressed all of them:

  • Blocking delta updates and in-place UpdateBlockAsync when encryption is enabled (ciphertext regions aren't positionally independent — my version would have silently corrupted files)
  • Blocking raw GetWriteStream (my version left an open bypass that could write plaintext to an "encrypted" database)
  • Validating checksums against ciphertext rather than plaintext (my approach was backwards and could mask on-disk corruption)
  • Enforcing encryption-mode consistency on open (my version would silently misinterpret a file opened with the wrong mode)
  • Zeroizing the cipher on dispose

My submission was a prototype. Yours is production-grade. The attention to edge cases — especially the ones that would have caused silent data corruption rather than loud failures — is exactly what this kind of security-critical path needs.

Thanks for taking the time to do it right. Moving forward with my integration on v1.9.7 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] SingleFile (.scdb) mode ignores DatabaseOptions.EncryptionKey, writing data in plaintext

2 participants