Enhance URL handling in ClientRequest for got Options

Author: Napalys

javascript/ql/lib/semmle/javascript/frameworks/ClientRequests.qll

@@ -436,6 +436,13 @@ module ClientRequest {
     override DataFlow::Node getUrl() {
       result = this.getArgument(0) and
       not exists(this.getOptionArgument(1, "baseUrl"))
+      or
+      // Handle URL when passed as options
+      exists(API::InvokeNode optionsCall |
+        optionsCall = API::moduleImport("got").getMember("Options").getAnInvocation() and
+        optionsCall.getReturn().getAValueReachableFromSource() = this.getAnArgument() and
+        result = optionsCall.getParameter(0).getMember("url").asSink()
+      )
     }
 
     override DataFlow::Node getHost() {

javascript/ql/test/library-tests/frameworks/ClientRequests/ClientRequests.expected

@@ -99,7 +99,9 @@ test_ClientRequest
 | tst.js:321:5:321:32 | superag ... st(url) |
 | tst.js:328:5:328:38 | got(und ... ptions) |
 | tst.js:329:5:329:45 | got(und ... {url})) |
+| tst.js:332:5:332:46 | got.ext ... ).get() |
 | tst.js:337:5:337:20 | jsonClient.get() |
+| tst.js:340:5:340:21 | jsonClient2.get() |
 test_getADataNode
 | axiosTest.js:12:5:17:6 | axios({ ... \\n    }) | axiosTest.js:15:18:15:55 | { 'Cont ... json' } |
 | axiosTest.js:12:5:17:6 | axios({ ... \\n    }) | axiosTest.js:16:15:16:35 | {x: 'te ... 'test'} |
@@ -257,8 +259,10 @@ test_getUrl
 | tst.js:319:5:319:26 | superag ... ', url) | tst.js:319:23:319:25 | url |
 | tst.js:320:5:320:23 | superagent.del(url) | tst.js:320:20:320:22 | url |
 | tst.js:321:5:321:32 | superag ... st(url) | tst.js:321:29:321:31 | url |
+| tst.js:328:5:328:38 | got(und ... ptions) | tst.js:327:34:327:36 | url |
 | tst.js:328:5:328:38 | got(und ... ptions) | tst.js:328:9:328:17 | undefined |
 | tst.js:329:5:329:45 | got(und ... {url})) | tst.js:329:9:329:17 | undefined |
+| tst.js:329:5:329:45 | got(und ... {url})) | tst.js:329:40:329:42 | url |
 test_getAResponseDataNode
 | axiosTest.js:4:5:7:6 | axios({ ... \\n    }) | axiosTest.js:4:5:7:6 | axios({ ... \\n    }) | json | true |
 | axiosTest.js:12:5:17:6 | axios({ ... \\n    }) | axiosTest.js:12:5:17:6 | axios({ ... \\n    }) | json | true |
@@ -341,4 +345,6 @@ test_getAResponseDataNode
 | tst.js:321:5:321:32 | superag ... st(url) | tst.js:321:5:321:32 | superag ... st(url) | stream | true |
 | tst.js:328:5:328:38 | got(und ... ptions) | tst.js:328:5:328:38 | got(und ... ptions) | text | true |
 | tst.js:329:5:329:45 | got(und ... {url})) | tst.js:329:5:329:45 | got(und ... {url})) | text | true |
+| tst.js:332:5:332:46 | got.ext ... ).get() | tst.js:332:5:332:46 | got.ext ... ).get() | text | true |
 | tst.js:337:5:337:20 | jsonClient.get() | tst.js:337:5:337:20 | jsonClient.get() | text | true |
+| tst.js:340:5:340:21 | jsonClient2.get() | tst.js:340:5:340:21 | jsonClient2.get() | text | true |

javascript/ql/test/library-tests/frameworks/ClientRequests/tst.js

@@ -325,8 +325,8 @@ import { Options } from 'got';
 
 function gotTests(url){
     const options = new Options({url});
-    got(undefined, undefined, options); // undefined is flagged, but should be url from options
-    got(undefined, undefined, Options({url})); // undefined is flagged, but should be url from options
+    got(undefined, undefined, options);
+    got(undefined, undefined, Options({url}));
 
     const options2 = new Options({url});
     got.extend(options2).extend(options).get(); // call flagged not the actual url flow