Repository navigation
test(vcencrypt): differential fuzzing of the Go and Rust value codecs - #384
Conversation
Each decoder's own fuzz targets only prove it agrees with itself. These targets feed the same bytes to vcffi and to the Rust decoder in the wasm guest, and fail when one accepts what the other rejects, or when both accept but re-encode differently. No guest export is added. Calling vc_encrypt or vc_decrypt with a never-issued handle reports Rust's accept or reject verdict, because both decode before the handle lookup. Rust's re-encoding comes from decrypting a ciphertext that is a single passthrough node, which avoids AES and the cipher's sealing rules. Refs #383
The committed wasm guest is rebuilt by hand and can lag its source, so the nightly job rebuilds it before fuzzing, comparing Go with the Rust on the same commit. Document the approach, and mark the guest's decode-before-handle-lookup order as relied on. Closes #383
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fc15187c57
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| t.Fatalf("input %x: Rust re-encodes as %x, Go as %x", data, rustOut, goOut) | ||
| } | ||
| for i := range rustOut { | ||
| if rustOut[i] != goOut[i] && (rustOut[i] != 0x01 || goOut[i] != 0x00) { |
There was a problem hiding this comment.
When a codec divergence changes an ordinary payload byte from 0x01 to 0x00—for example, inside a Bytes value—this raw byte comparison incorrectly accepts it as the documented Undefined-to-Null projection. That leaves the differential fuzzer blind to real data-corruption mismatches whenever the changed bytes have this pair of values; normalize parsed Undefined nodes or track tag positions instead of permitting the substitution at every byte offset.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 6150839. The comparison now walks Rust's re-encoding to find its Undefined value-node tags, nested ones included, and requires Go's re-encoding to equal Rust's with exactly those bytes changed to Null. Your example, Rust 04 01 00 00 00 against Go 04 00 00 00 00, now fails, as does a 01 changing inside a Bytes payload or an object key. TestUndefinedTags pins the walker. The same weakness was in vcffi's own fuzz check on #381 and is fixed there in 8b8102a; this module can't import that test helper, so it has its own copy. Both differential targets ran clean for a minute each with the stricter check.
There was a problem hiding this comment.
🟡 Changes recommended
The byte-level Undefined exception can conceal genuine codec disagreements in non-tag payload bytes.
1 open finding
What changed in this PR
Adds differential fuzzing to detect divergences between Go and Rust value/ciphertext codecs.
Changes:
- Adds value and ciphertext differential fuzz targets.
- Documents and tests decode-before-handle-lookup behavior.
- Expands nightly fuzzing to rebuild and test the Rust guest.
| File | Description |
|---|---|
.github/workflows/fuzz.yml |
Runs all four fuzz targets nightly. |
bindings/go/vcffi/README.md |
References differential fuzzing. |
bindings/go/vcencrypt/README.md |
Documents fuzzing behavior and mechanism. |
bindings/go/vcencrypt/differential_fuzz_test.go |
Implements targets, tests, and benchmark. |
bindings/go/vcencrypt/guest/src/abi.rs |
Documents required decode ordering. |
bindings/go/vcencrypt/testdata/fuzz/FuzzDifferentialValue/27c1e73ef072e6f1 |
Adds a regression seed. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…ned tags The differential comparison accepted a 0x01 to 0x00 change at any offset, so Rust Int32(1) against Go Int32(0) passed as the Undefined projection. Walk Rust's re-encoding to find its Undefined value-node tags and require Go's to differ at exactly those bytes. Claude-Session: https://claude.ai/code/session_012zHhtTEo968WazExKxA5oT
Closes #383. Stacked on #381.
What
There are two new fuzz targets in
bindings/go/vcencrypt/differential_fuzz_test.go. Each feeds the same bytes to Go's decoder (vcffi) and to the Rust decoder inside the embedded wasm guest, and fails when they disagree.FuzzDifferentialValuerequires both sides to accept or reject the same value bytes. When both accept, Rust must re-encode the input exactly, and Go's re-encoding must match Rust's except for Go's documentedundefined→nullswap.FuzzDifferentialCipherTextrequires both sides to accept or reject the same ciphertext bytes. This target can only compare accept/reject, because fuzzed seals never authenticate.How, without a new guest export
vc_encryptandvc_decryptdecode their input before they look up the cipher handle. Called with handle 0, which is never issued, they returnErrEncodingif Rust rejects the bytes andErrBadHandleif it accepts them, with no AES work.TestUnissuedHandleReportsDecodingpins that order, and a comment in the guest marks it as relied on.0x05) carrying the input. The guest decodes the value and re-encodes it as a value passthrough (0xF2), with no AES and none of the cipher's rules about what may be sealed.My first version encrypted and then decrypted, as #383 proposed. The fuzzer broke it within seconds:
[passthrough(undefined)]is valid, but the cipher refuses to seal any non-empty container whose children are all passthrough. That refusal is deliberate (seeAesSeqCipher::end), so the encrypt step can't be used to read Rust's view of a value. The input is kept as a regression seed.Nightly
fuzz.ymlnow runs the four Go targets, each in its own directory. For the differential targets it rebuilds the guest from source first, so the nightly run compares Go with the Rust on the same commit rather than with the committed binary. That binary is rebuilt by hand and can lag its source. PR CI runs the new targets over their seeds against the committed binary, through the existinggo test ./...step.Throughput and results
Measured on an M-series laptop with 10 workers:
FuzzDifferentialValueFuzzDifferentialCipherTextEach target ran clean for 2.5 minutes after the fix described above. The existing 5-minute nightly default looks right.
Open points from #383
Separate observation (not changed here)
The refusal above is also reachable from the public Go API.
Encrypt(map[string]any{"id": vcvalue.Plain{V: int64(42)}})returnsErrInternal, the same error a guest panic gives, for what is an intentional refusal. A distinct error would be clearer, but that's an API change outside this PR.