fix(server): scope the zero-byte put workaround to buffered bodies; bind test MinIO to localhost

This commit is contained in:
jubnl
2026-08-19 21:54:44 +02:00
parent 0054ae8287
commit 0a4c36e1fb
3 changed files with 27 additions and 24 deletions
+8 -4
View File
@@ -4,14 +4,18 @@
# TREK_TEST_S3_BUCKET=trek-test TREK_TEST_S3_ACCESS_KEY_ID=trekci \
# TREK_TEST_S3_SECRET_ACCESS_KEY=trekci-secret \
# npx vitest run tests/unit/nest/storage/s3.driver.contract.test.ts
# 127.0.0.1 is load-bearing: aws-lite picks path-style only for IP/localhost
# hosts, and stock MinIO only speaks path-style (no MINIO_DOMAIN here).
# 127.0.0.1 is load-bearing on both sides: aws-lite picks path-style only for
# IP/localhost hosts (stock MinIO only speaks path-style, no MINIO_DOMAIN
# here), and the port publish below binds 127.0.0.1 only — not 0.0.0.0 — so
# this fixed-credential test instance is never reachable from the LAN.
services:
minio:
image: minio/minio:RELEASE.2025-09-07T16-13-09Z # pinned — repo frozen upstream, tag is stable
# Docker Hub's minio/minio repo is archived — quay.io is the maintained
# registry going forward. Pinned tag: repo is stable/frozen upstream.
image: quay.io/minio/minio:RELEASE.2025-09-07T16-13-09Z.hotfix.7aa24e772
command: server /data
ports:
- "9000:9000"
- "127.0.0.1:9000:9000"
environment:
- MINIO_ROOT_USER=trekci
- MINIO_ROOT_PASSWORD=trekci-secret
+19 -10
View File
@@ -103,10 +103,11 @@ function isNotFound(err: unknown): boolean {
}
/**
* Workaround for a verified upstream signing defect: a zero-byte PutObject
* body fails against real S3-compatible servers (confirmed against MinIO)
* with 403 SignatureDoesNotMatch, while any non-empty body succeeds via the
* identical code path. Root cause, traced through the installed deps:
* Workaround for a verified upstream signing defect, scoped ONLY to the
* buffered zero-byte `Body` path in `put()` below — a zero-byte PutObject
* `Body: Buffer` fails against real S3-compatible servers (confirmed against
* MinIO) with 403 SignatureDoesNotMatch, while any non-empty `Body` succeeds
* via the identical code path. Root cause, traced through the installed deps:
*
* - `@aws-lite/s3`'s PutObject (node_modules/@aws-lite/s3/src/put-object.mjs:99,
* the default `!ApplyChecksum` "unsigned payload" branch) always sets
@@ -117,8 +118,11 @@ function isNotFound(err: unknown): boolean {
* `!0 === true`, so aws4 treats the already-set `0` as "unset" and injects
* a SECOND, differently-cased `Content-Length` header onto the same object.
* JS object keys are case-sensitive, so both survive; aws4's
* canonicalHeaders()/signedHeaders() (aws4.js:311-322) don't dedupe by
* lowercased name, so `content-length` is signed *twice*
* `filterHeaders()` (aws4.js:302-308) lowercases each header entry for
* canonicalization WITHOUT deduping by that lowercased name — the two
* distinct keys stay two distinct entries — so `canonicalHeaders()`/
* `signedHeaders()` (aws4.js:311-322, which just map/join whatever
* `filterHeaders()` produced) sign `content-length` *twice*
* (`SignedHeaders=content-length;content-length;...`, confirmed live
* against MinIO in the repro below). MinIO's own canonicalization of the
* headers it actually received doesn't reproduce that duplicate, so the
@@ -127,6 +131,14 @@ function isNotFound(err: unknown): boolean {
* and no duplicate is ever added — hence the bug is exactly the size-0
* boundary, nothing else.
*
* The `File` route (`put-object.mjs:87`, `payload = createReadStream(File)`)
* is NOT affected and must NOT get this workaround: aws-lite's request
* builder (`@aws-lite/client/src/request/index.js:60,90`) sets
* `params.body = undefined` for any stream payload, so aws4.js:151's
* `request.body && ...` guard never even evaluates truthy there — a
* zero-byte temp file signs correctly today. Only the buffered `Body: Buffer`
* call site below needs this.
*
* `ApplyChecksum: true` routes PutObject through @aws-lite/s3's OTHER branch
* (put-object.mjs:106-118), which never pre-sets Content-Length itself —
* aws4 then adds it exactly once, and independently computes a correct
@@ -181,10 +193,7 @@ export class S3Driver implements StorageDriver {
try {
const { size } = await fs.promises.stat(source.tmpPath);
if (size < MULTIPART_THRESHOLD) {
await this.deadlined(
s3.PutObject({ ...base, File: source.tmpPath, ...zeroByteChecksumWorkaround(size) }),
`put '${key}'`,
);
await this.deadlined(s3.PutObject({ ...base, File: source.tmpPath }), `put '${key}'`);
} else {
await this.uploadMonitored(s3, base, fs.createReadStream(source.tmpPath), key);
}
@@ -381,16 +381,6 @@ describe('S3Driver put — LocalTempFile ownership', () => {
expect(api.PutObject).toHaveBeenCalledWith(
expect.objectContaining({ File: tmp, Key: 'a/t.bin', ContentType: 'image/png' }),
);
expect((api.PutObject as ReturnType<typeof vi.fn>).mock.calls[0][0].ApplyChecksum).toBeUndefined();
expect(fs.existsSync(tmp)).toBe(false);
});
it('applies the zero-byte checksum workaround to a zero-byte temp file too (same aws4 signing defect)', async () => {
const api = makeMockApi({ PutObject: vi.fn().mockResolvedValue({}) });
const tmp = await makeTmp(0);
await makeDriver(api).put('a/empty-tmp.bin', { tmpPath: tmp });
expect(api.PutObject).toHaveBeenCalledWith(
expect.objectContaining({ File: tmp, Key: 'a/empty-tmp.bin', ApplyChecksum: true }),
);
expect(fs.existsSync(tmp)).toBe(false);
});
it('routes a threshold-sized temp file to Upload', async () => {