fix(security): tighten secret-file perms, optional key pepper, validate allowedIps, proxy-aware queue-UI auth

CHANGELOG.md

@@ -7,6 +7,24 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 
+### Security
+
+- **Generated secret files are written owner-only (0600).** `data/.env.generated` (which holds S3/DB/Redis
+  credentials) and `data/.api-key` (the raw admin key) were created world-readable; they are now `0600`, and
+  any pre-existing looser file is tightened on startup.
+- **Optional API-key pepper.** Set `API_KEY_PEPPER` to hash API keys with HMAC-SHA256 instead of plain
+  SHA-256, so a leak of the key database alone can't precompute candidates. Off by default — existing keys are
+  unaffected; enabling it is a deploy-time choice that requires re-issuing keys.
+- **`allowedIps` entries are validated as an IP address or CIDR range.** A malformed entry (e.g. a bare
+  hostname or `10.0.0.0/33`) is now rejected when creating/updating a key, instead of being stored silently
+  and never matching any request.
+- **The queue dashboard (Bull Board) auth uses the same trusted-proxy IP model as the API.** It now resolves
+  the client IP via `TRUSTED_PROXIES`, so an `allowedIps`-restricted ADMIN key is enforced consistently behind
+  a reverse proxy (previously it read the socket address directly).
+- **The production startup secret-guard inspects the S3 variables the app actually uses.** It now checks
+  `S3_ACCESS_KEY_ID`/`S3_SECRET_ACCESS_KEY` (falling back to the legacy `S3_ACCESS_KEY`/`S3_SECRET_KEY`),
+  matching the storage layer — closing a gap where a placeholder in the canonical variable was not caught.
+
 ## [0.4.2] - 2026-06-19
 
 Bug-fix and hardening release: access-control tightening, session-lifecycle resilience, data-migration

src/common/security/bull-board-auth.middleware.spec.ts

@@ -1,4 +1,5 @@
 import { UnauthorizedException, ForbiddenException } from '@nestjs/common';
+import { ConfigService } from '@nestjs/config';
 import { Request, Response } from 'express';
 import { BullBoardAuthMiddleware } from './bull-board-auth.middleware';
 import { AuthService } from '../../modules/auth/auth.service';
