sneak/vaultik
1
0
Fork 0
Code Issues 18 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

Negative and boundary tests for internal/blobgen and internal/types (and internal/crypto if it survives) #170

New Issue
Closed
opened 2026-09-22 00:55:14 +02:00 by clawbot · 1 comment
clawbot commented 2026-09-22 00:55:14 +02:00
Collaborator
Copy Link
Copy Source

This is definition-of-done item 2 of the security review #73. Severity of the gap: low to medium; the compress, encrypt, hash writer and its reader have no negative test at all.

What exists today

  • No test in internal/blobgen calls NewReader. Reader is exercised once, indirectly, with a 48-byte input (internal/vaultik/blob_fetch_hash_test.go).
  • Writer.Sum256 equals SHA256(SHA256(input)) for one 1 MiB input; same input twice gives the same hash and different ciphertext (writer_test.go).
  • CompressStream and CompressData tests assert only no error and non-empty output; the output is never decrypted and the hash value never checked. TestCompressStreamEmptyInput does not assert written == 0 or that the output decrypts to empty.
  • internal/types has no test file. internal/crypto tests cover only Encryptor, which production never calls.

Order

Do #151 first: it deletes internal/crypto and the unused secret types, which removes their test obligations. If the owner keeps internal/crypto, apply the same wrong-identity, truncation, corruption and size cases to Decryptor.Decrypt and DecryptStream.

Two cases fail today and belong to the issues that fix them, as their regression tests. Do not commit them here skipped or weakened:

  • a stream cut exactly after the age header and nonce: #152
  • a secret key passed as a recipient must not appear in the error: #153

Tests to add in internal/blobgen (package blobgen_test, identities from age.GenerateX25519Identity, table-driven)

  1. Writer to Reader round trip for sizes 0, 1, 65535, 65536, 65537 and 4*65536+123, once with random data (so the compressed payload crosses 64 KiB age segments) and once with compressible data; output equals input.
  2. For each size: Reader.Sum256 == SHA256(output); the second hash of it equals Writer.Sum256; BytesRead == BytesWritten == len(input).
  3. Zero-length input through Writer with no Write, and through CompressStream and CompressData if they are kept: size 0, hash equals hex(SHA256(SHA256(""))), output reads back as empty with no error.
  4. NewReader with a different identity than the recipient: errors.As *age.NoIdentityMatchError.
  5. Two recipients: each identity reads the blob back.
  6. Truncation of a multi-segment blob: inside the age header; inside the nonce; inside the first segment; exactly at the end of the first 64 KiB+16 segment; last byte removed. Each gives a non-nil error from NewReader or io.ReadAll, never data with a nil error.
  7. One flipped byte in a header stanza, the header MAC line, the nonce, a body segment and the final 16-byte tag: each errors.
  8. Bytes appended after a valid blob; empty input; random garbage: each errors.
  9. NewWriter with level 0, -1 and 20: errors.Is ErrInvalidCompressionLevel and nothing written to the destination. Levels 1 and 19 round trip.
  10. NewWriter with nil and empty recipient lists, and with an invalid recipient string: error.
  11. A destination writer that fails: the error surfaces from Write or Close.
  12. In internal/vaultik: a validly encrypted blob with different content stored under another blob's name makes verify --deep fail with errBlobHashMismatch.

Reword or remove the two test messages that say a single hash "would allow content confirmation attacks" (writer_test.go:63-66); the double hash does not prevent that.

Tests to add in internal/types/types_test.go

For FileID and BlobID: Value then Scan returns the same id; Scan of string and of []byte; Scan of NULL gives the zero id; Scan of a non-string type and of malformed text returns an error; ParseFileID and ParseBlobID accept and reject cases; IsZero. Leave the alternate text forms uuid.Parse accepts out: no behaviour has been decided for them and rows written by Value are always canonical.

Definition of done

  1. Every numbered case above is covered; no existing assertion weakened; no test skipped.
  2. make check green.

Restore-level corrupt and truncated blob scenarios are #72; do not duplicate them.

Line numbers are as of next at 6fcd8e1.

model: fable-5-1

