Skip to content

JS: Refactor Nest test suite with inline expectations #19514

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Empty file.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ import { Foo } from "./foo.interface";

export class FooImpl extends Foo {
fooMethod(x: string) {
sink(x); // $ hasValueFlow=x
sink(x);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@ export class Controller {
) { }

@Get()
route1(@Query('x') validatedObj: Struct, @Query('y') unvalidated: string) {
if (Math.random()) return unvalidated; // NOT OK
return validatedObj.key; // OK
}
route1(@Query('x') validatedObj: Struct, @Query('y') unvalidated: string) {
if (Math.random()) return unvalidated; // $responseSendArgument
return validatedObj.key; // $responseSendArgument
} // $routeHandler

@Get()
route2(@Query('x') x: string) {
this.foo.fooMethod(x);
}
} // $routeHandler
}

class Struct {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { Get, createParamDecorator, ExecutionContext } from '@nestjs/common';

export const SneakyQueryParam = createParamDecorator(
(data: unknown, ctx: ExecutionContext) => {
const request = ctx.switchToHttp().getRequest();
const request = ctx.switchToHttp().getRequest(); // $requestSource
return request.query.sneakyQueryParam;
},
);
Expand All @@ -16,11 +16,11 @@ export const SafeParam = createParamDecorator(
export class Controller {
@Get()
sneaky(@SneakyQueryParam() value) {
return value; // NOT OK
}
return value; // $responseSendArgument
} // $routeHandler

@Get()
safe(@SafeParam() value) {
return value; // OK
}
return value; // $responseSendArgument
} // $routeHandler
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,33 +18,33 @@ export class CustomPropagatingPipe implements PipeTransform {
export class Controller {
@Get()
sanitizingPipe1(@Query('x', CustomSanitizingPipe) sanitized: number): string {
return '' + sanitized; // OK
}
return '' + sanitized; // $responseSendArgument
} // $routeHandler

@Get()
sanitizingPipe2(@Query('x', new CustomSanitizingPipe()) sanitized: number): string {
return '' + sanitized; // OK
}
return '' + sanitized; // $responseSendArgument
} // $routeHandler

@Get()
@UsePipes(CustomSanitizingPipe)
sanitizingPipe3(@Query('x') sanitized: number): string {
return '' + sanitized; // OK
}
return '' + sanitized; // $responseSendArgument
} // $routeHandler

@Get()
propagatingPipe1(@Query('x', CustomPropagatingPipe) unsanitized: string): string {
return '' + unsanitized; // NOT OK
}
return '' + unsanitized; // $responseSendArgument
} // $routeHandler

@Get()
propagatingPipe2(@Query('x', new CustomPropagatingPipe()) unsanitized: string): string {
return '' + unsanitized; // NOT OK
}
return '' + unsanitized; // $responseSendArgument
} // $routeHandler

@Get()
@UsePipes(CustomPropagatingPipe)
propagatingPipe3(@Query('x') unsanitized: string): string {
return '' + unsanitized; // NOT OK
}
return '' + unsanitized; // $responseSendArgument
} // $routeHandler
}
46 changes: 23 additions & 23 deletions javascript/ql/test/library-tests/frameworks/Nest/local/routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,70 +5,70 @@ export class TestController {
@Get('foo')
getFoo() {
return 'foo';
}
} // $routeHandler

@Post('foo')
postFoo() {
return 'foo';
}
} // $routeHandler

@Get()
getRoot() {
return 'foo';
}
} // $routeHandler

@All('bar')
bar() {
return 'bar';
}
} // $routeHandler

@Get('requestInputs/:x')
requestInputs(
@Param('x') x,
@Query() queryObj,
@Query('name') name,
@Req() req
@Req() req // $requestSource
) {
if (Math.random()) return x; // NOT OK
if (Math.random()) return queryObj; // NOT OK
if (Math.random()) return name; // NOT OK
if (Math.random()) return req.query.abc; // NOT OK
if (Math.random()) return x; // $responseSendArgument
if (Math.random()) return queryObj; // $responseSendArgument
if (Math.random()) return name; // $responseSendArgument
if (Math.random()) return req.query.abc; // $responseSendArgument
return;
}
} // $routeHandler

@Post('post')
post(@Body() body) {
return body.x; // NOT OK
}
return body.x; // $responseSendArgument
} // $routeHandler

@Get('redir')
@Redirect('https://example.com')
redir() {
return {
url: '//other.example.com' // OK
url: '//other.example.com' // $redirectSink
};
}
} // $routeHandler

@Get('redir')
@Redirect('https://example.com')
redir2(@Query('redirect') target) {
return {
url: target // NOT OK
url: target // $redirectSink
};
}
} // $routeHandler