@@ -7,14 +8,16 @@ import { ApiKeyRole } from '../../modules/auth/entities/api-key.entity';
 describe('BullBoardAuthMiddleware', () => {
   let mw: BullBoardAuthMiddleware;
   let authService: { validateApiKey: jest.Mock; hasPermission: jest.Mock };
+  let configService: { get: jest.Mock };
   const res = {} as Response;
 
   const reqWith = (headers: Record<string, unknown> = {}, query: Record<string, unknown> = {}): Request =>
     ({ headers, query, ip: '127.0.0.1', socket: { remoteAddress: '127.0.0.1' } }) as unknown as Request;
 
   beforeEach(() => {
     authService = { validateApiKey: jest.fn(), hasPermission: jest.fn() };
-    mw = new BullBoardAuthMiddleware(authService as unknown as AuthService);
+    configService = { get: jest.fn().mockReturnValue(undefined) }; // no trusted proxies by default
+    mw = new BullBoardAuthMiddleware(authService as unknown as AuthService, configService as unknown as ConfigService);
   });
 
   it('rejects when no API key is provided', async () => {
@@ -56,6 +59,26 @@ describe('BullBoardAuthMiddleware', () => {
     expect(authService.validateApiKey).toHaveBeenCalledWith('abc', '127.0.0.1');
   });
 
+  it('honors X-Forwarded-For only behind a configured trusted proxy (allowedIps parity with the guard)', async () => {
+    configService.get.mockReturnValue(['127.0.0.1']); // the socket peer is a trusted proxy
+    authService.validateApiKey.mockResolvedValue({ role: ApiKeyRole.ADMIN });
+    authService.hasPermission.mockReturnValue(true);
+
+    await mw.use(reqWith({ 'x-api-key': 'admin', 'x-forwarded-for': '203.0.113.5' }), res, jest.fn());
+
+    expect(authService.validateApiKey).toHaveBeenCalledWith('admin', '203.0.113.5');
+  });
+
+  it('ignores a spoofed X-Forwarded-For when no trusted proxy is configured (uses the socket address)', async () => {
+    configService.get.mockReturnValue([]); // no trusted proxies — XFF is attacker-controlled
+    authService.validateApiKey.mockResolvedValue({ role: ApiKeyRole.ADMIN });
+    authService.hasPermission.mockReturnValue(true);
+
+    await mw.use(reqWith({ 'x-api-key': 'admin', 'x-forwarded-for': '203.0.113.5' }), res, jest.fn());
+
+    expect(authService.validateApiKey).toHaveBeenCalledWith('admin', '127.0.0.1');
+  });
+
   it('rejects an ?apiKey query param (no key in the URL)', async () => {
     const next = jest.fn();
     await mw.use(reqWith({}, { apiKey: 'qkey' }), res, next);

src/common/security/bull-board-auth.middleware.ts

@@ -1,7 +1,9 @@
 import { Injectable, NestMiddleware, UnauthorizedException, ForbiddenException } from '@nestjs/common';
+import { ConfigService } from '@nestjs/config';
 import { Request, Response, NextFunction } from 'express';
 import { AuthService } from '../../modules/auth/auth.service';
 import { ApiKeyRole } from '../../modules/auth/entities/api-key.entity';
+import { resolveClientIp } from '../utils/ip';
 
 /**
  * Protects the Bull Board UI (/admin/queues).
@@ -16,7 +18,10 @@ import { ApiKeyRole } from '../../modules/auth/entities/api-key.entity';
  */
 @Injectable()
 export class BullBoardAuthMiddleware implements NestMiddleware {
-  constructor(private readonly authService: AuthService) {}
+  constructor(
+    private readonly authService: AuthService,
+    private readonly configService: ConfigService,
+  ) {}
 
   async use(req: Request, _res: Response, next: NextFunction): Promise<void> {
     try {
@@ -49,6 +54,9 @@ export class BullBoardAuthMiddleware implements NestMiddleware {
   }
 
   private getClientIp(req: Request): string {
-    return req.socket?.remoteAddress || req.ip || '';
+    // Mirror the ApiKeyGuard's IP model so allowedIps is enforced consistently for the queue UI:
+    // X-Forwarded-For is honored only behind a configured trusted proxy.
+    const trustedProxies = this.configService.get<string[]>('security.trustedProxies') ?? [];
+    return resolveClientIp(req, trustedProxies);
   }
 }

src/common/utils/secret-file.spec.ts

@@ -0,0 +1,31 @@
+import { mkdtempSync, rmSync, statSync, writeFileSync } from 'fs';
+import { tmpdir } from 'os';
+import { join } from 'path';
+import { writeSecretFile } from './secret-file';
+
+describe('writeSecretFile', () => {
+  let dir: string;
+  beforeEach(() => {
+    dir = mkdtempSync(join(tmpdir(), 'owa-secret-'));
+  });
+  afterEach(() => {
+    rmSync(dir, { recursive: true, force: true });
+  });
+
+  const mode = (p: string): number => statSync(p).mode & 0o777;
+
+  it('writes a new secret file owner-only (no group/other access)', () => {
+    const p = join(dir, 'secret');
+    writeSecretFile(p, 'topsecret');
+    expect(mode(p) & 0o077).toBe(0);
+  });
+
+  it('tightens an already-existing world-readable file (writeFileSync mode only applies on create)', () => {
+    const p = join(dir, 'legacy');
+    writeFileSync(p, 'old', { mode: 0o644 });
+    expect(mode(p) & 0o077).not.toBe(0); // precondition: loose
+
+    writeSecretFile(p, 'new');
+    expect(mode(p) & 0o077).toBe(0);
+  });
+});

src/common/utils/secret-file.ts

@@ -0,0 +1,24 @@
+import { writeFileSync, chmodSync } from 'fs';
+
+/**
+ * Write a secret file (e.g. the generated `.env`, the raw admin key) with owner-only permissions.
+ *
+ * `writeFileSync`'s `mode` is honored only when the file is CREATED — on an overwrite it keeps the
+ * existing permissions. So we chmod to 0o600 BEFORE writing too: if the file already exists with
+ * looser perms, the new secret content is never briefly world-readable during the rewrite. The
+ * post-write chmod is a backstop. Both chmods are best-effort (a mount that can't chmod, or an
+ * absent file on the pre-write call, shouldn't break the write — create-mode covers new files).
+ */
+export function writeSecretFile(filePath: string, content: string): void {
+  try {
+    chmodSync(filePath, 0o600);
+  } catch {
+    /* file not present yet, or unsupported — create-mode below covers a new file */
+  }
+  writeFileSync(filePath, content, { mode: 0o600 });
+  try {
+    chmodSync(filePath, 0o600);
+  } catch {
+    /* best-effort */
+  }
+}

src/main.ts

@@ -1,5 +1,6 @@
 import { NestFactory } from '@nestjs/core';
 import { ValidationPipe } from '@nestjs/common';
+import { ConfigService } from '@nestjs/config';
 import { SwaggerModule } from '@nestjs/swagger';
 import helmet from 'helmet';
 import { AppModule, DASHBOARD_DIST, dashboardServingEnabled, dashboardBuildPresent } from './app.module';
@@ -15,6 +16,7 @@ import {
 import { BullBoardAuthMiddleware } from './common/security/bull-board-auth.middleware';
 import { AuthService } from './modules/auth/auth.service';
 import { Request, Response, NextFunction, json, urlencoded } from 'express';
+import { writeSecretFile } from './common/utils/secret-file';
 import * as dotenv from 'dotenv';
 import * as fs from 'fs';
 import * as path from 'path';
@@ -36,6 +38,18 @@ if (!fs.existsSync(dataDir)) {
   fs.mkdirSync(dataDir, { recursive: true });
 }
 
+// Tighten any pre-existing secret files written before per-file 0600 perms (best-effort) — the
+// generated .env holds S3/DB/Redis secrets and .api-key holds the raw admin key.
+for (const secret of [generatedEnvPath, path.resolve(dataDir, '.api-key')]) {
+  if (fs.existsSync(secret)) {
+    try {
+      fs.chmodSync(secret, 0o600);
+    } catch {
+      /* best-effort */
+    }
+  }
+}
+
 // 2. User-managed .env (does not override real process env)
 if (fs.existsSync(userEnvPath)) {
   console.log('[Bootstrap] Loading .env from:', userEnvPath);
@@ -70,7 +84,7 @@ STORAGE_PATH=./data/media
 
 # Docker Profiles: none (minimal setup)
 `;
-  fs.writeFileSync(generatedEnvPath, minimalConfig);
+  writeSecretFile(generatedEnvPath, minimalConfig);
   console.log('[Bootstrap] Created default configuration at:', generatedEnvPath);
   dotenv.config({ path: generatedEnvPath, override: false });
 }
@@ -96,8 +110,10 @@ async function bootstrap() {
     databaseType: process.env.DATABASE_TYPE,
     databasePassword: process.env.DATABASE_PASSWORD,
     storageType: process.env.STORAGE_TYPE,
-    s3AccessKey: process.env.S3_ACCESS_KEY,
-    s3SecretKey: process.env.S3_SECRET_KEY,
+    // Mirror storage.service's canonical-with-legacy fallback so the guard inspects the var the app
+    // actually uses (it reads S3_ACCESS_KEY_ID/S3_SECRET_ACCESS_KEY first).
+    s3AccessKey: process.env.S3_ACCESS_KEY_ID || process.env.S3_ACCESS_KEY,
+    s3SecretKey: process.env.S3_SECRET_ACCESS_KEY || process.env.S3_SECRET_KEY,
     apiMasterKey: process.env.API_MASTER_KEY,
     allowDevApiKey: process.env.ALLOW_DEV_API_KEY,
   });
@@ -215,7 +231,7 @@ async function bootstrap() {
   // @bull-board/nestjs as raw Express middleware that the global ApiKeyGuard
   // does not cover; registering this before app.listen() ensures it runs ahead
   // of the Bull Board router. Requires a valid ADMIN API key.
-  const bullBoardAuth = new BullBoardAuthMiddleware(app.get(AuthService));
+  const bullBoardAuth = new BullBoardAuthMiddleware(app.get(AuthService), app.get(ConfigService));
   app.use('/api/admin/queues', (req: Request, res: Response, next: NextFunction) => {
     void bullBoardAuth.use(req, res, next);
   });

src/modules/auth/api-key-hash.spec.ts

@@ -0,0 +1,19 @@
+import { createHash, createHmac } from 'crypto';
+import { hashApiKey } from './api-key-hash';
+
+describe('hashApiKey', () => {
+  it('uses plain SHA-256 when no pepper is set (preserves existing stored hashes)', () => {
+    expect(hashApiKey('owa_secret')).toBe(createHash('sha256').update('owa_secret').digest('hex'));
+    expect(hashApiKey('owa_secret', undefined)).toBe(createHash('sha256').update('owa_secret').digest('hex'));
+  });
+
+  it('uses HMAC-SHA256 with the pepper when set, distinct from the un-peppered hash', () => {
+    const peppered = hashApiKey('owa_secret', 'server-pepper');
+    expect(peppered).toBe(createHmac('sha256', 'server-pepper').update('owa_secret').digest('hex'));
+    expect(peppered).not.toBe(hashApiKey('owa_secret'));
+  });
+
+  it('is deterministic for the same key + pepper', () => {
+    expect(hashApiKey('k', 'p')).toBe(hashApiKey('k', 'p'));
+  });
+});

src/modules/auth/api-key-hash.ts

@@ -0,0 +1,14 @@
+import { createHash, createHmac } from 'crypto';
+
+/**
+ * Hash an API key for storage/lookup. With a server-side pepper (`API_KEY_PEPPER`) set, uses HMAC so
+ * a database leak alone can't precompute candidate hashes against a guessed/user-chosen key. Without
+ * a pepper it falls back to plain SHA-256 — unchanged behaviour, so existing stored hashes still
+ * validate. NOTE: enabling (or changing) the pepper invalidates keys hashed before it was set, so it
+ * is a deploy-time choice; rotate/re-issue keys when turning it on.
+ */
+export function hashApiKey(rawKey: string, pepper?: string): string {
+  return pepper
+    ? createHmac('sha256', pepper).update(rawKey).digest('hex')
+    : createHash('sha256').update(rawKey).digest('hex');
+}

src/modules/auth/auth.service.spec.ts

@@ -2,7 +2,7 @@ import { Test, TestingModule } from '@nestjs/testing';
 import { getRepositoryToken } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
 import { UnauthorizedException, NotFoundException } from '@nestjs/common';
-import { createHash } from 'crypto';
+import { createHash, createHmac } from 'crypto';
 import { AuthService, resolveSeedApiKey } from './auth.service';
 import { ApiKey, ApiKeyRole } from './entities/api-key.entity';
 
@@ -407,4 +407,35 @@ describe('AuthService', () => {
       expect(r2.id).toBe(key.id);
     });
   });
+
+  // ── API_KEY_PEPPER wiring ─────────────────────────────────────────
+  // Proves the service's hashing path actually reads the env var (not just the pure helper). We
+  // assert on the keyHash the service QUERIES findOne with, since the mock returns regardless.
+  describe('hashKey reads API_KEY_PEPPER', () => {
+    const ORIGINAL_ENV = process.env;
+    afterEach(() => {
+      process.env = ORIGINAL_ENV;
+    });
+
+    const queriedHash = async (rawKey: string): Promise<string> => {
+      (repository.findOne as jest.Mock).mockResolvedValue(null);
+      await expect(service.validateApiKey(rawKey)).rejects.toThrow(UnauthorizedException);
+      const calls = (repository.findOne as jest.Mock).mock.calls as Array<[{ where: { keyHash: string } }]>;
+      return calls[0][0].where.keyHash;
+    };
+
+    it('hashes with HMAC-SHA256 when the pepper is set', async () => {
+      process.env = { ...ORIGINAL_ENV, API_KEY_PEPPER: 'server-pepper' };
+      const queried = await queriedHash('owa_raw_key');
+      expect(queried).toBe(createHmac('sha256', 'server-pepper').update('owa_raw_key').digest('hex'));
+      expect(queried).not.toBe(createHash('sha256').update('owa_raw_key').digest('hex'));
+    });
+
+    it('hashes with plain SHA-256 when the pepper is unset (existing keys keep validating)', async () => {
+      process.env = { ...ORIGINAL_ENV };
+      delete process.env.API_KEY_PEPPER;
+      const queried = await queriedHash('owa_raw_key');
+      expect(queried).toBe(createHash('sha256').update('owa_raw_key').digest('hex'));
+    });
+  });
 });

src/modules/auth/auth.service.ts

@@ -1,9 +1,11 @@
 import { Injectable, NotFoundException, UnauthorizedException, OnModuleInit } from '@nestjs/common';
 import { InjectRepository } from '@nestjs/typeorm';
 import { Repository } from 'typeorm';
-import { createHash, randomBytes } from 'crypto';
-import { existsSync, writeFileSync, readFileSync } from 'fs';
+import { randomBytes } from 'crypto';
+import { existsSync, readFileSync } from 'fs';
 import { join } from 'path';
+import { writeSecretFile } from '../../common/utils/secret-file';
+import { hashApiKey } from './api-key-hash';
 import { ApiKey, ApiKeyRole } from './entities/api-key.entity';
 import { CreateApiKeyDto, UpdateApiKeyDto } from './dto';
 import { createLogger } from '../../common/services/logger.service';
@@ -53,9 +55,9 @@ export class AuthService implements OnModuleInit {
       await this.seedApiKey(displayKey, 'Default Admin Key', ApiKeyRole.ADMIN);
       isNewKey = true;
 
-      // Save raw key to file for startup script to read
+      // Save raw key to file for startup script to read (owner-only — it's the raw admin key).
       try {
-        writeFileSync(API_KEY_FILE, displayKey, 'utf-8');
+        writeSecretFile(API_KEY_FILE, displayKey);
       } catch (err) {
         this.logger.warn('Could not save API key file', { error: String(err) });
       }
@@ -243,7 +245,7 @@ export class AuthService implements OnModuleInit {
   }
 
   private hashKey(rawKey: string): string {
-    return createHash('sha256').update(rawKey).digest('hex');
+    return hashApiKey(rawKey, process.env.API_KEY_PEPPER);
   }
 
   private isIpAllowed(clientIp: string, allowedIps: string[]): boolean {

src/modules/auth/dto/api-key.dto.ts

@@ -1,6 +1,7 @@
-import { IsString, IsOptional, IsEnum, IsArray, IsDateString, MinLength, MaxLength } from 'class-validator';
+import { IsString, IsOptional, IsEnum, IsArray, IsDateString, MinLength, MaxLength, Validate } from 'class-validator';
 import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
 import { ApiKeyRole } from '../entities/api-key.entity';
+import { IsIpOrCidrConstraint } from './is-ip-or-cidr.validator';
 
 export class CreateApiKeyDto {
   @ApiProperty({
@@ -28,6 +29,7 @@ export class CreateApiKeyDto {
   @IsOptional()
   @IsArray()
   @IsString({ each: true })
+  @Validate(IsIpOrCidrConstraint, { each: true })
   allowedIps?: string[];
 
   @ApiPropertyOptional({
@@ -110,6 +112,7 @@ export class UpdateApiKeyDto {
   @IsOptional()
   @IsArray()
   @IsString({ each: true })
+  @Validate(IsIpOrCidrConstraint, { each: true })
   allowedIps?: string[];
 
   @ApiPropertyOptional()