security(proxy-path): set Vary on public routes that echo x-proxy-path (cache poisoning)

src/node/hooks/express/specialpages.ts

@@ -25,6 +25,21 @@ let ioI: { sockets: { sockets: any[]; }; } | null = null
 // rules. Reused by admin.ts so both call sites share one definition.
 import {sanitizeProxyPath} from '../../utils/sanitizeProxyPath';
 
+// Public routes echo the proxy-path headers into rendered URLs, social-preview
+// metadata, manifest links and the legacy timeslider redirect. Advertise the
+// headers in Vary so a shared cache/CDN in front of Etherpad keys on them and
+// can't serve a proxy-path injected by one client to another (cache poisoning).
+// Mirrors the admin-route fix in admin.ts (GHSA-fjgc-3mj7-8rg8).
+//
+// Only vary on the headers sanitizeProxyPath() actually consults for the
+// current config: x-proxy-path is always honored, but x-forwarded-prefix and
+// x-ingress-path are ignored unless trustProxy is enabled — varying on them
+// then would only fragment shared caches without affecting the response.
+const varyOnProxyPath = (res: any) => {
+  res.vary('x-proxy-path');
+  if (settings.trustProxy) res.vary(['x-forwarded-prefix', 'x-ingress-path']);
+};
+
 
 exports.socketio = (hookName: string, {io}: any) => {
   ioI = io
@@ -173,6 +188,7 @@ const handleLiveReload = async (args: ArgsExpressType, padString: string, timeSl
       })
       setRouteHandler('/', (req: any, res: any) => {
         const proxyPath = sanitizeProxyPath(req);
+        varyOnProxyPath(res);
         const socialMetaHtml = renderSocialMeta({
           req, settings, availableLangs: i18n.availableLangs, locales: i18n.locales, kind: 'home',
           proxyPath,
@@ -202,6 +218,7 @@ const handleLiveReload = async (args: ArgsExpressType, padString: string, timeSl
         });
 
         const proxyPath = sanitizeProxyPath(req);
+        varyOnProxyPath(res);
         const socialMetaHtml = renderSocialMeta({
           req, settings, availableLangs: i18n.availableLangs, locales: i18n.locales, kind: 'pad', padName: req.params.pad,
           proxyPath,
@@ -246,6 +263,7 @@ const handleLiveReload = async (args: ArgsExpressType, padString: string, timeSl
         });
 
         const proxyPath = sanitizeProxyPath(req);
+        varyOnProxyPath(res);
         const socialMetaHtml = renderSocialMeta({
           req, settings, availableLangs: i18n.availableLangs, locales: i18n.locales, kind: 'timeslider', padName: req.params.pad,
           proxyPath,
@@ -369,6 +387,7 @@ exports.expressCreateServer = async (_hookName: string, args: ArgsExpressType, c
     // serve index.html under /
     args.app.get('/', (req: any, res: any) => {
       const proxyPath = sanitizeProxyPath(req);
+      varyOnProxyPath(res);
       const socialMetaHtml = renderSocialMeta({
         req, settings, availableLangs: i18n.availableLangs, locales: i18n.locales, kind: 'home',
         proxyPath,
@@ -389,6 +408,7 @@ exports.expressCreateServer = async (_hookName: string, args: ArgsExpressType, c
       });
 
       const proxyPath = sanitizeProxyPath(req);
+      varyOnProxyPath(res);
       const socialMetaHtml = renderSocialMeta({
         req, settings, availableLangs: i18n.availableLangs, locales: i18n.locales, kind: 'pad', padName: req.params.pad,
         proxyPath,
@@ -417,6 +437,7 @@ exports.expressCreateServer = async (_hookName: string, args: ArgsExpressType, c
         // technically well-defined but Firefox dropped a trailing-slash
         // case once that flaked the legacy-URL test (#7710).
         const proxyPath = sanitizeProxyPath(req);
+        varyOnProxyPath(res);
         return res.redirect(302, `${proxyPath}/p/${encodeURIComponent(req.params.pad)}`);
       }
       ensureAuthorTokenCookie(req, res, settings);
@@ -425,6 +446,7 @@ exports.expressCreateServer = async (_hookName: string, args: ArgsExpressType, c
       });
 
       const proxyPath = sanitizeProxyPath(req);
+      varyOnProxyPath(res);
       const socialMetaHtml = renderSocialMeta({
         req, settings, availableLangs: i18n.availableLangs, locales: i18n.locales, kind: 'timeslider', padName: req.params.pad,
         proxyPath,

src/tests/backend/specs/proxyPathVary.ts

@@ -0,0 +1,93 @@
+'use strict';
+
+/**
+ * Cache-poisoning hardening for the `x-proxy-path` header on public routes.
+ *
+ * Etherpad echoes the (sanitised) `x-proxy-path` prefix into rendered URLs,
+ * social-preview metadata, PWA manifest links, and the legacy timeslider
+ * redirect on `/`, `/p/:pad`, and `/p/:pad/timeslider`. If a shared cache /
+ * reverse proxy in front of Etherpad stores these responses keyed on URL alone,
+ * a value injected by one client is served to every other client. These
+ * responses must therefore advertise `Vary: x-proxy-path` so a conforming cache
+ * keeps prefixed and non-prefixed variants separate. Reported by `meifukun`;
+ * companion to the already-fixed admin-route variant (GHSA-fjgc-3mj7-8rg8).
+ */
+
+const assert = require('assert').strict;
+const common = require('../common');
+const padManager = require('../../../node/db/PadManager');
+import settings from '../../../node/utils/Settings';
+
+let agent: any;
+
+const varyList = (res: any): string[] =>
+  String(res.headers.vary || '').toLowerCase().split(',').map((s: string) => s.trim());
+
+const assertVariesOnProxyPath = (res: any, where: string) => {
+  if (!varyList(res).includes('x-proxy-path')) {
+    throw new Error(
+        `${where}: response must advertise "Vary: x-proxy-path" so a shared ` +
+        `cache cannot serve a poisoned proxy-path to another user ` +
+        `(got Vary: "${res.headers.vary || ''}")`);
+  }
+};
+
+describe(__filename, function () {
+  this.timeout(30000);
+  const padId = 'ProxyPathVaryTest';
+  let trustProxyBackup: any;
+
+  before(async function () {
+    agent = await common.init();
+    await padManager.getPad(padId, 'test content');
+    trustProxyBackup = settings.trustProxy;
+  });
+
+  after(function () {
+    settings.trustProxy = trustProxyBackup;
+  });
+
+  it('sets Vary: x-proxy-path on the home page', async function () {
+    const res = await agent.get('/').expect(200);
+    assertVariesOnProxyPath(res, 'GET /');
+  });
+
+  it('sets Vary: x-proxy-path on the pad page', async function () {
+    const res = await agent.get(`/p/${padId}`).expect(200);
+    assertVariesOnProxyPath(res, `GET /p/${padId}`);
+  });
+
+  it('sets Vary: x-proxy-path on the timeslider embed page', async function () {
+    const res = await agent.get(`/p/${padId}/timeslider?embed=1`).expect(200);
+    assertVariesOnProxyPath(res, `GET /p/${padId}/timeslider?embed=1`);
+  });
+
+  it('sets Vary: x-proxy-path on the legacy timeslider redirect', async function () {
+    const res = await agent.get(`/p/${padId}/timeslider`).expect(302);
+    assertVariesOnProxyPath(res, `GET /p/${padId}/timeslider (redirect)`);
+  });
+
+  it('does NOT vary on the trust-gated headers when trustProxy is false', async function () {
+    // sanitizeProxyPath ignores x-forwarded-prefix / x-ingress-path unless
+    // trustProxy is enabled, so varying on them would only fragment caches.
+    settings.trustProxy = false;
+    const res = await agent.get('/').expect(200);
+    const vary = varyList(res);
+    assert.ok(vary.includes('x-proxy-path'), `expected x-proxy-path, got "${res.headers.vary}"`);
+    assert.ok(!vary.includes('x-forwarded-prefix'),
+        `must not vary on x-forwarded-prefix when trustProxy=false (got "${res.headers.vary}")`);
+    assert.ok(!vary.includes('x-ingress-path'),
+        `must not vary on x-ingress-path when trustProxy=false (got "${res.headers.vary}")`);
+  });
+
+  it('varies on all proxy-path headers when trustProxy is true', async function () {
+    settings.trustProxy = true;
+    const res = await agent.get('/').expect(200);
+    const vary = varyList(res);
+    assert.ok(vary.includes('x-proxy-path'), `expected x-proxy-path, got "${res.headers.vary}"`);
+    assert.ok(vary.includes('x-forwarded-prefix'),
+        `expected x-forwarded-prefix when trustProxy=true (got "${res.headers.vary}")`);
+    assert.ok(vary.includes('x-ingress-path'),
+        `expected x-ingress-path when trustProxy=true (got "${res.headers.vary}")`);
+  });
+});