@Get()
explicitSend(@Req() req, @Res() res) {
res.send(req.query.x) // NOT OK
}
explicitSend(@Req() req, @Res() res) { // $requestSource $responseSource
res.send(req.query.x) // $responseSource $responseSendArgument
} // $routeHandler

@Post()
upload(@UploadedFile() file) {
return file.originalname; // NOT OK
}
return file.originalname; // $responseSendArgument
} // $routeHandler

@Post()
uploadMany(@UploadedFiles() files) {
return files[0].originalname; // NOT OK
}
return files[0].originalname; // $responseSendArgument
} // $routeHandler
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,45 +4,45 @@ import { IsIn } from 'class-validator';
export class Controller {
@Get()
route1(@Query('x', new ValidationPipe()) validatedObj: Struct) {
return validatedObj.key; // OK
}
return validatedObj.key; // $responseSendArgument
} // $routeHandler

@Get()
route2(@Query('x', ValidationPipe) validatedObj: Struct) {
return validatedObj.key; // OK
}
return validatedObj.key; // $responseSendArgument
} // $routeHandler

@Get()
@UsePipes(new ValidationPipe())
route3(@Query('x') validatedObj: Struct, @Query('y') unvalidated: string) {
if (Math.random()) return validatedObj.key; // OK
return unvalidated; // NOT OK
}
if (Math.random()) return validatedObj.key; // $responseSendArgument
return unvalidated; // $responseSendArgument
} // $routeHandler

@Get()
@UsePipes(ValidationPipe)
route4(@Query('x') validatedObj: Struct, @Query('y') unvalidated: string) {
if (Math.random()) return validatedObj.key; // OK
return unvalidated; // NOT OK
}
if (Math.random()) return validatedObj.key; // $responseSendArgument
return unvalidated; // $responseSendArgument
} // $routeHandler
}

@UsePipes(new ValidationPipe())
export class Controller2 {
@Get()
route5(@Query('x') validatedObj: Struct, @Query('y') unvalidated: string) {
if (Math.random()) return validatedObj.key; // OK
return unvalidated; // NOT OK
}
if (Math.random()) return validatedObj.key; // $responseSendArgument
return unvalidated; // $responseSendArgument
} // $routeHandler
}

@UsePipes(ValidationPipe)
export class Controller3 {
@Get()
route6(@Query('x') validatedObj: Struct, @Query('y') unvalidated: string) {
if (Math.random()) return validatedObj.key; // OK
return unvalidated; // NOT OK
}
if (Math.random()) return validatedObj.key; // $responseSendArgument
return unvalidated; // $responseSendArgument
} // $routeHandler
}

