mirror of
https://github.com/nestjs/nest.git
synced 2026-10-28 09:45:43 +08:00
fix(core,websockets): run transient enhancers bound to static gateways
A default-scoped gateway with no scoped global enhancers builds its message handlers once, in the static context and without an inquirer id, and the injector constructed the transient enhancers of a static provider for entry providers only. A transient guard bound to such a gateway with @UseGuards() was then looked up as the placeholder whose constructor never ran. With an arrow-function canActivate it was skipped, so the handler ran for any client; with a prototype method it ran without its dependencies. The bootstrap load of every static provider now constructs its transient enhancers with the provider as the inquirer, and the static handlers of a gateway are created with its id as the inquirer, as those of controllers and entry providers are. Such guards now run. Discovered by Anthropic's OSS Scanner, as vulnerability ANT-2026-J6VYP0AI.
This commit is contained in:
5 files changed
+109
-10
No files matched your search
@@ -37,10 +37,13 @@ import {
|
||||
Payload,
|
||||
Transport,
|
||||
} from '@nestjs/microservices';
|
||||
import { WsAdapter } from '@nestjs/platform-ws';
|
||||
import { Test } from '@nestjs/testing';
|
||||
import { SubscribeMessage, WebSocketGateway } from '@nestjs/websockets';
|
||||
import { firstValueFrom, of } from 'rxjs';
|
||||
import { map } from 'rxjs/operators';
|
||||
import request from 'supertest';
|
||||
import WebSocket from 'ws';
|
||||
import { listenOnLoopback } from '../../_support/listen-on-loopback.js';
|
||||
|
||||
const TCP_PORT = 3792;
|
||||
@@ -233,6 +236,29 @@ class EntryProviderConsumerGuard implements CanActivate {
|
||||
}
|
||||
}
|
||||
|
||||
@Injectable({ scope: Scope.TRANSIENT })
|
||||
class TransientArrowDenyGuard implements CanActivate {
|
||||
constructor(private readonly gate: Gate) {}
|
||||
|
||||
// An instance property, so a guard whose constructor never ran has none.
|
||||
canActivate = () => this.gate.isOpen();
|
||||
}
|
||||
|
||||
@WebSocketGateway({ path: '/transient' })
|
||||
class TransientGuardedGateway {
|
||||
@UseGuards(TransientArrowDenyGuard)
|
||||
@SubscribeMessage('arrow')
|
||||
arrow() {
|
||||
return { event: 'arrow', data: 'granted' };
|
||||
}
|
||||
|
||||
@UseGuards(TrackingDenyGuard)
|
||||
@SubscribeMessage('method')
|
||||
method() {
|
||||
return { event: 'method', data: 'granted' };
|
||||
}
|
||||
}
|
||||
|
||||
const createModule = (
|
||||
providers: Provider[],
|
||||
controllers: Type<unknown>[] = [FirstController],
|
||||
@@ -512,4 +538,47 @@ describe('Transient global enhancers', () => {
|
||||
await expect(handler()).rejects.toThrow('Forbidden resource');
|
||||
});
|
||||
});
|
||||
|
||||
// A default-scoped gateway with no scoped global enhancers builds its
|
||||
// handlers once, in the static context.
|
||||
describe('gateway', () => {
|
||||
let app: INestApplication;
|
||||
|
||||
afterEach(async () => {
|
||||
await app?.close();
|
||||
});
|
||||
|
||||
const send = async (event: 'arrow' | 'method') => {
|
||||
const moduleRef = await Test.createTestingModule({
|
||||
imports: [createModule([TransientGuardedGateway], [])],
|
||||
}).compile();
|
||||
app = moduleRef.createNestApplication({ logger: false });
|
||||
app.useWebSocketAdapter(new WsAdapter(app));
|
||||
await app.listen(0, '127.0.0.1');
|
||||
|
||||
const client = new WebSocket(
|
||||
`${(await app.getUrl()).replace('http', 'ws')}/transient`,
|
||||
);
|
||||
await new Promise(resolve => client.once('open', resolve));
|
||||
const reply = new Promise<string>(resolve =>
|
||||
client.once('message', data => resolve(data.toString())),
|
||||
);
|
||||
client.send(JSON.stringify({ event, data: {} }));
|
||||
try {
|
||||
return JSON.parse(await reply);
|
||||
} finally {
|
||||
client.close();
|
||||
}
|
||||
};
|
||||
|
||||
it.each(['arrow', 'method'] as const)(
|
||||
'should deny the message with a transient guard bound by class (%s)',
|
||||
async event => {
|
||||
const reply = await send(event);
|
||||
|
||||
expect(reply.event).toBe('exception');
|
||||
expect(reply.data.message).toBe('Forbidden resource');
|
||||
},
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -347,11 +347,11 @@ export class Injector {
|
||||
if (
|
||||
resolutionContext.contextId === STATIC_CONTEXT &&
|
||||
!resolutionContext.inquirer &&
|
||||
moduleRef.entryProviders.includes(wrapper) &&
|
||||
wrapper.isDependencyTreeStatic()
|
||||
) {
|
||||
// A static entry provider never gets a per-request load either, so its
|
||||
// transient enhancers must exist for the provider before the first call.
|
||||
// A static provider that hosts handlers (e.g., an entry provider or a
|
||||
// gateway) never gets a per-request load either, so its transient
|
||||
// enhancers must exist for the provider before the first call.
|
||||
// Only its bootstrap load (no inquirer) waits for them, as they may depend
|
||||
// on a provider that is itself waiting for this one. The non-transient
|
||||
// ones are shared, and get loaded with the injectables of the module.
|
||||
|
||||
@@ -437,7 +437,7 @@ describe('Injector', () => {
|
||||
expect(loadEnhancersPerContext).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
describe('enhancers of a static entry provider', () => {
|
||||
describe('enhancers of a static provider', () => {
|
||||
@SetMetadata(ENTRY_PROVIDER_WATERMARK, true)
|
||||
class EntryProvider {}
|
||||
|
||||
@@ -500,15 +500,21 @@ describe('Injector', () => {
|
||||
expect(loadInstance).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it('should not load the enhancers of a provider that is not an entry provider', async () => {
|
||||
addEnhancer(PlainProvider, Scope.TRANSIENT);
|
||||
it('should also load them for a provider that is not an entry provider (e.g., a gateway)', async () => {
|
||||
const enhancer = addEnhancer(PlainProvider, Scope.TRANSIENT);
|
||||
const provider = moduleRef.providers.get(PlainProvider)!;
|
||||
|
||||
await injector.loadProvider(
|
||||
moduleRef.providers.get(PlainProvider)!,
|
||||
await injector.loadProvider(provider, moduleRef);
|
||||
|
||||
expect(loadInstance).toHaveBeenCalledWith(
|
||||
enhancer,
|
||||
moduleRef.injectables,
|
||||
moduleRef,
|
||||
expect.objectContaining({
|
||||
contextId: STATIC_CONTEXT,
|
||||
inquirer: provider,
|
||||
}),
|
||||
);
|
||||
|
||||
expect(loadInstance).toHaveBeenCalledOnce();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { Scope } from '@nestjs/common';
|
||||
import { REQUEST } from '@nestjs/core';
|
||||
import { STATIC_CONTEXT } from '@nestjs/core/injector/constants.js';
|
||||
import { Injector } from '@nestjs/core/injector/injector.js';
|
||||
import { Module } from '@nestjs/core/injector/module.js';
|
||||
import { InstanceWrapper } from '@nestjs/core/injector/instance-wrapper.js';
|
||||
@@ -205,6 +206,28 @@ describe('WebSocketsController', () => {
|
||||
server.server,
|
||||
);
|
||||
});
|
||||
it('should create the handlers of a static gateway with the gateway as the inquirer', () => {
|
||||
instance.subscribeToServerEvents(
|
||||
{
|
||||
instance: gateway,
|
||||
metatype: Test,
|
||||
id: 'instanceWrapperId',
|
||||
isDependencyTreeStatic: () => true,
|
||||
} as any,
|
||||
{ namespace },
|
||||
port,
|
||||
'moduleKey',
|
||||
'instanceWrapperId',
|
||||
);
|
||||
expect(untypedInstance.contextCreator.create).toHaveBeenCalledWith(
|
||||
gateway,
|
||||
handlerCallback,
|
||||
'moduleKey',
|
||||
'methodName',
|
||||
STATIC_CONTEXT,
|
||||
'instanceWrapperId',
|
||||
);
|
||||
});
|
||||
it('should call "subscribeEvents" with expected arguments', () => {
|
||||
instance.subscribeToServerEvents(
|
||||
{
|
||||
|
||||
@@ -171,6 +171,7 @@ export class WebSocketsController {
|
||||
moduleKey,
|
||||
methodName,
|
||||
STATIC_CONTEXT,
|
||||
instanceWrapper.id,
|
||||
)
|
||||
: this.createRequestScopedHandler(
|
||||
instanceWrapper,
|
||||
|
||||
Reference in new issue
Block a user