feat(http-utils): add anonymousEndpoints override and drop GET /slack/events from the anonymous auth bypass

packages/spacecat-shared-http-utils/src/auth/auth-wrapper.js

@@ -16,20 +16,86 @@ import { isObject } from '@adobe/spacecat-shared-utils';
 import AuthenticationManager from './authentication-manager.js';
 import { checkScopes } from './check-scopes.js';
 
+/**
+ * Route-based entries that bypass authentication.
+ *
+ * SECURITY CONTRACT (VULN-39365): an entry here means this library performs NO authentication
+ * for that route, so the consuming service MUST authenticate it by other means. For
+ * `POST /slack/events` that means verifying the Slack request signature (`X-Slack-Signature` +
+ * `X-Slack-Request-Timestamp`) before the payload reaches any handler. spacecat-api-service does
+ * this in `slackSignatureWrapper`, which is mounted OUTSIDE this wrapper so it runs first.
+ *
+ * IMPORTANT: this list is NOT the whole unauthenticated surface. Two further bypasses are
+ * unconditional clauses in the wrapper below and are not represented here or overridable via
+ * `anonymousEndpoints`:
+ *   - every `OPTIONS` request (CORS preflight), and
+ *   - any route matching the `POST /hooks/site-detection/` prefix.
+ *
+ * Historically this list also contained `GET /slack/events`. That was removed because Slack only
+ * ever POSTs events and interactive payloads, and a GET carries no body to sign — so a GET could
+ * never be signature-verified and existed purely as an unauthenticated entry point.
+ *
+ * Do NOT add entries here. A service that needs an unauthenticated route should pass its own
+ * `anonymousEndpoints` (see below) rather than widening the default for every consumer.
+ */
 const ANONYMOUS_ENDPOINTS = [
-  'GET /slack/events',
   'POST /slack/events',
 ];
 
