Skip to content

Commit 5e7a29a

Browse files
committed
Ruby: Use API graph subclassing in Rails modelling
Now that API graphs have basic subclassing support, we can simplify some of the ActiveRecord and ActionController code.
1 parent 8419daa commit 5e7a29a

File tree

3 files changed

+20
-48
lines changed

3 files changed

+20
-48
lines changed

ruby/ql/lib/codeql/ruby/frameworks/ActionController.qll

Lines changed: 8 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,22 +4,9 @@ private import codeql.ruby.controlflow.CfgNodes
44
private import codeql.ruby.DataFlow
55
private import codeql.ruby.dataflow.RemoteFlowSources
66
private import codeql.ruby.ast.internal.Module
7+
private import codeql.ruby.ApiGraphs
78
private import ActionView
89

9-
private class ActionControllerBaseAccess extends ConstantReadAccess {
10-
ActionControllerBaseAccess() {
11-
this.getName() = "Base" and
12-
this.getScopeExpr().(ConstantAccess).getName() = "ActionController"
13-
}
14-
}
15-
16-
// ApplicationController extends ActionController::Base, but we
17-
// treat it separately in case the ApplicationController definition
18-
// is not in the database
19-
private class ApplicationControllerAccess extends ConstantReadAccess {
20-
ApplicationControllerAccess() { this.getName() = "ApplicationController" }
21-
}
22-
2310
/**
2411
* A `ClassDeclaration` for a class that extends `ActionController::Base`.
2512
* For example,
@@ -35,16 +22,13 @@ private class ApplicationControllerAccess extends ConstantReadAccess {
3522
*/
3623
class ActionControllerControllerClass extends ClassDeclaration {
3724
ActionControllerControllerClass() {
38-
// class FooController < ActionController::Base
39-
this.getSuperclassExpr() instanceof ActionControllerBaseAccess
40-
or
41-
// class FooController < ApplicationController
42-
this.getSuperclassExpr() instanceof ApplicationControllerAccess
43-
or
44-
// class BarController < FooController
45-
exists(ActionControllerControllerClass other |
46-
other.getModule() = resolveConstantReadAccess(this.getSuperclassExpr())
47-
)
25+
this.getSuperclassExpr() =
26+
[
27+
API::getTopLevelMember("ActionController").getMember("Base"),
28+
// In Rails applications `ApplicationController` typically extends `ActionController::Base`, but we
29+
// treat it separately in case the `ApplicationController` definition is not in the database.
30+
API::getTopLevelMember("ApplicationController")
31+
].getASubclass*().getAUse().asExpr().getExpr()
4832
}
4933

5034
/**

ruby/ql/lib/codeql/ruby/frameworks/ActiveRecord.qll

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,20 +7,6 @@ private import codeql.ruby.ast.internal.Module
77
private import codeql.ruby.ApiGraphs
88
private import codeql.ruby.frameworks.StandardLibrary
99

10-
private class ActiveRecordBaseAccess extends ConstantReadAccess {
11-
ActiveRecordBaseAccess() {
12-
this.getName() = "Base" and
13-
this.getScopeExpr().(ConstantAccess).getName() = "ActiveRecord"
14-
}
15-
}
16-
17-
// ApplicationRecord extends ActiveRecord::Base, but we
18-
// treat it separately in case the ApplicationRecord definition
19-
// is not in the database
20-
private class ApplicationRecordAccess extends ConstantReadAccess {
21-
ApplicationRecordAccess() { this.getName() = "ApplicationRecord" }
22-
}
23-
2410
/// See https://api.rubyonrails.org/classes/ActiveRecord/Persistence.html
2511
private string activeRecordPersistenceInstanceMethodName() {
2612
result =
@@ -41,26 +27,28 @@ private predicate isBuiltInMethodForActiveRecordModelInstance(string methodName)
4127
}
4228

4329
/**
44-
* A `ClassDeclaration` for a class that extends `ActiveRecord::Base`. For example,
30+
* A `ClassDeclaration` for a class that inherits from `ActiveRecord::Base`. For example,
4531
*
4632
* ```rb
4733
* class UserGroup < ActiveRecord::Base
4834
* has_many :users
4935
* end
36+
*
37+
* class SpecialUserGroup < UserGroup
38+
* end
5039
* ```
5140
*/
5241
class ActiveRecordModelClass extends ClassDeclaration {
5342
ActiveRecordModelClass() {
5443
// class Foo < ActiveRecord::Base
55-
this.getSuperclassExpr() instanceof ActiveRecordBaseAccess
56-
or
57-
// class Foo < ApplicationRecord
58-
this.getSuperclassExpr() instanceof ApplicationRecordAccess
59-
or
6044
// class Bar < Foo
61-
exists(ActiveRecordModelClass other |
62-
other.getModule() = resolveConstantReadAccess(this.getSuperclassExpr())
63-
)
45+
this.getSuperclassExpr() =
46+
[
47+
API::getTopLevelMember("ActiveRecord").getMember("Base"),
48+
// In Rails applications `ApplicationRecord` typically extends `ActiveRecord::Base`, but we
49+
// treat it separately in case the `ApplicationRecord` definition is not in the database.
50+
API::getTopLevelMember("ApplicationRecord")
51+
].getASubclass*().getAUse().asExpr().getExpr()
6452
}
6553

6654
// Gets the class declaration for this class and all of its super classes

ruby/ql/test/library-tests/frameworks/ActiveRecordInjection.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -93,4 +93,4 @@ class BazController < BarController
9393
def yet_another_handler
9494
Admin.delete_by(params[:admin_condition])
9595
end
96-
end
96+
end

0 commit comments

Comments
 (0)