Skip to content

Commit eb1da15

Browse files
committed
Python: Rewrite ClickHouse SQL lib modeling
This did turn into a few changes, that maybe could have been split into separate PRs 🤷 * Rename `ClickHouseDriver` => `ClickhouseDriver`, to better follow import name in `.qll` name * Rewrote modeling to use API graphs * Split modeling of `aioch` into separate `.qll` file, which does re-use the `getExecuteMethodName` predicate. I feel that sharing code between the modeling like this was the best approach, and stuck the `INTERNAL: Do not use.` labels on both modules. * I also added handling of keyword arguments (see change in .py files)
1 parent c9a9535 commit eb1da15

File tree

7 files changed

+128
-96
lines changed

7 files changed

+128
-96
lines changed
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
/**
2+
* Provides classes modeling security-relevant aspects of the `aioch` PyPI package (an
3+
* async-io version of the `clickhouse-driver` PyPI package).
4+
*
5+
* See https://pypi.org/project/aioch/
6+
*/
7+
8+
private import python
9+
private import semmle.python.Concepts
10+
private import semmle.python.ApiGraphs
11+
private import semmle.python.frameworks.PEP249
12+
private import experimental.semmle.python.frameworks.ClickhouseDriver
13+
14+
/**
15+
* INTERNAL: Do not use.
16+
*
17+
* Provides models for `aioch` PyPI package (an async-io version of the
18+
* `clickhouse-driver` PyPI package).
19+
*
20+
* See https://pypi.org/project/aioch/
21+
*/
22+
module Aioch {
23+
/** Provides models for `aioch.Client` class and subclasses. */
24+
module Client {
25+
/** Gets a reference to the `aioch.Client` class or any subclass. */
26+
API::Node subclassRef() {
27+
result = API::moduleImport("aioch").getMember("Client").getASubclass*()
28+
}
29+
30+
/** Gets a reference to an instance of `clickhouse_driver.Client` or any subclass. */
31+
API::Node instance() { result = subclassRef().getReturn() }
32+
}
33+
34+
/**
35+
* A call to any of the the execute methods on a `aioch.Client`, which are just async
36+
* versions of the methods in the `clickhouse-driver` PyPI package.
37+
*
38+
* See
39+
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute
40+
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_iter
41+
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_with_progress
42+
*/
43+
class ClientExecuteCall extends SqlExecution::Range, DataFlow::CallCfgNode {
44+
ClientExecuteCall() {
45+
exists(string methodName | methodName = ClickhouseDriver::getExecuteMethodName() |
46+
this = Client::instance().getMember(methodName).getACall()
47+
)
48+
}
49+
50+
override DataFlow::Node getSql() { result in [this.getArg(0), this.getArgByName("query")] }
51+
}
52+
}

python/ql/src/experimental/semmle/python/frameworks/ClickHouseDriver.qll

