fix(storage): bound tar imports, contain storage keys across backends, stop export-file accumulation

.env.example

@@ -166,6 +166,10 @@ WEBHOOK_SSRF_PROTECT=true
 # Server-side media size/time limits:
 # MEDIA_DOWNLOAD_MAX_BYTES=52428800   # cap remote-URL sends AND inbound media (default 50 MiB; oversized inbound media is dropped, message kept)
 # MEDIA_DOWNLOAD_TIMEOUT_MS=30000     # abort a slow media download (default 30s)
+# Storage import/export limits (ADMIN /infra/storage/* endpoints):
+# STORAGE_IMPORT_MAX_BYTES=209715200  # per-entry cap for a tar.gz import; aborts on overflow (default 200 MiB)
+# STORAGE_IMPORT_MAX_ENTRIES=100000   # max entries in an import archive; aborts beyond this (default 100000)
+# STORAGE_EXPORT_TTL_MS=3600000       # auto-delete an export archive after this long (default 1h)
 
 # =============================================================================
 # RATE LIMITING

CHANGELOG.md

@@ -7,6 +7,27 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+
+- **Storage import is bounded against decompression bombs.** A `tar.gz` import buffered each entry in
+  memory with no size or entry-count limit, so a crafted archive could exhaust process memory. Imports
+  now enforce a per-entry byte cap (`STORAGE_IMPORT_MAX_BYTES`, default 200 MiB) and a maximum entry
+  count (`STORAGE_IMPORT_MAX_ENTRIES`, default 100000), aborting the whole import on breach.
+- **Storage-key containment is enforced for the S3 backend too.** The local backend already rejected
+  tar entry names that traversed the storage root; the S3 path did not. Containment is now checked at the
+  backend-agnostic `putFile` boundary, so an object key can't escape the intended `media/` prefix.
+- **Plugin storage keys are sandbox-contained.** A plugin's `ctx.storage` get/set/delete built a file
+  path from the raw key, so a key containing `..` could read/write/delete outside the plugin's own data
+  directory. Keys that escape the sandbox are now rejected (ordinary JID-style keys are preserved).
+
+### Fixed
+
+- **Storage export no longer accumulates copies on the data volume.** `GET /infra/storage/export` wrote
+  a timestamped `tar.gz` of all media into `data/` (alongside the live databases and session state) and
+  never deleted it, so repeated exports could fill the disk and destabilize the gateway. The export now
+  writes to the OS temp directory and is removed after a TTL (`STORAGE_EXPORT_TTL_MS`, default 1h), and the
+  export read yields the event loop per file instead of blocking it with a synchronous read.
+
 ## [0.4.2] - 2026-06-19
 
 Bug-fix and hardening release: access-control tightening, session-lifecycle resilience, data-migration

docs/14-migration-guide.md

@@ -156,7 +156,9 @@ curl -s 'http://localhost:2785/api/infra/storage/files/count' \
 # Step 2: Export all files as tar.gz
 curl -s 'http://localhost:2785/api/infra/storage/export' \
   -H 'X-API-Key: YOUR_KEY'
-# Response: { "message": "Storage export completed", "download": "/app/data/storage-export-xxx.tar.gz" }
+# Response: { "message": "Storage export completed", "download": "/app/data/exports/storage-export-xxx.tar.gz" }
+# The archive is auto-removed after STORAGE_EXPORT_TTL_MS (default 1h), so re-import it before then.
+# It is written under data/ so it survives the restart in Step 4 and stays import-able.
 
 # Step 3: Change storage configuration
 # From: STORAGE_TYPE=local
@@ -170,7 +172,7 @@ docker compose up -d
 curl -X POST 'http://localhost:2785/api/infra/storage/import' \
   -H 'X-API-Key: YOUR_KEY' \
   -H 'Content-Type: application/json' \
