extract() error path closes the archive descriptor instead of the output file

[alecgibson]

Note

This issue was researched and written by Claude Code.
The reproduction below was run and verified against node-stream-zip@1.16.0.

When an entry's stream errors before the output file has finished opening, extract()
closes the archive descriptor instead of the output descriptor it just opened.

Three things go wrong at once:

1. The output descriptor (fdFile) leaks.
2. The archive descriptor is closed behind StreamZip's back — fd is left dangling
   rather than set to null, and closed is never set.
3. A later zip.close() therefore calls fs.close() on a descriptor number the process
   no longer owns. Here it surfaces as EBADF; if the OS has since reassigned that
   number, it closes an unrelated file instead.

Why it happens

In extract():

fs.open(outPath, 'w', (err, fdFile) => {
    if (err) {
        return callback(err);
    }
    if (errThrown) {
        fs.close(fd, () => {          // <-- `fd` is the archive; `fdFile` is leaked
            callback(errThrown);
        });
        return;
    }
    fsStm = fs.createWriteStream(outPath, {fd: fdFile});

fd is the closure-scoped archive descriptor assigned in open(). fdFile is the
descriptor this callback was just handed. The errThrown branch closes the former and
drops the latter.

The window is real because the entry stream starts flowing before fs.open is called —
this.stream() pipes through zlib.createInflateRaw() for deflated entries, so
inflation begins immediately and a corrupt entry can emit 'error' while the output
open is still in flight.

Reproduction

Uses the repo's own test/err/corrupt_entry.zip. setFs here delays only the output
open, to make the existing race deterministic — it changes no library logic, and the
flags === 'w' check means the archive open is untouched.

const fs = require('fs');
const StreamZip = require('node-stream-zip');

const realFs = {...fs};
StreamZip.setFs({
	...realFs,
	open(p, flags, cb) {
		if (flags === 'w') return setTimeout(() => realFs.open(p, flags, cb), 150);
		return realFs.open(p, flags, cb);
	},
});

const zip = new StreamZip({file: 'test/err/corrupt_entry.zip'});
zip.on('error', () => {});
zip.on('ready', () => {
	zip.extract('doc/api_assets/logo.svg', './out.txt', (error) => {
		console.log('extract error:', error && error.message);
		setTimeout(() => {
			zip.close((closeError) => {
				console.log('zip.close() error:', closeError && closeError.code);
			});
		}, 100);
	});
});

Actual, on node-stream-zip@1.16.0 / Node 24.14.1:

extract error: invalid distance too far back
zip.close() error: EBADF

Inspecting /proc/self/fd at that point also shows the archive descriptor already gone
and out.txt still open.

Expected: extract() closes fdFile, leaves the archive descriptor alone, and
zip.close() succeeds.

How likely is this in practice

Without the injected delay I measured 0/40 on a local SSD — the inflate error and the
output open are close enough that the open usually wins. It needs the output open to
be the slower of the two, so a network or fuse filesystem, a loaded machine, or
on-access AV scanning on Windows would be the realistic triggers. So: latent rather than
common, but the code is unambiguously closing the wrong descriptor, and the failure mode
(closing a descriptor the process may have reassigned) is nastier than a plain leak.

Possible fix

if (errThrown) {
    fs.close(fdFile, () => {
        callback(errThrown);
    });
    return;
}

Happy to send a PR if you'd like.