Lines changed: 0 additions & 85 deletions
This file was deleted.
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* Provides classes modeling security-relevant aspects of the `clickhouse-driver` PyPI package.
3+
* See
4+
* - https://pypi.org/project/clickhouse-driver/
5+
* - https://clickhouse-driver.readthedocs.io/en/latest/
6+
*/
7+
8+
private import python
9+
private import semmle.python.Concepts
10+
private import semmle.python.ApiGraphs
11+
private import semmle.python.frameworks.PEP249
12+
13+
/**
14+
* INTERNAL: Do not use.
15+
*
16+
* Provides models for `clickhouse-driver` PyPI package (imported as `clickhouse_driver`).
17+
* See
18+
* - https://pypi.org/project/clickhouse-driver/
19+
* - https://clickhouse-driver.readthedocs.io/en/latest/
20+
*/
21+
module ClickhouseDriver {
22+
/**
23+
* `clickhouse_driver` implements PEP249,
24+
* providing ways to execute SQL statements against a database.
25+
*/
26+
class ClickHouseDriverPEP249 extends PEP249ModuleApiNode {
27+
ClickHouseDriverPEP249() { this = API::moduleImport("clickhouse_driver") }
28+
}
29+
30+
/** Provides models for `clickhouse_driver.Client` class and subclasses. */
31+
module Client {
32+
/** Gets a reference to the `clickhouse_driver.Client` class or any subclass. */
33+
API::Node subclassRef() {
34+
exists(API::Node classRef |
35+
// canonical definition
36+
classRef = API::moduleImport("clickhouse_driver").getMember("client").getMember("Client")
37+
or
38+
// commonly used alias
39+
classRef = API::moduleImport("clickhouse_driver").getMember("Client")
40+
|
41+
result = classRef.getASubclass*()
42+
)
43+
}
44+
45+
/** Gets a reference to an instance of `clickhouse_driver.Client` or any subclass. */
46+
API::Node instance() { result = subclassRef().getReturn() }
47+
}
48+
49+
/** `clickhouse_driver.Client` execute method names */
50+
string getExecuteMethodName() { result in ["execute_with_progress", "execute", "execute_iter"] }
51+
52+
/**
53+
* A call to any of the the execute methods on a `clickhouse_driver.Client` method
54+
*
55+
* See
56+
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute
57+
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_iter
58+
* - https://clickhouse-driver.readthedocs.io/en/latest/api.html#clickhouse_driver.Client.execute_with_progress
59+
*/
60+
class ClientExecuteCall extends SqlExecution::Range, DataFlow::CallCfgNode {
61+
ClientExecuteCall() { this = Client::instance().getMember(getExecuteMethodName()).getACall() }
62+
63+
override DataFlow::Node getSql() { result in [this.getArg(0), this.getArgByName("query")] }
64+
}
65+
}
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
import python
22
import experimental.meta.ConceptsTest
3-
import experimental.semmle.python.frameworks.ClickHouseDriver
3+
import experimental.semmle.python.frameworks.Aioch

python/ql/test/experimental/semmle/python/frameworks/aioch/sql_test.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ async def aioch_test():
88
client = aioch.Client("localhost")
99

1010
await client.execute(SQL) # $ getSql=SQL
11-
await client.execute(query=SQL) # $ MISSING: getSql=SQL
11+
await client.execute(query=SQL) # $ getSql=SQL
1212

1313
await client.execute_with_progress(SQL) # $ getSql=SQL
14-
await client.execute_with_progress(query=SQL) # $ MISSING: getSql=SQL
14+
await client.execute_with_progress(query=SQL) # $ getSql=SQL
1515

1616
await client.execute_iter(SQL) # $ getSql=SQL
17-
await client.execute_iter(query=SQL) # $ MISSING: getSql=SQL
17+
await client.execute_iter(query=SQL) # $ getSql=SQL
1818

1919

2020
# Using custom client (this has been seen done for the blocking version in
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
import python
22
import experimental.meta.ConceptsTest
3-
import experimental.semmle.python.frameworks.ClickHouseDriver
3+
import experimental.semmle.python.frameworks.ClickhouseDriver

python/ql/test/experimental/semmle/python/frameworks/clickhouse_driver/sql_test.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@
77
# Normal operation
88
client = clickhouse_driver.client.Client("localhost")
99

10-
client.execute(SQL) # $ MISSING: getSql=SQL
11-
client.execute(query=SQL) # $ MISSING: getSql=SQL
10+
client.execute(SQL) # $ getSql=SQL
11+
client.execute(query=SQL) # $ getSql=SQL
1212

13-
client.execute_with_progress(SQL) # $ MISSING: getSql=SQL
14-
client.execute_with_progress(query=SQL) # $ MISSING: getSql=SQL
13+
client.execute_with_progress(SQL) # $ getSql=SQL
14+
client.execute_with_progress(query=SQL) # $ getSql=SQL
1515

16-
client.execute_iter(SQL) # $ MISSING: getSql=SQL
17-
client.execute_iter(query=SQL) # $ MISSING: getSql=SQL
16+
client.execute_iter(SQL) # $ getSql=SQL
17+
client.execute_iter(query=SQL) # $ getSql=SQL
1818

1919

2020
# commonly used alias

0 commit comments

Comments
 (0)