fix(core): let exclusions cover routes excluded from the global prefix (#18209)

Middleware and CSRF exclusions now also cover the routes excluded from the global prefix that they match, which the router serves without the prefix. Such a route now skips the middleware or the CSRF check, as the same exclusion already does for routes under the prefix.

integration/hello-world/e2e/middleware-global-prefix-exclude.spec.ts

@@ -152,3 +152,76 @@ describe.each([
     );
   },
 );
+
+@Controller('health')
+class ProbesController {
+  @Get()
+  live() {
+    return RETURN_VALUE;
+  }
+
+  @Get('ready')
+  ready() {
+    return RETURN_VALUE;
+  }
+}
+
+describe.each([
+  ['Express', () => undefined],
+  ['Fastify', () => new FastifyAdapter()],
+])(
+  'Middleware - exclusion covering routes excluded from the global prefix (%s)',
+  (_, createAdapter) => {
+    let app: INestApplication;
+
+    const createApp = async (exclusion: string, prefixExclusions: string[]) => {
+      @Module({
+        controllers: [ProbesController, ReportsController, UsersController],
+      })
+      class ProbesModule {
+        configure(consumer: MiddlewareConsumer) {
+          consumer
+            .apply((req, res, next) => res.end(MIDDLEWARE_VALUE))
+            .exclude(exclusion)
+            .forRoutes('{*path}');
+        }
+      }
+
+      app = (
+        await Test.createTestingModule({ imports: [ProbesModule] }).compile()
+      ).createNestApplication(createAdapter());
+      app.setGlobalPrefix('api', { exclude: prefixExclusions });
+      await listenOnLoopback(app);
+    };
+
+    afterEach(async () => {
+      await app.close();
+    });
+
+    it.each(['health/{*path}', 'health/:check', 'health/(.*)'])(
+      `exclude('%s') should skip "/health/ready" excluded from the prefix`,
+      async exclusion => {
+        await createApp(exclusion, ['health', 'health/ready']);
+        await request(app.getHttpServer())
+          .get('/health/ready')
+          .expect(200, RETURN_VALUE);
+        await request(app.getHttpServer())
+          .get('/health')
+          .expect(200, MIDDLEWARE_VALUE);
+        await request(app.getHttpServer())
+          .get('/api/users/1')
+          .expect(200, MIDDLEWARE_VALUE);
+      },
+    );
+
+    it(`exclude('reports') should skip "/reports" excluded by a wildcard`, async () => {
+      await createApp('reports', ['reports/{*path}']);
+      await request(app.getHttpServer())
+        .get('/reports')
+        .expect(200, RETURN_VALUE);
+      await request(app.getHttpServer())
+        .get('/reports/1')
+        .expect(200, MIDDLEWARE_VALUE);
+    });
+  },
+);

integration/security/e2e/csrf-protection.spec.ts

@@ -332,6 +332,29 @@ describe.each(adapters)('CSRF protection (%s)', (_name, createAdapter) => {
     });
   });
 