class Struct {
Expand Down
77 changes: 38 additions & 39 deletions javascript/ql/test/library-tests/frameworks/Nest/test.expected
Original file line number Diff line number Diff line change
@@ -1,39 +1,6 @@
testFailures
routeHandler
| global/validation.ts:11:3:14:3 | route1( ... OK\\n } |
| global/validation.ts:17:3:19:3 | route2( ... x);\\n } |
| local/customDecorator.ts:18:3:20:3 | sneaky( ... OK\\n } |
| local/customDecorator.ts:23:3:25:3 | safe(@S ... OK\\n } |
| local/customPipe.ts:20:5:22:5 | sanitiz ... K\\n } |
| local/customPipe.ts:25:5:27:5 | sanitiz ... K\\n } |
| local/customPipe.ts:31:5:33:5 | sanitiz ... K\\n } |
| local/customPipe.ts:36:5:38:5 | propaga ... K\\n } |
| local/customPipe.ts:41:5:43:5 | propaga ... K\\n } |
| local/customPipe.ts:47:5:49:5 | propaga ... K\\n } |
| local/routes.ts:6:3:8:3 | getFoo( ... o';\\n } |
| local/routes.ts:11:3:13:3 | postFoo ... o';\\n } |
| local/routes.ts:16:3:18:3 | getRoot ... o';\\n } |
| local/routes.ts:21:3:23:3 | bar() { ... r';\\n } |
| local/routes.ts:26:3:37:3 | request ... rn;\\n } |
| local/routes.ts:40:3:42:3 | post(@B ... OK\\n } |
| local/routes.ts:46:3:50:3 | redir() ... };\\n } |
| local/routes.ts:54:3:58:3 | redir2( ... };\\n } |
| local/routes.ts:61:3:63:3 | explici ... OK\\n } |
| local/routes.ts:66:3:68:3 | upload( ... OK\\n } |
| local/routes.ts:71:3:73:3 | uploadM ... OK\\n } |
| local/validation.ts:6:3:8:3 | route1( ... OK\\n } |
| local/validation.ts:11:3:13:3 | route2( ... OK\\n } |
| local/validation.ts:17:3:20:3 | route3( ... OK\\n } |
| local/validation.ts:24:3:27:3 | route4( ... OK\\n } |
| local/validation.ts:33:3:36:3 | route5( ... OK\\n } |
| local/validation.ts:42:3:45:3 | route6( ... OK\\n } |
requestSource
| local/customDecorator.ts:5:21:5:51 | ctx.swi ... quest() |
| local/routes.ts:30:12:30:14 | req |
| local/routes.ts:61:23:61:25 | req |
responseSource
| local/routes.ts:61:35:61:37 | res |
| local/routes.ts:62:5:62:25 | res.sen ... uery.x) |
redirectSink
| local/routes.ts:48:12:48:32 | '//othe ... le.com' |
| local/routes.ts:56:12:56:17 | target |
requestInputAccess
| body | local/routes.ts:40:16:40:19 | body |
| body | local/routes.ts:66:26:66:29 | file |
Expand All @@ -60,6 +27,10 @@ requestInputAccess
| parameter | local/validation.ts:33:56:33:66 | unvalidated |
| parameter | local/validation.ts:42:22:42:33 | validatedObj |
| parameter | local/validation.ts:42:56:42:66 | unvalidated |
requestSource
| local/customDecorator.ts:5:21:5:51 | ctx.swi ... quest() |
| local/routes.ts:30:12:30:14 | req |
| local/routes.ts:61:23:61:25 | req |
responseSendArgument
| global/validation.ts:12:31:12:41 | unvalidated |
| global/validation.ts:13:12:13:27 | validatedObj.key |
Expand Down Expand Up @@ -89,6 +60,34 @@ responseSendArgument
| local/validation.ts:35:12:35:22 | unvalidated |
| local/validation.ts:43:31:43:46 | validatedObj.key |
| local/validation.ts:44:12:44:22 | unvalidated |
redirectSink
| local/routes.ts:48:12:48:32 | '//othe ... le.com' |
| local/routes.ts:56:12:56:17 | target |
responseSource
| local/routes.ts:61:35:61:37 | res |
| local/routes.ts:62:5:62:25 | res.sen ... uery.x) |
routeHandler
| global/validation.ts:11:3:14:3 | route1( ... ent\\n } |
| global/validation.ts:17:3:19:3 | route2( ... x);\\n } |
| local/customDecorator.ts:18:3:20:3 | sneaky( ... ent\\n } |
| local/customDecorator.ts:23:3:25:3 | safe(@S ... ent\\n } |
| local/customPipe.ts:20:5:22:5 | sanitiz ... t\\n } |
| local/customPipe.ts:25:5:27:5 | sanitiz ... t\\n } |
| local/customPipe.ts:31:5:33:5 | sanitiz ... t\\n } |
| local/customPipe.ts:36:5:38:5 | propaga ... t\\n } |
| local/customPipe.ts:41:5:43:5 | propaga ... t\\n } |
| local/customPipe.ts:47:5:49:5 | propaga ... t\\n } |
| local/routes.ts:6:3:8:3 | getFoo( ... o';\\n } |
| local/routes.ts:11:3:13:3 | postFoo ... o';\\n } |
| local/routes.ts:16:3:18:3 | getRoot ... o';\\n } |
| local/routes.ts:21:3:23:3 | bar() { ... r';\\n } |
| local/routes.ts:26:3:37:3 | request ... rn;\\n } |
| local/routes.ts:40:3:42:3 | post(@B ... ent\\n } |
| local/routes.ts:46:3:50:3 | redir() ... };\\n } |
| local/routes.ts:54:3:58:3 | redir2( ... };\\n } |
| local/routes.ts:61:3:63:3 | explici ... ent\\n } |
| local/routes.ts:66:3:68:3 | upload( ... ent\\n } |
| local/routes.ts:71:3:73:3 | uploadM ... ent\\n } |
| local/validation.ts:6:3:8:3 | route1( ... ent\\n } |
| local/validation.ts:11:3:13:3 | route2( ... ent\\n } |
| local/validation.ts:17:3:20:3 | route3( ... ent\\n } |
| local/validation.ts:24:3:27:3 | route4( ... ent\\n } |
| local/validation.ts:33:3:36:3 | route5( ... ent\\n } |
| local/validation.ts:42:3:45:3 | route6( ... ent\\n } |
3 changes: 0 additions & 3 deletions javascript/ql/test/library-tests/frameworks/Nest/test.ql
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import javascript
private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomizations
private import utils.test.InlineFlowTest

query Http::RouteHandler routeHandler() { any() }

Expand All @@ -26,5 +25,3 @@ module TestConfig implements DataFlow::ConfigSig {
exists(DataFlow::CallNode call | call.getCalleeName() = "sink" and node = call.getArgument(0))
}
}

import ValueFlowTest<TestConfig>
2 changes: 2 additions & 0 deletions javascript/ql/test/library-tests/frameworks/Nest/test.qlref
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
query: test.ql
postprocess: utils/test/InlineExpectationsTestQuery.ql