fix(security)!: sanitize unmapped S3 errors and stop mid-stream socket leaks

README.md

@@ -103,7 +103,10 @@ app.get('/*splat', (req, res) => proxy.pipe(req, res));
 ```
 
 Both resolve when the body finishes streaming, and neither rejects on a
-classified S3 failure; they write the correct status instead. Use the
+classified S3 failure; they write the correct status instead. If S3 fails
+mid-stream (after the headers are already sent), the response is terminated
+rather than cleanly ended, so a truncated body surfaces as a broken
+connection instead of a silently short success. Use the
 explicit `fetch()` + try/catch form (below) when you need a custom error
 body (XML/HTML), structured logging, or a non-Express framework.
 

examples/fastify-basic.ts

@@ -5,6 +5,7 @@
  * v4 proxy.fetch() API: pure data fetch, frameworks own the response.
  */
 
+import { pipeline } from 'node:stream';
 import { XmlNode, XmlText } from '@aws-sdk/xml-builder';
 import Fastify from 'fastify';
 import { S3Proxy } from '../src/index.js';
@@ -60,18 +61,12 @@ fastify.head('/*', async (request, reply) => {
 fastify.get('/*', async (request, reply) => {
   const { stream, status, headers } = await proxy.fetch(request.raw as unknown as HttpRequest);
   reply.raw.writeHead(status, headers);
-  stream.on('error', (err: S3Error) => {
-    const errorXml = createErrorXml(err, request.url);
-    if (!reply.raw.headersSent) {
-      reply
-        .status(err.statusCode || 500)
-        .type('application/xml')
-        .send(errorXml);
-    } else {
-      reply.raw.end();
-    }
+  // pipeline (not .pipe) destroys the S3 source stream if the client disconnects
+  // mid-transfer, so the underlying S3 socket isn't leaked. Headers are already
+  // sent by writeHead above, so on a stream error we can only end the response.
+  pipeline(stream, reply.raw, (err) => {
+    if (err && !reply.raw.writableEnded) reply.raw.end();
   });
-  stream.pipe(reply.raw);
   return reply.hijack();
 });
 

examples/http.ts

@@ -1,4 +1,5 @@
 import { createServer } from 'node:http';
+import { pipeline } from 'node:stream';
 import { S3Proxy } from '../src/index.js';
 import type { HttpRequest } from '../src/types.js';
 
@@ -18,7 +19,11 @@ const server = createServer(async (req, res) => {
   try {
     const { stream, status, headers } = await proxy.fetch(req as unknown as HttpRequest);
     res.writeHead(status, headers);
-    stream.on('error', () => res.end()).pipe(res);
+    // pipeline (not .pipe) destroys the S3 source stream if the client
+    // disconnects mid-transfer, so the underlying S3 socket isn't leaked.
+    pipeline(stream, res, (err) => {
+      if (err && !res.writableEnded) res.end();
+    });
   } catch (error) {
     const statusCode =
       typeof error === 'object' &&

src/index.ts

@@ -1,5 +1,5 @@
 import { EventEmitter } from 'node:events';
-import { Readable } from 'node:stream';
+import { pipeline, Readable } from 'node:stream';
 import { GetObjectCommand, HeadBucketCommand, HeadObjectCommand } from '@aws-sdk/client-s3';
 import { S3Forbidden, S3NotFound, S3ProxyError } from './errors.js';
 import { mapHeaderToParam, parseRequest } from './request-parser.js';
@@ -147,6 +147,12 @@ export class S3Proxy extends EventEmitter {
    * Resolves once the body has finished streaming (or the error response
    * has been written). Never rejects for classified S3 failures — it
    * renders them. It only rejects if `res.writeHead` itself throws.
+   *
+   * If S3 fails *mid-stream* — after the status and headers have already
+   * gone out — the response is terminated (destroyed), not cleanly ended:
+   * the client sees a broken connection instead of a silently truncated
+   * "successful" body, and the S3 source socket is torn down. (Before v4.2.2
+   * the response was `end()`-ed, which could mask a short read as success.)
    */
   public pipe(req: HttpRequest, res: HttpResponse): Promise<void> {
     return this.fetch(req).then(
@@ -170,12 +176,12 @@ export class S3Proxy extends EventEmitter {
   ): Promise<void> {
     res.writeHead(status, headers);
     return new Promise<void>((resolve) => {
-      stream.on('error', () => {
-        res.end();
-        resolve();
-      });
-      stream.on('end', resolve);
-      stream.pipe(res);
+      // pipeline (not .pipe) destroys the S3 source stream when the client
+      // disconnects mid-transfer or a write fails, so the underlying S3 socket
+      // isn't leaked. Headers are already sent, so a mid-stream failure can only
+      // be handled by ending the response — hence resolve (never reject) in all
+      // cases; middleware()/pipe() treat a rendered/terminated response as done.
+      pipeline(stream, res, () => resolve());
     });
   }
 

src/s3-gateway.ts

@@ -11,7 +11,7 @@ import {
   S3Client,
   S3ServiceException,
 } from '@aws-sdk/client-s3';
-import { S3Forbidden, S3InvalidRange, S3NotFound, type S3ProxyError } from './errors.js';
+import { S3Forbidden, S3InvalidRange, S3NotFound, S3ProxyError } from './errors.js';
 import type { S3FetchResponse, S3ProxyOptions } from './types.js';
 import { UserException } from './UserException.js';
 
@@ -75,15 +75,21 @@ function outputToHeaders(output: SupportedOutput): Record<string, string> {
   return h;
 }
 
-function mapError(e: unknown, target: string): S3ProxyError | never {
+function mapError(e: unknown, target: string): S3ProxyError {
   if (e instanceof NoSuchKey || e instanceof NoSuchBucket) {
     return new S3NotFound(target, { cause: e });
   }
   if (e instanceof S3ServiceException) {
     if (e.name === 'AccessDenied') return new S3Forbidden(target, { cause: e });
     if (e.name === 'InvalidRange') return new S3InvalidRange(target, { cause: e });
   }
-  throw e;
+  // Anything else — an unmapped S3ServiceException (e.g. PermanentRedirect,
+  // SlowDown, InternalError), a network error, or an unexpected throw — is
+  // wrapped in a generic 500. Raw AWS details (bucket region/endpoint,
+  // requestId, internal messages) must never reach the client; the original
+  // is preserved on `cause` for server-side logging only. Consumers that log
+  // errors should log `err.cause`, never surface it in a response body.
+  return new S3ProxyError(`S3 request failed: ${target}`, 500, { cause: e });
 }
 
 /**

test/fetch.test.ts

@@ -1,6 +1,6 @@
 import { GetObjectCommand, S3ServiceException } from '@aws-sdk/client-s3';
 import { afterEach, beforeEach, describe, expect, it } from 'vitest';
-import { S3Forbidden, S3InvalidRange, S3NotFound, S3Proxy } from '../src/index.js';
+import { S3Forbidden, S3InvalidRange, S3NotFound, S3Proxy, S3ProxyError } from '../src/index.js';
 import { s3Mock, setupS3Mocks, teardownS3Mocks } from './helpers/aws-mock.js';
 import { catchError, makeReq, readAll } from './helpers/http-mocks.js';
 
@@ -88,20 +88,29 @@ describe('proxy.fetch', () => {
     expect((err as S3InvalidRange).cause).toBe(invalidRange);
   });
 
-  it('rethrows non-AWS errors unchanged', async () => {
+  it('wraps non-AWS errors in a sanitized 500 (original preserved on cause)', async () => {
+    // Unmapped errors (network failures, unexpected AWS exceptions like
+    // PermanentRedirect/SlowDown) must not leak their raw message to the client.
     const networkError = new Error('Network error');
     s3Mock.on(GetObjectCommand, { Bucket: '.test-bucket', Key: 'flaky.txt' }).rejects(networkError);
-    await expect(proxy.fetch(makeReq('/flaky.txt'))).rejects.toThrow('Network error');
+    const err = await catchError(proxy.fetch(makeReq('/flaky.txt')));
+    expect(err).toBeInstanceOf(S3ProxyError);
+    expect((err as S3ProxyError).statusCode).toBe(500);
+    expect((err as Error).message).not.toContain('Network error');
+    expect((err as S3ProxyError).cause).toBe(networkError);
   });
 
-  it('throws for unrecognized body type', async () => {
+  it('wraps an unrecognized body type in a sanitized 500 (original on cause)', async () => {
     s3Mock.on(GetObjectCommand, { Bucket: '.test-bucket', Key: 'weird.txt' }).resolves({
       Body: { invalid: 'body' } as never,
       ContentLength: 100,
       ContentType: 'text/plain',
       $metadata: { httpStatusCode: 200, requestId: 'mock' },
     });
-    await expect(proxy.fetch(makeReq('/weird.txt'))).rejects.toThrow('unrecognized type');
+    const err = await catchError(proxy.fetch(makeReq('/weird.txt')));
+    expect(err).toBeInstanceOf(S3ProxyError);
+    expect((err as S3ProxyError).statusCode).toBe(500);
+    expect(((err as S3ProxyError).cause as Error).message).toBe('unrecognized type');
   });
 
   it('does not write to a response (pure)', async () => {

test/pipe.test.ts

@@ -82,7 +82,7 @@ describe('proxy.pipe / proxy.middleware (convenience adapter)', () => {
   });
 
   describe('mid-stream failure', () => {
-    it('ends the response and resolves when the S3 body errors after headers are sent', async () => {
+    it('terminates the response and resolves when the S3 body errors after headers are sent', async () => {
       s3Mock.on(GetObjectCommand, { Bucket: '.test-bucket', Key: 'flaky-stream.bin' }).resolves({
         Body: new Readable({
           read() {
@@ -95,7 +95,10 @@ describe('proxy.pipe / proxy.middleware (convenience adapter)', () => {
       const res = new FakeResponse();
       await expect(proxy.pipe(makeReq('/flaky-stream.bin'), res)).resolves.toBeUndefined();
       expect(res.status).toBe(200); // headers already went out before the stream broke
-      expect(res.writableEnded).toBe(true);
+      // pipeline() destroys the response on a mid-stream source error rather than
+      // end()-ing it: a truncated body is surfaced as a broken connection, not a
+      // silently-short "successful" response. destroy() also tears down the S3 source.
+      expect(res.destroyed).toBe(true);
     });
   });
 });

test/s3proxy.test.ts

@@ -1,8 +1,9 @@
 import { HeadBucketCommand } from '@aws-sdk/client-s3';
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
-import { S3NotFound, S3Proxy, UserException } from '../src/index.js';
+import { S3NotFound, S3Proxy, S3ProxyError, UserException } from '../src/index.js';
 import type { S3ProxyConfig } from '../src/types.js';
 import { s3Mock, setupS3Mocks, teardownS3Mocks } from './helpers/aws-mock.js';
+import { catchError } from './helpers/http-mocks.js';
 
 describe('S3Proxy', () => {
   beforeEach(() => {
@@ -72,8 +73,14 @@ describe('S3Proxy', () => {
       const errorSpy = vi.fn();
       proxy.on('error', errorSpy);
 
-      await expect(proxy.init()).rejects.toThrow('Init failed');
-      expect(errorSpy).toHaveBeenCalledWith(initError);
+      // init() rejects with a sanitized S3ProxyError (raw message not exposed);
+      // the original is preserved on `cause` and the same error is emitted.
+      const err = await catchError(proxy.init());
+      expect(err).toBeInstanceOf(S3ProxyError);
+      expect((err as S3ProxyError).statusCode).toBe(500);
+      expect((err as Error).message).not.toContain('Init failed');
+      expect((err as S3ProxyError).cause).toBe(initError);
+      expect(errorSpy).toHaveBeenCalledWith(err);
     });
   });
 
@@ -99,10 +106,14 @@ describe('S3Proxy', () => {
   });
 
   describe('verifyOnInit', () => {
-    it('defaults to true: rejects when the bucket is unreachable', async () => {
+    it('defaults to true: rejects with a sanitized error when the bucket is unreachable', async () => {
       const proxy = new S3Proxy({ bucket: '.test-bucket' });
-      s3Mock.on(HeadBucketCommand).rejectsOnce(new Error('unreachable'));
-      await expect(proxy.init()).rejects.toThrow('unreachable');
+      const cause = new Error('unreachable');
+      s3Mock.on(HeadBucketCommand).rejectsOnce(cause);
+      const err = await catchError(proxy.init());
+      expect(err).toBeInstanceOf(S3ProxyError);
+      expect((err as Error).message).not.toContain('unreachable');
+      expect((err as S3ProxyError).cause).toBe(cause);
     });
 
     it('false: init() resolves without sending a HeadBucket', async () => {