Skip to content

Commit 4164096

Browse files
committed
Use the version from package.json in the runner
Update the ql queries to account for change in how we look for runner Previously, we guarded blocks of code to be run by the runner or the action using if statements like this: ```js if (mode === "actions") ... ``` We are no longer doing this. And now, the `unguarded-action-lib.ql` query is out of date. This query checks that runner code does not unintentionally access actions-only methods in the libraries. With these changes, we now ensure that code scanning is happy.
1 parent 4758879 commit 4164096

1 file changed

Lines changed: 85 additions & 21 deletions

File tree

queries/unguarded-action-lib.ql

Lines changed: 85 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -90,17 +90,88 @@ class RunnerEntrypoint extends Function {
9090
}
9191
}
9292

93+
/**
94+
* A generic check to see if we are in actions or runner mode in a particular block of code.
95+
*/
96+
abstract class ActionsGuard extends IfStmt {
97+
98+
/**
99+
* Get a statement block that is only executed on actions
100+
*/
101+
abstract Stmt getActionsBlock();
102+
103+
/**
104+
* Gets an expr that is only executed on actions
105+
*/
106+
final Expr getAnActionsExpr() { getActionsBlock().getAChildStmt*().getAChildExpr*() = result }
107+
108+
}
109+
110+
/**
111+
* A check of whether we are in actions mode or runner mode, based on
112+
* the presense of a call to `isActions()` in the condition of an if statement.
113+
*/
114+
class IsActionsGuard extends ActionsGuard {
115+
IsActionsGuard() {
116+
getCondition().(CallExpr).getCalleeName() = "isActions"
117+
}
118+
119+
/**
120+
* Get the "then" block that is the "actions" path.
121+
*/
122+
override Stmt getActionsBlock() {
123+
result = getThen()
124+
}
125+
}
126+
127+
/**
128+
* A check of whether we are in actions mode or runner mode, based on
129+
* the presense of a call to `!isActions()` in the condition of an if statement.
130+
*/
131+
class NegatedIsActionsGuard extends ActionsGuard {
132+
NegatedIsActionsGuard() {
133+
getCondition().(LogNotExpr).getOperand().(CallExpr).getCalleeName() = "isActions"
134+
}
135+
136+
/**
137+
* Get the "else" block that is the "actions" path.
138+
*/
139+
override Stmt getActionsBlock() {
140+
result = getElse()
141+
}
142+
}
143+
144+
class ModeAccess extends PropAccess {
145+
ModeAccess() {
146+
(
147+
// eg- Mode.actions
148+
getBase().(Identifier).getName() = "Mode" or
149+
// eg- actionUtil.Mode.actions
150+
getBase().(PropAccess).getPropertyName() = "Mode"
151+
) and
152+
(getPropertyName() = "actions" or getPropertyName() = "runner")
153+
}
154+
155+
predicate isActions() {
156+
getPropertyName() = "actions"
157+
}
158+
159+
predicate isRunner() {
160+
getPropertyName() = "runner"
161+
}
162+
}
163+
93164
/**
94165
* A check of whether we are in actions mode or runner mode.
95166
*/
96-
class ModeGuard extends IfStmt {
167+
class ModeGuard extends ActionsGuard {
97168
ModeGuard() {
98-
getCondition().(EqualityTest).getAnOperand().(StringLiteral).getValue() = "actions" or
99-
getCondition().(EqualityTest).getAnOperand().(StringLiteral).getValue() = "runner"
169+
getCondition().(EqualityTest).getAnOperand().(ModeAccess).isActions() or
170+
getCondition().(EqualityTest).getAnOperand().(ModeAccess).isRunner()
100171
}
101172

102-
string getOperand() {
103-
result = getCondition().(EqualityTest).getAnOperand().(StringLiteral).getValue()
173+
ModeAccess getOperand() {
174+
result = getCondition().(EqualityTest).getAnOperand()
104175
}
105176

106177
predicate isPositive() {
@@ -110,21 +181,14 @@ class ModeGuard extends IfStmt {
110181
/**
111182
* Get the then or else block that is the "actions" path.
112183
*/
113-
Stmt getActionsBlock() {
114-
(getOperand() = "actions" and isPositive() and result = getThen())
184+
override Stmt getActionsBlock() {
185+
(getOperand().isActions() and isPositive() and result = getThen())
115186
or
116-
(getOperand() = "runner" and not isPositive() and result = getThen())
187+
(getOperand().isRunner() and not isPositive() and result = getThen())
117188
or
118-
(getOperand() = "actions" and not isPositive() and result = getElse())
189+
(getOperand().isActions() and not isPositive() and result = getElse())
119190
or
120-
(getOperand() = "runner" and isPositive() and result = getElse())
121-
}
122-
123-
/**
124-
* Get an expr that is only executed on actions
125-
*/
126-
Expr getAnActionsExpr() {
127-
getActionsBlock().getAChildStmt*().getAChildExpr*() = result
191+
(getOperand().isRunner() and isPositive() and result = getElse())
128192
}
129193
}
130194

@@ -133,7 +197,7 @@ class ModeGuard extends IfStmt {
133197
* and is not only called on actions.
134198
*/
135199
Expr getAFunctionChildExpr(Function f) {
136-
not exists(ModeGuard guard | guard.getAnActionsExpr() = result) and
200+
not exists(ActionsGuard guard | guard.getAnActionsExpr() = result) and
137201
result.getContainer() = f
138202
}
139203

@@ -145,16 +209,16 @@ Function calledBy(Function f) {
145209
exists(InvokeExpr invokeExpr |
146210
invokeExpr = getAFunctionChildExpr(f) and
147211
invokeExpr.getResolvedCallee() = result and
148-
not exists(ModeGuard guard | guard.getAnActionsExpr() = invokeExpr)
212+
not exists(ActionsGuard guard | guard.getAnActionsExpr() = invokeExpr)
149213
)
150214
or
151215
// Assume outer function causes inner function to be called
152216
(result instanceof Expr and
153217
result.getEnclosingContainer() = f and
154-
not exists(ModeGuard guard | guard.getAnActionsExpr() = result))
218+
not exists(ActionsGuard guard | guard.getAnActionsExpr() = result))
155219
}
156220

157-
from VarAccess v, ActionsLibImport actionsLib, RunnerEntrypoint runnerEntry
221+
from VarAccess v, ActionsLibImport actionsLib, RunnerEntrypoint runnerEntry
158222
where actionsLib.getAProvidedVariable() = v.getVariable()
159223
and getAFunctionChildExpr(calledBy*(runnerEntry)) = v
160224
and not (isSafeActionLibWithActionsEnvVars(actionsLib.getName()) and runnerEntry.setsActionsEnvVars())

0 commit comments

Comments
 (0)