Skip to content

Commit 9db235a

Browse files
committed
JS: Improve @google-cloud/spanner model
1 parent 35f294f commit 9db235a

File tree

4 files changed

+69
-27
lines changed

4 files changed

+69
-27
lines changed

javascript/ql/src/semmle/javascript/frameworks/SQL.qll

Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -543,68 +543,90 @@ private module Spanner {
543543
API::Node database() {
544544
result =
545545
spanner().getReturn().getMember("instance").getReturn().getMember("database").getReturn()
546+
or
547+
result = API::Node::ofType("@google-cloud/spanner", "Database")
546548
}
547549

548550
/**
549551
* Gets a node that refers to an instance of the `v1.SpannerClient` class.
550552
*/
551553
API::Node v1SpannerClient() {
552554
result = spanner().getMember("v1").getMember("SpannerClient").getInstance()
555+
or
556+
result = API::Node::ofType("@google-cloud/spanner", "v1.SpannerClient")
553557
}
554558

555559
/**
556560
* Gets a node that refers to a transaction object.
557561
*/
558562
API::Node transaction() {
559-
result = database().getMember("runTransaction").getParameter(0).getParameter(1)
563+
result =
564+
database()
565+
.getMember(["runTransaction", "runTransactionAsync"])
566+
.getParameter([0, 1])
567+
.getParameter(1)
568+
or
569+
result = API::Node::ofType("@google-cloud/spanner", "Transaction")
570+
}
571+
572+
/** Gets an API node referring to a `BatchTransaction` object. */
573+
API::Node batchTransaction() {
574+
result = database().getMember("batchTransaction").getReturn()
575+
or
576+
result = database().getMember("createBatchTransaction").getReturn().getPromised()
577+
or
578+
result = API::Node::ofType("@google-cloud/spanner", "BatchTransaction")
560579
}
561580

562581
/**
563582
* A call to a Spanner method that executes a SQL query.
564583
*/
565-
abstract class SqlExecution extends DatabaseAccess, DataFlow::InvokeNode {
566-
/**
567-
* Gets the position of the query argument; default is zero, which can be overridden
568-
* by subclasses.
569-
*/
570-
int getQueryArgumentPosition() { result = 0 }
571-
572-
override DataFlow::Node getAQueryArgument() {
573-
result = getArgument(getQueryArgumentPosition()) or
574-
result = getOptionArgument(getQueryArgumentPosition(), "sql")
575-
}
576-
}
584+
abstract class SqlExecution extends DatabaseAccess, DataFlow::InvokeNode { }
577585

578586
/**
579-
* A call to `Database.run`, `Database.runPartitionedUpdate` or `Database.runStream`.
587+
* A SQL execution that takes the input directly in the first argument or in the `sql` option.
580588
*/
581-
class DatabaseRunCall extends SqlExecution {
582-
DatabaseRunCall() {
589+
class SqlExecutionDirect extends SqlExecution {
590+
SqlExecutionDirect() {
583591
this = database().getMember(["run", "runPartitionedUpdate", "runStream"]).getACall()
592+
or
593+
this = transaction().getMember(["run", "runStream", "runUpdate"]).getACall()
594+
or
595+
this = batchTransaction().getMember("createQueryPartitions").getACall()
596+
}
597+
598+
override DataFlow::Node getAQueryArgument() {
599+
result = getArgument(0)
600+
or
601+
result = getOptionArgument(0, "sql")
584602
}
585603
}
586604

587605
/**
588-
* A call to `Transaction.run`, `Transaction.runStream` or `Transaction.runUpdate`.
606+
* A SQL execution that takes an array of SQL strings or { sql: string } objects.
589607
*/
590-
class TransactionRunCall extends SqlExecution {
591-
TransactionRunCall() {
592-
this = transaction().getMember(["run", "runStream", "runUpdate"]).getACall()
608+
class SqlExecutionBatch extends SqlExecution, API::CallNode {
609+
SqlExecutionBatch() { this = transaction().getMember("batchUpdate").getACall() }
610+
611+
override DataFlow::Node getAQueryArgument() {
612+
// just use the whole array as the query argument, as arrays becomes tainted if one of the elements
613+
// are tainted
614+
result = getArgument(0)
615+
or
616+
result = getParameter(0).getUnknownMember().getMember("sql").getARhs()
593617
}
594618
}
595619

596620
/**
597-
* A call to `v1.SpannerClient.executeSql` or `v1.SpannerClient.executeStreamingSql`.
621+
* A SQL execution that only takes the input in the `sql` option, and do not accept query strings
622+
* directly.
598623
*/
599-
class ExecuteSqlCall extends SqlExecution {
600-
ExecuteSqlCall() {
624+
class SqlExecutionWithOption extends SqlExecution {
625+
SqlExecutionWithOption() {
601626
this = v1SpannerClient().getMember(["executeSql", "executeStreamingSql"]).getACall()
602627
}
603628

604-
override DataFlow::Node getAQueryArgument() {
605-
// `executeSql` and `executeStreamingSql` do not accept query strings directly
606-
result = getOptionArgument(0, "sql")
607-
}
629+
override DataFlow::Node getAQueryArgument() { result = getOptionArgument(0, "sql") }
608630
}
609631

610632
/**

javascript/ql/test/library-tests/frameworks/SQL/SqlString.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
| sequelizeImport.js:3:17:3:118 | 'SELECT ... Y name' |
4242
| spanner2.js:5:26:5:35 | "SQL code" |
4343
| spanner2.js:7:35:7:44 | "SQL code" |
44+
| spanner-types.ts:4:12:4:23 | 'SELECT 123' |
4445
| spanner.js:6:8:6:17 | "SQL code" |
4546
| spanner.js:7:8:7:26 | { sql: "SQL code" } |
4647
| spanner.js:7:15:7:24 | "SQL code" |
@@ -59,6 +60,8 @@
5960
| spanner.js:18:16:18:25 | "SQL code" |
6061
| spanner.js:19:16:19:34 | { sql: "SQL code" } |
6162
| spanner.js:19:23:19:32 | "SQL code" |
63+
| spanner.js:23:12:23:23 | 'SELECT 123' |
64+
| spanner.js:26:12:26:38 | 'UPDATE ... = @baz' |
6265
| spannerImport.js:4:8:4:17 | "SQL code" |
6366
| sqlite-types.ts:4:12:4:49 | "UPDATE ... id = ?" |
6467
| sqlite.js:7:8:7:45 | "UPDATE ... id = ?" |
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import { Database } from "@google-cloud/spanner";
2+
3+
export function doSomething(db: Database) {
4+
db.run('SELECT 123');
5+
}

javascript/ql/test/library-tests/frameworks/SQL/spanner.js

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,18 @@ db.runTransaction((err, tx) => {
1717
tx.runStream({ sql: "SQL code" });
1818
tx.runUpdate("SQL code");
1919
tx.runUpdate({ sql: "SQL code" });
20+
21+
const queries = [
22+
{
23+
sql: 'SELECT 123',
24+
},
25+
{
26+
sql: 'UPDATE foo SET bar = @baz',
27+
params: {key: 'baz', value: '123'}
28+
}
29+
];
30+
31+
tx.batchUpdate(queries, () => {});
2032
});
2133

2234
exports.instance = instance;

0 commit comments

Comments
 (0)