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
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
@@ -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
@@ -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 {
Original file line number Diff line number Diff line change
@@ -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;
},
);
@@ -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
@@ -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
@@ -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
@@ -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 {
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 |
@@ -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 |
@@ -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() }

@@ -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