diff --git a/docker-compose.minio-test.yml b/docker-compose.minio-test.yml index 2b5315eb5..aeb7a6612 100644 --- a/docker-compose.minio-test.yml +++ b/docker-compose.minio-test.yml @@ -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 diff --git a/server/src/nest/storage/drivers/s3.driver.ts b/server/src/nest/storage/drivers/s3.driver.ts index 4bb294e69..b789efee4 100644 --- a/server/src/nest/storage/drivers/s3.driver.ts +++ b/server/src/nest/storage/drivers/s3.driver.ts @@ -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); } diff --git a/server/tests/unit/nest/storage/s3.driver.test.ts b/server/tests/unit/nest/storage/s3.driver.test.ts index d5c84392b..8c745ebe4 100644 --- a/server/tests/unit/nest/storage/s3.driver.test.ts +++ b/server/tests/unit/nest/storage/s3.driver.test.ts @@ -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).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 () => {