From 27ee61b223e13ee03afed8cd1f09494a10b9df50 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 1 Sep 2026 12:11:57 +0200 Subject: [PATCH 1/3] format: a tar.gz pads where Go can read it, which is not where it padded before O163, and a breaking change the owner asked for. Sizes do not move and every size that was reachable still is. What moves is where the padding sits inside the gzip header, and with it the bytes of every .tar.gz this tool writes. The fault was invisible for a month and each reason it stayed that way is worth naming, because none of them was carelessness. Go's compress/gzip reads a header comment into a fixed buffer: 511 bytes it takes, 512 it refuses. This format padded through that comment up to four kilobytes, so 4134 of the 11 260 reachable sizes produced an archive no Go program could open. The message it gives is "gzip: invalid header", which reads like a corrupt file rather than like a field the reader will not take, so anybody meeting it would suspect the fixture rather than the tool that made it. Testers write tools in Go. Every reference tool took those files. 7-Zip, GNU tar, bsdtar, Python and node accept a comment of ten megabytes without a word, so the oracle was green on all of them. The channel had been measured carefully in August and written up, and the measurement asked five readers - the language this is written in was not among them. That is the same shape as trusting a document instead of measuring: the question was right and the sample was one entry short. Both fixes named in the observation were measured and rejected. Capping the comment at what Go takes gives zero unreadable archives and takes 831 sizes out of reach, because a tar filler entry cannot be smaller than one 512 byte block and nothing bridges the gap under it. Trading 4134 unreadable files for 831 refusals is not a fix, and it would have looked like one in a summary. So the padding moved to the gzip extra field, which was measured the same day rather than assumed: Go takes 65 531 bytes of it, to the last one, beside a comment, at a cost of exactly two bytes more than it carries. Byte granular, where the filler entry is aligned to 512. The comment keeps the label and stretches by at most one byte, which is the one amount the extra field cannot do, so the pair together reach every size. After: 11 260 reachable, 0 unreachable, 0 unreadable. Reachability identical to before, which is the number that decided it. 7-Zip, GNU tar and Python were run on real output afterwards and all three open it. Three golden hashes moved, recorded through the remeasured list with the measurement, because editing them quietly is the single move that turns that guard into decoration. There is no way back to the old bytes and the changelog says so: the old bytes are the ones Go cannot read, so a switch for them would be a switch for the fault. TestEveryArchiveThisToolWritesCanBeReadByTheStandardLibrary now asks both containers, across the band where the padding channel changes shape, whether the standard library opens what we wrote. 1762 archives. It is the guard that would have caught this on the day it went in, and the reason it did not exist is that every check we had asked somebody else's reader. The sweep that priced all of this is tools/probes/targzpadding, and it asks two questions rather than one: which sizes are REACHABLE and which of those are READABLE. Run it before and after any change to a padding limit. Reachability is the one that gets worse quietly. validCost, commentOfCost and commentCapacity had no readers left afterwards and are gone. The measurement commentCapacity carried moved into the comment on the limit that still binds, because a fact does not stop being true when the constant holding it does. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 23 +++ internal/format/targz/size.go | 142 ++++++++++++------ internal/format/targz/targz.go | 77 ++++++---- internal/guard/archivereadable_test.go | 108 +++++++++++++ internal/guard/testdata/generator-golden.json | 21 ++- 5 files changed, 292 insertions(+), 79 deletions(-) create mode 100644 internal/guard/archivereadable_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index f14132a..7794ba8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,29 @@ because it turns other people's test suites red. ### Breaking +- **A generated `.tar.gz` has different bytes, because a lot of them could + not be opened by a Go program.** Sizes are unchanged, every size that + worked before still works, and every reader that took these files still + takes them. What moved is where the padding sits inside the header. + + The padding used to ride in the gzip header comment. Go's own + `compress/gzip` reads that field into a fixed buffer and refuses a comment + of 512 bytes or more, so **4134 of the 11 260 reachable sizes produced an + archive no Go program could open** - and the message it gives, + `gzip: invalid header`, reads like a corrupt file rather than like a field + the reader will not take. 7-Zip, GNU tar, bsdtar, Python and node all took + those files without a word, which is why it was not noticed sooner. + + The padding now rides in the gzip extra field, which Go reads to the end + of. After the change: 11 260 sizes reachable, none unreachable, none + unreadable. + + **There is no way back to the old bytes**, and that is the difference + between this and the other two entries here. The old bytes are the ones a + Go program cannot read, so keeping a switch for them would be keeping a + switch for the fault. A suite pinning `.tar.gz` hashes will go red once + and then stay green. + - **A generated log now advances through time, so its bytes are different.** Every entry used to carry the same instant. Ten thousand requests all landing at one moment is not a log anybody can test a time window, a rate alert or a diff --git a/internal/format/targz/size.go b/internal/format/targz/size.go index c981a1c..61c5d65 100644 --- a/internal/format/targz/size.go +++ b/internal/format/targz/size.go @@ -93,40 +93,68 @@ func commentCost(comment string) int64 { // archiveSize is the exact size of the archive described by m. func archiveSize(m memo) int64 { - return gzipFixed(tarLength(m)) + commentCost(m.comment) + return gzipFixed(tarLength(m)) + commentCost(m.comment) + extraCost(m.withExtra, m.extraLen) } -// maxCost is the largest comment cost we allow, from the limit on its length. -func maxCost() int64 { return commentPaddingLimit + 1 } - -// validCost says whether a comment of exactly this cost can be built. +// extraCost is what a gzip extra field of n bytes costs in the file. // -// There is a gap at one, and it is a property of the writer rather than of the -// format. A zero length comment with the flag set is legal and costs the one -// terminating byte - measured, and all five readers accept it - but Go leaves -// the flag off for an empty string, so the smallest comment it will emit is one -// character and costs two. That gap only bites without a label, because a label -// already costs more than that and grows a byte at a time from there. -func validCost(label string, cost int64) bool { - if label == "" { - return cost == 0 || cost >= 2 +// Measured rather than read off the specification: two bytes of length and +// then the bytes themselves, so an EMPTY field still costs two and no field +// at all costs nothing. That difference is the whole reason noExtra is not +// simply zero. +func extraCost(present bool, n int64) int64 { + if !present { + return 0 } - return cost >= int64(len(label))+1 + return n + 2 } -// commentOfCost builds a comment that costs exactly cost and starts with the -// label. +// maxCost is the largest comment cost we allow, from the limit on its length. +// maxCost is the most padding the header can take, across both channels. +// +// The comment reaches commentPaddingLimit beyond the label and the extra +// field reaches extraPaddingLimit plus its own two bytes. Anything past +// this needs a filler entry inside the tar. +func maxCost() int64 { return commentPaddingLimit + extraPaddingLimit + 2 } + +// place works out where a given number of padding bytes goes. +// +// Two channels, and which one is used is decided by size rather than by +// preference. The extra field is where bulk goes: it holds 65 531 bytes, it +// is byte granular, and Go reads all of it - which the comment does not, and +// that is O163. But it cannot cost one byte, because its own length field +// costs two, so a single byte of padding has nowhere to go there. // -// The cost has to have passed validCost, which is the same question asked -// before the decision to use one - both callers ask it. Repeating it here would -// be a third copy of one rule, and a copy is where two answers come from. What -// happens without it is a panic inside strings.Repeat rather than a wrong file, -// which is the failure this project prefers of the two. -func commentOfCost(label string, cost int64) string { - if cost == 0 { - return "" +// The comment closes that. It already carries the label, so lengthening it by +// one is free of any structural minimum, and one byte is exactly what the +// extra field cannot do. Without a label there is no comment to lengthen and +// the gap at one stays - which is the gap this format has always had, written +// up in MVP-FORMATS.md section 3.1 as a property rather than a fault. +// +// Returns false when the amount cannot be built at all, so the caller refuses +// rather than producing an archive of the wrong size. +func place(label string, padding int64) (comment string, withExtra bool, extraLen int64, ok bool) { + switch { + case padding < 0 || padding > maxCost(): + return "", false, 0, false + case padding == 0: + return label, false, 0, true + case padding == 1: + if label == "" { + // Nothing to lengthen, and neither channel starts at one. + return "", false, 0, false + } + return label + " ", false, 0, true + case padding <= extraPaddingLimit+2: + return label, true, padding - 2, true + } + // Past what the field holds, the comment takes the remainder. It is + // capped well under what Go reads, so the pair together stay readable. + rest := padding - (extraPaddingLimit + 2) + if label == "" || rest > commentPaddingLimit { + return "", false, 0, false } - return label + strings.Repeat(" ", int(cost)-1-len(label)) + return label + strings.Repeat(" ", int(rest)), true, extraPaddingLimit, true } // pad decides where the difference between the bare archive and the size that @@ -145,48 +173,66 @@ func pad(m *memo, p *format.Plan, target int64, label string, groups []format.Co } } - // What the comment would have to cost to land on the target on its own. - want := target - fixed + // What the header would have to carry to land on the target on its own. + // The label is already counted in bare, so this is padding and nothing + // else - which is the change O163 brought: the comment holds the label + // and the extra field holds the padding, rather than one field holding + // both and growing past what a Go reader will take. + want := target - bare if want <= maxCost() { - if !validCost(label, want) { + comment, withExtra, extra, ok := place(label, want) + if !ok { return &format.BelowMinimumError{ Format: "TAR.GZ", Requested: target, Minimum: bare + 2, - Reason: "the gzip comment that would make up the difference cannot be one byte long, so this size sits in a gap just above the smallest archive", - Hint: fmt.Sprintf("Ask for %d B or more, or keep the label on and any size from %d B works.", bare+2, bare), + Reason: "the padding that would make up the difference cannot be one byte long, " + + "so this size sits in a gap just above the smallest archive", + Hint: fmt.Sprintf("Ask for %d B or more, or keep the label on and any size from %d B works.", bare+2, bare), } } - m.comment = commentOfCost(label, want) + m.comment, m.withExtra, m.extraLen = comment, withExtra, extra return nil } // Above what the comment holds, the bulk goes into a stored entry inside // the tar. A tar entry is aligned to 512 bytes, so it never lands on an // exact size by itself - the comment closes the last few bytes. - size, comment, ok := solveFiller(tarLength(*m), target, label) + size, header, ok := solveFiller(tarLength(*m), target, label) if !ok { return fmt.Errorf("targz: no arrangement of padding reaches exactly %d B", target) } m.withFiller = true m.fillerSize = size - m.comment = comment + m.comment = header.comment + m.withExtra = header.withExtra + m.extraLen = header.extraLen p.Properties["padding_entry"] = fillerName return nil } -// solveFiller picks how big the padding entry is and what the comment says. +// headerPadding is where a solved arrangement puts the bytes the filler +// entry could not carry. A record rather than three return values, because +// three of them in a row is where a reader stops being able to tell which is +// which. +type headerPadding struct { + comment string + withExtra bool + extraLen int64 +} + +// solveFiller picks how big the padding entry is and where the rest goes. // // The padding entry is a whole number of tar blocks, so it moves the total in // steps of 512. The largest step that still fits is taken first, and whatever -// is left over becomes comment. Stepping back down by a block adds 512 to that -// leftover, which is how the one byte gap is stepped over when there is no -// label. -func solveFiller(base, target int64, label string) (size int64, comment string, ok bool) { +// is left over goes into the header. Stepping back down by a block adds 512 to +// that leftover, which is how the one byte gap is stepped over when there is +// no label to lengthen. +func solveFiller(base, target int64, label string) (size int64, header headerPadding, ok bool) { minCost := commentCost(label) withHeader := base + tarBlock if gzipFixed(withHeader)+minCost > target { - return 0, "", false + return 0, headerPadding{}, false } lo, hi := int64(0), target/tarBlock+2 @@ -203,12 +249,14 @@ func solveFiller(base, target int64, label string) (size int64, comment string, // the comment holds several thousand. for k := lo; k >= 0 && k > lo-3; k-- { blocks := k * tarBlock - cost := target - gzipFixed(withHeader+blocks) - if validCost(label, cost) && cost <= maxCost() { - return blocks, commentOfCost(label, cost), true + // What is left for the header once the filler has taken its blocks. + // minCost is the label, which the comment carries either way. + padding := target - gzipFixed(withHeader+blocks) - minCost + if comment, withExtra, extra, ok := place(label, padding); ok { + return blocks, headerPadding{comment: comment, withExtra: withExtra, extraLen: extra}, true } } - return 0, "", false + return 0, headerPadding{}, false } // build writes the archive. @@ -223,6 +271,12 @@ func build(ctx context.Context, w io.Writer, m memo) error { return fmt.Errorf("targz: the archive could not be started: %w", err) } zw.Comment = m.comment + // The extra field is where padding rides. It precedes everything, like + // the comment, so how long it is has to be settled before the first byte + // goes out - which is what the arithmetic above is for. + if m.withExtra { + zw.Extra = make([]byte, m.extraLen) + } tw := tar.NewWriter(zw) for _, c := range m.children { diff --git a/internal/format/targz/targz.go b/internal/format/targz/targz.go index 7a5f1a6..133ae70 100644 --- a/internal/format/targz/targz.go +++ b/internal/format/targz/targz.go @@ -21,37 +21,43 @@ import ( const ( generatorVersion = "1" - // commentCapacity is the largest gzip header comment any reader here - // accepts. RFC 1952 gives the field no length at all, and the document - // carried that as "no limit" until it was measured on 2026-08-04. + // commentPaddingLimit is how much of the comment padding may use. // - // Measured, by bisection, on two builds of the same archiver: + // What every OTHER reader takes is far more than this and was measured by + // bisection on 2026-08-04: 7-Zip 26.02 and p7zip 23.01 both accept 65 535 + // and refuse 65 536, while GNU gzip, GNU tar, bsdtar, Python and node take + // ten megabytes without a word. None of that is the binding number any + // more, so it is written here rather than kept as a constant nothing uses. // - // 7-Zip 26.02 on Windows accepts 65 535, refuses 65 536 - // p7zip 23.01 on Linux accepts 65 535, refuses 65 536 + // It was 4096 until 2026-09-01, and that number could not be read by a + // very ordinary reader. Go's compress/gzip takes a header comment of 511 + // bytes and refuses 512, because it reads the field into a fixed buffer - + // so 4134 of the 11 260 reachable sizes in a twenty kilobyte sweep produced + // an archive no Go program could open, with a message that reads like a + // corrupt file. 7-Zip, GNU tar, bsdtar, Python and node all took them + // without a word, which is why nobody noticed for a month. O163. // - // The refusal is "Is not archive" on the whole file rather than a warning, - // so a fixture past that line is not degraded, it is unreadable. GNU gzip - // 1.12, GNU tar 1.35, bsdtar 3.8.4, Python and node take ten megabytes - // without a word, and bsdtar draws its own line at 1 048 566 - one mebibyte - // less the fixed ten byte header, so that one caps the whole header rather - // than this field. - commentCapacity = 65535 - - // commentPaddingLimit is how much of that we actually use. + // The comment now carries the label and at most a byte or two beyond it. + // Bulk padding moved to the extra field, which holds sixteen times more and + // which Go reads to the end of. + commentPaddingLimit = 480 + + // extraPaddingLimit is how many bytes the gzip extra field may carry. // - // Not the measured ceiling, and the reason is local rather than borrowed. - // The second stage below pads through a tar entry, and a tar entry is - // aligned to 512 bytes, so it delivers bulk and never the last few bytes. - // The comment is what reaches an exact size, and a few kilobytes of it is - // more than enough for that - the gap it has to close is under one block. + // The field's own limit is 65 535, since XLEN is two bytes wide. This build + // stops at 65 531, which is the number measured across every reader on + // 2026-08-04 and re-measured against Go on 2026-09-01 - accepted to the + // last byte, beside a comment, with the cost exactly two bytes more than + // what is carried. // - // Which leaves no reason to sit on the edge of a two byte length field, and - // one reason not to: that edge is where a known bad build of p7zip crashed - // on a ZIP comment filled to its maximum. That build is not installed on - // any machine here, so this is caution about something unmeasured rather - // than a measurement, and it costs nothing. - commentPaddingLimit = 4096 + // That cost is what makes this the right channel rather than a bigger one: + // it is byte granular. The comment was too, but it had to hold the label as + // well, and a tar filler entry is aligned to 512 bytes so it delivers bulk + // and never the last few. Capping the comment at what Go reads and leaving + // the rest to the filler was measured and rejected: 831 sizes in the same + // sweep stopped being reachable at all, because nothing could bridge the + // gap under one block. + extraPaddingLimit = 65531 // fillerName is the entry that carries padding the comment cannot hold. // Named plainly, because somebody opening the archive should see what it @@ -91,13 +97,18 @@ func init() { MinBytes: minimumBytes(), Padding: format.PaddingChannel{ - Name: "gzip header comment, then a stored filler entry above its limit", + Name: "gzip extra field, then a stored filler entry above its limit", Where: format.PlacementStart, - // The comment precedes everything, so how long it is has to be + // The extra field precedes everything, so how long it is has to be // settled before the first byte goes out. That is the whole reason // the size is worked out by arithmetic rather than by measuring // what came before. - Capacity: commentCapacity, + // + // It was the comment until 2026-09-01, and the comment is still + // here carrying the label. What moved is the padding, because Go + // reads a comment of 511 bytes and refuses 512 while it reads this + // field to the end. O163. + Capacity: extraPaddingLimit, }, Label: format.LabelInternal, Oracle: "7z", @@ -153,6 +164,14 @@ type memo struct { // The zero value is not the default - ReadOwnership fills it, because // the mode this format has always written is 644 rather than 0. own archive.Ownership + // withExtra says whether the header carries a gzip extra field at all, + // and extraLen says how many bytes it holds. Two fields rather than one + // with a sentinel, matching withFiller beside them, because an EMPTY + // field still costs its two byte length while no field costs nothing - + // and a sentinel makes the zero value of this struct wrong, which is a + // thing minimumBytes met on the first try. + withExtra bool + extraLen int64 } func (generator) Plan(r format.Request) (format.Plan, error) { diff --git a/internal/guard/archivereadable_test.go b/internal/guard/archivereadable_test.go new file mode 100644 index 0000000..de158ca --- /dev/null +++ b/internal/guard/archivereadable_test.go @@ -0,0 +1,108 @@ +package guard + +import ( + stdzip "archive/zip" + "bytes" + "compress/gzip" + "context" + "testing" + + "github.com/donislawdev/TestingFilesGenerator/internal/format" + _ "github.com/donislawdev/TestingFilesGenerator/internal/format/all" +) + +// Every archive this tool writes opens in the standard library of the language +// it is written in. +// +// This guard exists because that was not true for a month and nothing said so. +// TAR.GZ padded through the gzip header comment, up to four kilobytes of it, and +// Go's compress/gzip reads that field into a fixed buffer: it takes 511 bytes +// and refuses 512. Measured on 2026-09-01 across 11 260 reachable sizes, 4 134 +// of them produced an archive no Go program could open. O163. +// +// Three things made it invisible, and each one is a lesson this guard is the +// answer to. +// +// The reference tools all accepted the files. 7-Zip, GNU tar, bsdtar, Python +// and node take a comment of ten megabytes without a word, so the oracle was +// green on every one of them. An oracle says "would a real reader take this", +// and the answer was yes - for every reader anybody had thought to ask. +// +// The padding channel had been measured, carefully, and written up. The +// measurement asked five readers and Go was not among them, which is the same +// shape of miss as trusting a document instead of measuring: the question was +// right and the sample was short one entry. +// +// And the failure was silent in the worst way. compress/gzip says +// "gzip: invalid header", which reads like a corrupt file rather than like a +// field this reader will not take, so somebody meeting it would suspect the +// fixture rather than the tool that made it. Testers write tools in Go. +// +// Asked of every container and every size across the band where the padding +// channel changes shape, because the fault was in a band rather than at a +// point - the sizes just above where the header can no longer hold it all. +func TestEveryArchiveThisToolWritesCanBeReadByTheStandardLibrary(t *testing.T) { + checked := 0 + for _, d := range format.All() { + if !d.Container { + continue + } + read, ok := standardReaderFor(d.ID) + if !ok { + t.Errorf("%s is a container and nothing here knows how to open it with the standard "+ + "library, so this guard skips it silently - which is how the fault it was "+ + "written for survived", d.ID) + continue + } + // From the floor upwards, far enough past it that the header channel + // fills and the run has to reach for a filler entry. + for size := d.MinBytes; size <= d.MinBytes+20000; size += 13 { + plan, err := d.Generator.Plan(format.Request{Bytes: size, Seed: 7741, Label: true}) + if err != nil { + // Below the floor, or one of the sizes this format says it + // cannot reach. Neither is the subject. + continue + } + var buf bytes.Buffer + if err := d.Generator.Write(context.Background(), &buf, plan); err != nil { + t.Fatalf("%s at %d B could not be written: %v", d.ID, size, err) + } + checked++ + if err := read(buf.Bytes()); err != nil { + t.Fatalf("%s at %d B cannot be opened by the standard library: %v\n"+ + " every reference tool would take this file, so nothing else here would have said so", + d.ID, size, err) + } + } + } + + if checked == 0 { + t.Fatal("no archive was read back, so this proved nothing") + } + t.Logf("%d archives opened with the standard library", checked) +} + +// standardReaderFor is how the standard library opens one container, or false +// when this guard has no way to. +// +// A table rather than a switch inside the loop, so a container arriving without +// an entry is reported rather than skipped. A guard that quietly covers less +// than it says is the shape of the fault this file exists for. +func standardReaderFor(id string) (func([]byte) error, bool) { + switch id { + case "targz": + return func(b []byte) error { + r, err := gzip.NewReader(bytes.NewReader(b)) + if err != nil { + return err + } + return r.Close() + }, true + case "zip": + return func(b []byte) error { + _, err := stdzip.NewReader(bytes.NewReader(b), int64(len(b))) + return err + }, true + } + return nil, false +} diff --git a/internal/guard/testdata/generator-golden.json b/internal/guard/testdata/generator-golden.json index 57276e7..2b9c963 100644 --- a/internal/guard/testdata/generator-golden.json +++ b/internal/guard/testdata/generator-golden.json @@ -158,18 +158,18 @@ }, "targz_16kib": { "bytes": 16384, - "sha256": "03bba423e483a3606a8d43e40ea24e98356e1e07ef474c9178cb5cdf084384a6", - "measured_on": "2026-08-04" + "sha256": "47304109c42a00e600fb25cfa25bc218140a7bb5b3914b5310cfbb3874366baa", + "measured_on": "2026-09-01" }, "targz_past_the_comment_limit": { "bytes": 262144, - "sha256": "0031527861ad2753e25ab242a8415fdf1c027f0be377de948894778caa26e0eb", - "measured_on": "2026-08-04" + "sha256": "8b20c23fb1e2ed4dae11755ef7f356a65377a83b3323c7be20bfafa42f5511d2", + "measured_on": "2026-09-01" }, "targz_with_three_pdfs": { "bytes": 65536, - "sha256": "84a776cc5439d9a7edb29018dca7b0fb1462f7b57a374de34c21b0cf7f601eb5", - "measured_on": "2026-08-04" + "sha256": "01969ba9c8368dbc8af3810291cf37468359b1b6b2ba9bf224c1b72cde19c612", + "measured_on": "2026-09-01" }, "tiff_100kib_sized_to_fit": { "bytes": 102400, @@ -245,6 +245,15 @@ } }, "remeasured": [ + { + "on": "2026-09-01", + "why": "TAR.GZ pads through the gzip EXTRA field now, where it used to pad through the header comment. The comment keeps the label and at most a byte beyond it. Sizes are unchanged - every one of these archives is the same length it was, and the same sizes are reachable, measured across 11 260 of them. What moved is where the padding sits in the header. The reason is O163: Go's compress/gzip reads a header comment into a fixed buffer and refuses one of 512 bytes or more, so 4 134 of those 11 260 sizes produced an archive no Go program could open - with the message gzip: invalid header, which reads like a corrupt file rather than like a field the reader will not take. 7-Zip, GNU tar, bsdtar, Python and node all took them without a word, which is why it went a month unnoticed, and testers write tools in Go. The extra field was measured against Go on the same day and accepted to 65 531 bytes beside a comment, at a cost of exactly two bytes more than it carries, so it is byte granular where a tar filler entry is aligned to 512. The cheaper looking fix - capping the comment at 511 and leaving the rest to the filler - was measured and rejected: 831 sizes in the same sweep stopped being reachable at all, because nothing could bridge the gap under one block. After the change: 11 260 reachable, 0 unreachable, 0 unreadable. Guarded from now on by TestEveryArchiveThisToolWritesCanBeReadByTheStandardLibrary.", + "files": [ + "targz_16kib", + "targz_past_the_comment_limit", + "targz_with_three_pdfs" + ] + }, { "on": "2026-08-04", "why": "The files inside a container now default to 8 kB rather than 4 kB, which is what the registry always said they did. entry_size was declared as 8kb and the generator used 4096, so tfg formats printed one answer and generating without the setting gave the other - neither number wrong on its own, written in two places, and one of them drifted. The declaration is the half consumers believe, because AR9 makes the registry the place a consumer asks what a format accepts, so the generator was moved to the declaration rather than the other way round. Both values now come from one constant in each container, and the two containers use the same one, because two settings of the same name defaulting differently is a difference nobody would predict. Only the four cases that leave entry_size unset moved. Sizes are unchanged - the archives hold the same number of files and those files are bigger. Guarded from now on by TestADeclaredDefaultIsTheOneTheFormatUses, which generates with each declared default left out and written out and compares the bytes.", From e3dc83e32682362d160b32ffd8c50cfb16fcbb5b Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 1 Sep 2026 12:49:41 +0200 Subject: [PATCH 2/3] format: a zip can be locked with ZipCrypto, which is the lock worth having for what it breaks tfg generate --format zip --size 30kb --set entries=3 \ --set password=Secret123 --set encryption=zipcrypto It is here for what it does to a reader rather than for what it protects. Measured on our own output: .NET's ZipFile opens one of these, reports the entry at its true length of 8192, hands back a stream and fills it with ff c7 04 3e where the file holds "tfg - txt". It never says the entry was encrypted at all, so an application built on that library processes noise and calls it data. AES in the same library throws, which is the safer defect and the less interesting one. This is the fixture PRESETS.md section 4.12 has been describing since before there was anything to build it with. The cipher is not ours and it was not taken on trust. It went into tools/probes/zipcrypto BEFORE it went into the generator, pointed at an archive 7-Zip had written, and asked to decrypt it - check byte, plaintext CRC and the bytes themselves all came back right. That order was chosen because of what the AES work had shown a few hours earlier: a mutation proved the archiver guard could not see a keystream running backwards, since an AE-2 entry signs its ciphertext rather than its contents, so a file of noise passes every check a reader makes. A defect got through anyway, and the probe is what diagnosed it. Every locked entry declared method 99 - WinZip AES - while a ZipCrypto entry has to stay stored, because it changes nothing about how the bytes sit and only puts twelve in front of them. 7-Zip reported "Data Error in encrypted file. Wrong password?" which points at the password, the one thing that was right. The probe decrypted the same file perfectly, and that is what moved the suspicion off the cryptography and onto the header. So there is a guard for the header shape now, and it asks both schemes in both directions. ZipCrypto stores, carries the real plaintext CRC and no extra field. AES declares 99, carries a CRC of nought and a 0x9901 field. Getting either backwards produces a file that opens and then fails on something that sounds like the user's fault. The cost is named rather than hidden: ZipCrypto puts the high byte of the plaintext CRC in its header, so the contents have to be known before the first byte of the entry goes out - and the contents arrive as a stream. Each entry is therefore generated twice, once to be checksummed and once to be encrypted. Twice the processor and not a byte more memory, because buffering the entry would break the guard that says a generator does not hold a whole file. Three guards stopped carrying a hand written list of methods and read the registry instead. A list in a test is one somebody has to remember on the day a fourth scheme arrives, and this was that day. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 ++ internal/format/archive/archive.go | 8 +- internal/format/archive/lock.go | 52 +++++++++- internal/format/archive/zipcrypto.go | 126 ++++++++++++++++++++++++ internal/format/zip/zip.go | 63 ++++++++++-- internal/guard/archivelock_test.go | 138 +++++++++++++++++++++++---- web/public/formats/index.html | 2 +- web/public/pl/formaty/index.html | 2 +- 8 files changed, 364 insertions(+), 35 deletions(-) create mode 100644 internal/format/archive/zipcrypto.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 7794ba8..68b8e0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -70,6 +70,14 @@ because it turns other people's test suites red. ### Added +- **A zip can be locked with ZipCrypto, the old scheme.** `--set encryption=zipcrypto`. + + It is here for what it does to a reader rather than for what it protects. Measured: .NET's own `ZipFile` opens one of these, reports the entry at its true length, hands back a stream and fills it with the ENCRYPTED bytes - and never says the entry was encrypted at all. An application built on that library processes noise and calls it data. AES fails loudly in the same library, which is the safer defect and the less interesting one. + + So this is the fixture for finding out whether something in a pipeline waves an encrypted archive through. + + **It is not protection and it is not offered as any.** ZipCrypto has been broken for decades. Use `aes-256` when the point is that the contents are hard to read. + - **A zip can be locked with a password.** tfg generate --format zip --size 30kb --set entries=3 \ diff --git a/internal/format/archive/archive.go b/internal/format/archive/archive.go index 90458ea..9ae8101 100644 --- a/internal/format/archive/archive.go +++ b/internal/format/archive/archive.go @@ -61,6 +61,7 @@ const ( // The encryption methods, spelled the way a recipe writes them. const ( NoEncryption = "none" + ZipCrypto = "zipcrypto" AES128 = "aes-128" AES192 = "aes-192" AES256 = "aes-256" @@ -162,10 +163,11 @@ var axes = map[string]format.Property{ Encryption: { Name: Encryption, Kind: format.PropertyChoice, // Sorted, because a closed set has one order on every surface. - Choices: []string{AES128, AES192, AES256, NoEncryption}, + Choices: []string{AES128, AES192, AES256, NoEncryption, ZipCrypto}, Default: NoEncryption, - Detail: "How the archive is locked. This is the WinZip AES scheme, which 7-Zip and WinZip open " + - "and some other readers cannot open at all - .NET lists the files and then fails on reading one.", + Detail: "How the archive is locked. AES is the WinZip scheme, and some readers cannot open it " + + "at all. ZipCrypto is the old one every reader opens and nothing modern trusts, and some " + + "of them hand back the encrypted bytes without saying so.", }, } diff --git a/internal/format/archive/lock.go b/internal/format/archive/lock.go index afa77cc..a34d074 100644 --- a/internal/format/archive/lock.go +++ b/internal/format/archive/lock.go @@ -41,6 +41,12 @@ const ( authLen = 10 iterations = 1000 + // zipStore is a stored entry, and winZipAES is the number an AES entry + // declares instead. The second is not a compression at all: the real method + // sits in the 0x9901 field beside it, and this build always stores. + zipStore = 0 + winZipAES = 99 + // aesExtraLen is the 0x9901 field: two bytes of id, two of length and // seven of body. It is written into the local header AND the central // directory, so an entry pays for it twice. @@ -64,6 +70,22 @@ type Lock struct { // On says whether anything is encrypted. func (l Lock) On() bool { return l.Method != "" && l.Method != NoEncryption } +// NeedsPlaintextCRC says whether an entry cannot be started until its +// contents are known. +// +// True for ZipCrypto and false for AES, and the difference is measured rather +// than assumed. Read out of what 7-Zip writes: a ZipCrypto entry carries the +// real CRC of the plaintext and puts its high byte in the header, so a reader +// can reject a wrong password without decrypting anything. An AE-2 entry +// carries a CRC of zero, because its authentication code does that job. +// +// The cost falls on the caller and it is real: an entry that needs this is +// generated twice, once to be counted and once to be encrypted. Twice the +// processor and not a byte more memory, which is the trade this project takes +// every time - holding the file to hash it would break the guard that says a +// generator does not. +func (l Lock) NeedsPlaintextCRC() bool { return l.Method == ZipCrypto } + // keyLen is the AES key in bytes, and the salt is half of it. Zero for an // archive that is not locked with AES. func (l Lock) keyLen() int { @@ -98,6 +120,7 @@ func (l Lock) strength() byte { // Measured, and it agrees from two directions - the size of the whole file and // the compressed size field in the local header: // +// ZipCrypto +12 (eleven bytes that vary and one check byte) // AES-128 +20 (8 salt, 2 verifier, 10 authentication) // AES-192 +24 // AES-256 +28 @@ -106,12 +129,34 @@ func (l Lock) strength() byte { // headers are written for real during the counting pass, so the writer counts // those eleven bytes twice over on its own. func (l Lock) EntryOverhead() int64 { - if !l.On() { + switch { + case !l.On(): return 0 + case l.Method == ZipCrypto: + return zipCryptoHeader } return int64(l.saltLen() + pwvLen + authLen) } +// ZipMethod is the compression method the entry declares. Named for what it +// answers rather than for the field it reads, because Method is that field. +// +// AES entries declare 99, which is not a compression at all - the real +// method sits in the 0x9901 field beside it. ZipCrypto declares what it +// really is, because it changes nothing about how the bytes are stored, it +// only puts twelve bytes in front of them and scrambles what follows. +// +// Getting this wrong is quiet. An entry declaring 99 with no 0x9901 field +// beside it gets past the check byte and fails on the checksum, and 7-Zip +// reports "Data Error in encrypted file. Wrong password?" - which points at +// the password, the one thing that was right. +func (l Lock) ZipMethod() uint16 { + if l.keyLen() == 0 { + return zipStore + } + return winZipAES +} + // Extra is the 0x9901 field an AES entry carries, and nil for anything else. func (l Lock) Extra() []byte { if l.keyLen() == 0 { @@ -185,7 +230,10 @@ func ReadLock(id string, props map[string]string) (Lock, error) { // would give two runs of one recipe different bytes, which is untouchable rule // 3 - and the same recipe producing the same file is more of the product here // than the encryption is. -func (l Lock) NewEntryWriter(w io.Writer, seed uint64, index int) (io.WriteCloser, error) { +func (l Lock) NewEntryWriter(w io.Writer, seed uint64, index int, crc uint32) (io.WriteCloser, error) { + if l.Method == ZipCrypto { + return l.newZipCryptoWriter(w, seed, index, crc) + } if l.keyLen() == 0 { return nil, fmt.Errorf("archive: %q is not an encryption this build can write", l.Method) } diff --git a/internal/format/archive/zipcrypto.go b/internal/format/archive/zipcrypto.go new file mode 100644 index 0000000..e9684af --- /dev/null +++ b/internal/format/archive/zipcrypto.go @@ -0,0 +1,126 @@ +package archive + +import ( + "hash/crc32" + "io" + + "github.com/donislawdev/TestingFilesGenerator/internal/core" +) + +// The old ZIP encryption, the one everything opens and nothing modern trusts. +// +// It is here for what it does to a reader rather than for what it protects. +// Measured on 2026-09-01: .NET's own ZipFile opens a ZipCrypto archive, reports +// the entry at its true length, hands back a stream and fills it with +// CIPHERTEXT - 'ea 59 89 8e' where the file holds 'AAAA' - and never says the +// entry was encrypted at all. An application built on that library processes +// noise and calls it data. AES fails loudly in the same library, which is the +// safer defect and the less interesting one. +// +// So this is the fixture that finds a real class of fault, and it is the one +// the presets have been waiting for: PRESETS.md section 4.12 names "an archive +// with a password waved through without warning" as a thing to catch. +// +// The cipher itself is not ours and was not taken on trust. It was written into +// a probe first, pointed at an archive 7-Zip produced, and asked to decrypt it - +// the check byte, the plaintext CRC and the bytes themselves all came back +// right. tools/probes/zipcrypto, and it stays runnable, because a keystream +// that is subtly wrong produces a file some readers still accept. + +const ( + // The three keys PKWARE starts from and the multiplier it steps them with. + // Constants of the scheme rather than choices of ours. + key0Init = 305419896 + key1Init = 591751049 + key2Init = 878082192 + multiplier = 134775813 + + // zipCryptoHeader is the twelve bytes that precede an entry's data: eleven + // that vary and one that lets a reader reject a wrong password without + // decrypting anything else. It is the whole of what this scheme adds, which + // is why the overhead is a constant. + zipCryptoHeader = 12 +) + +var crcTable = crc32.MakeTable(crc32.IEEE) + +// pkware is the stream cipher, keyed by a password and then by every byte of +// plaintext that passes through it. +// +// The plaintext updates the keys in both directions, which is why encrypting +// and decrypting are two functions here rather than one - a stream cipher that +// is its own inverse would not need the distinction, and this one is not. +type pkware struct{ k0, k1, k2 uint32 } + +func newPKWARE(password string) *pkware { + c := &pkware{k0: key0Init, k1: key1Init, k2: key2Init} + for i := 0; i < len(password); i++ { + c.update(password[i]) + } + return c +} + +func (c *pkware) update(p byte) { + c.k0 = crcTable[(c.k0^uint32(p))&0xff] ^ (c.k0 >> 8) + c.k1 += c.k0 & 0xff + c.k1 = c.k1*multiplier + 1 + c.k2 = crcTable[(c.k2^(c.k1>>24))&0xff] ^ (c.k2 >> 8) +} + +func (c *pkware) keyByte() byte { + t := uint16(c.k2|2) & 0xffff + return byte((t * (t ^ 1)) >> 8) +} + +func (c *pkware) encrypt(p byte) byte { + x := p ^ c.keyByte() + c.update(p) + return x +} + +// zipCryptoWriter encrypts on the way through, header first. +type zipCryptoWriter struct { + out io.Writer + c *pkware +} + +// newZipCryptoWriter starts an entry, writing the twelve byte header before +// anything else. +// +// The eleven bytes that vary come from the run seed and never from crypto/rand, +// for the reason every other draw in this tool avoids it: two runs of one +// recipe have to give one file. The twelfth is the high byte of the plaintext +// CRC, and it is why this scheme needs the contents known before the first byte +// goes out - the whole reason a ZipCrypto entry is generated twice. +func (l Lock) newZipCryptoWriter(w io.Writer, seed uint64, index int, crc uint32) (io.WriteCloser, error) { + c := newPKWARE(l.Password) + rng := core.NewRand(core.FileSeed(seed, index)) + + header := make([]byte, zipCryptoHeader) + for i := range header[:zipCryptoHeader-1] { + header[i] = byte(rng.Uint32()) + } + header[zipCryptoHeader-1] = byte(crc >> 24) + for i := range header { + header[i] = c.encrypt(header[i]) + } + if _, err := w.Write(header); err != nil { + return nil, err + } + return &zipCryptoWriter{out: w, c: c}, nil +} + +func (z *zipCryptoWriter) Write(p []byte) (int, error) { + out := make([]byte, len(p)) + for i := range p { + out[i] = z.c.encrypt(p[i]) + } + if _, err := z.out.Write(out); err != nil { + return 0, err + } + return len(p), nil +} + +// Close has nothing to finish. ZipCrypto signs nothing - the entry's own CRC is +// what a reader checks, after decrypting, if it checks at all. +func (z *zipCryptoWriter) Close() error { return nil } diff --git a/internal/format/zip/zip.go b/internal/format/zip/zip.go index 160fbf0..08f939a 100644 --- a/internal/format/zip/zip.go +++ b/internal/format/zip/zip.go @@ -9,6 +9,7 @@ import ( stdzip "archive/zip" "context" "fmt" + "hash/crc32" "io" "strings" "time" @@ -48,11 +49,6 @@ const ( fillerName = "tfg-padding.bin" writeChunk = 32 * 1024 - - // winZipAES is the compression method a locked entry declares. It is - // not a compression at all - the real method sits in the 0x9901 field - // and is store, like everything else here. - winZipAES = 99 ) // A fixed timestamp on every entry. Taking one from the clock would make two @@ -414,6 +410,11 @@ type entryPlan struct { index int // withContents is false during the pass that only measures. withContents bool + // crc is the checksum of the contents, and it is only ever filled for a + // lock that needs the contents known before the first byte. Zero + // otherwise, which is what an AE-2 entry carries anyway and what the + // counting pass writes into a header nobody reads. + crc uint32 } // openEntry starts the next entry and gives back what its contents go to. @@ -447,14 +448,14 @@ func openEntry(zw *stdzip.Writer, m memo, e entryPlan) (io.Writer, func() error, h := &stdzip.FileHeader{ Name: e.name, - Method: winZipAES, + Method: m.lock.ZipMethod(), Modified: fixedTime, Extra: m.lock.Extra(), } // Bit 0 says the entry is encrypted. The CRC stays zero because AE-2 // carries none - which is what lets the contents be written in one pass. h.Flags |= 1 - h.CRC32 = 0 + h.CRC32 = e.crc h.CompressedSize64 = uint64(e.plain + m.lock.EntryOverhead()) h.UncompressedSize64 = uint64(e.plain) @@ -470,13 +471,42 @@ func openEntry(zw *stdzip.Writer, m memo, e entryPlan) (io.Writer, func() error, // which is what the engine caught when this was written the other way. return raw, nothingToShut, nil } - locked, err := m.lock.NewEntryWriter(raw, m.seed, e.index) + locked, err := m.lock.NewEntryWriter(raw, m.seed, e.index, e.crc) if err != nil { return nil, nil, err } return locked, locked.Close, nil } +// plaintextCRC is the checksum of what an entry is about to hold, worked out +// by generating it once and throwing the bytes away. +// +// Only ZipCrypto asks for this, and only when the contents are really being +// written. That scheme puts the high byte of the plaintext CRC in the twelve +// byte header it prepends, so a reader can turn away a wrong password without +// decrypting anything - which means the checksum has to be known before the +// first byte of the entry goes out, and the contents arrive as a stream. +// +// Generating twice rather than buffering, and that is the trade taken +// deliberately. Holding the entry to hash it would break the guard that says +// a generator does not keep a whole file in memory, and the second pass is +// free of risk because a generator producing different bytes on two calls in +// one process is itself a guarded impossibility. It costs processor time on +// locked archives and nothing at all on open ones. +func plaintextCRC(ctx context.Context, m memo, withContents bool, write func(io.Writer) error) (uint32, error) { + if !withContents || !m.lock.NeedsPlaintextCRC() { + return 0, nil + } + sum := crc32.NewIEEE() + if err := write(sum); err != nil { + return 0, err + } + if err := ctx.Err(); err != nil { + return 0, err + } + return sum.Sum32(), nil +} + // build writes the archive. // // withContents says whether the files inside are actually generated. The @@ -500,8 +530,14 @@ func build(ctx context.Context, w io.Writer, m memo, withContents bool) error { default: } + crc, err := plaintextCRC(ctx, m, withContents, func(w io.Writer) error { + return c.desc.Generator.Write(ctx, w, c.plan) + }) + if err != nil { + return fmt.Errorf("zip: the %s file inside could not be checksummed: %w", c.desc.ID, err) + } entry, shut, err := openEntry(zw, m, entryPlan{ - name: c.name, plain: c.plan.Bytes, index: i, withContents: withContents, + name: c.name, plain: c.plan.Bytes, index: i, withContents: withContents, crc: crc, }) if err != nil { return err @@ -520,8 +556,15 @@ func build(ctx context.Context, w io.Writer, m memo, withContents bool) error { // The filler is locked with everything else. An archive where one entry // opens without the password and the rest do not is a file nobody // asked for, and the arithmetic is the same either way. + crc, err := plaintextCRC(ctx, m, withContents, func(w io.Writer) error { + return writeFiller(ctx, w, m.seed, m.fillerSize) + }) + if err != nil { + return err + } entry, shut, err := openEntry(zw, m, entryPlan{ - name: fillerName, plain: m.fillerSize, index: len(m.children), withContents: withContents, + name: fillerName, plain: m.fillerSize, index: len(m.children), + withContents: withContents, crc: crc, }) if err != nil { return err diff --git a/internal/guard/archivelock_test.go b/internal/guard/archivelock_test.go index 33da77a..2d0a9f0 100644 --- a/internal/guard/archivelock_test.go +++ b/internal/guard/archivelock_test.go @@ -25,9 +25,34 @@ import ( // 2.16. Repeated here on purpose: a guard that reads its expectation out of the // code it is guarding proves the code agrees with itself. var lockOverhead = map[string]int64{ - archive.AES128: 20, - archive.AES192: 24, - archive.AES256: 28, + archive.ZipCrypto: 12, + archive.AES128: 20, + archive.AES192: 24, + archive.AES256: 28, +} + +// locksOffered is every encryption a format declares, apart from none. +// +// Read out of the registry rather than written out here, because a list in a +// test is a list somebody has to remember on the day a fourth scheme arrives - +// and three guards carried the same one until ZipCrypto was that day. +func locksOffered(t *testing.T, id string) []string { + t.Helper() + var out []string + for _, p := range descriptorFor(t, id).Properties { + if p.Name != archive.Encryption { + continue + } + for _, method := range p.Choices { + if method != archive.NoEncryption { + out = append(out, method) + } + } + } + if len(out) == 0 { + t.Fatalf("%s offers no encryption, so the guard asking about them proves nothing", id) + } + return out } // A locked archive is one a real archiver opens with the password and refuses @@ -54,7 +79,7 @@ func TestARealArchiverOpensALockedArchiveAndRefusesTheWrongPassword(t *testing.T const password = "Secret123" dir := t.TempDir() - for _, method := range []string{archive.AES128, archive.AES192, archive.AES256} { + for _, method := range locksOffered(t, "zip") { t.Run(method, func(t *testing.T) { path := filepath.Join(dir, "locked-"+method+".zip") writeLocked(t, path, 40*1024, method, password, 7741) @@ -163,6 +188,75 @@ func TestEveryLockTheRegistryOffersCanActuallyBeWritten(t *testing.T) { } } +// Each scheme declares the header shape it requires, and they are not the +// same shape. +// +// Written after getting it wrong. Every locked entry was declaring method 99, +// which is what WinZip AES uses - and a ZipCrypto entry has to stay stored, +// because it changes nothing about how the bytes sit, it only puts twelve in +// front of them. 7-Zip found it, and the way it reported it is the reason this +// guard exists: "Data Error in encrypted file. Wrong password?", pointing at +// the password, which was the one thing that was right. +// +// The CRC is the other half and it goes the opposite way between the two. +// ZipCrypto carries the real checksum of the plaintext and puts its high byte +// in the header, so a reader can turn away a wrong password without decrypting. +// AE-2 carries nought, because its authentication code does that job. Getting +// either backwards produces a file that opens and then fails on something that +// sounds like the user's fault. +func TestALockedEntryDeclaresTheShapeItsSchemeRequires(t *testing.T) { + dir := t.TempDir() + for _, method := range locksOffered(t, "zip") { + t.Run(method, func(t *testing.T) { + path := filepath.Join(dir, "shape-"+method+".zip") + writeLocked(t, path, 40*1024, method, "Secret123", 7741) + + r, err := stdzip.OpenReader(path) + if err != nil { + t.Fatal(err) + } + defer func() { _ = r.Close() }() + if len(r.File) == 0 { + t.Fatal("the archive holds nothing, so there is no header to read") + } + h := r.File[0].FileHeader + + if h.Flags&1 == 0 { + t.Errorf("the entry does not say it is encrypted: flags %04x", h.Flags) + } + if h.CompressedSize64 != h.UncompressedSize64+uint64(lockOverhead[method]) { + t.Errorf("the entry stores %d B for %d B of contents, and %s adds %d", + h.CompressedSize64, h.UncompressedSize64, method, lockOverhead[method]) + } + + if method == archive.ZipCrypto { + if h.Method != stdzip.Store { + t.Errorf("a ZipCrypto entry declares method %d and has to stay stored", h.Method) + } + if h.CRC32 == 0 { + t.Error("a ZipCrypto entry carries no checksum, so no reader can turn away " + + "a wrong password without decrypting the whole entry") + } + if len(h.Extra) != 0 { + t.Errorf("a ZipCrypto entry carries %d bytes of extra field and needs none", len(h.Extra)) + } + return + } + + const winZipAES = 99 + if h.Method != winZipAES { + t.Errorf("an AES entry declares method %d and the scheme is %d", h.Method, winZipAES) + } + if h.CRC32 != 0 { + t.Errorf("an AE-2 entry carries the checksum %08x and the scheme says nought", h.CRC32) + } + if len(h.Extra) == 0 { + t.Error("an AES entry carries no 0x9901 field, so nothing says which key length it used") + } + }) + } +} + // Locking does not cost the exact size, which is the promise the format makes. // // The reason it can be kept is the measurement: a stream cipher does not change @@ -176,7 +270,7 @@ func TestEveryLockTheRegistryOffersCanActuallyBeWritten(t *testing.T) { // enough that the padding entry is doing the work rather than the comment. func TestALockedArchiveStillHitsTheSizeToTheByte(t *testing.T) { dir := t.TempDir() - for _, method := range []string{archive.AES128, archive.AES192, archive.AES256} { + for _, method := range locksOffered(t, "zip") { for _, size := range []int64{12 * 1024, 12*1024 + 1, 40 * 1024, 300*1024 + 7} { path := filepath.Join(dir, "size.zip") writeLocked(t, path, size, method, "Secret123", 11) @@ -369,21 +463,29 @@ func TestTheManifestCarriesThePasswordSoATestCanOpenTheFile(t *testing.T) { func TestALockedArchiveIsTheSameFileForTheSameSeed(t *testing.T) { dir := t.TempDir() - first := filepath.Join(dir, "a.zip") - same := filepath.Join(dir, "b.zip") - other := filepath.Join(dir, "c.zip") - writeLocked(t, first, 40*1024, archive.AES256, "Secret123", 4242) - writeLocked(t, same, 40*1024, archive.AES256, "Secret123", 4242) - writeLocked(t, other, 40*1024, archive.AES256, "Secret123", 9999) - - a, b, c := readAll(t, first), readAll(t, same), readAll(t, other) - if string(a) != string(b) { - t.Error("the same recipe and seed gave two different archives, so the salt is drawn rather than derived") - } - if string(a) == string(c) { - t.Error("two different seeds gave the same archive, so the salt ignores the seed") + // Every scheme, because each draws bytes of its own: AES a salt, ZipCrypto + // eleven bytes at the head of the entry. Both come from the run. + for _, method := range locksOffered(t, "zip") { + first := filepath.Join(dir, method+"-a.zip") + same := filepath.Join(dir, method+"-b.zip") + other := filepath.Join(dir, method+"-c.zip") + writeLocked(t, first, 40*1024, method, "Secret123", 4242) + writeLocked(t, same, 40*1024, method, "Secret123", 4242) + writeLocked(t, other, 40*1024, method, "Secret123", 9999) + + a, b, c := readAll(t, first), readAll(t, same), readAll(t, other) + if string(a) != string(b) { + t.Errorf("%s: the same recipe and seed gave two different archives, "+ + "so what it draws is drawn rather than derived", method) + } + if string(a) == string(c) { + t.Errorf("%s: two different seeds gave the same archive", method) + } } + first := filepath.Join(dir, archive.AES256+"-a.zip") + other := filepath.Join(dir, archive.AES256+"-c.zip") + // The salt itself, not the file it is in. // // Comparing whole archives is too blunt to say anything about the salt: the diff --git a/web/public/formats/index.html b/web/public/formats/index.html index ca43f8e..c73d80d 100644 --- a/web/public/formats/index.html +++ b/web/public/formats/index.html @@ -563,7 +563,7 @@

Settings each format accepts

encryption - aes-128, aes-192, aes-256, none + aes-128, aes-192, aes-256, none, zipcrypto diff --git a/web/public/pl/formaty/index.html b/web/public/pl/formaty/index.html index d958606..a9da2ec 100644 --- a/web/public/pl/formaty/index.html +++ b/web/public/pl/formaty/index.html @@ -563,7 +563,7 @@

Ustawienia, które przyjmuje każdy format

encryption - aes-128, aes-192, aes-256, none + aes-128, aes-192, aes-256, none, zipcrypto From df62b38ee945c4494c49616c267931e1b931fcd3 Mon Sep 17 00:00:00 2001 From: DonislawDev Date: Tue, 1 Sep 2026 14:27:22 +0200 Subject: [PATCH 3/3] test: the full mutation run found five holes, and three of them are one mistake 691 entries, 74.0 minutes, five guards that stayed green while the thing they name was broken. Every one is closed and proven. Three of the five are the same mistake wearing different clothes: a guard asking "is this text in the file" stops meaning anything the day the text appears a second time. This tree has that trap written down and still collected three of them in one run. The window binary. The guard asked whether ci.yml mentions ./cmd/tfg-gui anywhere. It appears three times now - and the third arrived later, in the sbom job, which runs on Linux alone. Removing both builds from the platform matrix left the guard green, so a window that stops compiling on Windows or macOS would have passed in silence, which is the whole thing it watches for. It asks the matrix job now. The release notes. The notes carry two attestation commands and they answer different questions - what is inside a file, which needs the predicate type spelled out, and where a Linux archive came from, which must not carry it. Breaking one left the phrase present in the other. Both are required now, by count and by name. The menu. The mutation turned off the branch that widens a menu box to what a row of its open list needs, and measuring says that branch is unreachable for every menu this window has: the guard prints the same tightest slot with it on and off. It was reachable once - the comment beside it records the day twenty values came out cut off - so the data changed, not the rule. Deleting a defence in the window is not something to do in passing, so it is written up as O164 with three options and the price of each. The mutation moved to what the guard actually names: the room a row gives its words. Four real values come out cut off and it names each. The fourth is this branch's own doing. The comment terminator mutation used to move the floor, and the minimum guard watched the floor. Padding rode in the comment then, so an off by one in commentCost was absorbed - the solve built a comment one byte longer and the archive still came out exact. Padding rides in the extra field now, so the same byte lands on the size instead, and three other guards go red on it. The entry follows it to the arithmetic guard that owns the fact. The fifth is a guard whose sample was short. JXL checks that a named quality still fits the file, across five qualities and three sizes - and the break shows at quality 100 and 5000 B, which was not among them. Two sizes added. That is the same shape as O163 one file over: a measurement proves what its sample covers and says nothing about the gap beside it. CLAUDE.md gets the measured run time, because the sentence there was written at 512 entries. 74.0 min at 691, median 4.64 s an entry, 99% of it inside go test - and a note not to estimate it from the first dozen, which I did twice that day and was wrong by a factor of four both times. Co-Authored-By: Claude Opus 5 --- internal/guard/attestation_test.go | 20 ++++++++++++++ internal/guard/jxlladder_test.go | 12 +++++++- internal/guard/smallfixes_test.go | 44 ++++++++++++++++++++++++++++-- 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/internal/guard/attestation_test.go b/internal/guard/attestation_test.go index e427ba0..f12b917 100644 --- a/internal/guard/attestation_test.go +++ b/internal/guard/attestation_test.go @@ -245,6 +245,26 @@ func TestTheReleaseNotesSayHowToCheckWhatWasDownloaded(t *testing.T) { t.Errorf("the release notes never mention %q", want) } } + + // BOTH commands, because they answer different questions and the notes say + // so themselves. One asks what is inside a file and needs the predicate + // type spelled out, since gh asks for build provenance unless told + // otherwise. The other asks where a Linux archive came from and must not + // carry that flag. + // + // Asking only whether the words appear was not enough, and the full + // mutation run of 2026-09-01 said so: replacing one of the two left the + // guard green, because the other still carried the phrase. That is the + // third guard in this tree to fail the same way in one run - "is this text + // in the file" stops meaning anything the day the text appears twice. + if n := strings.Count(notes, "gh attestation verify"); n < 2 { + t.Errorf("the release notes give %d attestation command(s) and there are two things to check - "+ + "what is inside a file, and where a Linux archive came from", n) + } + if !strings.Contains(notes, "--predicate-type https://spdx.dev/Document/v2.3") { + t.Error("the release notes never name the predicate type, so somebody following them asks " + + "for build provenance and is told the bill of materials is not there") + } // The binaries are still unsigned, and the notes have said so since the // first release. Provenance is a different question and must not be read // as an answer to that one. diff --git a/internal/guard/jxlladder_test.go b/internal/guard/jxlladder_test.go index cf31035..b851fb0 100644 --- a/internal/guard/jxlladder_test.go +++ b/internal/guard/jxlladder_test.go @@ -216,8 +216,18 @@ func TestJxlStillFitsWhenTheQualityIsNotTheOneTheCeilingsWereMeasuredAt(t *testi t.Fatal(err) } + // The sizes matter as much as the qualities, and the first version of this + // guard had too few of them. It asked 2000, 20000 and 200000, and the full + // mutation run of 2026-09-01 called it a HOLE: turning the slow road off + // left it green. Measured afterwards by sweeping, the break shows at + // quality 100 and 5000 B, where a 160x120 picture codes to 6047 B and the + // file was to be 5000 - so the rung the table picked does not fit at all. + // + // The guard was right about what it watches and short on where it looked, + // which is the same shape as O163 one file over: a measurement proves what + // its sample covers and says nothing about the gap beside it. for _, quality := range []string{"1", "10", "70", "90", "100"} { - for _, want := range []int64{2000, 20000, 200000} { + for _, want := range []int64{2000, 5000, 20000, 60000, 200000} { p, err := d.Generator.Plan(format.Request{ Bytes: want, Seed: 7741, Label: true, Properties: map[string]string{"quality": quality}, diff --git a/internal/guard/smallfixes_test.go b/internal/guard/smallfixes_test.go index 87f4ce4..e1c5f09 100644 --- a/internal/guard/smallfixes_test.go +++ b/internal/guard/smallfixes_test.go @@ -208,9 +208,49 @@ func TestTheWindowBinaryIsBuiltSomewhere(t *testing.T) { if err != nil { t.Skipf("the workflow is not here: %v", err) } - if !strings.Contains(string(raw), "./cmd/tfg-gui") { - t.Error("nothing in CI builds ./cmd/tfg-gui, so it can stop compiling without a red run") + // Asked of the job that runs on every operating system, not of the file. + // + // It used to ask the file, and the full mutation run of 2026-09-01 called + // that a HOLE. Removing both builds from the platform matrix left the guard + // green, because a third build had appeared since - in the sbom job, which + // runs on Linux alone. So the substring was still there and the protection + // was gone: a window binary that stops compiling on Windows or macOS would + // have passed CI in silence, which is the whole thing this watches for. + // + // That is the failure a guard asking "is this text in the file" always has, + // and it arrives the day the text appears a second time. Nothing about the + // text changed - the tree grew a second writer of it. + const matrixJob = "\n test:\n" + at := strings.Index(string(raw), matrixJob) + if at < 0 { + t.Fatal("ci.yml has no job called test, so this guard is reading a file it does not understand") + } + // The job ends where the next one begins, at the next key on its own + // indentation. + block := string(raw)[at+1:] + if end := nextJobAfter(block); end > 0 { + block = block[:end] + } + if !strings.Contains(block, "./cmd/tfg-gui") { + t.Error("the job that runs on every operating system does not build ./cmd/tfg-gui, " + + "so the window can stop compiling on one of them without a red run.\n" + + " A build elsewhere is not the same promise - the sbom job runs on Linux alone.") + } +} + +// nextJobAfter is where the job starting at the top of block ends, which is the +// next key at the same indentation. Zero when it runs to the end of the file. +func nextJobAfter(block string) int { + for i := 1; i < len(block); i++ { + if block[i-1] != '\n' { + continue + } + rest := block[i:] + if len(rest) > 2 && rest[0] == ' ' && rest[1] == ' ' && rest[2] != ' ' && rest[2] != '#' { + return i + } } + return 0 } // The formatter leaves nothing beside the file it settled. It writes through a