From acef9b71114f58f6de3a7787d7e6aecf646e7dbe Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 4 Apr 2024 15:03:00 +0200 Subject: [PATCH 01/22] Dynamic/JS: Add library for exporting models --- .../javascript/endpoints/EndpointNaming.qll | 6 +- .../frameworks/data/ModelsAsData.qll | 96 ++++++++ .../data/internal/ApiGraphModels.qll | 2 +- .../data/internal/ApiGraphModelsSpecific.qll | 20 ++ .../ModelGeneration/ModelGeneration.expected | 3 + .../ModelGeneration/ModelGeneration.ext.yml | 6 + .../ModelGeneration/ModelGeneration.ql | 17 ++ .../ModelGeneration/reexport/package.json | 4 + .../ModelGeneration/reexport/reexport.js | 10 + shared/mad/codeql/mad/dynamic/GraphExport.qll | 230 ++++++++++++++++++ 10 files changed, 392 insertions(+), 2 deletions(-) create mode 100644 javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected create mode 100644 javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml create mode 100644 javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql create mode 100644 javascript/ql/test/library-tests/ModelGeneration/reexport/package.json create mode 100644 javascript/ql/test/library-tests/ModelGeneration/reexport/reexport.js create mode 100644 shared/mad/codeql/mad/dynamic/GraphExport.qll diff --git a/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll b/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll index fdb2b7ab9669..9e514d4c9f41 100644 --- a/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll +++ b/javascript/ql/lib/semmle/javascript/endpoints/EndpointNaming.qll @@ -147,7 +147,11 @@ private predicate isPrivateAssignment(DataFlow::Node node) { ) } -private predicate isPrivateLike(API::Node node) { isPrivateAssignment(node.asSink()) } +/** + * Holds if `node` is the sink node corresponding to the right-hand side of a private declaration, + * like a private field (`#field`) or class member with the `private` modifier. + */ +predicate isPrivateLike(API::Node node) { isPrivateAssignment(node.asSink()) } bindingset[name] private int getNameBadness(string name) { diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll index 7fb674dfdbaa..fd01b0719480 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll @@ -19,6 +19,7 @@ private import javascript private import internal.ApiGraphModels as Shared private import internal.ApiGraphModelsSpecific as Specific +private import semmle.javascript.endpoints.EndpointNaming as EndpointNaming import Shared::ModelInput as ModelInput import Shared::ModelOutput as ModelOutput @@ -55,3 +56,98 @@ private class TaintStepFromSummary extends TaintTracking::SharedTaintStep { summaryStepNodes(pred, succ, "taint") } } + +/** + * Specifies which parts of the API graph to export in `ModelExport`. + */ +signature module ModelExportSig { + /** + * Holds if the exported model should contain `node`, if it is publicly accessible. + * + * This ensures that all ways to access `node` will be exported in type models. + */ + predicate shouldContain(API::Node node); + + /** + * Holds if a named must be generated for `node` if it is to be included in the exported graph. + */ + default predicate mustBeNamed(API::Node node) { none() } +} + +/** + * Module for exporting type models for a given set of nodes in the API graph. + */ +module ModelExport { + private import codeql.mad.dynamic.GraphExport + + private module GraphExportConfig implements GraphExportSig { + predicate edge = Specific::apiGraphHasEdge/3; + + predicate shouldContain = S::shouldContain/1; + + predicate shouldNotContain(API::Node node) { + EndpointNaming::isPrivateLike(node) + or + node instanceof API::Use + } + + predicate mustBeNamed(API::Node node) { + node.getAValueReachingSink() instanceof DataFlow::ClassNode + or + node = API::Internal::getClassInstance(_) + or + S::mustBeNamed(node) + } + + predicate exposedName(API::Node node, string type, string path) { + node = API::moduleExport(type) and path = "" + } + + predicate suggestedName(API::Node node, string type) { + exists(string package, string name | + ( + EndpointNaming::sinkHasPrimaryName(node, package, name) and + not EndpointNaming::aliasDefinition(_, _, _, _, node) + or + EndpointNaming::aliasDefinition(_, _, package, name, node) + ) and + type = EndpointNaming::renderName(package, name) + ) + } + + bindingset[host] + predicate hasTypeSummary(API::Node host, string path) { + exists(string methodName | + functionReturnsReceiver(host.getMember(methodName).getAValueReachingSink()) and + path = "Member[" + methodName + "].ReturnValue" + ) + } + + pragma[nomagic] + private predicate functionReturnsReceiver(DataFlow::FunctionNode func) { + getAReceiverRef(func).flowsTo(func.getReturnNode()) + } + + pragma[nomagic] + private DataFlow::MethodCallNode getAReceiverCall(DataFlow::FunctionNode func) { + result = getAReceiverRef(func).getAMethodCall() + } + + pragma[nomagic] + private predicate callReturnsReceiver(DataFlow::MethodCallNode call) { + functionReturnsReceiver(call.getACallee().flow()) + } + + pragma[nomagic] + private DataFlow::SourceNode getAReceiverRef(DataFlow::FunctionNode func) { + result = func.getReceiver() + or + result = getAReceiverCall(func) and + callReturnsReceiver(result) + } + } + + private module ExportedGraph = GraphExport; + + import ExportedGraph +} diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll index dd4331527519..fe316217df0d 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll @@ -435,7 +435,7 @@ private API::Node getNodeFromType(string type) { * Gets the API node identified by the first `n` tokens of `path` in the given `(type, path)` tuple. */ pragma[nomagic] -private API::Node getNodeFromPath(string type, AccessPath path, int n) { +API::Node getNodeFromPath(string type, AccessPath path, int n) { isRelevantFullPath(type, path) and ( n = 0 and diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll index 664c040d57e8..5b86fba8d487 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll @@ -353,3 +353,23 @@ module ModelOutputSpecific { ) } } + +/** + * Holds if the edge `pred -> succ` labelled with `path` exists in the API graph. + */ +bindingset[pred] +predicate apiGraphHasEdge(API::Node pred, string path, API::Node succ) { + exists(string name | succ = pred.getMember(name) and path = "Member[" + name + "]") + or + succ = pred.getUnknownMember() and path = "AnyMember" + or + succ = pred.getInstance() and path = "Instance" + or + succ = pred.getReturn() and path = "ReturnValue" + or + exists(int n | succ = pred.getParameter(n) | + if pred instanceof API::Use then path = "Argument[" + n + "]" else path = "Parameter[" + n + "]" + ) + or + succ = pred.getPromised() and path = "Awaited" +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected new file mode 100644 index 000000000000..05753fb9cc8e --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -0,0 +1,3 @@ +typeModel +| (reexport).func | reexport | Member[func] | +summaryModel diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml new file mode 100644 index 000000000000..a608ae5a4939 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml @@ -0,0 +1,6 @@ +extensions: + - addsTo: + pack: codeql/javascript-all + extensible: typeModel + data: + - ["upstream-lib.XYZ", "upstream-lib", "Member[x].Member[y].Member[z]"] diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql new file mode 100644 index 000000000000..74caddf1a1ff --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql @@ -0,0 +1,17 @@ +private import javascript +private import semmle.javascript.endpoints.EndpointNaming as EndpointNaming +private import semmle.javascript.frameworks.data.internal.ApiGraphModels as Shared + +module ModelExportConfig implements ModelExportSig { + predicate shouldContain(API::Node node) { + node.getAValueReachingSink() instanceof DataFlow::FunctionNode + } + + predicate mustBeNamed(API::Node node) { shouldContain(node) } +} + +module Exported = ModelExport; + +query predicate typeModel = Exported::typeModel/3; + +query predicate summaryModel = Exported::summaryModel/5; diff --git a/javascript/ql/test/library-tests/ModelGeneration/reexport/package.json b/javascript/ql/test/library-tests/ModelGeneration/reexport/package.json new file mode 100644 index 000000000000..85258b89fa91 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/reexport/package.json @@ -0,0 +1,4 @@ +{ + "name": "reexport", + "main": "reexport.js" +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/reexport/reexport.js b/javascript/ql/test/library-tests/ModelGeneration/reexport/reexport.js new file mode 100644 index 000000000000..86b43f5820a7 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/reexport/reexport.js @@ -0,0 +1,10 @@ +import * as lib from "upstream-lib"; + +export { lib }; + +export const x = lib.x; +export const xy = lib.x.y; + +export function func() { + return lib; +} diff --git a/shared/mad/codeql/mad/dynamic/GraphExport.qll b/shared/mad/codeql/mad/dynamic/GraphExport.qll new file mode 100644 index 000000000000..3eeb50d099cf --- /dev/null +++ b/shared/mad/codeql/mad/dynamic/GraphExport.qll @@ -0,0 +1,230 @@ +/** + * Contains predicates for converting an arbitrary graph to a set of `typeModel` rows. + */ + +/** + * Concatenates two access paths, separating them by `.` unless one of them is empty. + */ +bindingset[x, y] +string join(string x, string y) { + if x = "" or y = "" then result = x + y else result = x + "." + y +} + +signature class NodeSig { + /** + * Holds if this node is located in file `path` between line `startline`, column `startcol`, + * and line `endline`, column `endcol`. + */ + predicate hasLocationInfo(string path, int startline, int startcol, int endline, int endcol); + + /** Gets a string representation of this node. */ + string toString(); +} + +/** + * Specifies a graph to export in `GraphExport`. + */ +signature module GraphExportSig { + /** + * Holds if an edge `pred -> succ` exist with the access path `path`. + */ + bindingset[pred] + predicate edge(Node pred, string path, Node succ); + + /** + * Holds if `node` is exposed to downstream packages with the given `(type, path)` tuple. + * + * A consumer of the exported graph should be able to interpret the `(type, path)` pair + * without having access to the current codebase. + */ + predicate exposedName(Node node, string type, string path); + + /** + * Holds if `name` is a good name for `node` that should be used in case the node needs + * to be named with a type name. + * + * Should not hold for nodes that are named via `exposedName`. + */ + predicate suggestedName(Node node, string name); + + /** + * Holds if `node` must be named if it part of the exported graph. + */ + predicate mustBeNamed(Node node); + + /** + * Holds if the exported graph should contain `node`, if it is reachable from an exposed node. + * + * This ensures that all paths leading from an exposed node to `node` will be exported. + */ + predicate shouldContain(Node node); + + /** + * Holds if `host` has a method that returns `this`, and `path` is the path from `host` + * to the method followed by the appropriate `ReturnValue` token. + * + * For example, if the method is named `m` then `path` should be `Member[m].ReturnValue` + * or `Method[m].ReturnValue`. + */ + bindingset[host] + predicate hasTypeSummary(Node host, string path); + + /** + * Holds if paths going through `node` should be blocked. + * + * For example, this can be the case for functions that are public at runtime + * but intended to be private. + */ + predicate shouldNotContain(Node node); +} + +/** + * Module for exporting an arbitrary graph as models-as-data rows. + */ +module GraphExport S> { + private import S + + private Node getAnExposedNode() { + not shouldNotContain(result) and + ( + exposedName(result, _, _) + or + edge(getAnExposedNode(), _, result) + ) + } + + pragma[nomagic] + private predicate exposedEdge(Node pred, string path, Node succ) { + // Materialize this relation so we can access 'edge' without binding set on 'pred' + pred = getAnExposedNode() and + edge(pred, path, succ) + } + + private Node getARelevantNode() { + result = getAnExposedNode() and + shouldContain(result) + or + exposedEdge(result, _, getARelevantNode()) + } + + final private class FinalNode = Node; + + private class RelevantNode extends FinalNode { + RelevantNode() { this = getARelevantNode() } + } + + pragma[inline] + private RelevantNode getAPredecessor(RelevantNode node, string path) { + exposedEdge(result, path, node) + } + + private predicate nodeMustBeNamed(RelevantNode node) { + exposedName(node, _, "") + or + S::mustBeNamed(node) + or + strictcount(getAPredecessor(node, _)) > 1 + } + + /** Gets a type-name to use as a prefix, in case we need to synthesize a name. */ + private string getAPrefixTypeName(RelevantNode node) { + result = + min(string prefix | + exists(string type, string path | + exposedName(node, type, path) and + prefix = join(type, path) + ) + or + suggestedName(node, prefix) + | + prefix + ) + or + not exposedName(node, _, _) and + not suggestedName(node, _) and + result = getAPrefixTypeName(getAPredecessor(node, _)) + } + + /** + * Holds if a named type exists or will be generated for `node`. + */ + private predicate isSyntheticallyNamedNode(RelevantNode node, string prefix) { + nodeMustBeNamed(node) and + not exposedName(node, _, "") and + not suggestedName(node, _) and + prefix = min(getAPrefixTypeName(node)) + } + + /** + * Gets a synthetic type name to generate for `node`. + */ + private string getSyntheticName(RelevantNode node) { + exists(int k, string prefixTypeName | + node = + rank[k](RelevantNode n, string path, int startline, int startcol, int endline, int endcol | + isSyntheticallyNamedNode(n, prefixTypeName) and + n.hasLocationInfo(path, startline, startcol, endline, endcol) + | + // Use location information for an arbitrary ordering + n order by path, startline, startcol, endline, endcol + ) and + result = prefixTypeName + "~expr" + k + ) + } + + private string getNodeName(RelevantNode node) { + nodeMustBeNamed(node) and + ( + exposedName(node, result, "") + or + suggestedName(node, result) and + not exposedName(node, _, "") + or + result = getSyntheticName(node) + ) + } + + /** + * Holds if `(type, path)` resolves to `node` in the exported graph. + */ + predicate pathToNode(string type, string path, RelevantNode node) { + type = getNodeName(node) and + path = "" + or + exposedName(node, type, path) + or + not nodeMustBeNamed(node) and + exists(string prevPath, string step | + pathToNode(type, prevPath, getAPredecessor(node, step)) and + path = join(prevPath, step) + ) + } + + /** + * Holds if `type1, type2, path` should be emitted as a type row. + * + * That is, `(type2, path)` leads to an value belonging to `type1`. + */ + predicate typeModel(string type1, string type2, string path) { + exists(string prevPath, string step, RelevantNode node | + type1 = getNodeName(node) and + pathToNode(type2, prevPath, getAPredecessor(node, step)) and + path = join(prevPath, step) + ) and + not (type1 = type2 and path = "") + } + + /** + * Holds if `type, path, input, output, kind` should be emitted as a summary row. + * + * This is only used to emit type propagation summaries, that is, summaries of kind `type`. + */ + predicate summaryModel(string type, string path, string input, string output, string kind) { + exists(RelevantNode host | + pathToNode(type, path, host) and + hasTypeSummary(host, output) and + input = "" and + kind = "type" + ) + } +} From c55e03c588fc80fbaa040d5fc3126e1dc27ef8b1 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 4 Apr 2024 15:06:19 +0200 Subject: [PATCH 02/22] Dynamic/JS: Add support for re-exporting type models --- .../frameworks/data/ModelsAsData.qll | 10 +- .../data/internal/ApiGraphModels.qll | 2 +- .../data/internal/ApiGraphModelsExport.qll | 122 ++++++++++++++++++ .../ModelGeneration/ModelGeneration.expected | 4 + .../ModelGeneration/ModelGeneration.ql | 2 + 5 files changed, 138 insertions(+), 2 deletions(-) create mode 100644 javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll index fd01b0719480..03193d0ee129 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll @@ -72,6 +72,13 @@ signature module ModelExportSig { * Holds if a named must be generated for `node` if it is to be included in the exported graph. */ default predicate mustBeNamed(API::Node node) { none() } + + /** + * Holds if the exported model should preserve all paths leading to an instance of `type`, + * including partial ones. It does not need to be closed transitively, `ModelExport` will + * extend this to include type models from which `type` can be derived. + */ + default predicate shouldContainType(string type) { none() } } /** @@ -79,6 +86,7 @@ signature module ModelExportSig { */ module ModelExport { private import codeql.mad.dynamic.GraphExport + private import internal.ApiGraphModelsExport private module GraphExportConfig implements GraphExportSig { predicate edge = Specific::apiGraphHasEdge/3; @@ -147,7 +155,7 @@ module ModelExport { } } - private module ExportedGraph = GraphExport; + private module ExportedGraph = TypeGraphExport; import ExportedGraph } diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll index fe316217df0d..8dea3d67bd8c 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModels.qll @@ -267,7 +267,7 @@ private predicate summaryModel(string type, string path, string input, string ou } /** Holds if a type model exists for the given parameters. */ -private predicate typeModel(string type1, string type2, string path) { +predicate typeModel(string type1, string type2, string path) { exists(string row | typeModel(row) and row.splitAt(";", 0) = type1 and diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll new file mode 100644 index 000000000000..f6901128c231 --- /dev/null +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll @@ -0,0 +1,122 @@ +/** + * Contains an extension of `GraphExport` that relies on API graph specific functionality. + */ + +private import ApiGraphModels as Shared +private import codeql.mad.dynamic.GraphExport +private import ApiGraphModelsSpecific as Specific + +private module API = Specific::API; + +private import Shared + +/** + * Holds if some proper prefix of `(type, path)` evaluated to `node`, where `remainingPath` + * is bound to the suffix of `path` that was not evaluated yet. + */ +bindingset[type, path] +predicate partiallyEvaluatedModel(string type, string path, API::Node node, string remainingPath) { + exists(int n, AccessPath accessPath | + accessPath = path and + getNodeFromPath(type, accessPath, n) = node and + n > 0 and + // Note that `n < accessPath.getNumToken()` is implied by the use of strictconcat() + remainingPath = + strictconcat(int k | + k = [n .. accessPath.getNumToken() - 1] + | + accessPath.getToken(k), "." order by k + ) + ) +} + +/** + * Holds if `type` and all types leading to `type` should be re-exported. + */ +signature predicate shouldContainTypeSig(string type); + +/** + * Wrapper around `GraphExport` that also exports information about re-exported types. + * + * ### JavaScript example 1 + * For example, suppose `shouldContainType("foo")` holds, and the following is the entry point for a package `bar`: + * ```js + * // bar.js + * module.exports.xxx = require('foo'); + * ``` + * then this would generate the following type model: + * ``` + * foo; bar; Member[xxx] + * ``` + * + * ### JavaScript example 2 + * For a more complex case, suppose the following type model exists: + * ``` + * foo.XYZ; foo; Member[x].Member[y].Member[z] + * ``` + * And the package exports something that matches a prefix of the access path above: + * ```js + * module.exports.blah = require('foo').x.y; + * ``` + * This would result in the following type model: + * ``` + * foo.XYZ; bar; Member[blah].Member[z] + * ``` + * Notice that the access path `Member[blah].Member[z]` consists of an access path generated from the API + * graph, with pieces of the access path from the original type model appended to it. + */ +module TypeGraphExport S, shouldContainTypeSig/1 shouldContainType> { + /** Like `shouldContainType` but includes types that lead to `type` via type models. */ + private predicate shouldContainTypeEx(string type) { + shouldContainType(type) + or + exists(string prevType | + shouldContainType(prevType) and + Shared::typeModel(prevType, type, _) + ) + } + + private module Config implements GraphExportSig { + import S + + predicate shouldContain(API::Node node) { + S::shouldContain(node) + or + exists(string type1 | shouldContainTypeEx(type1) | + ModelOutput::getATypeNode(type1).getAValueReachableFromSource() = node.asSink() + or + exists(string type2, string path | + Shared::typeModel(type1, type2, path) and + getNodeFromPath(type2, path, _).getAValueReachableFromSource() = node.asSink() + ) + ) + } + } + + private module ExportedGraph = GraphExport; + + import ExportedGraph + + /** + * Holds if `type1, type2, path` should be emitted as a type model, that is `(type2, path)` leads to an instance of `type1`. + */ + predicate typeModel(string type1, string type2, string path) { + ExportedGraph::typeModel(type1, type2, path) + or + shouldContainTypeEx(type1) and + exists(API::Node node | + // A relevant type is exported directly + ModelOutput::getATypeNode(type1).getAValueReachableFromSource() = node.asSink() and + ExportedGraph::pathToNode(type2, path, node) + or + // Something that leads to a relevant type, but didn't finish its access path, is exported + exists(string midType, string midPath, string remainingPath, string prefix, API::Node source | + Shared::typeModel(type1, midType, midPath) and + partiallyEvaluatedModel(midType, midPath, source, remainingPath) and + source.getAValueReachableFromSource() = node.asSink() and + ExportedGraph::pathToNode(type2, prefix, node) and + path = join(prefix, remainingPath) + ) + ) + } +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index 05753fb9cc8e..e5df99eb34c3 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -1,3 +1,7 @@ typeModel | (reexport).func | reexport | Member[func] | +| upstream-lib | (reexport).func | ReturnValue | +| upstream-lib | reexport | Member[lib] | +| upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] | +| upstream-lib.XYZ | reexport | Member[xy].Member[z] | summaryModel diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql index 74caddf1a1ff..2d5ac3eb4235 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ql @@ -8,6 +8,8 @@ module ModelExportConfig implements ModelExportSig { } predicate mustBeNamed(API::Node node) { shouldContain(node) } + + predicate shouldContainType(string type) { Shared::isRelevantType(type) } } module Exported = ModelExport; From 348c95ebe1e03379f7fc862e19d6258e001a8fb4 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 11:30:35 +0200 Subject: [PATCH 03/22] JS: Add a test case with fluent flow --- .../ModelGeneration/ModelGeneration.expected | 12 ++++++++++++ .../ModelGeneration/return-this/package.json | 4 ++++ .../ModelGeneration/return-this/return-this.js | 17 +++++++++++++++++ 3 files changed, 33 insertions(+) create mode 100644 javascript/ql/test/library-tests/ModelGeneration/return-this/package.json create mode 100644 javascript/ql/test/library-tests/ModelGeneration/return-this/return-this.js diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index e5df99eb34c3..53d49d69d90f 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -1,7 +1,19 @@ typeModel | (reexport).func | reexport | Member[func] | +| (return-this).FluentInterface | return-this | Member[FluentInterface] | +| (return-this).FluentInterface.prototype | (return-this).FluentInterface | Instance | +| (return-this).FluentInterface.prototype | (return-this).FluentInterface.prototype.bar | ReturnValue | +| (return-this).FluentInterface.prototype | (return-this).FluentInterface.prototype.baz | ReturnValue | +| (return-this).FluentInterface.prototype.bar | (return-this).FluentInterface.prototype | Member[bar] | +| (return-this).FluentInterface.prototype.baz | (return-this).FluentInterface.prototype | Member[baz] | +| (return-this).FluentInterface.prototype.foo | (return-this).FluentInterface.prototype | Member[foo] | +| (return-this).FluentInterface.prototype.notFluent | (return-this).FluentInterface.prototype | Member[notFluent] | +| (return-this).FluentInterface.prototype.notFluent2 | (return-this).FluentInterface.prototype | Member[notFluent2] | | upstream-lib | (reexport).func | ReturnValue | | upstream-lib | reexport | Member[lib] | | upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] | | upstream-lib.XYZ | reexport | Member[xy].Member[z] | summaryModel +| (return-this).FluentInterface.prototype | | | Member[bar].ReturnValue | type | +| (return-this).FluentInterface.prototype | | | Member[baz].ReturnValue | type | +| (return-this).FluentInterface.prototype | | | Member[foo].ReturnValue | type | diff --git a/javascript/ql/test/library-tests/ModelGeneration/return-this/package.json b/javascript/ql/test/library-tests/ModelGeneration/return-this/package.json new file mode 100644 index 000000000000..34b1e65149f1 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/return-this/package.json @@ -0,0 +1,4 @@ +{ + "name": "return-this", + "main": "return-this.js" +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/return-this/return-this.js b/javascript/ql/test/library-tests/ModelGeneration/return-this/return-this.js new file mode 100644 index 000000000000..6ad6004511a1 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/return-this/return-this.js @@ -0,0 +1,17 @@ +export class FluentInterface { + foo() { + return this; + } + bar() { + return this.foo(); + } + baz() { + return this.foo().bar().bar().foo(); + } + notFluent() { + this.foo(); + } + notFluent2() { + return this.notFluent2(); + } +} From 946f0b4dc47104dab06cefffa46ab1850d837d8a Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 11:34:39 +0200 Subject: [PATCH 04/22] JS: Add test for class with aliases --- .../ModelGeneration/ModelGeneration.expected | 8 ++++++++ .../library-tests/ModelGeneration/aliases/aliases.js | 9 +++++++++ .../library-tests/ModelGeneration/aliases/package.json | 4 ++++ 3 files changed, 21 insertions(+) create mode 100644 javascript/ql/test/library-tests/ModelGeneration/aliases/aliases.js create mode 100644 javascript/ql/test/library-tests/ModelGeneration/aliases/package.json diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index 53d49d69d90f..3aa3019fd026 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -1,4 +1,11 @@ typeModel +| (aliases).Alias1 | aliases | Member[Alias1] | +| (aliases).Alias1 | aliases | Member[Alias2] | +| (aliases).Alias1 | aliases | Member[Alias3].Member[x] | +| (aliases).Alias1 | aliases | Member[Alias4].Member[x].Member[x] | +| (aliases).Alias1 | aliases | Member[AliasedClass] | +| (aliases).Alias1.prototype | (aliases).Alias1 | Instance | +| (aliases).Alias1.prototype.foo | (aliases).Alias1.prototype | Member[foo] | | (reexport).func | reexport | Member[func] | | (return-this).FluentInterface | return-this | Member[FluentInterface] | | (return-this).FluentInterface.prototype | (return-this).FluentInterface | Instance | @@ -14,6 +21,7 @@ typeModel | upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] | | upstream-lib.XYZ | reexport | Member[xy].Member[z] | summaryModel +| (aliases).Alias1.prototype | | | Member[foo].ReturnValue | type | | (return-this).FluentInterface.prototype | | | Member[bar].ReturnValue | type | | (return-this).FluentInterface.prototype | | | Member[baz].ReturnValue | type | | (return-this).FluentInterface.prototype | | | Member[foo].ReturnValue | type | diff --git a/javascript/ql/test/library-tests/ModelGeneration/aliases/aliases.js b/javascript/ql/test/library-tests/ModelGeneration/aliases/aliases.js new file mode 100644 index 000000000000..a9874a887446 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/aliases/aliases.js @@ -0,0 +1,9 @@ +export class AliasedClass { + foo() { return this; } +} + +export const Alias1 = AliasedClass; +export const Alias2 = AliasedClass; + +export const Alias3 = { x: AliasedClass }; +export const Alias4 = { x: Alias3 }; diff --git a/javascript/ql/test/library-tests/ModelGeneration/aliases/package.json b/javascript/ql/test/library-tests/ModelGeneration/aliases/package.json new file mode 100644 index 000000000000..2356a50733f8 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/aliases/package.json @@ -0,0 +1,4 @@ +{ + "name": "aliases", + "main": "aliases.js" +} From f4e05cc621c28b46740538e8468463db37ced885 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 11:39:38 +0200 Subject: [PATCH 05/22] JS: Add tests with semi-internal class problem --- .../ModelGeneration/ModelGeneration.expected | 11 ++++++++++ .../semi-internal-class/package.json | 4 ++++ .../semi-internal-class.js | 21 +++++++++++++++++++ 3 files changed, 36 insertions(+) create mode 100644 javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/package.json create mode 100644 javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/semi-internal-class.js diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index 3aa3019fd026..394f7e10600d 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -16,6 +16,17 @@ typeModel | (return-this).FluentInterface.prototype.foo | (return-this).FluentInterface.prototype | Member[foo] | | (return-this).FluentInterface.prototype.notFluent | (return-this).FluentInterface.prototype | Member[notFluent] | | (return-this).FluentInterface.prototype.notFluent2 | (return-this).FluentInterface.prototype | Member[notFluent2] | +| (semi-internal-class).PublicClass | semi-internal-class | Member[PublicClass] | +| (semi-internal-class).PublicClass.prototype | (semi-internal-class).PublicClass | Instance | +| (semi-internal-class).PublicClass.prototype | (semi-internal-class).SemiInternalClass.prototype.method | ReturnValue | +| (semi-internal-class).PublicClass.prototype | (semi-internal-class).getAnonymous~expr2 | ReturnValue | +| (semi-internal-class).PublicClass.prototype.publicMethod | (semi-internal-class).PublicClass.prototype | Member[publicMethod] | +| (semi-internal-class).SemiInternalClass.prototype | (semi-internal-class).get | ReturnValue | +| (semi-internal-class).SemiInternalClass.prototype.method | (semi-internal-class).SemiInternalClass.prototype | Member[method] | +| (semi-internal-class).get | semi-internal-class | Member[get] | +| (semi-internal-class).getAnonymous | semi-internal-class | Member[getAnonymous] | +| (semi-internal-class).getAnonymous~expr1 | (semi-internal-class).getAnonymous | ReturnValue | +| (semi-internal-class).getAnonymous~expr2 | (semi-internal-class).getAnonymous~expr1 | Member[method] | | upstream-lib | (reexport).func | ReturnValue | | upstream-lib | reexport | Member[lib] | | upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] | diff --git a/javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/package.json b/javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/package.json new file mode 100644 index 000000000000..18a689163b5b --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/package.json @@ -0,0 +1,4 @@ +{ + "name": "semi-internal-class", + "main": "semi-internal-class.js" +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/semi-internal-class.js b/javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/semi-internal-class.js new file mode 100644 index 000000000000..962f19330324 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/semi-internal-class/semi-internal-class.js @@ -0,0 +1,21 @@ +export class PublicClass { + publicMethod() {} +} + +class SemiInternalClass { + method() { + return new PublicClass(); + } +} + +export function get() { + return new SemiInternalClass(); +} + +export function getAnonymous() { + return new (class { + method() { + return new PublicClass(); + } + }); +} From ab3c03d2d6458721bd93f378c81df58bef485204 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 11:48:18 +0200 Subject: [PATCH 06/22] JS: Add test where root export object is a function --- .../ModelGeneration/ModelGeneration.expected | 4 ++++ .../ModelGeneration/root-function/package.json | 4 ++++ .../ModelGeneration/root-function/root-function.js | 9 +++++++++ 3 files changed, 17 insertions(+) create mode 100644 javascript/ql/test/library-tests/ModelGeneration/root-function/package.json create mode 100644 javascript/ql/test/library-tests/ModelGeneration/root-function/root-function.js diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index 394f7e10600d..182f3f608b70 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -16,6 +16,10 @@ typeModel | (return-this).FluentInterface.prototype.foo | (return-this).FluentInterface.prototype | Member[foo] | | (return-this).FluentInterface.prototype.notFluent | (return-this).FluentInterface.prototype | Member[notFluent] | | (return-this).FluentInterface.prototype.notFluent2 | (return-this).FluentInterface.prototype | Member[notFluent2] | +| (root-function).PublicClass | root-function | Member[PublicClass] | +| (root-function).PublicClass.prototype | (root-function).PublicClass | Instance | +| (root-function).PublicClass.prototype | root-function | ReturnValue | +| (root-function).PublicClass.prototype.method | (root-function).PublicClass.prototype | Member[method] | | (semi-internal-class).PublicClass | semi-internal-class | Member[PublicClass] | | (semi-internal-class).PublicClass.prototype | (semi-internal-class).PublicClass | Instance | | (semi-internal-class).PublicClass.prototype | (semi-internal-class).SemiInternalClass.prototype.method | ReturnValue | diff --git a/javascript/ql/test/library-tests/ModelGeneration/root-function/package.json b/javascript/ql/test/library-tests/ModelGeneration/root-function/package.json new file mode 100644 index 000000000000..46b6d4e2bda4 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/root-function/package.json @@ -0,0 +1,4 @@ +{ + "name": "root-function", + "main": "root-function.js" +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/root-function/root-function.js b/javascript/ql/test/library-tests/ModelGeneration/root-function/root-function.js new file mode 100644 index 000000000000..0d33f5536814 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/root-function/root-function.js @@ -0,0 +1,9 @@ +class C { + method() {} +} + +module.exports = function() { + return new C(); +} + +module.exports.PublicClass = C; From 3022c59654ca457ac887b3e92a7d7dd5739b8762 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 14:26:14 +0200 Subject: [PATCH 07/22] JS: Add access path alias test --- .../ModelGeneration/ModelGeneration.expected | 3 +++ .../long-access-path/long-access-path.js | 11 +++++++++++ .../ModelGeneration/long-access-path/package.json | 4 ++++ 3 files changed, 18 insertions(+) create mode 100644 javascript/ql/test/library-tests/ModelGeneration/long-access-path/long-access-path.js create mode 100644 javascript/ql/test/library-tests/ModelGeneration/long-access-path/package.json diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index 182f3f608b70..74712e39c340 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -6,6 +6,9 @@ typeModel | (aliases).Alias1 | aliases | Member[AliasedClass] | | (aliases).Alias1.prototype | (aliases).Alias1 | Instance | | (aliases).Alias1.prototype.foo | (aliases).Alias1.prototype | Member[foo] | +| (long-access-path).a.shortcut.d | long-access-path | Member[a].Member[b].Member[c].Member[d] | +| (long-access-path).a.shortcut.d | long-access-path | Member[a].Member[shortcut].Member[d] | +| (long-access-path).a.shortcut.d.e | (long-access-path).a.shortcut.d | Member[e] | | (reexport).func | reexport | Member[func] | | (return-this).FluentInterface | return-this | Member[FluentInterface] | | (return-this).FluentInterface.prototype | (return-this).FluentInterface | Instance | diff --git a/javascript/ql/test/library-tests/ModelGeneration/long-access-path/long-access-path.js b/javascript/ql/test/library-tests/ModelGeneration/long-access-path/long-access-path.js new file mode 100644 index 000000000000..e596ca26ea38 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/long-access-path/long-access-path.js @@ -0,0 +1,11 @@ +export const a = { + b: { + c: { + d: { + e: function() {} + } + } + } +}; + +a.shortcut = a.b.c; diff --git a/javascript/ql/test/library-tests/ModelGeneration/long-access-path/package.json b/javascript/ql/test/library-tests/ModelGeneration/long-access-path/package.json new file mode 100644 index 000000000000..7b45f4d38994 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/long-access-path/package.json @@ -0,0 +1,4 @@ +{ + "name": "long-access-path", + "main": "long-access-path.js" +} From ef7767b6cd698f08e46af862706b89984fb938e6 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 14:26:27 +0200 Subject: [PATCH 08/22] JS: Add partial test for subclassing --- .../ModelGeneration/ModelGeneration.expected | 9 +++++++++ .../ModelGeneration/subclass/package.json | 4 ++++ .../ModelGeneration/subclass/subclass.js | 11 +++++++++++ 3 files changed, 24 insertions(+) create mode 100644 javascript/ql/test/library-tests/ModelGeneration/subclass/package.json create mode 100644 javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index 74712e39c340..b39ed5454368 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -34,6 +34,15 @@ typeModel | (semi-internal-class).getAnonymous | semi-internal-class | Member[getAnonymous] | | (semi-internal-class).getAnonymous~expr1 | (semi-internal-class).getAnonymous | ReturnValue | | (semi-internal-class).getAnonymous~expr2 | (semi-internal-class).getAnonymous~expr1 | Member[method] | +| (subclass).A | subclass | Member[A] | +| (subclass).A.prototype | (subclass).A | Instance | +| (subclass).A.prototype.a | (subclass).A.prototype | Member[a] | +| (subclass).B | subclass | Member[B] | +| (subclass).B.prototype | (subclass).B | Instance | +| (subclass).B.prototype.b | (subclass).B.prototype | Member[b] | +| (subclass).C | subclass | Member[C] | +| (subclass).C.prototype | (subclass).C | Instance | +| (subclass).C.prototype.c | (subclass).C.prototype | Member[c] | | upstream-lib | (reexport).func | ReturnValue | | upstream-lib | reexport | Member[lib] | | upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] | diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/package.json b/javascript/ql/test/library-tests/ModelGeneration/subclass/package.json new file mode 100644 index 000000000000..0764886a0dbd --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/package.json @@ -0,0 +1,4 @@ +{ + "name": "subclass", + "main": "subclass.js" +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js new file mode 100644 index 000000000000..c48e21798fb3 --- /dev/null +++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js @@ -0,0 +1,11 @@ +export class A { + a() {} +} + +export class B { + b() {} +} + +export class C { + c() {} +} From 9313564e64d57220870c01eb29c62a7ffa9f34a1 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 15:05:10 +0200 Subject: [PATCH 09/22] JS: Add subclassing test and fix lack of subclassing handling --- .../frameworks/data/internal/ApiGraphModelsSpecific.qll | 6 ++++++ .../library-tests/ModelGeneration/ModelGeneration.expected | 2 ++ .../test/library-tests/ModelGeneration/subclass/subclass.js | 4 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll index 5b86fba8d487..3c3c3673e2bb 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll @@ -372,4 +372,10 @@ predicate apiGraphHasEdge(API::Node pred, string path, API::Node succ) { ) or succ = pred.getPromised() and path = "Awaited" + or + exists(DataFlow::ClassNode cls | + pred = API::Internal::getClassInstance(cls.getADirectSubClass()) and + succ = API::Internal::getClassInstance(cls) and + path = "" + ) } diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index b39ed5454368..e383335372ed 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -36,9 +36,11 @@ typeModel | (semi-internal-class).getAnonymous~expr2 | (semi-internal-class).getAnonymous~expr1 | Member[method] | | (subclass).A | subclass | Member[A] | | (subclass).A.prototype | (subclass).A | Instance | +| (subclass).A.prototype | (subclass).B.prototype | | | (subclass).A.prototype.a | (subclass).A.prototype | Member[a] | | (subclass).B | subclass | Member[B] | | (subclass).B.prototype | (subclass).B | Instance | +| (subclass).B.prototype | (subclass).C.prototype | | | (subclass).B.prototype.b | (subclass).B.prototype | Member[b] | | (subclass).C | subclass | Member[C] | | (subclass).C.prototype | (subclass).C | Instance | diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js index c48e21798fb3..a219be46ea03 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js +++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js @@ -2,10 +2,10 @@ export class A { a() {} } -export class B { +export class B extends A { b() {} } -export class C { +export class C extends B { c() {} } From f2ea88aa4ce25dbe073b908621601455aae716b3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 15:11:37 +0200 Subject: [PATCH 10/22] JS: Add test showing missing re-export of base class relationship --- .../ModelGeneration/ModelGeneration.expected | 3 +++ .../library-tests/ModelGeneration/ModelGeneration.ext.yml | 1 + .../library-tests/ModelGeneration/subclass/subclass.js | 8 ++++++++ 3 files changed, 12 insertions(+) diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index e383335372ed..3613cce6acc9 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -45,6 +45,9 @@ typeModel | (subclass).C | subclass | Member[C] | | (subclass).C.prototype | (subclass).C | Instance | | (subclass).C.prototype.c | (subclass).C.prototype | Member[c] | +| (subclass).D | subclass | Member[D] | +| (subclass).D.prototype | (subclass).D | Instance | +| (subclass).D.prototype.d | (subclass).D.prototype | Member[d] | | upstream-lib | (reexport).func | ReturnValue | | upstream-lib | reexport | Member[lib] | | upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] | diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml index a608ae5a4939..22113126a9c2 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.ext.yml @@ -4,3 +4,4 @@ extensions: extensible: typeModel data: - ["upstream-lib.XYZ", "upstream-lib", "Member[x].Member[y].Member[z]"] + - ["upstream-lib.Type", "upstream-lib", "Member[Type].Instance"] diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js index a219be46ea03..c119a5c71ad7 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js +++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js @@ -9,3 +9,11 @@ export class B extends A { export class C extends B { c() {} } + +import * as upstream from "upstream-lib"; + +// TODO: needs to emit type model: [upstream.Type; (subclass).D.prototype; ""] +// The getAValueReachableFromSource() logic does not handle the base class -> instance step +export class D extends upstream.Type { + d() {} +} From 56ebe6c7273facd69a229dae9d4ca686ad9f70f3 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 16:01:03 +0200 Subject: [PATCH 11/22] JS: More re-export logic to handle subclass export --- .../data/internal/ApiGraphModelsExport.qll | 4 +-- .../data/internal/ApiGraphModelsSpecific.qll | 25 +++++++++++++++++++ .../ModelGeneration/ModelGeneration.expected | 1 + .../ModelGeneration/subclass/subclass.js | 2 -- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll index f6901128c231..ca4e470fddb5 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll @@ -106,14 +106,14 @@ module TypeGraphExport S, shouldContainTypeSig/1 shoul shouldContainTypeEx(type1) and exists(API::Node node | // A relevant type is exported directly - ModelOutput::getATypeNode(type1).getAValueReachableFromSource() = node.asSink() and + Specific::sourceFlowsToSink(ModelOutput::getATypeNode(type1), node) and ExportedGraph::pathToNode(type2, path, node) or // Something that leads to a relevant type, but didn't finish its access path, is exported exists(string midType, string midPath, string remainingPath, string prefix, API::Node source | Shared::typeModel(type1, midType, midPath) and partiallyEvaluatedModel(midType, midPath, source, remainingPath) and - source.getAValueReachableFromSource() = node.asSink() and + Specific::sourceFlowsToSink(source, node) and ExportedGraph::pathToNode(type2, prefix, node) and path = join(prefix, remainingPath) ) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll index 3c3c3673e2bb..a599af743aeb 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll @@ -379,3 +379,28 @@ predicate apiGraphHasEdge(API::Node pred, string path, API::Node succ) { path = "" ) } + +/** + * Holds if the value of `source` is exposed at `sink`. + */ +bindingset[source] +predicate sourceFlowsToSink(API::Node source, API::Node sink) { + source.getAValueReachableFromSource() = sink.asSink() + or + // Handle the case of an upstream class being the base class of an exposed own class + // + // class Foo extends external.BaseClass {} + // + // Here we want to ensure that `Instance(Foo)` is seen as subtype of `Instance(external.BaseClass)`. + // + // Although we have a dedicated sink node for `Instance(Foo)` we don't have dedicate source node for `Instance(external.BaseClass)`. + // + // However, there is always an `Instance` edge from the base class expression (`external.BaseClass`) + // to the receiver node in subclass constructor (the implicit constructor of `Foo`), which always exists. + // So we use the constructor receiver as the representative for `Instance(external.BaseClass)`. + // (This will get simplified when migrating to Ruby-style API graphs, as both sides will have explicit API nodes). + exists(DataFlow::ClassNode cls | + source.asSource() = cls.getConstructor().getReceiver() and + sink = API::Internal::getClassInstance(cls) + ) +} diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index 3613cce6acc9..c9ce52281ac3 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -50,6 +50,7 @@ typeModel | (subclass).D.prototype.d | (subclass).D.prototype | Member[d] | | upstream-lib | (reexport).func | ReturnValue | | upstream-lib | reexport | Member[lib] | +| upstream-lib.Type | (subclass).D.prototype | | | upstream-lib.XYZ | reexport | Member[x].Member[y].Member[z] | | upstream-lib.XYZ | reexport | Member[xy].Member[z] | summaryModel diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js index c119a5c71ad7..17c975a3d2aa 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js +++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js @@ -12,8 +12,6 @@ export class C extends B { import * as upstream from "upstream-lib"; -// TODO: needs to emit type model: [upstream.Type; (subclass).D.prototype; ""] -// The getAValueReachableFromSource() logic does not handle the base class -> instance step export class D extends upstream.Type { d() {} } From 29a61458e00cb85910179ea433f90c253710baf7 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 16:08:57 +0200 Subject: [PATCH 12/22] JS: Add test case showing problem with chains going through internal classes --- .../ModelGeneration/ModelGeneration.expected | 3 +++ .../library-tests/ModelGeneration/subclass/subclass.js | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index c9ce52281ac3..e3883154c344 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -48,6 +48,9 @@ typeModel | (subclass).D | subclass | Member[D] | | (subclass).D.prototype | (subclass).D | Instance | | (subclass).D.prototype.d | (subclass).D.prototype | Member[d] | +| (subclass).ExposedMidSubClass | subclass | Member[ExposedMidSubClass] | +| (subclass).ExposedMidSubClass.prototype | (subclass).ExposedMidSubClass | Instance | +| (subclass).ExposedMidSubClass.prototype.m | (subclass).ExposedMidSubClass.prototype | Member[m] | | upstream-lib | (reexport).func | ReturnValue | | upstream-lib | reexport | Member[lib] | | upstream-lib.Type | (subclass).D.prototype | | diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js index 17c975a3d2aa..5ebab4f5fb2d 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js +++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js @@ -15,3 +15,11 @@ import * as upstream from "upstream-lib"; export class D extends upstream.Type { d() {} } + +// Test case where subclass chain goes through an internal class +// TODO: we miss the subclass chain between ExposedMidSubClass and A +class InternalMidClass extends A {} + +export class ExposedMidSubClass extends InternalMidClass { + m() {} +} From 81b96a804173627fda8b1315d60a5973f69dce34 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 5 Apr 2024 16:18:47 +0200 Subject: [PATCH 13/22] JS: Ensure MkClassInstance exists for base classes --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 20 +++++++++++-------- .../ModelGeneration/ModelGeneration.expected | 2 ++ .../ModelGeneration/subclass/subclass.js | 1 - 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 27c1632358e6..252e5a4d9f8a 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -696,14 +696,7 @@ module API { or any(Type t).hasUnderlyingType(m, _) } or - MkClassInstance(DataFlow::ClassNode cls) { - hasSemantics(cls) and - ( - cls = trackDefNode(_) - or - cls.getAnInstanceReference() = trackDefNode(_) - ) - } or + MkClassInstance(DataFlow::ClassNode cls) { needsDefNode(cls) } or MkDef(DataFlow::Node nd) { rhs(_, _, nd) } or MkUse(DataFlow::Node nd) { use(_, _, nd) } or /** A use of a TypeScript type. */ @@ -716,6 +709,17 @@ module API { trackUseNode(src, true, bound, "").flowsTo(nd.getCalleeNode()) } + private predicate needsDefNode(DataFlow::ClassNode cls) { + hasSemantics(cls) and + ( + cls = trackDefNode(_) + or + cls.getAnInstanceReference() = trackDefNode(_) + or + needsDefNode(cls.getADirectSubClass()) + ) + } + class TDef = MkModuleDef or TNonModuleDef; class TNonModuleDef = MkModuleExport or MkClassInstance or MkDef or MkSyntheticCallbackArg; diff --git a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected index e3883154c344..e6c93a1a0e8a 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected +++ b/javascript/ql/test/library-tests/ModelGeneration/ModelGeneration.expected @@ -37,6 +37,7 @@ typeModel | (subclass).A | subclass | Member[A] | | (subclass).A.prototype | (subclass).A | Instance | | (subclass).A.prototype | (subclass).B.prototype | | +| (subclass).A.prototype | (subclass).ExposedMidSubClass.prototype~expr1 | | | (subclass).A.prototype.a | (subclass).A.prototype | Member[a] | | (subclass).B | subclass | Member[B] | | (subclass).B.prototype | (subclass).B | Instance | @@ -51,6 +52,7 @@ typeModel | (subclass).ExposedMidSubClass | subclass | Member[ExposedMidSubClass] | | (subclass).ExposedMidSubClass.prototype | (subclass).ExposedMidSubClass | Instance | | (subclass).ExposedMidSubClass.prototype.m | (subclass).ExposedMidSubClass.prototype | Member[m] | +| (subclass).ExposedMidSubClass.prototype~expr1 | (subclass).ExposedMidSubClass.prototype | | | upstream-lib | (reexport).func | ReturnValue | | upstream-lib | reexport | Member[lib] | | upstream-lib.Type | (subclass).D.prototype | | diff --git a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js index 5ebab4f5fb2d..6cbfbabad0e6 100644 --- a/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js +++ b/javascript/ql/test/library-tests/ModelGeneration/subclass/subclass.js @@ -17,7 +17,6 @@ export class D extends upstream.Type { } // Test case where subclass chain goes through an internal class -// TODO: we miss the subclass chain between ExposedMidSubClass and A class InternalMidClass extends A {} export class ExposedMidSubClass extends InternalMidClass { From 8cb80d60141932302c51fa3d0e835e031b1251e8 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 8 Apr 2024 11:36:49 +0200 Subject: [PATCH 14/22] JS: Switch from hasLocationInfo to Location --- .../ql/lib/semmle/javascript/ApiGraphs.qll | 15 ++++++++--- .../frameworks/data/ModelsAsData.qll | 2 +- .../data/internal/ApiGraphModelsExport.qll | 8 +++--- .../data/internal/ApiGraphModelsSpecific.qll | 2 ++ shared/mad/codeql/mad/dynamic/GraphExport.qll | 26 +++++++++++-------- shared/mad/qlpack.yml | 3 ++- 6 files changed, 37 insertions(+), 19 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll index 252e5a4d9f8a..ded107e7b191 100644 --- a/javascript/ql/lib/semmle/javascript/ApiGraphs.qll +++ b/javascript/ql/lib/semmle/javascript/ApiGraphs.qll @@ -501,16 +501,25 @@ module API { } /** + * Gets the location of this API node, if it corresponds to a program element with a source location. + */ + final Location getLocation() { result = this.getInducingNode().getLocation() } + + /** + * DEPRECATED: Use `getLocation().hasLocationInfo()` instead. + * * Holds if this node is located in file `path` between line `startline`, column `startcol`, * and line `endline`, column `endcol`. * * For nodes that do not have a meaningful location, `path` is the empty string and all other * parameters are zero. */ - predicate hasLocationInfo(string path, int startline, int startcol, int endline, int endcol) { - this.getInducingNode().hasLocationInfo(path, startline, startcol, endline, endcol) + deprecated predicate hasLocationInfo( + string path, int startline, int startcol, int endline, int endcol + ) { + this.getLocation().hasLocationInfo(path, startline, startcol, endline, endcol) or - not exists(this.getInducingNode()) and + not exists(this.getLocation()) and path = "" and startline = 0 and startcol = 0 and diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll index 03193d0ee129..a109d2b4eadb 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll @@ -88,7 +88,7 @@ module ModelExport { private import codeql.mad.dynamic.GraphExport private import internal.ApiGraphModelsExport - private module GraphExportConfig implements GraphExportSig { + private module GraphExportConfig implements GraphExportSig { predicate edge = Specific::apiGraphHasEdge/3; predicate shouldContain = S::shouldContain/1; diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll index ca4e470fddb5..477c0a5d2679 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll @@ -65,7 +65,9 @@ signature predicate shouldContainTypeSig(string type); * Notice that the access path `Member[blah].Member[z]` consists of an access path generated from the API * graph, with pieces of the access path from the original type model appended to it. */ -module TypeGraphExport S, shouldContainTypeSig/1 shouldContainType> { +module TypeGraphExport< + GraphExportSig S, shouldContainTypeSig/1 shouldContainType> +{ /** Like `shouldContainType` but includes types that lead to `type` via type models. */ private predicate shouldContainTypeEx(string type) { shouldContainType(type) @@ -76,7 +78,7 @@ module TypeGraphExport S, shouldContainTypeSig/1 shoul ) } - private module Config implements GraphExportSig { + private module Config implements GraphExportSig { import S predicate shouldContain(API::Node node) { @@ -93,7 +95,7 @@ module TypeGraphExport S, shouldContainTypeSig/1 shoul } } - private module ExportedGraph = GraphExport; + private module ExportedGraph = GraphExport; import ExportedGraph diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll index a599af743aeb..8f8f06225eaa 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsSpecific.qll @@ -27,6 +27,8 @@ module API = JS::API; import JS::DataFlow as DataFlow +class Location = JS::Location; + /** * Holds if `rawType` represents the JavaScript type `qualifiedName` from the given NPM `package`. * diff --git a/shared/mad/codeql/mad/dynamic/GraphExport.qll b/shared/mad/codeql/mad/dynamic/GraphExport.qll index 3eeb50d099cf..87f78278d518 100644 --- a/shared/mad/codeql/mad/dynamic/GraphExport.qll +++ b/shared/mad/codeql/mad/dynamic/GraphExport.qll @@ -2,6 +2,8 @@ * Contains predicates for converting an arbitrary graph to a set of `typeModel` rows. */ +private import codeql.util.Location + /** * Concatenates two access paths, separating them by `.` unless one of them is empty. */ @@ -10,21 +12,20 @@ string join(string x, string y) { if x = "" or y = "" then result = x + y else result = x + "." + y } -signature class NodeSig { - /** - * Holds if this node is located in file `path` between line `startline`, column `startcol`, - * and line `endline`, column `endcol`. - */ - predicate hasLocationInfo(string path, int startline, int startcol, int endline, int endcol); +private module WithLocation { + signature class NodeSig { + /** Gets the location of this node if it has one. */ + Location getLocation(); - /** Gets a string representation of this node. */ - string toString(); + /** Gets a string representation of this node. */ + string toString(); + } } /** * Specifies a graph to export in `GraphExport`. */ -signature module GraphExportSig { +signature module GraphExportSig::NodeSig Node> { /** * Holds if an edge `pred -> succ` exist with the access path `path`. */ @@ -81,7 +82,9 @@ signature module GraphExportSig { /** * Module for exporting an arbitrary graph as models-as-data rows. */ -module GraphExport S> { +module GraphExport< + LocationSig Location, WithLocation::NodeSig Node, GraphExportSig S> +{ private import S private Node getAnExposedNode() { @@ -163,9 +166,10 @@ module GraphExport S> { node = rank[k](RelevantNode n, string path, int startline, int startcol, int endline, int endcol | isSyntheticallyNamedNode(n, prefixTypeName) and - n.hasLocationInfo(path, startline, startcol, endline, endcol) + n.getLocation().hasLocationInfo(path, startline, startcol, endline, endcol) | // Use location information for an arbitrary ordering + // TODO: improve support for nodes without a location, currently they can cause FNs n order by path, startline, startcol, endline, endcol ) and result = prefixTypeName + "~expr" + k diff --git a/shared/mad/qlpack.yml b/shared/mad/qlpack.yml index 97c5f74aead8..1bdf6602e3d0 100644 --- a/shared/mad/qlpack.yml +++ b/shared/mad/qlpack.yml @@ -2,5 +2,6 @@ name: codeql/mad version: 0.2.14-dev groups: shared library: true -dependencies: null +dependencies: + codeql/util: ${workspace} warnOnImplicitThis: true From 82101434fd5fd78e171a11099e90ce767ab97797 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 8 Apr 2024 15:02:24 +0200 Subject: [PATCH 15/22] Dynamic: Add hasPrettyName() --- shared/mad/codeql/mad/dynamic/GraphExport.qll | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/shared/mad/codeql/mad/dynamic/GraphExport.qll b/shared/mad/codeql/mad/dynamic/GraphExport.qll index 87f78278d518..adcfcd747edc 100644 --- a/shared/mad/codeql/mad/dynamic/GraphExport.qll +++ b/shared/mad/codeql/mad/dynamic/GraphExport.qll @@ -121,6 +121,12 @@ module GraphExport< exposedEdge(result, path, node) } + private predicate hasPrettyName(RelevantNode node) { + exposedName(node, _, "") + or + suggestedName(node, _) + } + private predicate nodeMustBeNamed(RelevantNode node) { exposedName(node, _, "") or @@ -143,8 +149,7 @@ module GraphExport< prefix ) or - not exposedName(node, _, _) and - not suggestedName(node, _) and + not hasPrettyName(node) and result = getAPrefixTypeName(getAPredecessor(node, _)) } @@ -153,8 +158,7 @@ module GraphExport< */ private predicate isSyntheticallyNamedNode(RelevantNode node, string prefix) { nodeMustBeNamed(node) and - not exposedName(node, _, "") and - not suggestedName(node, _) and + not hasPrettyName(node) and prefix = min(getAPrefixTypeName(node)) } From f5355cfa98cae4e7401d3de03377b1101dba224f Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 9 Apr 2024 14:37:20 +0200 Subject: [PATCH 16/22] Dynamic: Sync ApiGraphModels.qll --- .../semmle/python/frameworks/data/internal/ApiGraphModels.qll | 4 ++-- .../codeql/ruby/frameworks/data/internal/ApiGraphModels.qll | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll index dd4331527519..8dea3d67bd8c 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll @@ -267,7 +267,7 @@ private predicate summaryModel(string type, string path, string input, string ou } /** Holds if a type model exists for the given parameters. */ -private predicate typeModel(string type1, string type2, string path) { +predicate typeModel(string type1, string type2, string path) { exists(string row | typeModel(row) and row.splitAt(";", 0) = type1 and @@ -435,7 +435,7 @@ private API::Node getNodeFromType(string type) { * Gets the API node identified by the first `n` tokens of `path` in the given `(type, path)` tuple. */ pragma[nomagic] -private API::Node getNodeFromPath(string type, AccessPath path, int n) { +API::Node getNodeFromPath(string type, AccessPath path, int n) { isRelevantFullPath(type, path) and ( n = 0 and diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll index dd4331527519..8dea3d67bd8c 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll @@ -267,7 +267,7 @@ private predicate summaryModel(string type, string path, string input, string ou } /** Holds if a type model exists for the given parameters. */ -private predicate typeModel(string type1, string type2, string path) { +predicate typeModel(string type1, string type2, string path) { exists(string row | typeModel(row) and row.splitAt(";", 0) = type1 and @@ -435,7 +435,7 @@ private API::Node getNodeFromType(string type) { * Gets the API node identified by the first `n` tokens of `path` in the given `(type, path)` tuple. */ pragma[nomagic] -private API::Node getNodeFromPath(string type, AccessPath path, int n) { +API::Node getNodeFromPath(string type, AccessPath path, int n) { isRelevantFullPath(type, path) and ( n = 0 and From 15eabb42ef99377c020310358399b990ee0b3f3b Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 12 Apr 2024 11:35:10 +0200 Subject: [PATCH 17/22] JS: Address review comments --- .../frameworks/data/internal/ApiGraphModelsExport.qll | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll index 477c0a5d2679..3b3b08be31f8 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/internal/ApiGraphModelsExport.qll @@ -13,9 +13,13 @@ private import Shared /** * Holds if some proper prefix of `(type, path)` evaluated to `node`, where `remainingPath` * is bound to the suffix of `path` that was not evaluated yet. + * + * See concrete examples in `TypeGraphExport`. */ bindingset[type, path] -predicate partiallyEvaluatedModel(string type, string path, API::Node node, string remainingPath) { +private predicate partiallyEvaluatedModel( + string type, string path, API::Node node, string remainingPath +) { exists(int n, AccessPath accessPath | accessPath = path and getNodeFromPath(type, accessPath, n) = node and From 330229c463a21ca443621363f2d1734de5ea34ae Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 12 Apr 2024 15:00:17 +0200 Subject: [PATCH 18/22] Update javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll Co-authored-by: Rasmus Wriedt Larsen --- .../ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll index a109d2b4eadb..adfd683a4977 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/data/ModelsAsData.qll @@ -69,7 +69,7 @@ signature module ModelExportSig { predicate shouldContain(API::Node node); /** - * Holds if a named must be generated for `node` if it is to be included in the exported graph. + * Holds if `node` must be named if it is part of the exported graph. */ default predicate mustBeNamed(API::Node node) { none() } From 3949ae41234a254339fc607a42f411ada931f539 Mon Sep 17 00:00:00 2001 From: Asger F Date: Fri, 12 Apr 2024 15:00:24 +0200 Subject: [PATCH 19/22] Update shared/mad/codeql/mad/dynamic/GraphExport.qll Co-authored-by: Rasmus Wriedt Larsen --- shared/mad/codeql/mad/dynamic/GraphExport.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/mad/codeql/mad/dynamic/GraphExport.qll b/shared/mad/codeql/mad/dynamic/GraphExport.qll index adcfcd747edc..1a7c7edf4d89 100644 --- a/shared/mad/codeql/mad/dynamic/GraphExport.qll +++ b/shared/mad/codeql/mad/dynamic/GraphExport.qll @@ -49,7 +49,7 @@ signature module GraphExportSig::No predicate suggestedName(Node node, string name); /** - * Holds if `node` must be named if it part of the exported graph. + * Holds if `node` must be named if it is part of the exported graph. */ predicate mustBeNamed(Node node); From 844b29b6371ce77ec4675d4aacd1fb0ba7c23c85 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 16 Apr 2024 20:09:26 +0200 Subject: [PATCH 20/22] Update shared/mad/codeql/mad/dynamic/GraphExport.qll Co-authored-by: Rasmus Wriedt Larsen --- shared/mad/codeql/mad/dynamic/GraphExport.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/mad/codeql/mad/dynamic/GraphExport.qll b/shared/mad/codeql/mad/dynamic/GraphExport.qll index 1a7c7edf4d89..06420a06d658 100644 --- a/shared/mad/codeql/mad/dynamic/GraphExport.qll +++ b/shared/mad/codeql/mad/dynamic/GraphExport.qll @@ -163,7 +163,7 @@ module GraphExport< } /** - * Gets a synthetic type name to generate for `node`. + * Gets a synthetic type name to generate for `node`, if it doesn't have a pretty name. */ private string getSyntheticName(RelevantNode node) { exists(int k, string prefixTypeName | From ee5cb6f3d8f4f3d6baa143d1cb671774167eb3f2 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 16 Apr 2024 20:10:51 +0200 Subject: [PATCH 21/22] Update shared/mad/codeql/mad/dynamic/GraphExport.qll --- shared/mad/codeql/mad/dynamic/GraphExport.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/mad/codeql/mad/dynamic/GraphExport.qll b/shared/mad/codeql/mad/dynamic/GraphExport.qll index 06420a06d658..e28c82f47ab3 100644 --- a/shared/mad/codeql/mad/dynamic/GraphExport.qll +++ b/shared/mad/codeql/mad/dynamic/GraphExport.qll @@ -154,7 +154,7 @@ module GraphExport< } /** - * Holds if a named type exists or will be generated for `node`. + * Holds if a synthetic name must be generated for `node`. */ private predicate isSyntheticallyNamedNode(RelevantNode node, string prefix) { nodeMustBeNamed(node) and From 3335d48154fa8871eadf5ade02f1e7666468ad62 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 16 Apr 2024 20:26:41 +0200 Subject: [PATCH 22/22] Sync files --- .../python/frameworks/data/internal/ApiGraphModels.qll | 10 ---------- .../ruby/frameworks/data/internal/ApiGraphModels.qll | 10 ---------- 2 files changed, 20 deletions(-) diff --git a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll index bdcea5a797a9..b850a4acea04 100644 --- a/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll +++ b/python/ql/lib/semmle/python/frameworks/data/internal/ApiGraphModels.qll @@ -341,18 +341,8 @@ private predicate summaryModel( } /** Holds if a type model exists for the given parameters. */ -<<<<<<< HEAD predicate typeModel(string type1, string type2, string path) { - exists(string row | - typeModel(row) and - row.splitAt(";", 0) = type1 and - row.splitAt(";", 1) = type2 and - row.splitAt(";", 2) = path - ) -======= -private predicate typeModel(string type1, string type2, string path) { any(DeprecationAdapter a).typeModel(type1, type2, path) ->>>>>>> main or Extensions::typeModel(type1, type2, path) } diff --git a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll index bdcea5a797a9..b850a4acea04 100644 --- a/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll +++ b/ruby/ql/lib/codeql/ruby/frameworks/data/internal/ApiGraphModels.qll @@ -341,18 +341,8 @@ private predicate summaryModel( } /** Holds if a type model exists for the given parameters. */ -<<<<<<< HEAD predicate typeModel(string type1, string type2, string path) { - exists(string row | - typeModel(row) and - row.splitAt(";", 0) = type1 and - row.splitAt(";", 1) = type2 and - row.splitAt(";", 2) = path - ) -======= -private predicate typeModel(string type1, string type2, string path) { any(DeprecationAdapter a).typeModel(type1, type2, path) ->>>>>>> main or Extensions::typeModel(type1, type2, path) }