zipfile append mode corrupted four of six product zips and only testzip() noticed

The symptom

I needed to add one AGENTS.md file to six product zips that people pay for. The obvious way:

import zipfile
with zipfile.ZipFile(path, "a") as z:
    z.write("AGENTS.md", "AGENTS.md")

No exception, six clean runs, and the files opened fine afterwards. namelist() returned everything that was there before plus AGENTS.md.

Then, because these are products and a corrupt download is the worst bug I can ship, I ran the check that is not part of anybody’s normal workflow:

with zipfile.ZipFile(path) as z:
    bad = z.testzip()
    print(path, bad)

Four of the six named a broken entry: files like supabase/migrations/0001_rate_limiting.sql, and on three of them a bare supabase/functions/ directory entry. These had been fine minutes earlier, in archives I had only appended to, in files I had not touched. The other two appended cleanly with the same code, in the same run, on the same machine.

What I tried first

My first thought was that I had broken the source file, so I checked AGENTS.md. It was fine, and in any case the entries testzip() complained about were not it. My second thought was that the archives had been damaged before I got to them, which would have been a better outcome, since it would mean the build step was the bug and not the repack step. So I copied an original byte for byte and tested it:

shutil.copy2(original, tmp)
zipfile.ZipFile(tmp).testzip()   # None
zipfile.ZipFile(appended).testzip()  # 'supabase/migrations/0001_rate_limiting.sql'

The copy passes and the appended version does not, so the append did it.

The inconsistency made me distrust my own test for a while. Four failed and two did not, with no pattern in the two that survived. Six out of six I would have believed in a minute; a partial failure looks like a flaky test, and the instinct is to re-run it rather than accept it.

What was happening

Append mode on a zip is more involved than it looks. Python has to locate the central directory at the end of the existing file, position the write so the new local file header lands where the directory used to start, and then rewrite the whole directory. When the existing archive has anything unusual in its layout, and directory entries with zero-length data qualify, that arithmetic can land in the wrong place and overwrite bytes belonging to entries that were already there. That is why the damage shows up in files you never named: you appended AGENTS.md, and the entry that broke is a migration you have not edited in weeks.

It also explains every misleading signal. namelist() reads only the central directory, which was rewritten correctly, so the list of names is right. Opening the file works, because opening reads that same directory. getinfo() is fine and the metadata is all consistent. The compressed data is what got trodden on, and nothing reads that until you decompress it, which is to say until your customer unzips it. testzip() is the only standard-library call that decompresses every member and checks its CRC, so it is the only thing in the normal toolkit that looks at the part that was damaged.

The fix

Never append. Read every entry out of the original and write a new archive:

import os, zipfile

def repack(path, add_path, add_name):
    tmp = path + ".tmp"
    with zipfile.ZipFile(path) as src, \
         zipfile.ZipFile(tmp, "w", zipfile.ZIP_DEFLATED) as dst:
        for info in src.infolist():
            dst.writestr(info, src.read(info.filename))
        dst.write(add_path, add_name)
    os.replace(tmp, path)

Passing the whole ZipInfo to writestr rather than just the name preserves timestamps, permissions and the compression type per entry, so the rebuilt archive is not subtly different from the one you tested. os.replace is atomic, so a crash halfway through leaves the original intact instead of a truncated product file.

Then verify the artifact:

def verify(before, after, expected_added):
    import hashlib
    with zipfile.ZipFile(before) as a, zipfile.ZipFile(after) as b:
        assert b.testzip() is None, b.testzip()
        old = {i.filename for i in a.infolist()}
        new = {i.filename for i in b.infolist()}
        assert new - old == expected_added, new - old
        assert old - new == set(), old - new
        for name in old:
            assert hashlib.md5(a.read(name)).hexdigest() == \
                   hashlib.md5(b.read(name)).hexdigest(), name

Each assertion catches a different failure. testzip() catches the corruption. The set comparison catches “added the wrong thing” and “lost something”. The per-entry MD5 catches silent content changes in files that were supposed to be carried across untouched, which is exactly the damage here. That check is worth keeping permanently in any script that mutates a distributable.

The lesson

If you are mutating a distributable artifact in place, verify the artifact rather than the operation. The operation reported success on all six and the artifact was broken on four. Everything cheap and convenient to check, the exception that was not raised, the file that opened, the name list that was correct, was reading metadata, and the metadata was never the damaged part.

When a format has a structural index separate from its payload, every quick sanity check you can think of reads the index. Reading the payload is slower and it is the only thing that answers the question. It matters most for a product zip, because the buyer is the one who finds out.