+// Entries are compared verbatim against `${METHOD.toUpperCase()} ${suffix}`, so they must be an
+// upper-case method, a single space, then an absolute path.
+const ANONYMOUS_ROUTE_SHAPE = /^[A-Z]+ \/\S*$/;
+
+/**
+ * Wraps a function with authentication.
+ *
+ * @param {UniversalFunction} fn - the function to wrap.
+ * @param {object} [opts] - options.
+ * @param {Array} [opts.authHandlers] - the authentication handler classes to try, in order.
+ * @param {string[]} [opts.anonymousEndpoints] - overrides the default set of **route-based**
+ *   anonymous entries, as `'METHOD /path'` strings (exact match, method upper-case).
+ *
+ *   Pass `[]` to remove the route-based entries. Note this does NOT authenticate everything:
+ *   `OPTIONS` requests and `POST /hooks/site-detection/*` bypass authentication
+ *   unconditionally and are not affected by this option. A service that does not verify Slack
+ *   request signatures SHOULD pass `[]`, otherwise it inherits an unauthenticated
+ *   `POST /slack/events` it may not be defending.
+ *
+ *   Supplying a value that is not an array of strings THROWS at wrapper-construction time
+ *   rather than falling back to the default. This is security-sensitive configuration: a typo
+ *   by a service trying to *disable* the bypass must not silently re-enable it. Entries are
+ *   copied, so mutating the caller's array afterwards cannot widen the bypass.
+ * @returns {UniversalFunction} the wrapped function.
+ * @throws {Error} when `opts.anonymousEndpoints` is present but not an array of strings.
+ */
 export function authWrapper(fn, opts = {}) {
   let authenticationManager;
+  let anonymousEndpoints = ANONYMOUS_ENDPOINTS;
+
+  if (opts.anonymousEndpoints !== undefined) {
+    if (!Array.isArray(opts.anonymousEndpoints)
+      || opts.anonymousEndpoints.some((route) => typeof route !== 'string')) {
+      throw new Error('authWrapper: anonymousEndpoints must be an array of "METHOD /path" strings');
+    }
+    // Shape-check each entry too. Matching is an exact string compare against
+    // `${METHOD.toUpperCase()} ${suffix}`, so an entry like 'post /slack/events' or a missing
+    // leading slash would validate, never match, and silently fail closed -- a debugging trap
+    // for a service that believes it has allowed a route. Same rationale as the throw above:
+    // security-sensitive config must not misconfigure quietly.
+    const malformed = opts.anonymousEndpoints.filter((route) => !ANONYMOUS_ROUTE_SHAPE.test(route));
+    if (malformed.length > 0) {
+      throw new Error(`authWrapper: anonymousEndpoints entries must look like "METHOD /path" with an upper-case method; got: ${malformed.join(', ')}`);
+    }
+    // Defensive copy: the caller must not be able to widen the bypass after construction.
+    anonymousEndpoints = [...opts.anonymousEndpoints];
+  }
 
   return async (request, context) => {
     const { log, pathInfo: { method, suffix } } = context;
 
     const route = `${method.toUpperCase()} ${suffix}`;
 
-    if (ANONYMOUS_ENDPOINTS.includes(route)
+    if (anonymousEndpoints.includes(route)
         || route.startsWith('POST /hooks/site-detection/')
         || method.toUpperCase() === 'OPTIONS') {
       return fn(request, context);

packages/spacecat-shared-http-utils/src/auth/readme.md

@@ -50,6 +50,29 @@ export const main = wrap(run)
   .with(auth, { authHandlers: [LegacyApiKeyHandler, AdobeImsHandler] });
 ```
 
+#### Anonymous routes (`anonymousEndpoints`)
+
+Some routes bypass authentication. By default that is `POST /slack/events`, which the
+consuming service is expected to authenticate by other means — spacecat-api-service verifies
+the Slack request signature in a wrapper mounted outside this one (VULN-39365).
+
+A service that does **not** verify Slack request signatures should opt out explicitly, so it
+does not inherit an unauthenticated route it is not defending:
+
+```javascript
+export const main = wrap(run)
+  .with(auth, { authHandlers: [...], anonymousEndpoints: [] });
+```
+
+Two caveats:
+
+- `anonymousEndpoints` governs only the **route-based** list (exact `'METHOD /path'` matches).
+  `OPTIONS` requests and `POST /hooks/site-detection/*` bypass authentication unconditionally
+  and are **not** affected by this option — `[]` does not mean "authenticate everything".
+- Passing anything other than an array of strings **throws at construction** rather than
+  falling back to the default, so a typo while locking things down cannot silently re-open the
+  bypass.
+
 ### Implementing an Authentication Handler
 
 To implement a new authentication handler, extend the `AbstractHandler` class and implement the `checkAuth` method.

packages/spacecat-shared-http-utils/test/auth/auth-wrapper.test.js

@@ -76,14 +76,125 @@ describe('auth wrapper', () => {
   });
 
   it('passes anonymous route', async () => {
+    // Slack only ever POSTs; GET /slack/events is no longer anonymous (VULN-39365).
     context.pathInfo.suffix = '/slack/events';
 
-    const resp = await action(new Request('https://space.cat/slack/events'), context);
+    const resp = await action(new Request('https://space.cat/slack/events', { method: 'POST' }), context);
 
     expect(resp).to.equal(42);
     expect(context.attributes.authInfo).to.be.undefined;
   });
 
+  it('does NOT treat GET /slack/events as anonymous (VULN-39365)', async () => {
+    // A GET carries no body to sign, so it could never be Slack-signature-verified. It must
+    // fall through to the authentication manager, which rejects it.
+    context.pathInfo.suffix = '/slack/events';
+
+    const resp = await action(new Request('https://space.cat/slack/events'), context);
+
+    expect(resp.status).to.equal(401);
+    expect(context.attributes.authInfo).to.be.undefined;
+  });
+
+  it('honours an anonymousEndpoints override that disables the bypass', async () => {
+    const locked = wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: [] })
+      .with(enrichPathInfo);
+    context.pathInfo.suffix = '/slack/events';
+
+    const resp = await locked(new Request('https://space.cat/slack/events', { method: 'POST' }), context);
+
+    expect(resp.status).to.equal(401);
+  });
+
+  // The option governs ONLY the route-based list. These pin the two unconditional bypasses it
+  // does not reach, so the documented contract cannot silently drift from the behaviour.
+  it('anonymousEndpoints: [] does NOT authenticate OPTIONS requests', async () => {
+    const locked = wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: [] })
+      .with(enrichPathInfo);
+    context.pathInfo.suffix = '/sites';
+
+    const resp = await locked(new Request('https://space.cat/sites', { method: 'OPTIONS' }), context);
+
+    expect(resp).to.equal(42);
+  });
+
+  it('anonymousEndpoints: [] does NOT authenticate POST /hooks/site-detection/*', async () => {
+    const locked = wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: [] })
+      .with(enrichPathInfo);
+    context.pathInfo.suffix = '/hooks/site-detection/cdn/some-secret';
+
+    const resp = await locked(
+      new Request('https://space.cat/hooks/site-detection/cdn/some-secret', { method: 'POST' }),
+      context,
+    );
+
+    expect(resp).to.equal(42);
+  });
+
+  it('a non-empty override drops the default POST /slack/events', async () => {
+    const custom = wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: ['POST /custom/hook'] })
+      .with(enrichPathInfo);
+    context.pathInfo.suffix = '/slack/events';
+
+    const resp = await custom(new Request('https://space.cat/slack/events', { method: 'POST' }), context);
+
+    expect(resp.status).to.equal(401);
+  });
+
+  it('copies anonymousEndpoints so later mutation cannot widen the bypass', async () => {
+    const caller = [];
+    const locked = wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: caller })
+      .with(enrichPathInfo);
+    caller.push('POST /slack/events');
+    context.pathInfo.suffix = '/slack/events';
+
+    const resp = await locked(new Request('https://space.cat/slack/events', { method: 'POST' }), context);
+
+    expect(resp.status).to.equal(401);
+  });
+
+  it('honours an anonymousEndpoints override that names a different route', async () => {
+    const custom = wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: ['POST /custom/hook'] })
+      .with(enrichPathInfo);
+    context.pathInfo.suffix = '/custom/hook';
+
+    const resp = await custom(new Request('https://space.cat/custom/hook', { method: 'POST' }), context);
+
+    expect(resp).to.equal(42);
+  });
+
+  it('throws on a non-array anonymousEndpoints instead of silently using the default', () => {
+    // Security-sensitive config: a typo by a service trying to DISABLE the bypass must not
+    // silently re-enable an unauthenticated POST /slack/events.
+    expect(() => wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: 'POST /slack/events' })
+      .with(enrichPathInfo)).to.throw('anonymousEndpoints must be an array');
+  });
+
+  it('throws on an anonymousEndpoints array containing a non-string', () => {
+    expect(() => wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: ['POST /slack/events', 42] })
+      .with(enrichPathInfo)).to.throw('anonymousEndpoints must be an array');
+  });
+
+  it('throws on a lower-case method, which would validate but never match', () => {
+    expect(() => wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: ['post /slack/events'] })
+      .with(enrichPathInfo)).to.throw('upper-case method');
+  });
+
+  it('throws on an entry with no leading slash on the path', () => {
+    expect(() => wrap(() => 42)
+      .with(authWrapper, { authHandlers: [DummyHandler], anonymousEndpoints: ['POST slack/events'] })
+      .with(enrichPathInfo)).to.throw('upper-case method');
+  });
+
   it('passes options method', async () => {
     context.pathInfo.method = 'OPTIONS';
     context.pathInfo.suffix = '/sites';