mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-23 15:36:33 +00:00
fix(server): S3 driver zero-byte put — work around aws4 duplicate Content-Length signing defect
This commit is contained in:
@@ -102,6 +102,44 @@ function isNotFound(err: unknown): boolean {
|
||||
return statusCode(err) === 404 || code === 'NoSuchKey' || code === 'NotFound';
|
||||
}
|
||||
|
||||
/**
|
||||
* 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:
|
||||
*
|
||||
* - `@aws-lite/s3`'s PutObject (node_modules/@aws-lite/s3/src/put-object.mjs:99,
|
||||
* the default `!ApplyChecksum` "unsigned payload" branch) always sets
|
||||
* `headers['content-length'] = dataSize` — a *number*, including `0`.
|
||||
* - `aws4`'s signer (node_modules/aws4/aws4.js:151-152, aws4@1.13.2) then runs
|
||||
* `if (request.body && !headers['Content-Length'] && !headers['content-length'])
|
||||
* headers['Content-Length'] = Buffer.byteLength(request.body)`.
|
||||
* `!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*
|
||||
* (`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
|
||||
* signatures disagree — 403. Any non-empty body makes
|
||||
* `headers['content-length']` a truthy number, so aws4's guard never fires
|
||||
* and no duplicate is ever added — hence the bug is exactly the size-0
|
||||
* boundary, nothing else.
|
||||
*
|
||||
* `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
|
||||
* `x-amz-content-sha256` for the (empty) body instead of `UNSIGNED-PAYLOAD`.
|
||||
* Verified end-to-end against real MinIO: 200 OK, correct empty-object ETag.
|
||||
* The extra checksum cost is nil for a 0-byte body. Delete this workaround
|
||||
* once aws4 dedupes header keys case-insensitively before signing (or
|
||||
* @aws-lite/s3 stops pre-setting a falsy-numeric Content-Length).
|
||||
*/
|
||||
function zeroByteChecksumWorkaround(size: number): { ApplyChecksum: true } | Record<string, never> {
|
||||
return size === 0 ? { ApplyChecksum: true } : {};
|
||||
}
|
||||
|
||||
export class S3Driver implements StorageDriver {
|
||||
readonly id: string;
|
||||
private readonly bucket: string;
|
||||
@@ -143,7 +181,10 @@ 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 }), `put '${key}'`);
|
||||
await this.deadlined(
|
||||
s3.PutObject({ ...base, File: source.tmpPath, ...zeroByteChecksumWorkaround(size) }),
|
||||
`put '${key}'`,
|
||||
);
|
||||
} else {
|
||||
await this.uploadMonitored(s3, base, fs.createReadStream(source.tmpPath), key);
|
||||
}
|
||||
@@ -167,7 +208,11 @@ export class S3Driver implements StorageDriver {
|
||||
// the PassThrough — but ended means the chunks already hold everything.
|
||||
if (peeked.ended || source.readableEnded) {
|
||||
try {
|
||||
await this.deadlined(s3.PutObject({ ...base, Body: Buffer.concat(peeked.chunks) }), `put '${key}'`);
|
||||
const body = Buffer.concat(peeked.chunks);
|
||||
await this.deadlined(
|
||||
s3.PutObject({ ...base, Body: body, ...zeroByteChecksumWorkaround(body.length) }),
|
||||
`put '${key}'`,
|
||||
);
|
||||
} catch (err) {
|
||||
throw this.wrap(err, `put failed for '${key}'`);
|
||||
}
|
||||
|
||||
@@ -251,6 +251,18 @@ describe('S3Driver put — bounded peek routing', () => {
|
||||
await makeDriver(api).put('a/empty.bin', Readable.from([]));
|
||||
const input = (api.PutObject as ReturnType<typeof vi.fn>).mock.calls[0][0];
|
||||
expect(Buffer.isBuffer(input.Body) && input.Body.length).toBe(0);
|
||||
// aws4@1.13.2 signing workaround (see zeroByteChecksumWorkaround in
|
||||
// s3.driver.ts): a zero-byte Body must set ApplyChecksum: true so
|
||||
// @aws-lite/s3's PutObject never pre-sets a falsy-numeric Content-Length
|
||||
// that aws4 then duplicates, breaking the signature against real S3.
|
||||
expect(input.ApplyChecksum).toBe(true);
|
||||
});
|
||||
it('does not apply the zero-byte checksum workaround to a 1-byte stream (boundary is exactly size 0)', async () => {
|
||||
const api = makeMockApi({ PutObject: vi.fn().mockResolvedValue({}) });
|
||||
await makeDriver(api).put('a/one-byte.bin', Readable.from(Buffer.from([7])));
|
||||
const input = (api.PutObject as ReturnType<typeof vi.fn>).mock.calls[0][0];
|
||||
expect(input.Body.length).toBe(1);
|
||||
expect(input.ApplyChecksum).toBeUndefined();
|
||||
});
|
||||
it('routes a stream exceeding the threshold to Upload with the peeked prefix re-attached', async () => {
|
||||
let uploaded: Buffer | null = null;
|
||||
@@ -369,6 +381,16 @@ 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 () => {
|
||||
|
||||
Reference in New Issue
Block a user