-  -d '{"filePath": "/app/data/storage-export-xxx.tar.gz"}'
+  -d '{"filePath": "/app/data/exports/storage-export-xxx.tar.gz"}'
 ```
 
 | Scenario                     | Support | Method                   |

src/common/storage/storage.service.spec.ts

@@ -88,3 +88,95 @@ describe('StorageService (local) path traversal protection', () => {
     expect(count).toBe(1);
   });
 });
+
+function makeLocalService(): { service: StorageService; baseDir: string; localPath: string } {
+  const baseDir = fs.mkdtempSync(path.join(os.tmpdir(), 'owa-storage-'));
+  const localPath = path.join(baseDir, 'media');
+  const configService = {
+    get: (key: string) => (key === 'storage.type' ? 'local' : key === 'storage.localPath' ? localPath : undefined),
+  } as unknown as ConfigService;
+  return { service: new StorageService(configService), baseDir, localPath };
+}
+
+describe('StorageService put/getFile containment is backend-agnostic', () => {
+  // Force S3 routing with a stub client so the assertion proves the guard runs BEFORE any S3 call
+  // (i.e. it lives in put/getFile, so the otherwise-unguarded S3 backend is contained too).
+  function s3Stub(service: StorageService): jest.Mock {
+    const sendMock = jest.fn();
+    const internal = service as unknown as { storageType: string; s3Client: unknown; s3Available: boolean };
+    internal.storageType = 's3';
+    internal.s3Client = { send: sendMock };
+    internal.s3Available = true;
+    return sendMock;
+  }
+
+  it('putFile rejects an unsafe key before reaching the S3 backend', async () => {
+    const { service, baseDir } = makeLocalService();
+    const sendMock = s3Stub(service);
+
+    await expect(service.putFile('../evil', Buffer.from('x'))).rejects.toThrow();
+    expect(sendMock).not.toHaveBeenCalled();
+
+    fs.rmSync(baseDir, { recursive: true, force: true });
+  });
+
+  it('getFile rejects an unsafe key before reaching the S3 backend', async () => {
+    const { service, baseDir } = makeLocalService();
+    const sendMock = s3Stub(service);
+
+    await expect(service.getFile('../../etc/passwd')).rejects.toThrow();
+    expect(sendMock).not.toHaveBeenCalled();
+
+    fs.rmSync(baseDir, { recursive: true, force: true });
+  });
+});
+
+describe('StorageService import resource caps (decompression-bomb defense)', () => {
+  let baseDir: string;
+  let localPath: string;
+  let service: StorageService;
+
+  beforeEach(() => {
+    ({ service, baseDir, localPath } = makeLocalService());
+  });
+
+  afterEach(() => {
+    fs.rmSync(baseDir, { recursive: true, force: true });
+    delete process.env.STORAGE_IMPORT_MAX_BYTES;
+    delete process.env.STORAGE_IMPORT_MAX_ENTRIES;
+  });
+
+  it('aborts an entry that exceeds the per-entry byte cap, writing nothing', async () => {
+    process.env.STORAGE_IMPORT_MAX_BYTES = '8';
+    const gz = await makeTarGz([{ name: 'bomb.bin', data: 'far-more-than-eight-bytes' }]);
+
+    await expect(service.importFromStream(Readable.from(gz))).rejects.toThrow(/byte|cap|exceed|large/i);
+    expect(fs.existsSync(path.join(localPath, 'bomb.bin'))).toBe(false);
+  });
+
+  it('aborts when the archive exceeds the max entry count', async () => {
+    process.env.STORAGE_IMPORT_MAX_ENTRIES = '1';
+    const gz = await makeTarGz([
+      { name: 'a.txt', data: 'a' },
+      { name: 'b.txt', data: 'b' },
+    ]);
+
+    await expect(service.importFromStream(Readable.from(gz))).rejects.toThrow(/entr/i);
+  });
+
+  it('aborts a large multi-chunk entry mid-stream (the payload spans several stream chunks)', async () => {
+    process.env.STORAGE_IMPORT_MAX_BYTES = '1024';
+    // 256 KiB easily spans multiple 64 KiB stream chunks, so this proves the running accumulator
+    // aborts mid-stream rather than only after the whole entry is buffered.
+    const gz = await makeTarGz([{ name: 'big.bin', data: 'x'.repeat(256 * 1024) }]);
+
+    await expect(service.importFromStream(Readable.from(gz))).rejects.toThrow(/byte|cap|exceed|large/i);
+    expect(fs.existsSync(path.join(localPath, 'big.bin'))).toBe(false);
+  });
+
+  it('imports normally within the (generous default) caps', async () => {
+    const gz = await makeTarGz([{ name: 'ok.txt', data: 'fine' }]);
+    const count = await service.importFromStream(Readable.from(gz));
+    expect(count).toBe(1);
+  });
+});

src/common/storage/storage.service.ts

@@ -15,7 +15,7 @@ import {
   CreateBucketCommand,
 } from '@aws-sdk/client-s3';
 import { createLogger } from '../services/logger.service';
-import { isPathWithin } from '../utils/path-safety';
+import { isPathWithin, isSafeStorageKey } from '../utils/path-safety';
 
 interface S3Config {
   endpoint?: string;
@@ -25,6 +25,16 @@ interface S3Config {
   bucket?: string;
 }
 
+/** Per-entry buffer cap for an import (200 MiB — 4× the inbound media cap). Bounds a decompression bomb. */
+const DEFAULT_IMPORT_MAX_BYTES = 200 * 1024 * 1024;
+/** Max number of entries an import archive may contain. Bounds an entry-count DoS. */
+const DEFAULT_IMPORT_MAX_ENTRIES = 100_000;
+
+function positiveIntFromEnv(name: string, fallback: number): number {
+  const parsed = Number.parseInt(process.env[name] ?? '', 10);
+  return Number.isInteger(parsed) && parsed > 0 ? parsed : fallback;
+}
+
 @Injectable()
 export class StorageService {
   private readonly logger = createLogger('StorageService');
@@ -114,13 +124,23 @@ export class StorageService {
   }
 
   async getFile(filePath: string): Promise<Buffer> {
+    // Mirror putFile: getLocalFile has its own isPathWithin guard, but getS3File builds
+    // `media/${filePath}` with none — contain both read backends at this boundary.
+    if (!isSafeStorageKey(filePath)) {
+      throw new Error(`Refusing to read an unsafe storage key: ${filePath}`);
+    }
     if (this.storageType === 's3' && this.s3Client && this.s3Available) {
       return this.getS3File(filePath);
     }
     return this.getLocalFile(filePath);
   }
 
   async putFile(filePath: string, data: Buffer): Promise<void> {
+    // Centralized containment so BOTH backends inherit it: putLocalFile has its own isPathWithin
+    // guard, but putS3File builds `media/${filePath}` with none — reject a traversing key here.
+    if (!isSafeStorageKey(filePath)) {
+      throw new Error(`Refusing to store an unsafe storage key: ${filePath}`);
+    }
     if (this.storageType === 's3' && this.s3Client && this.s3Available) {
       return this.putS3File(filePath, data);
     }
@@ -191,18 +211,59 @@ export class StorageService {
   // Import - Extract tar.gz stream to current storage
   // ============================================================================
 
+  // Best-effort, NOT atomic: a single bad/traversing entry is skipped and the rest still import, and a
+  // resource-cap breach aborts the rest but KEEPS the entries already written (no rollback). Callers
+  // re-running an import is safe (putFile overwrites). A staging-dir + atomic promote would make it
+  // transactional, but is out of scope here.
   async importFromStream(inputStream: Readable): Promise<number> {
     let importedCount = 0;
+    let entryCount = 0;
+    const maxEntryBytes = positiveIntFromEnv('STORAGE_IMPORT_MAX_BYTES', DEFAULT_IMPORT_MAX_BYTES);
+    const maxEntries = positiveIntFromEnv('STORAGE_IMPORT_MAX_ENTRIES', DEFAULT_IMPORT_MAX_ENTRIES);
 
     const extract = tar.extract();
     const gunzip = createGunzip();
 
     return new Promise<number>((resolve, reject) => {
+      let settled = false;
+      // Abort the whole import: a per-entry overflow or too many entries is a (zip-bomb) attack, not
+      // a per-file skip — tear down the pipeline and reject so nothing further is buffered or written.
+      const fail = (err: Error): void => {
+        if (settled) return;
+        settled = true;
+        extract.destroy();
+        reject(err);
+      };
+
       extract.on('entry', (header, stream, next) => {
+        if (settled) {
+          stream.resume();
+          return;
+        }
+        if (++entryCount > maxEntries) {
+          stream.resume();
+          fail(new Error(`Import aborted: archive exceeds the ${maxEntries}-entry limit`));
+          return;
+        }
+
         const chunks: Buffer[] = [];
+        let entryBytes = 0;
+        let entryAborted = false;
+
+        stream.on('data', (chunk: Buffer) => {
+          if (entryAborted || settled) return;
+          entryBytes += chunk.length;
+          if (entryBytes > maxEntryBytes) {
+            entryAborted = true;
+            stream.resume(); // drain the remainder so the source can end
+            fail(new Error(`Import aborted: entry "${header.name}" exceeds the ${maxEntryBytes}-byte per-entry cap`));
+          } else {
+            chunks.push(chunk);
+          }
+        });
 
-        stream.on('data', (chunk: Buffer) => chunks.push(chunk));
         stream.on('end', () => {
+          if (entryAborted || settled) return;
           const data = Buffer.concat(chunks);
           this.putFile(header.name, data)
             .then(() => {
@@ -219,13 +280,15 @@ export class StorageService {
       });
 
       extract.on('finish', () => {
+        if (settled) return;
+        settled = true;
         this.logger.log(`Import completed: ${importedCount} files`);
         resolve(importedCount);
       });
 
       extract.on('error', (err: Error) => {
         this.logger.error('Import failed', String(err));
-        reject(err);
+        fail(err);
       });
 
       inputStream.pipe(gunzip).pipe(extract);
@@ -264,7 +327,9 @@ export class StorageService {
       throw new Error(`Refusing to read outside storage root: ${filePath}`);
     }
     const fullPath = path.join(this.localPath, filePath);
-    return Promise.resolve(fs.readFileSync(fullPath));
+    // Async read so the export loop (the only caller) yields the event loop per file instead of
+    // blocking it with a synchronous read for every media file.
+    return fs.promises.readFile(fullPath);
   }
 
   private putLocalFile(filePath: string, data: Buffer): Promise<void> {

src/common/utils/path-safety.spec.ts

@@ -1,5 +1,5 @@
 import * as path from 'path';
-import { isPathWithin } from './path-safety';
+import { isPathWithin, isSafeStorageKey } from './path-safety';
 
 describe('isPathWithin', () => {
   const root = path.resolve('/srv/app/data');
@@ -25,3 +25,32 @@ describe('isPathWithin', () => {
     expect(isPathWithin('/srv/app/data', '/srv/app/data-evil/x')).toBe(false);
   });
 });
+
+describe('isSafeStorageKey', () => {
+  it.each([
+    'file.jpg',
+    'sessionId/messageId.jpg',
+    'a/b/c.txt',
+    'group:sid:123@g.us.json', // plugin/JID-style keys must survive (':' '@' '.' '-')
+  ])('accepts the safe relative key %s', k => {
+    expect(isSafeStorageKey(k)).toBe(true);
+  });
+
+  it.each([
+    '../evil.txt',
+    'a/../../etc/passwd',
+    'media/../../../secret',
+    '/etc/passwd', // absolute
+    '..',
+    'a\\..\\b', // backslash traversal (pins the split on '\\' too)
+    '', // empty
+  ])('rejects the traversing/absolute key %j', k => {
+    expect(isSafeStorageKey(k)).toBe(false);
+  });
+
+  it('rejects a key containing a NUL or other control character (it reaches the raw S3 object key)', () => {
+    expect(isSafeStorageKey(`foo${String.fromCharCode(0)}.txt`)).toBe(false); // NUL
+    expect(isSafeStorageKey(`bar${String.fromCharCode(9)}.txt`)).toBe(false); // tab
+    expect(isSafeStorageKey(`baz${String.fromCharCode(31)}.txt`)).toBe(false); // unit separator
+  });
+});

src/common/utils/path-safety.ts

@@ -15,3 +15,19 @@ export function isPathWithin(root: string, target: string): boolean {
   const resolvedTarget = path.resolve(resolvedRoot, target);
   return resolvedTarget === resolvedRoot || resolvedTarget.startsWith(resolvedRoot + path.sep);
 }
+
+/**
+ * Returns true if `key` is a safe, contained relative storage key: a non-empty relative path with no
+ * `..` traversal segment. Used to validate untrusted archive entry names / object keys at the
+ * backend-agnostic `putFile`/`getFile` boundary so an S3 key (which has no host filesystem root to
+ * check against `isPathWithin`) still can't escape the intended `media/` prefix. Ordinary keys —
+ * including plugin/JID-style ones with `:`, `@`, `.`, `-` — are preserved.
+ */
+export function isSafeStorageKey(key: string): boolean {
+  if (typeof key !== 'string' || key.length === 0) return false;
+  // Reject NUL / control chars: harmless on the local FS but a NUL would reach the raw S3 object Key.
+  // eslint-disable-next-line no-control-regex
+  if (/[\u0000-\u001f]/.test(key)) return false;
+  if (path.isAbsolute(key)) return false;
+  return !key.split(/[/\\]/).includes('..');
+}