+  describe('with an exclusion covering a route excluded from the global prefix', () => {
+    beforeEach(() =>
+      init(app => {
+        app.setGlobalPrefix('api', { exclude: ['webhooks/stripe'] });
+        app.enableCsrfProtection({
+          exclude: [{ path: 'webhooks/*path', method: RequestMethod.POST }],
+        });
+      }),
+    );
+
+    it('allows a cross-site POST to the route served without the prefix', async () => {
+      const post = (path: string) =>
+        request(app.getHttpServer())
+          .post(path)
+          .set('Host', HOST)
+          .set('Sec-Fetch-Site', 'cross-site')
+          .set('Origin', 'https://api.stripe.example');
+
+      await post('/webhooks/stripe').expect(201, { received: true });
+      await post('/api/items').expect(403);
+    });
+  });
+
   describe('with a global exception filter', () => {
     beforeEach(() =>
       init(app => {

packages/core/middleware/builder.ts

@@ -61,18 +61,13 @@ export class MiddlewareBuilder implements MiddlewareConsumer {
     ): MiddlewareConfigProxy {
       this.excludedRoutes = [
         ...this.excludedRoutes,
-        ...this.getRoutesFlatList(routes).reduce((excludedRoutes, route) => {
-          for (const routePath of this.routeInfoPathExtractor.extractPathFrom(
-            route,
-          )) {
-            excludedRoutes.push({
-              ...route,
-              path: routePath,
-            });
-          }
-
-          return excludedRoutes;
-        }, [] as RouteInfo[]),
+        ...this.getRoutesFlatList(routes).flatMap(route =>
+          this.routeInfoPathExtractor.extractExclusionsFrom(route, {
+            caseSensitive:
+              this.builder.getHttpAdapter().getRouteMatchingOptions?.()
+                ?.caseSensitive ?? true,
+          }),
+        ),
       ];
 
       return this;

packages/core/middleware/route-info-path-extractor.ts

@@ -82,12 +82,15 @@ export class RouteInfoPathExtractor {
    * route's own path, which an exclusion ending in an optional segment such
    * as "reports/{*path}" takes out of the prefix (see
    * `isExcludedWithTrailingSlash()`).
+   *
+   * The route matches case-insensitively unless `caseSensitive` is set: a
+   * caller that skips something for the covered routes (an exclusion) must
+   * match them as strictly as the router does.
    */
-  public extractExcludedRoutesCoveredBy({
-    path,
-    method,
-    version,
-  }: RouteInfo): RouteInfo[] {
+  public extractExcludedRoutesCoveredBy(
+    { path, method, version }: RouteInfo,
+    { caseSensitive = false }: { caseSensitive?: boolean } = {},
+  ): RouteInfo[] {
     if (
       !Array.isArray(this.excludedGlobalPrefixRoutes) ||
       this.isAWildcard(path) ||
@@ -99,6 +102,7 @@ export class RouteInfoPathExtractor {
     try {
       routeRegex = pathToRegexp(
         LegacyRouteConverter.tryConvert(addLeadingSlash(path), { logs: false }),
+        { sensitive: caseSensitive },
       ).regexp;
     } catch {
       // The adapter reports a path it cannot mount
@@ -134,6 +138,29 @@ export class RouteInfoPathExtractor {
     });
   }
 
+  /**
+   * Returns the routes that an exclusion (`MiddlewareConsumer.exclude()`, or
+   * the `exclude` option of the CSRF protection) skips, with the paths they
+   * are served at: the route itself and the routes excluded from the global
+   * prefix that it covers, which the router serves without the prefix (see
+   * `extractExcludedRoutesCoveredBy()`). Exclusions match case-sensitively
+   * unless told otherwise, as matching too much skips the middleware.
+   */
+  public extractExclusionsFrom(
+    route: RouteInfo,
+    { caseSensitive = true }: { caseSensitive?: boolean } = {},
+  ): RouteInfo[] {
+    return [
+      route,
+      ...this.extractExcludedRoutesCoveredBy(route, { caseSensitive }),
+    ].flatMap(excludedRoute =>
+      this.extractPathFrom(excludedRoute).map(path => ({
+        ...excludedRoute,
+        path,
+      })),
+    );
+  }
+
   /**
    * Returns the paths the router serves a route excluded from the global
    * prefix at: those of its pattern and, for a pattern ending in an optional

packages/core/security/cross-origin-protection.ts

@@ -278,16 +278,20 @@ export class CrossOriginProtection<TRequest = any> {
    * of `RouteInfo.version` (with another versioning type, only the requests
    * for that version are excluded). Called by `app.init()`, once the global
    * prefix and versioning are final; until then nothing is excluded.
+   * The routes excluded from the global prefix that a route covers are
+   * excluded too, at the paths the router serves them at.
    */
   public resolveExclusions(pathExtractor: RouteInfoPathExtractor): void {
     this.resolvedExclusions = this.excludedRoutes.flatMap(route =>
-      pathExtractor.extractPathFrom(route).map(path => {
-        const exclusion = compileExclusion(path, route.method, true);
-        if (route.version) {
-          exclusion.version = route.version;
-        }
-        return exclusion;
-      }),
+      pathExtractor
+        .extractExclusionsFrom(route, { caseSensitive: true })
+        .map(({ path, method }) => {
+          const exclusion = compileExclusion(path, method, true);
+          if (route.version) {
+            exclusion.version = route.version;
+          }
+          return exclusion;
+        }),
     );
     this.ignoreTrailingSlash =
       !!this.httpAdapter.getRouteMatchingOptions?.()?.ignoreTrailingSlash;

packages/core/test/middleware/builder.spec.ts

@@ -273,5 +273,60 @@ describe('MiddlewareBuilder', () => {
         },
       ]);
     });
+
+    it('should also exclude the routes excluded from the global prefix that a route covers', () => {
+      const appConfig = new ApplicationConfig();
+      appConfig.setGlobalPrefix('api');
+      appConfig.setGlobalPrefixOptions({
+        exclude: mapToExcludeRoute(['health', 'health/ready']),
+      });
+      builder = new MiddlewareBuilder(
+        new RoutesMapper(new NestContainer(), appConfig),
+        new NoopHttpAdapter({}),
+        new RouteInfoPathExtractor(appConfig),
+      );
+      const proxy: any = builder.apply().exclude('health/{*path}', 'health');
+
+      expect(proxy.getExcludedRoutes()).toEqual([
+        { path: '/api/health/{*path}', method: -1 },
+        { path: '/health/ready', method: -1 },
+        { path: '/health', method: -1 },
+      ]);
+    });
+
+    it('should match the routes excluded from the global prefix with the case sensitivity of the router', () => {
+      const appConfig = new ApplicationConfig();
+      appConfig.setGlobalPrefix('api');
+      appConfig.setGlobalPrefixOptions({
+        exclude: mapToExcludeRoute(['health/ready']),
+      });
+      const createBuilder = (caseSensitive: boolean) => {
+        const httpAdapter = new NoopHttpAdapter({});
+        httpAdapter.getRouteMatchingOptions = () => ({
+          caseSensitive,
+          ignoreTrailingSlash: false,
+        });
+        return new MiddlewareBuilder(
+          new RoutesMapper(new NestContainer(), appConfig),
+          httpAdapter,
+          new RouteInfoPathExtractor(appConfig),
+        );
+      };
+
+      const sensitive: any = createBuilder(true)
+        .apply()
+        .exclude('HEALTH/:check');
+      expect(sensitive.getExcludedRoutes()).toEqual([
+        { path: '/api/HEALTH/:check', method: -1 },
+      ]);
+
+      const insensitive: any = createBuilder(false)
+        .apply()
+        .exclude('HEALTH/:check');
+      expect(insensitive.getExcludedRoutes()).toEqual([
+        { path: '/api/HEALTH/:check', method: -1 },
+        { path: '/health/ready', method: -1 },
+      ]);
+    });
   });
 });

packages/core/test/middleware/route-info-path-extractor.spec.ts

@@ -431,6 +431,111 @@ describe('RouteInfoPathExtractor', () => {
     });
   });
 
+  describe('extractExclusionsFrom', () => {
+    let routeInfoPathExtractor: RouteInfoPathExtractor;
+
+    beforeEach(() => {
+      const appConfig = new ApplicationConfig();
+      appConfig.setGlobalPrefix('api');
+      appConfig.setGlobalPrefixOptions({
+        exclude: mapToExcludeRoute([
+          'health',
+          'health/ready',
+          { path: 'webhooks/stripe', method: RequestMethod.POST },
+          'reports/{*path}',
+        ]),
+      });
+      routeInfoPathExtractor = new RouteInfoPathExtractor(appConfig);
+    });
+
+    it(`should also return the routes excluded from the global prefix that the route covers`, () => {
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom({
+          path: '/health/{*path}',
+          method: -1 as RequestMethod,
+        }),
+      ).toEqual([
+        { path: '/api/health/{*path}', method: -1 },
+        { path: '/health/ready', method: -1 },
+      ]);
+
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom({
+          path: '/health/(.*)',
+          method: -1 as RequestMethod,
+        }),
+      ).toEqual([
+        { path: '/api/health/(.*)', method: -1 },
+        { path: '/health/ready', method: -1 },
+      ]);
+
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom({
+          path: '/webhooks/:provider',
+          method: RequestMethod.ALL,
+        }),
+      ).toEqual([
+        { path: '/api/webhooks/:provider', method: RequestMethod.ALL },
+        { path: '/webhooks/stripe', method: RequestMethod.POST },
+      ]);
+    });
+
+    it(`should not return the excluded routes of another request method`, () => {
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom({
+          path: '/webhooks/:provider',
+          method: RequestMethod.GET,
+        }),
+      ).toEqual([
+        { path: '/api/webhooks/:provider', method: RequestMethod.GET },
+      ]);
+    });
+
+    it(`should return the root route of a controller excluded by an optional wildcard`, () => {
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom({
+          path: '/reports',
+          method: -1 as RequestMethod,
+        }),
+      ).toEqual([
+        { path: '/api/reports', method: -1 },
+        { path: '/reports/', method: -1 },
+      ]);
+    });
+
+    it(`should match the excluded routes case-sensitively unless told otherwise`, () => {
+      const route = { path: '/HEALTH/:check', method: -1 as RequestMethod };
+
+      expect(routeInfoPathExtractor.extractExclusionsFrom(route)).toEqual([
+        { path: '/api/HEALTH/:check', method: -1 },
+      ]);
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom(route, {
+          caseSensitive: false,
+        }),
+      ).toEqual([
+        { path: '/api/HEALTH/:check', method: -1 },
+        { path: '/health/ready', method: -1 },
+      ]);
+    });
+
+    it(`should only return the route itself when it is excluded or a wildcard`, () => {
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom({
+          path: '/health/ready',
+          method: -1 as RequestMethod,
+        }),
+      ).toEqual([{ path: '/health/ready', method: -1 }]);
+
+      expect(
+        routeInfoPathExtractor.extractExclusionsFrom({
+          path: '/{*path}',
+          method: -1 as RequestMethod,
+        }),
+      ).toEqual([{ path: '/{*path}', method: -1 }]);
+    });
+  });
+
   describe('extractPathsFrom with a route excluded by an optional wildcard', () => {
     it(`should also return the parent path of the excluded route`, () => {
       const appConfig = new ApplicationConfig();

packages/core/test/security/cross-origin-protection.spec.ts

@@ -473,6 +473,36 @@ describe('CrossOriginProtection', () => {
       expect(isExcluded(protection, '/api/health')).toBe(false);
     });
 
+    it('also excludes the routes excluded from the global prefix that a route covers', () => {
+      const protection = createProtection(['webhooks/*path'], config => {
+        config.setGlobalPrefix('api');
+        config.setGlobalPrefixOptions({
+          exclude: mapToExcludeRoute([
+            { path: 'webhooks/stripe', method: RequestMethod.POST },
+          ]),
+        });
+      });
+      expect(isExcluded(protection, '/webhooks/stripe')).toBe(true);
+      expect(isExcluded(protection, '/webhooks/stripe', 'PUT')).toBe(false);
+      expect(isExcluded(protection, '/webhooks/github')).toBe(false);
+      expect(isExcluded(protection, '/api/webhooks/github')).toBe(true);
+    });
+
+    it('matches the routes excluded from the global prefix case-sensitively', () => {
+      const protection = createProtection(
+        [{ path: 'WEBHOOKS/*path', method: RequestMethod.POST }],
+        config => {
+          config.setGlobalPrefix('api');
+          config.setGlobalPrefixOptions({
+            exclude: mapToExcludeRoute(['webhooks/stripe']),
+          });
+        },
+      );
+      expect(isExcluded(protection, '/webhooks/stripe')).toBe(false);
+      expect(isExcluded(protection, '/api/webhooks/github')).toBe(false);
+      expect(isExcluded(protection, '/api/WEBHOOKS/github')).toBe(true);
+    });
+
     it('adds the version segment of RouteInfo.version (URI versioning)', () => {
       const protection = createProtection(
         [{ path: 'webhooks', method: RequestMethod.POST, version: '1' }],