fix: stop trusting the unprefixed sidebar-app frame cookie when COOKIE_SECURE=true (GHSA-cjm9-m826-mh78)

lib/__tests__/proxy-security-headers.test.ts

@@ -191,6 +191,44 @@ describe('frame-ancestors of the mail UI', () => {
   });
 });
 
+// A same-site sibling host can plant the unprefixed Sidebar Apps cookie; only
+// the __Host- one is trusted over HTTPS. Behind a TLS proxy that sends no
+// X-Forwarded-Proto the request looks like plain HTTP, so COOKIE_SECURE=true
+// has to count as HTTPS here too (GHSA-cjm9-m826-mh78).
+describe('Sidebar Apps frame-src cookie', () => {
+  afterEach(() => vi.unstubAllEnvs());
+
+  const frameSrcWith = async (url: string, cookie: string, extra: Record<string, string> = {}) => {
+    const res = await proxy(new NextRequest(url + '/en/mail', {
+      headers: { 'sec-fetch-dest': 'document', 'sec-fetch-mode': 'navigate', 'sec-fetch-site': 'none', cookie, ...extra },
+    }));
+    return /frame-src [^;]*/.exec(res.headers.get('content-security-policy') ?? '')?.[0];
+  };
+  const planted = 'bulwark_app_frame_origins=https%3A%2F%2Fevil.example';
+
+  it('trusts the unprefixed cookie on plain HTTP', async () => {
+    expect(await frameSrcWith('http://localhost:3000', planted)).toContain('https://evil.example');
+  });
+
+  it('ignores the unprefixed cookie when COOKIE_SECURE=true, even without X-Forwarded-Proto', async () => {
+    vi.stubEnv('COOKIE_SECURE', 'true');
+    expect(await frameSrcWith('http://webmail:3000', planted)).not.toContain('evil.example');
+    expect(await frameSrcWith('http://webmail:3000', '__Host-bulwark_app_frame_origins=https%3A%2F%2Fboard.example'))
+      .toContain('https://board.example');
+  });
+
+  it('ignores the unprefixed cookie behind a chain of proxies that starts with https', async () => {
+    expect(await frameSrcWith('http://webmail:3000', planted, { 'x-forwarded-proto': 'https, http' }))
+      .not.toContain('evil.example');
+  });
+
+  it('sends HSTS when COOKIE_SECURE=true says the site is served over TLS', async () => {
+    vi.stubEnv('COOKIE_SECURE', 'true');
+    const res = await load('/en/mail');
+    expect(res.headers.get('strict-transport-security')).toBe('max-age=63072000');
+  });
+});
+
 describe('isStaticAssetPath', () => {
   it('matches only root-level files and the public/ asset folders', () => {
     for (const path of STATIC_ASSETS) expect(isStaticAssetPath(path), path).toBe(true);

lib/__tests__/setup-session-cookie.test.ts

@@ -0,0 +1,25 @@
+import { afterEach, describe, expect, it, vi } from 'vitest';
+import { NextRequest } from 'next/server';
+import { buildSessionCookieAttributes } from '@/lib/setup/session';
+
+// The wizard cookie carries the setup token. Like the admin cookie, its Secure
+// flag follows the request unless COOKIE_SECURE says otherwise: behind a TLS
+// proxy that sends no X-Forwarded-Proto the request looks like plain HTTP.
+describe('setup wizard cookie Secure flag', () => {
+  afterEach(() => vi.unstubAllEnvs());
+
+  const secureFor = (url: string, headers: Record<string, string> = {}) =>
+    buildSessionCookieAttributes(new NextRequest(url, { headers })).secure;
+
+  it('follows the request', () => {
+    expect(secureFor('http://app:3000/api/setup/token', { 'x-forwarded-proto': 'https' })).toBe(true);
+    expect(secureFor('http://app:3000/api/setup/token')).toBe(false);
+  });
+
+  it('lets COOKIE_SECURE win over the request', () => {
+    vi.stubEnv('COOKIE_SECURE', 'true');
+    expect(secureFor('http://app:3000/api/setup/token')).toBe(true);
+    vi.stubEnv('COOKIE_SECURE', 'false');
+    expect(secureFor('http://app:3000/api/setup/token', { 'x-forwarded-proto': 'https' })).toBe(false);
+  });
+});

lib/admin/session.ts

@@ -1,5 +1,5 @@
 import { cookies } from 'next/headers';
-import { isHttpsRequest } from '@/lib/security/request-protocol';
+import { secureCookieFlag } from '@/lib/security/request-protocol';
 import { NextResponse } from 'next/server';
 import { createCipheriv, createDecipheriv, randomBytes, createHash } from 'node:crypto';
 import { getSessionSecret } from '@/lib/auth/session-secret';
@@ -130,9 +130,7 @@ export async function setAdminSessionCookie(request?: Request): Promise<void> {
   const cookieStore = await cookies();
   cookieStore.set(ADMIN_SESSION_COOKIE, token, {
     httpOnly: true,
-    secure: process.env.COOKIE_SECURE !== undefined
-      ? process.env.COOKIE_SECURE === 'true'
-      : request ? isHttpsRequest(request) : process.env.NODE_ENV === 'production',
+    secure: secureCookieFlag(request),
     sameSite: 'lax',
     path: '/',
     maxAge: getSessionTTL(),

lib/security/request-protocol.ts

@@ -18,3 +18,24 @@ export function isHttpsRequest(request: Request): boolean {
     return false;
   }
 }
+
+/**
+ * Whether the site is served over HTTPS, for decisions a missing proxy header
+ * must not weaken: which cookie name to trust, whether to send HSTS. A TLS
+ * proxy that sends no X-Forwarded-Proto makes every request look like plain
+ * HTTP, so COOKIE_SECURE=true counts as the operator saying it is HTTPS.
+ */
+export function isServedOverHttps(request: Request): boolean {
+  return process.env.COOKIE_SECURE === 'true' || isHttpsRequest(request);
+}
+
+/**
+ * The `Secure` flag for a cookie the server sets. An explicit COOKIE_SECURE
+ * wins, as it does for the mailbox cookies; otherwise it follows the request,
+ * because the browser drops Secure cookies on plain HTTP. Without a request
+ * the production default stands.
+ */
+export function secureCookieFlag(request?: Request): boolean {
+  if (process.env.COOKIE_SECURE !== undefined) return process.env.COOKIE_SECURE === 'true';
+  return request ? isHttpsRequest(request) : process.env.NODE_ENV === 'production';
+}

lib/setup/session.ts

@@ -1,7 +1,7 @@
 import { cookies } from 'next/headers';
 import type { NextRequest } from 'next/server';
 import { verifySetupToken } from './token';
-import { isHttpsRequest } from '@/lib/security/request-protocol';
+import { secureCookieFlag } from '@/lib/security/request-protocol';
 
 export const SETUP_COOKIE = 'bulwark_setup_token';
 const COOKIE_MAX_AGE = 60 * 60; // 1 hour, matches token TTL
@@ -28,12 +28,12 @@ export function buildSessionCookieAttributes(request?: NextRequest) {
   // on plain HTTP, so unconditionally setting Secure in production breaks
   // setup over HTTP - the operator gets "Wizard session required" on every
   // step. The wizard surfaces a cleartext-credentials warning in the UI when
-  // HTTPS isn't in use.
+  // HTTPS isn't in use. COOKIE_SECURE wins, as for the admin cookie.
   return {
     name: SETUP_COOKIE,
     httpOnly: true,
     sameSite: 'lax' as const,
-    secure: request ? isHttpsRequest(request) : process.env.NODE_ENV === 'production',
+    secure: secureCookieFlag(request),
     path: '/',
     maxAge: COOKIE_MAX_AGE,
   };

proxy.ts

@@ -12,6 +12,7 @@ import {
 import { configManager } from "./lib/admin/config-manager";
 import { getWopiEditorOrigins } from "./lib/wopi/discovery";
 import { detectSetupState } from "./lib/setup/state";
+import { isServedOverHttps } from "./lib/security/request-protocol";
 
 const intlMiddleware = createIntlMiddleware(routing);
 
@@ -307,7 +308,7 @@ export async function proxy(request: NextRequest) {
   const appFrameOrigins = sidebarAppsEnabled
     ? parseAppFrameOrigins(pickAppFrameOriginsCookie(
         (name) => request.cookies.get(name)?.value,
-        request.nextUrl.protocol === "https:" || request.headers.get("x-forwarded-proto") === "https",
+        isServedOverHttps(request),
       ))
     : [];
 
@@ -412,11 +413,12 @@ export async function proxy(request: NextRequest) {
 
   response.headers.set("Referrer-Policy", "strict-origin-when-cross-origin");
   response.headers.set("X-XSS-Protection", "0");
-  // Only on a request that arrived over TLS (directly or via the reverse
-  // proxy's X-Forwarded-Proto): a plain-http dev or LAN install must not be
-  // pinned to https by a header it cannot honour. No includeSubDomains, which
-  // would speak for the operator's other hosts.
-  if (request.nextUrl.protocol === "https:" || request.headers.get("x-forwarded-proto") === "https") {
+  // Only on a request that arrived over TLS (directly, via the reverse
+  // proxy's X-Forwarded-Proto, or as COOKIE_SECURE=true declares): a
+  // plain-http dev or LAN install must not be pinned to https by a header it
+  // cannot honour. No includeSubDomains, which would speak for the operator's
+  // other hosts.
+  if (isServedOverHttps(request)) {
     response.headers.set("Strict-Transport-Security", "max-age=63072000");
   }
   response.headers.set(