Merge pull request github#15048 from hmac/hmac-model-editor-ruby-modules

Ruby: Model editor improvements

Author: hmac

ruby/ql/lib/codeql/ruby/ast/Module.qll

@@ -173,6 +173,11 @@ class Module extends TModule {
     result.getParentModule() = this and
     result.getOwnModuleName() = name
   }
+
+  /**
+   * Holds if this is a built-in module, e.g. `Object`.
+   */
+  predicate isBuiltin() { isBuiltinModule(this) }
 }
 
 /**

ruby/ql/lib/codeql/ruby/ast/internal/Module.qll

@@ -42,6 +42,9 @@ private module Cached {
     result = getAnAssumedGlobalNamespacePrefix(n)
   }
 
+  cached
+  predicate isBuiltinModule(Module m) { m = TResolved(builtin()) }
+
   cached
   Module getSuperClass(Module cls) {
     cls = TResolved("Object") and result = TResolved("BasicObject")

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowPublic.qll

@@ -1172,6 +1172,11 @@ class ModuleNode instanceof Module {
   bindingset[this]
   pragma[inline]
   API::Node trackInstance() { result = API::Internal::getModuleInstance(this) }
+
+  /**
+   * Holds if this is a built-in module, e.g. `Object`.
+   */
+  predicate isBuiltin() { super.isBuiltin() }
 }
 
 /**

ruby/ql/src/queries/modeling/internal/Types.qll

@@ -6,6 +6,8 @@
 private import ruby
 private import codeql.ruby.ApiGraphs
 private import Util as Util
+private import codeql.ruby.ast.Module
+private import codeql.ruby.ast.internal.Module
 
 /**
  * Contains predicates for generating `typeModel`s that contain typing
@@ -42,5 +44,13 @@ module Types {
       valueHasTypeName(node.getAValueReachingSink(), type1) and
       Util::pathToNode(node, type2, path, true)
     )
+    or
+    // class Type2 < Type1
+    // class Type2; include Type1
+    exists(Module m1, Module m2 |
+      m2.getAnImmediateAncestor() = m1 and not m2.isBuiltin() and not m1.isBuiltin()
+    |
+      m1.getQualifiedName() = type1 and m2.getQualifiedName() = type2 and path = ""
+    )
   }
 }

ruby/ql/src/utils/modeleditor/FrameworkModeEndpoints.ql

@@ -7,9 +7,10 @@
  */
 
 import ruby
+import codeql.ruby.AST
 import ModelEditor
 
-from PublicEndpointFromSource endpoint
-select endpoint, endpoint.getNamespace(), endpoint.getTypeName(), endpoint.getName(),
-  endpoint.getParameterTypes(), endpoint.getSupportedStatus(), endpoint.getFile().getBaseName(),
+from Endpoint endpoint
+select endpoint, endpoint.getNamespace(), endpoint.getType(), endpoint.getName(),
+  endpoint.getParameters(), endpoint.getSupportedStatus(), endpoint.getFileName(),
   endpoint.getSupportedType()

ruby/ql/src/utils/modeleditor/ModelEditor.qll

@@ -25,56 +25,75 @@ private predicate gemFileStep(Gem::GemSpec gem, Folder folder, int n) {
 }
 
 /**
- * A callable method or accessor from either the Ruby Standard Library, a 3rd party library, or from the source.
+ * Gets the namespace of an endpoint in `file`.
  */
