From 90acf181f33391d92bc26f9e9dbb83d70d01bb4c Mon Sep 17 00:00:00 2001
From: Linus Rath <139418639+rathlinus@users.noreply.github.com>
Date: Tue, 28 Apr 2026 01:44:37 +0200
Subject: [PATCH] fix: harden plugin config, TOTP token exchange, and branding
 file serving

---
 app/admin/layout.tsx                       |  2 ++
 app/api/admin/branding/[filename]/route.ts |  8 +++++
 app/api/admin/plugins/[id]/config/route.ts | 36 ++++++++++++++++---
 app/api/auth/totp-token-exchange/route.ts  | 40 ++++++++++++++++------
 instrumentation.node.ts                    |  3 +-
 lib/oauth/discovery.ts                     | 15 +++++++-
 6 files changed, 86 insertions(+), 18 deletions(-)

diff --git a/app/admin/layout.tsx b/app/admin/layout.tsx
index 41fe9e05..7d8a819b 100644
--- a/app/admin/layout.tsx
+++ b/app/admin/layout.tsx
@@ -14,6 +14,7 @@ import {
   KeyRound,
   Puzzle,
   SwatchBook,
+  Activity,
   Mail,
   Calendar,
   BookUser,
@@ -56,6 +57,7 @@ const NAV_GROUPS = [
   {
     label: 'System',
     items: [
+      { href: '/admin/telemetry', label: 'Telemetry', icon: Activity },
       { href: '/admin/logs', label: 'Audit Log', icon: ScrollText },
     ],
   },
diff --git a/app/api/admin/branding/[filename]/route.ts b/app/api/admin/branding/[filename]/route.ts
index af8ab53a..d4994c50 100644
--- a/app/api/admin/branding/[filename]/route.ts
+++ b/app/api/admin/branding/[filename]/route.ts
@@ -53,11 +53,19 @@ export async function GET(
 
     const buffer = await readFile(resolved);
 
+    // SVG can carry inline <script> and event handlers that execute when the
+    // file is fetched as a top-level document. Defense in depth on top of
+    // admin-only upload: nosniff blocks MIME confusion, the CSP forces a
+    // sandboxed unique origin so any script in an SVG is inert and cannot
+    // touch app cookies or storage.
     return new NextResponse(buffer, {
       headers: {
         'Content-Type': contentType,
         'Cache-Control': 'public, max-age=3600, must-revalidate',
         'Content-Length': String(buffer.length),
+        'X-Content-Type-Options': 'nosniff',
+        'Content-Security-Policy':
+          "default-src 'none'; img-src 'self' data:; style-src 'unsafe-inline'; sandbox",
       },
     });
   } catch {
diff --git a/app/api/admin/plugins/[id]/config/route.ts b/app/api/admin/plugins/[id]/config/route.ts
index 7260956f..b8ea0029 100644
--- a/app/api/admin/plugins/[id]/config/route.ts
+++ b/app/api/admin/plugins/[id]/config/route.ts
@@ -2,15 +2,20 @@ import { NextRequest, NextResponse } from 'next/server';
 import { getPlugin } from '@/lib/admin/plugin-registry';
 import { getPluginConfig, setPluginConfig, deletePluginConfigKey } from '@/lib/admin/plugin-config';
 import { requireAdminAuth } from '@/lib/admin/session';
+import { getStalwartCredentials } from '@/lib/stalwart/credentials';
 
 /**
- * GET /api/admin/plugins/[id]/config - Read all config for a plugin
+ * GET /api/admin/plugins/[id]/config - Read plugin config
  *
- * Returns the full config object for admin-configured plugin settings.
- * This endpoint is accessible from the client-side plugin API.
+ * - Admin sessions receive every field, including those declared
+ *   `type: 'secret'` in the plugin's configSchema.
+ * - Authenticated mailbox users (the plugin running in their browser)
+ *   receive only non-secret fields.
+ * - Anonymous callers are rejected so unauthenticated visitors cannot
+ *   enumerate plugin secrets.
  */
 export async function GET(
-  _request: NextRequest,
+  request: NextRequest,
   { params }: { params: Promise<{ id: string }> },
 ) {
   try {
@@ -20,13 +25,34 @@ export async function GET(
       return NextResponse.json({ error: 'Invalid plugin ID' }, { status: 400 });
     }
 
+    const adminAuth = await requireAdminAuth();
+    const isAdmin = !('error' in adminAuth);
+
+    if (!isAdmin) {
+      const creds = await getStalwartCredentials(request);
+      if (!creds) {
+        return NextResponse.json({ error: 'Not authenticated' }, { status: 401 });
+      }
+    }
+
     const plugin = await getPlugin(id);
     if (!plugin) {
       return NextResponse.json({ error: 'Plugin not found' }, { status: 404 });
     }
 
     const config = await getPluginConfig(id);
-    return NextResponse.json(config, {
+
+    let response: Record<string, unknown> = config;
+    if (!isAdmin && plugin.configSchema) {
+      response = {};
+      for (const [key, value] of Object.entries(config)) {
+        const field = plugin.configSchema[key];
+        if (field?.type === 'secret') continue;
+        response[key] = value;
+      }
+    }
+
+    return NextResponse.json(response, {
       headers: { 'Cache-Control': 'no-store' },
     });
   } catch {
diff --git a/app/api/auth/totp-token-exchange/route.ts b/app/api/auth/totp-token-exchange/route.ts
index 2ee7226b..4cede47a 100644
--- a/app/api/auth/totp-token-exchange/route.ts
+++ b/app/api/auth/totp-token-exchange/route.ts
@@ -6,6 +6,7 @@ import { refreshTokenCookieName } from '@/lib/oauth/tokens';
 import { getCookieOptions } from '@/lib/oauth/cookie-config';
 import { readFileEnv } from '@/lib/read-file-env';
 import { configManager } from '@/lib/admin/config-manager';
+import { isPublicHttpUrl } from '@/lib/security/url-guard';
 
 /**
  * Exchange basic auth credentials (with TOTP appended) for OAuth tokens.
@@ -84,19 +85,36 @@ export async function POST(request: NextRequest) {
 
     const slot = typeof bodySlot === 'number' && bodySlot >= 0 && bodySlot <= 4 ? bodySlot : 0;
 
-    // Use the server-side JMAP_SERVER_URL if set (may differ from the
-    // public URL the browser uses, e.g. inside Docker).
-    const internalServerUrl = process.env.JMAP_SERVER_URL || process.env.NEXT_PUBLIC_JMAP_SERVER_URL || serverUrl;
+    // Pin the upstream URL to the configured JMAP server so an unauthenticated
+    // caller cannot point this route at internal hosts. Only when no server
+    // URL is configured (and the deployment explicitly allows custom JMAP
+    // endpoints) do we fall back to the user-supplied URL — and even then
+    // it must resolve to a public address.
+    await configManager.ensureLoaded();
+    const configuredServerUrl =
+      configManager.get<string>('jmapServerUrl', '') ||
+      process.env.JMAP_SERVER_URL ||
+      process.env.NEXT_PUBLIC_JMAP_SERVER_URL ||
+      '';
+    const allowCustomEndpoint = configManager.get<boolean>('allowCustomJmapEndpoint', false);
 
-    const tokenEndpoint = await findTokenEndpoint(internalServerUrl);
-    if (!tokenEndpoint) {
-      // Also try with the client-provided URL in case the internal one differs
-      const clientEndpoint = internalServerUrl !== serverUrl ? await findTokenEndpoint(serverUrl) : null;
-      if (!clientEndpoint) {
-        logger.warn('TOTP token exchange: no token endpoint found', { serverUrl, internalServerUrl });
-        return NextResponse.json({ error: 'no_token_endpoint', detail: 'Could not discover OAuth token endpoint on the mail server' }, { status: 404 });
+    let upstreamUrl: string;
+    if (configuredServerUrl) {
+      upstreamUrl = configuredServerUrl;
+    } else if (allowCustomEndpoint) {
+      if (!(await isPublicHttpUrl(serverUrl))) {
+        logger.warn('TOTP token exchange: rejected non-public server URL');
+        return NextResponse.json({ error: 'invalid_server_url' }, { status: 400 });
       }
-      return await attemptAllStrategies(clientEndpoint, username, password, slot);
+      upstreamUrl = serverUrl;
+    } else {
+      return NextResponse.json({ error: 'jmap_server_not_configured' }, { status: 500 });
+    }
+
+    const tokenEndpoint = await findTokenEndpoint(upstreamUrl);
+    if (!tokenEndpoint) {
+      logger.warn('TOTP token exchange: no token endpoint found');
+      return NextResponse.json({ error: 'no_token_endpoint', detail: 'Could not discover OAuth token endpoint on the mail server' }, { status: 404 });
     }
 
     return await attemptAllStrategies(tokenEndpoint, username, password, slot);
diff --git a/instrumentation.node.ts b/instrumentation.node.ts
index 3a286b78..3546ba4c 100644
--- a/instrumentation.node.ts
+++ b/instrumentation.node.ts
@@ -52,7 +52,8 @@ configManager.load()
     console.info("Admin dashboard initialized");
   })
   .then(async () => {
-    // Anonymous telemetry - opt-in, off until admin consents.
+    // Anonymous telemetry - on by default. Admins can disable via the
+    // admin UI, the BULWARK_TELEMETRY env var, or by clearing the endpoint.
     // See https://bulwarkmail.org/docs/legal/privacy/telemetry
     const { startScheduler, markProcessStart } = await import("./lib/telemetry");
     markProcessStart();
diff --git a/lib/oauth/discovery.ts b/lib/oauth/discovery.ts
index 36f869b0..8a7eab55 100644
--- a/lib/oauth/discovery.ts
+++ b/lib/oauth/discovery.ts
@@ -7,8 +7,21 @@ export interface OAuthMetadata {
 }
 
 const CACHE_TTL_MS = 10 * 60 * 1000;
+const CACHE_MAX_ENTRIES = 64;
 const metadataCache = new Map<string, { metadata: OAuthMetadata; expiresAt: number }>();
 
+function rememberMetadata(serverUrl: string, metadata: OAuthMetadata): void {
+  // Bound the cache so callers that can supply arbitrary serverUrl values
+  // (e.g. unauthenticated routes that fall back to user input) cannot
+  // exhaust memory. Map preserves insertion order, so the oldest entry is
+  // always the first one yielded by keys().
+  if (metadataCache.size >= CACHE_MAX_ENTRIES) {
+    const oldest = metadataCache.keys().next().value;
+    if (oldest !== undefined) metadataCache.delete(oldest);
+  }
+  metadataCache.set(serverUrl, { metadata, expiresAt: Date.now() + CACHE_TTL_MS });
+}
+
 export async function discoverOAuth(serverUrl: string): Promise<OAuthMetadata | null> {
   const cached = metadataCache.get(serverUrl);
   if (cached && cached.expiresAt > Date.now()) return cached.metadata;
@@ -38,7 +51,7 @@ export async function discoverOAuth(serverUrl: string): Promise<OAuthMetadata |
           revocation_endpoint: data.revocation_endpoint,
           end_session_endpoint: data.end_session_endpoint,
         };
-        metadataCache.set(serverUrl, { metadata, expiresAt: Date.now() + CACHE_TTL_MS });
+        rememberMetadata(serverUrl, metadata);
         return metadata;
       }
       errors.push(`${url} response missing required endpoints`);