This is definition-of-done item 2 of the security review https://git.eeqj.de/sneak/vaultik/issues/73. Severity of the gap: **low to medium**; the compress, encrypt, hash writer and its reader have no negative test at all. ## What exists today - No test in `internal/blobgen` calls `NewReader`. `Reader` is exercised once, indirectly, with a 48-byte input (`internal/vaultik/blob_fetch_hash_test.go`). - `Writer.Sum256` equals `SHA256(SHA256(input))` for one 1 MiB input; same input twice gives the same hash and different ciphertext (`writer_test.go`). - `CompressStream` and `CompressData` tests assert only no error and non-empty output; the output is never decrypted and the hash value never checked. `TestCompressStreamEmptyInput` does not assert `written == 0` or that the output decrypts to empty. - `internal/types` has no test file. `internal/crypto` tests cover only `Encryptor`, which production never calls. ## Order Do https://git.eeqj.de/sneak/vaultik/issues/151 first: it deletes `internal/crypto` and the unused secret types, which removes their test obligations. If the owner keeps `internal/crypto`, apply the same wrong-identity, truncation, corruption and size cases to `Decryptor.Decrypt` and `DecryptStream`. Two cases fail today and belong to the issues that fix them, as their regression tests. Do not commit them here skipped or weakened: - a stream cut exactly after the age header and nonce: https://git.eeqj.de/sneak/vaultik/issues/152 - a secret key passed as a recipient must not appear in the error: https://git.eeqj.de/sneak/vaultik/issues/153 ## Tests to add in `internal/blobgen` (package `blobgen_test`, identities from `age.GenerateX25519Identity`, table-driven) 1. Writer to Reader round trip for sizes 0, 1, 65535, 65536, 65537 and 4*65536+123, once with random data (so the compressed payload crosses 64 KiB age segments) and once with compressible data; output equals input. 2. For each size: `Reader.Sum256 == SHA256(output)`; the second hash of it equals `Writer.Sum256`; `BytesRead == BytesWritten == len(input)`. 3. Zero-length input through `Writer` with no `Write`, and through `CompressStream` and `CompressData` if they are kept: size 0, hash equals `hex(SHA256(SHA256("")))`, output reads back as empty with no error. 4. `NewReader` with a different identity than the recipient: `errors.As` `*age.NoIdentityMatchError`. 5. Two recipients: each identity reads the blob back. 6. Truncation of a multi-segment blob: inside the age header; inside the nonce; inside the first segment; exactly at the end of the first 64 KiB+16 segment; last byte removed. Each gives a non-nil error from `NewReader` or `io.ReadAll`, never data with a nil error. 7. One flipped byte in a header stanza, the header MAC line, the nonce, a body segment and the final 16-byte tag: each errors. 8. Bytes appended after a valid blob; empty input; random garbage: each errors. 9. `NewWriter` with level 0, -1 and 20: `errors.Is ErrInvalidCompressionLevel` and nothing written to the destination. Levels 1 and 19 round trip. 10. `NewWriter` with nil and empty recipient lists, and with an invalid recipient string: error. 11. A destination writer that fails: the error surfaces from `Write` or `Close`. 12. In `internal/vaultik`: a validly encrypted blob with different content stored under another blob's name makes `verify --deep` fail with `errBlobHashMismatch`. Reword or remove the two test messages that say a single hash "would allow content confirmation attacks" (`writer_test.go:63-66`); the double hash does not prevent that. ## Tests to add in `internal/types/types_test.go` For `FileID` and `BlobID`: `Value` then `Scan` returns the same id; `Scan` of string and of `[]byte`; `Scan` of NULL gives the zero id; `Scan` of a non-string type and of malformed text returns an error; `ParseFileID` and `ParseBlobID` accept and reject cases; `IsZero`. Leave the alternate text forms `uuid.Parse` accepts out: no behaviour has been decided for them and rows written by `Value` are always canonical. ## Definition of done 1. Every numbered case above is covered; no existing assertion weakened; no test skipped. 2. `make check` green. Restore-level corrupt and truncated blob scenarios are https://git.eeqj.de/sneak/vaultik/issues/72; do not duplicate them. Line numbers are as of `next` at `6fcd8e1`. model: fable-5-1
clawbot commented 2026-09-22 14:04:30 +02:00
Author
Collaborator
Copy Link
Copy Source

PR: #192

Test-only. Added negative and boundary coverage for internal/blobgen (round trips across the 64 KiB age-segment edges with hashes checked by decrypting; wrong identity; truncation and one-byte corruption of a multi-segment blob; trailing/empty/garbage input; compression-level and recipient validation; failing destination) and internal/types (Value/Scan/Parse/IsZero for FileID and BlobID). Reworded the two overstated double-hash messages; no assertion weakened.

Scoped to the two test-file trees per the task brief, so case 12 (verify --deep hash mismatch in internal/vaultik) is not here. The "cut right after the age header and nonce" truncation is left out as the regression case for #152. internal/crypto and CompressStream/CompressData, removed by #151, are skipped. Multi-recipient read-back is already covered by the existing test.

make check green.

Model: opus-4-8

PR: https://git.eeqj.de/sneak/vaultik/pulls/192 Test-only. Added negative and boundary coverage for `internal/blobgen` (round trips across the 64 KiB age-segment edges with hashes checked by decrypting; wrong identity; truncation and one-byte corruption of a multi-segment blob; trailing/empty/garbage input; compression-level and recipient validation; failing destination) and `internal/types` (`Value`/`Scan`/`Parse`/`IsZero` for `FileID` and `BlobID`). Reworded the two overstated double-hash messages; no assertion weakened. Scoped to the two test-file trees per the task brief, so case 12 (`verify --deep` hash mismatch in `internal/vaultik`) is not here. The "cut right after the age header and nonce" truncation is left out as the regression case for https://git.eeqj.de/sneak/vaultik/issues/152. `internal/crypto` and `CompressStream`/`CompressData`, removed by https://git.eeqj.de/sneak/vaultik/issues/151, are skipped. Multi-recipient read-back is already covered by the existing test. `make check` green. Model: opus-4-8
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
All checks pass, reviewed, rebased, ready for merge
 needs-checks
make check does not pass cleanly
 needs-rebase
PR has merge conflicts or is behind main
 needs-review
Code review not yet done
 needs-rework
Code review found issues to fix
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
clawbot sneak (Jeffrey Paul)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/vaultik#170