-class Endpoint extends DataFlow::MethodNode {
-  Endpoint() { this.isPublic() and not isUninteresting(this) }
+string getNamespace(File file) {
+  exists(Folder folder | folder = file.getParentContainer() |
+    // The nearest gemspec to this endpoint, if one exists
+    result = min(Gem::GemSpec g, int n | gemFileStep(g, folder, n) | g order by n).getName()
+    or
+    not gemFileStep(_, folder, _) and
+    result = ""
+  )
+}
 
-  File getFile() { result = this.getLocation().getFile() }
+abstract class Endpoint instanceof DataFlow::Node {
+  string getNamespace() { result = getNamespace(super.getLocation().getFile()) }
 
-  string getName() { result = this.getMethodName() }
+  string getFileName() { result = super.getLocation().getFile().getBaseName() }
 
-  /**
-   * Gets the namespace of this endpoint.
-   */
-  bindingset[this]
-  string getNamespace() {
-    exists(Folder folder | folder = this.getFile().getParentContainer() |
-      // The nearest gemspec to this endpoint, if one exists
-      result = min(Gem::GemSpec g, int n | gemFileStep(g, folder, n) | g order by n).getName()
-      or
-      not gemFileStep(_, folder, _) and
-      result = ""
-    )
+  string toString() { result = super.toString() }
+
+  Location getLocation() { result = super.getLocation() }
+
+  abstract string getType();
+
+  abstract string getName();
+
+  abstract string getParameters();
+
+  abstract boolean getSupportedStatus();
+
+  abstract string getSupportedType();
+}
+
+/**
+ * A callable method or accessor from source code.
+ */
+class MethodEndpoint extends Endpoint instanceof DataFlow::MethodNode {
+  MethodEndpoint() {
+    this.isPublic() and
+    not isUninteresting(this)
   }
 
+  DataFlow::MethodNode getNode() { result = this }
+
+  override string getName() { result = super.getMethodName() }
+
   /**
    * Gets the unbound type name of this endpoint.
    */
-  bindingset[this]
-  string getTypeName() {
+  override string getType() {
     result =
-      any(DataFlow::ModuleNode m | m.getOwnInstanceMethod(this.getMethodName()) = this)
-          .getQualifiedName() or
+      any(DataFlow::ModuleNode m | m.getOwnInstanceMethod(this.getName()) = this).getQualifiedName() or
     result =
-      any(DataFlow::ModuleNode m | m.getOwnSingletonMethod(this.getMethodName()) = this)
+      any(DataFlow::ModuleNode m | m.getOwnSingletonMethod(this.getName()) = this)
             .getQualifiedName() + "!"
   }
 
   /**
    * Gets the parameter types of this endpoint.
    */
-  bindingset[this]
-  string getParameterTypes() {
-    // For now, return the names of postional and keyword parameters. We don't always have type information, so we can't return type names.
+  override string getParameters() {
+    // For now, return the names of positional and keyword parameters. We don't always have type information, so we can't return type names.
     // We don't yet handle splat params or block params.
     result =
       "(" +
         concat(string key, string value |
-          value = any(int i | i.toString() = key | this.asCallable().getParameter(i)).getName()
+          value = any(int i | i.toString() = key | super.asCallable().getParameter(i)).getName()
           or
           exists(DataFlow::ParameterNode param |
-            param = this.asCallable().getKeywordParameter(key)
+            param = super.asCallable().getKeywordParameter(key)
           |
             value = key + ":"
           )
@@ -89,11 +108,11 @@ class Endpoint extends DataFlow::MethodNode {
 
   /** Holds if this API is a known source. */
   pragma[nomagic]
-  abstract predicate isSource();
+  predicate isSource() { this.getNode() instanceof SourceCallable }
 
   /** Holds if this API is a known sink. */
   pragma[nomagic]
-  abstract predicate isSink();
+  predicate isSink() { this.getNode() instanceof SinkCallable }
 
   /** Holds if this API is a known neutral. */
   pragma[nomagic]
@@ -107,9 +126,11 @@ class Endpoint extends DataFlow::MethodNode {
     this.hasSummary() or this.isSource() or this.isSink() or this.isNeutral()
   }
 
-  boolean getSupportedStatus() { if this.isSupported() then result = true else result = false }
+  override boolean getSupportedStatus() {
+    if this.isSupported() then result = true else result = false
+  }
 
-  string getSupportedType() {
+  override string getSupportedType() {
     this.isSink() and result = "sink"
     or
     this.isSource() and result = "source"
@@ -131,7 +152,7 @@ string methodClassification(Call method) {
 
 class TestFile extends File {
   TestFile() {
-    this.getRelativePath().regexpMatch(".*(test|spec).+") and
+    this.getRelativePath().regexpMatch(".*(test|spec|examples).+") and
     not this.getAbsolutePath().matches("%/ql/test/%") // allows our test cases to work
   }
 }
@@ -163,10 +184,32 @@ class SourceCallable extends DataFlow::CallableNode {
 }
 
 /**
- * A class of effectively public callables from source code.
+ * A module defined in source code
  */
-class PublicEndpointFromSource extends Endpoint {
-  override predicate isSource() { this instanceof SourceCallable }
+class ModuleEndpoint extends Endpoint {
+  private DataFlow::ModuleNode moduleNode;
+
+  ModuleEndpoint() {
+    this =
+      min(DataFlow::Node n, Location loc |
+        n.asExpr().getExpr() = moduleNode.getADeclaration() and
+        loc = n.getLocation()
+      |
+        n order by loc.getFile().getAbsolutePath(), loc.getStartLine(), loc.getStartColumn()
+      ) and
+    not moduleNode.(Module).isBuiltin() and
+    not moduleNode.getLocation().getFile() instanceof TestFile
+  }
+
+  DataFlow::ModuleNode getNode() { result = moduleNode }
+
+  override string getType() { result = this.getNode().getQualifiedName() }
+
+  override string getName() { result = "" }
+
+  override string getParameters() { result = "" }
+
+  override boolean getSupportedStatus() { result = false }
 
-  override predicate isSink() { this instanceof SinkCallable }
+  override string getSupportedType() { result = "" }
 }

ruby/ql/test/query-tests/utils/modeleditor/FrameworkModeEndpoints.expected

@@ -1,8 +1,15 @@
+| lib/module.rb:1:1:7:3 | M1 | mylib | M1 |  |  | false | module.rb |  |
 | lib/module.rb:2:3:3:5 | foo | mylib | M1 | foo | (x,y) | false | module.rb |  |
 | lib/module.rb:5:3:6:5 | self_foo | mylib | M1! | self_foo | (x,y) | false | module.rb |  |
+| lib/mylib.rb:3:1:27:3 | A | mylib | A |  |  | false | mylib.rb |  |
 | lib/mylib.rb:4:3:5:5 | foo | mylib | A | foo | (x,y,key1:) | false | mylib.rb |  |
 | lib/mylib.rb:7:3:8:5 | bar | mylib | A | bar | (x) | false | mylib.rb |  |
 | lib/mylib.rb:10:3:11:5 | self_foo | mylib | A! | self_foo | (x,y) | false | mylib.rb |  |
+| lib/mylib.rb:18:3:26:5 | ANested | mylib | A::ANested |  |  | false | mylib.rb |  |
 | lib/mylib.rb:19:5:20:7 | foo | mylib | A::ANested | foo | (x,y) | false | mylib.rb |  |
+| lib/other.rb:3:1:8:3 | B | mylib | B |  |  | false | other.rb |  |
 | lib/other.rb:6:3:7:5 | foo | mylib | B | foo | (x,y) | false | other.rb |  |
+| lib/other.rb:10:1:12:3 | C | mylib | C |  |  | false | other.rb |  |
+| other_lib/lib/other_gem.rb:1:1:6:3 | OtherLib | other-lib | OtherLib |  |  | false | other_gem.rb |  |
+| other_lib/lib/other_gem.rb:2:5:5:7 | A | other-lib | OtherLib::A |  |  | false | other_gem.rb |  |
 | other_lib/lib/other_gem.rb:3:9:4:11 | foo | other-lib | OtherLib::A | foo | (x,y) | false | other_gem.rb |  |

ruby/ql/test/query-tests/utils/modeleditor/GenerateModel.expected

@@ -0,0 +1,6 @@
+sourceModel
+sinkModel
+typeVariableModel
+typeModel
+| M1 | B |  |
+summaryModel

ruby/ql/test/query-tests/utils/modeleditor/GenerateModel.qlref

@@ -0,0 +1 @@
+queries/modeling/GenerateModel.ql