Add TAP coverage for corrupt WAL summary and backup manifest files#76
Draft
NikolayS wants to merge 1 commit into
Draft
Add TAP coverage for corrupt WAL summary and backup manifest files#76NikolayS wants to merge 1 commit into
NikolayS wants to merge 1 commit into
Conversation
Commit ee65441 hardened the block reference table reader (src/common/blkreftable.c) with fork-number and chunk-size sanity checks after external fuzzing found that a corrupted file could index off the end of the fork-number or chunk-entry arrays and crash. No regression test exercised the "corrupt file in, clean error out" contract, so nothing guards against those checks (or the surrounding truncation/overflow handling) regressing. Add two deterministic mutation-based TAP tests. Both generate a real input file, apply fixed-offset/fixed-value mutations (no randomness), run the consuming tool on each mutant, and assert it exits nonzero with a clean error message and is never killed by a signal (crash / assertion failure / core dump) -- which command_fails_like() alone does not verify. pg_walsummary/t/003_corrupt.pl: produces a real WAL summary file (node with summarize_wal=on, DML, wait for pg_wal/summaries), parses its on-disk structure, and mutates it: truncation at structural boundaries (0 bytes, mid-magic, mid-header, mid-chunk, mid-CRC), bit flips in the magic/fork-number/limit-block fields, an oversized chunk-size value, and oversized/large nchunks counts. This directly targets the fields ee65441 validates (fork numbers, chunk sizes) plus the boundaries it does not (magic, CRC, EOF, plausible-but-too-large counts). pg_verifybackup/t/011_manifest_mutation.pl: takes a real base backup and mutates its full-sized backup_manifest at the byte level. The existing 005_bad_manifest.pl feeds hand-written tiny manifests to hit individual JSON parse errors, and 003_corruption.pl appends one byte to the manifest; neither truncates or bit-flips a real manifest. This adds truncation at fixed offsets, structural JSON bit flips at located anchor points, and bogus size/checksum fields, asserting a clean, signal-free failure in every case. No mutation produced a crash, confirming the reader rejects malformed input cleanly. Co-Authored-By: Claude Fable 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP
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
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.
What
Two new deterministic, mutation-based TAP tests that prove the "corrupt file in, clean error out" contract: generate a real input file, apply fixed-offset / fixed-value mutations (no randomness), run the consuming tool on each mutant, and assert it exits nonzero with a clean error message and is never killed by a signal (crash / assertion failure / core dump).
src/bin/pg_walsummary/t/003_corrupt.plsrc/bin/pg_verifybackup/t/011_manifest_mutation.plmeson.buildWhy
Commit
ee654419d5("Add additional sanity checks when reading a blkreftable") hardenedsrc/common/blkreftable.cwith fork-number and chunk-size validation after external fuzzing found that a corrupted file could index off the end of the fork-number or chunk-entry arrays and crash. No regression test exercised that path, so nothing guards those checks (or the surrounding truncation / overflow handling) against regressing. The mutations here target exactly the fieldsee654419d5validates (fork numbers, chunk sizes) plus the structural boundaries it does not (magic, CRC, EOF, plausible-but-too-large counts).Note:
command_fails_like()only checks nonzero exit + stderr — a signal death still counts as "fails". These tests capture$?directly and assert the low 7 bits (signal) are zero, so a crash is caught as a distinct failure.Existing vs. added coverage
pg_walsummary — previously only
001_basic.pl(help/version/usage) and002_blocks.pl(happy-path parsing of a valid summary). No corrupt-input coverage existed.003_corrupt.plis entirely new.Mutation matrix (parses the real file's on-disk layout, then mutates precise offsets):
ends unexpectedlywrong magic numberinvalid fork numberwrong checksumchunk N has invalid sizeoversized chunk size arrayends unexpectedlypg_verifybackup —
005_bad_manifest.plalready feeds hand-written tiny manifests to hit individual JSON parse / checksum / size errors, and003_corruption.plappends one byte to a real manifest (→ checksum mismatch) and swaps manifests. What was missing: truncating and bit-flipping a real, full-sized manifest at the byte level.011_manifest_mutation.pladds only those missing shapes:{→[, corrupt version key, corrupt file-object startcould not parse backup manifestmanifest checksum mismatchTest evidence
Built with
meson setup build --buildtype=debug -Dcassert=true -Dtap_tests=enabled(asserts on, so an out-of-bounds access would trap as SIGABRT).Neighbours confirmed still green:
pg_walsummary/001_basic,002_blocks,pg_verifybackup/005_bad_manifest.No mutation produced a crash or signal exit — every corrupt input was rejected cleanly, confirming the reader/parser hardening holds. No live bug found.
Closes #70
🤖 Generated with Claude Code
https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP
Generated by Claude Code