fix: use allow list also for image proxy and update readme recommendations (#141)

Author: JannikStreek

.env.example

@@ -5,6 +5,10 @@ NODE_ENV=development
 # Supports wildcards for subdomains, e.g. *.example.com
 CORS_ALLOWED_ORIGINS=http://localhost:5173
 
+# Number of trusted reverse proxy hops (default: 0 = disabled, max: 10)
+# Set to 1 when behind a single reverse proxy (e.g. nginx) so rate limiting uses the real client IP
+# TRUST_PROXY=0
+
 # Rate Limiting / Throttle Configuration
 # THROTTLE_TTL=60000  # Time-to-live in milliseconds (default: 60000 = 1 minute)
 # THROTTLE_LIMIT=10  # Maximum requests per TTL window (default: 10)
@@ -62,6 +66,15 @@ NOSTR_AMB_RELAY_URL=ws://amb-relay:3334
 # Example: PUBLIC_BASE_URL=https://oer.example.com
 # PUBLIC_BASE_URL=
 
+# Asset Proxy Security Configuration
+# Timeout for proxied asset fetches in milliseconds (default: 15000, range: 1000-30000)
+# ASSET_PROXY_TIMEOUT_MS=15000
+
+# Comma-separated domain allowlist for asset proxy (empty = allow all)
+# Subdomains are matched automatically. Strongly recommended in production to prevent SSRF.
+# Example: ASSET_PROXY_ALLOWED_DOMAINS=arasaac.org,upload.wikimedia.org,live.staticflickr.com
+# ASSET_PROXY_ALLOWED_DOMAINS=
+
 # RPI-Virtuell Materialpool Configuration (optional, has default)
 # GraphQL API endpoint for RPI-Virtuell Materialpool
 # Default: https://material.rpi-virtuell.de/graphql

AGENTS.md

@@ -75,8 +75,10 @@ All adapters implement `SourceAdapter` from `oer-adapter-core` and normalize res
 | `ASSET_PROXY_ALLOWED_DOMAINS` | `''` | Comma-separated domain allowlist for asset proxy (empty = allow all). Subdomains are matched automatically |
 | `PUBLIC_BASE_URL` | `''` | Base URL for signed asset URLs (falls back to localhost) |
 | `CORS_ALLOWED_ORIGINS` | `''` | Comma-separated allowed origins (empty = allow all). Supports wildcards e.g. `*.example.com` |
+| `TRUST_PROXY` | `0` | Number of trusted reverse proxy hops (0 = disabled, max 10) |
 | `THROTTLE_TTL` | `60000` | Rate limit window (ms) |
 | `THROTTLE_LIMIT` | `30` | Requests per window |
+| `THROTTLE_BLOCK_DURATION` | `60000` | Block duration (ms) after exceeding limit |
 
 ## Build & Test
 

README.md

@@ -172,6 +172,8 @@ pnpm add @edufeed-org/oer-finder-plugin
 - 🔒 **Rate Limiting** - Per-IP rate limiting for API protection
 - 🔌 **Extensible** - Add custom adapters to integrate any external OER API
 
+> **Security Note — Nostr AMB Relay adapter:** If you plan to use the `nostr-amb-relay` adapter, it is recommended to use it in **direct-client mode** (browser-side) only. If you need to use it through the proxy server, configure **imgproxy** and set `ASSET_PROXY_ALLOWED_DOMAINS` to restrict which domains the proxy may contact. AMB relay events can contain arbitrary URLs; when the proxy fetches these server-side, a malicious event could point to internal network resources (SSRF). See the [Server Setup Guide](./docs/server-setup.md#nostr-amb-relay-security) for details.
+
 ## API Example
 
 ```bash

docs/server-setup.md

@@ -62,6 +62,17 @@ The application uses environment variables for configuration. See `.env.example`
 | Variable | Default | Description |
 |----------|---------|-------------|
 | `PORT` | `3000` | HTTP server port |
+| `NODE_ENV` | `development` | `development`, `production`, or `test` |
+| `CORS_ALLOWED_ORIGINS` | - | Comma-separated allowed origins (empty = allow all). Supports wildcards e.g. `*.example.com` |
+| `TRUST_PROXY` | `0` | Number of trusted reverse proxy hops (0 = disabled, max 10). Set to `1` when behind a single reverse proxy (e.g. nginx) so rate limiting uses the real client IP |
+
+### Rate Limiting
+
+| Variable | Default | Description |
+|----------|---------|-------------|
+| `THROTTLE_TTL` | `60000` | Rate limit window in milliseconds |
+| `THROTTLE_LIMIT` | `30` | Maximum requests per window |
+| `THROTTLE_BLOCK_DURATION` | `60000` | Block duration in milliseconds after exceeding the limit |
 
 ### Source Adapter Configuration
 
@@ -76,7 +87,7 @@ Source adapters forward search requests to external OER sources. Adapters are en
 
 | Adapter ID | Description | Additional Config |
 |------------|-------------|-------------------|
-| `nostr-amb-relay` | AMB Nostr relay for educational metadata | `NOSTR_AMB_RELAY_URL` required |
+| `nostr-amb-relay` | AMB Nostr relay for educational metadata | `NOSTR_AMB_RELAY_URL` required. See [security note](#nostr-amb-relay-security) |
 | `arasaac` | ARASAAC AAC pictograms (CC BY-NC-SA 4.0) | None required |
 | `openverse` | Openverse openly licensed media | None required |
 | `rpi-virtuell` | RPI-Virtuell religious education materials | Optional `RPI_VIRTUELL_API_URL` |
@@ -93,6 +104,12 @@ NOSTR_AMB_RELAY_URL=ws://amb-relay:3334
 ADAPTER_TIMEOUT_MS=5000
 ```
 
+#### Nostr AMB Relay Security
+
+> **Recommendation:** If you plan to use the `nostr-amb-relay` adapter, it is recommended to use it in **direct-client mode only** (running in the browser). If you need to use it through the proxy server, **configure imgproxy** and set `ASSET_PROXY_ALLOWED_DOMAINS` to restrict which domains the proxy may contact.
+>
+> **Why:** AMB relay events can contain arbitrary URLs (e.g. thumbnails, resource links). When the proxy fetches these URLs server-side, a malicious event could include URLs pointing to internal network resources (e.g. `http://169.254.169.254/...`, `http://localhost:...`), causing the proxy to make requests to services that should not be publicly reachable (SSRF). In direct-client mode, these requests come from the user's browser, which does not have access to the server's internal network. When using imgproxy, asset fetching is handled by imgproxy's own safeguards rather than the proxy application directly.
+
 When adapters are enabled:
 - The `source` query parameter selects which adapter to query (required)
 - With `source=nostr-amb-relay`: queries the AMB Nostr relay
@@ -127,6 +144,12 @@ The proxy supports optional [imgproxy](https://imgproxy.net/) integration. When
 - **Insecure**: Set only `IMGPROXY_BASE_URL`. URLs are generated without signatures.
 - **Secure**: Set all three variables. URLs are signed with HMAC-SHA256.
 
+**SSRF hardening**: imgproxy fetches arbitrary source URLs server-side. While imgproxy has built-in loopback protections, [past CVEs have shown bypasses](https://github.com/imgproxy/imgproxy/security/advisories/GHSA-j2hp-6m75-v4j4). To mitigate SSRF risks:
+1. **Set `ASSET_PROXY_ALLOWED_DOMAINS`** — the proxy applies this allowlist *before* any URL reaches imgproxy, blocking URLs to non-allowlisted domains at the application layer.
+2. **Network isolation** — run imgproxy in a network segment that cannot reach internal services (e.g. cloud metadata endpoints, databases, admin interfaces). In Docker Compose, use a dedicated network with no access to internal services.
+3. **Keep imgproxy updated** — patches close known bypasses.
+4. **Use URL signing** — prevents attackers from crafting arbitrary imgproxy URLs.
+
 ```bash
 # Generate secure keys (Linux/macOS)
 echo $(xxd -g 2 -l 64 -p /dev/random | tr -d '\n')
@@ -167,6 +190,15 @@ ASSET_SIGNING_TTL_SECONDS=3600
 PUBLIC_BASE_URL=https://oer.example.com
 ```
 
+#### Asset Proxy Security
+
+| Variable | Default | Description |
+|----------|---------|-------------|
+| `ASSET_PROXY_TIMEOUT_MS` | `15000` | Per-asset proxy fetch timeout in ms (range 1000–30000) |
+| `ASSET_PROXY_ALLOWED_DOMAINS` | - | Comma-separated domain allowlist for asset proxy (empty = allow all). Subdomains are matched automatically |
+
+When the proxy fetches assets on behalf of clients, these settings control timeouts and restrict which external domains are allowed. Setting `ASSET_PROXY_ALLOWED_DOMAINS` is **strongly recommended** in production to prevent the proxy from being used to probe internal network resources (SSRF).
+
 **Priority**: When both imgproxy and asset signing are configured, imgproxy takes priority for generating image URLs from source URLs. Asset signing is still used to sign adapter-provided image URLs.
 
 ## API Documentation

src/oer/controllers/asset-proxy.controller.spec.ts

@@ -7,6 +7,7 @@ import { PassThrough } from 'node:stream';
 import * as dnsPromises from 'node:dns/promises';
 import { AssetProxyController } from './asset-proxy.controller';
 import { AssetSigningService } from '../services/asset-signing.service';
+import { DomainAllowlistService } from '../services/domain-allowlist.service';
 import { AssetProxyEnabledGuard } from '../guards/asset-proxy-enabled.guard';
 
 jest.mock('node:dns/promises', () => ({
@@ -95,14 +96,19 @@ describe('AssetProxyController', () => {
       controllers: [AssetProxyController],
       providers: [
         { provide: AssetSigningService, useValue: assetSigningService },
+        {
+          provide: DomainAllowlistService,
+          useValue: {
+            isDomainAllowed: jest.fn().mockReturnValue(true),
+          },
+        },
         {
           provide: ConfigService,
           useValue: {
             get: jest
               .fn()
               .mockImplementation((key: string, defaultValue: unknown) => {
                 if (key === 'app.assetProxy.timeoutMs') return 15000;
-                if (key === 'app.assetProxy.allowedDomains') return [];
                 return defaultValue;
               }),
           },
@@ -814,6 +820,7 @@ describe('AssetProxyController', () => {
 describe('AssetProxyController (domain allowlist)', () => {
   let controller: AssetProxyController;
   let assetSigningService: jest.Mocked<AssetSigningService>;
+  let domainAllowlistService: jest.Mocked<DomainAllowlistService>;
   let mockFetch: jest.SpyInstance;
 
   beforeEach(async () => {
@@ -824,19 +831,25 @@ describe('AssetProxyController (domain allowlist)', () => {
       generateSignedUrl: jest.fn(),
     } as unknown as jest.Mocked<AssetSigningService>;
 
+    domainAllowlistService = {
+      isDomainAllowed: jest.fn().mockReturnValue(true),
+    } as unknown as jest.Mocked<DomainAllowlistService>;
+
     const module: TestingModule = await Test.createTestingModule({
       controllers: [AssetProxyController],
       providers: [
         { provide: AssetSigningService, useValue: assetSigningService },
+        {
+          provide: DomainAllowlistService,
+          useValue: domainAllowlistService,
+        },
         {
           provide: ConfigService,
           useValue: {
             get: jest
               .fn()
               .mockImplementation((key: string, defaultValue: unknown) => {
                 if (key === 'app.assetProxy.timeoutMs') return 15000;
-                if (key === 'app.assetProxy.allowedDomains')
-                  return ['example.com', 'cdn.images.org'];
                 return defaultValue;
               }),
           },
@@ -915,6 +928,7 @@ describe('AssetProxyController (domain allowlist)', () => {
     assetSigningService.verify.mockReturnValue(
       'https://evil.attacker.com/image.jpg',
     );
+    domainAllowlistService.isDomainAllowed.mockReturnValue(false);
 
     const res = createMockResponse();
     const req = createMockRequest();
@@ -932,10 +946,10 @@ describe('AssetProxyController (domain allowlist)', () => {
     );
   });
 
-  it('should not match partial domain names', async () => {
-    assetSigningService.verify.mockReturnValue(
-      'https://notexample.com/image.jpg',
-    );
+  it('should call isDomainAllowed with the verified URL', async () => {
+    const verifiedUrl = 'https://example.com/image.jpg';
+    assetSigningService.verify.mockReturnValue(verifiedUrl);
+    domainAllowlistService.isDomainAllowed.mockReturnValue(false);
 
     const res = createMockResponse();
     const req = createMockRequest();
@@ -951,5 +965,9 @@ describe('AssetProxyController (domain allowlist)', () => {
         ),
       HttpStatus.FORBIDDEN,
     );
+
+    expect(domainAllowlistService.isDomainAllowed).toHaveBeenCalledWith(
+      verifiedUrl,
+    );
   });
 });

src/oer/controllers/asset-proxy.controller.ts

@@ -24,6 +24,7 @@ import type { Request, Response } from 'express';
 import { Readable, Transform } from 'node:stream';
 import { lookup } from 'node:dns/promises';
 import { AssetSigningService } from '../services/asset-signing.service';
+import { DomainAllowlistService } from '../services/domain-allowlist.service';
 import { AssetCorpExceptionFilter } from '../filters/asset-corp-exception.filter';
 import { AssetProxyEnabledGuard } from '../guards/asset-proxy-enabled.guard';
 import { isPrivateIp } from '../utils/is-private-ip';
@@ -47,20 +48,16 @@ const ALLOWED_IMAGE_TYPES = new Set([
 export class AssetProxyController {
   private readonly logger = new Logger(AssetProxyController.name);
   private readonly timeoutMs: number;
-  private readonly allowedDomains: ReadonlyArray<string>;
 
   constructor(
     private readonly assetSigningService: AssetSigningService,
+    private readonly domainAllowlistService: DomainAllowlistService,
     private readonly configService: ConfigService,
   ) {
     this.timeoutMs = this.configService.get<number>(
       'app.assetProxy.timeoutMs',
       15000,
     );
-    this.allowedDomains = this.configService.get<string[]>(
-      'app.assetProxy.allowedDomains',
-      [],
-    );
   }
 
   @Get(':signature')
@@ -177,23 +174,17 @@ export class AssetProxyController {
     const parsed = new URL(url);
     const hostnameValue = parsed.hostname.toLowerCase();
 
-    if (this.allowedDomains.length > 0) {
-      const isAllowed = this.allowedDomains.some(
-        (domain) =>
-          hostnameValue === domain || hostnameValue.endsWith(`.${domain}`),
+    if (!this.domainAllowlistService.isDomainAllowed(url)) {
+      this.logger.warn(
+        `Blocked proxy to non-allowlisted domain: ${hostnameValue}`,
+      );
+      throw new HttpException(
+        {
+          statusCode: HttpStatus.FORBIDDEN,
+          message: 'Domain not allowed',
+        },
+        HttpStatus.FORBIDDEN,
       );
-      if (!isAllowed) {
-        this.logger.warn(
-          `Blocked proxy to non-allowlisted domain: ${hostnameValue}`,
-        );
-        throw new HttpException(
-          {
-            statusCode: HttpStatus.FORBIDDEN,
-            message: 'Domain not allowed',
-          },
-          HttpStatus.FORBIDDEN,
-        );
-      }
     }
 
     // Check if hostname is an IP literal

src/oer/oer.module.ts

@@ -3,6 +3,7 @@ import { OerQueryService } from './services/oer-query.service';
 import { ImgproxyService } from './services/imgproxy.service';
 import { AssetSigningService } from './services/asset-signing.service';
 import { AssetUrlService } from './services/asset-url.service';
+import { DomainAllowlistService } from './services/domain-allowlist.service';
 import { OerController } from './controllers/oer.controller';
 import { AssetProxyController } from './controllers/asset-proxy.controller';
 import { AdapterModule } from '../adapter';
@@ -15,6 +16,7 @@ import { AdapterModule } from '../adapter';
     ImgproxyService,
     AssetSigningService,
     AssetUrlService,
+    DomainAllowlistService,
   ],
   exports: [
     OerQueryService,