Skip to content

Commit 96e41bb

Browse files
author
Alvaro Muñoz
authored
Merge pull request #1 from GitHubSecurityLab/reusable_workflows
Add support for Reusable workflows
2 parents db41336 + 3152ed7 commit 96e41bb

File tree

14 files changed

+445
-78
lines changed

14 files changed

+445
-78
lines changed

‎ql/lib/codeql/actions/Ast.qll

+175-8
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,75 @@ class WorkflowStmt extends Statement instanceof Actions::Workflow {
3434
JobStmt getAJob() { result = super.getJob(_) }
3535

3636
JobStmt getJob(string id) { result = super.getJob(id) }
37+
38+
predicate isReusable() { this instanceof ReusableWorkflowStmt }
39+
}
40+
41+
class ReusableWorkflowStmt extends WorkflowStmt {
42+
YamlValue workflow_call;
43+
44+
ReusableWorkflowStmt() {
45+
this.(Actions::Workflow).getOn().getNode("workflow_call") = workflow_call
46+
}
47+
48+
InputsStmt getInputs() { result = workflow_call.(YamlMapping).lookup("inputs") }
49+
50+
OutputsStmt getOutputs() { result = workflow_call.(YamlMapping).lookup("outputs") }
51+
52+
string getName() { result = this.getLocation().getFile().getRelativePath() }
53+
}
54+
55+
class InputsStmt extends Statement instanceof YamlMapping {
56+
InputsStmt() {
57+
exists(Actions::On on | on.getNode("workflow_call").(YamlMapping).lookup("inputs") = this)
58+
}
59+
60+
/**
61+
* Gets a specific parameter expression (YamlMapping) by name.
62+
* eg:
63+
* on:
64+
* workflow_call:
65+
* inputs:
66+
* config-path:
67+
* required: true
68+
* type: string
69+
* secrets:
70+
* token:
71+
* required: true
72+
*/
73+
InputExpr getInputExpr(string name) {
74+
result.(YamlString).getValue() = name and
75+
this.(YamlMapping).maps(result, _)
76+
}
3777
}
3878

79+
class InputExpr extends Expression instanceof YamlString { }
80+
81+
class OutputsStmt extends Statement instanceof YamlMapping {
82+
OutputsStmt() {
83+
exists(Actions::On on | on.getNode("workflow_call").(YamlMapping).lookup("outputs") = this)
84+
}
85+
86+
/**
87+
* Gets a specific parameter expression (YamlMapping) by name.
88+
* eg:
89+
* on:
90+
* workflow_call:
91+
* outputs:
92+
* firstword:
93+
* description: "The first output string"
94+
* value: ${{ jobs.example_job.outputs.output1 }}
95+
* secondword:
96+
* description: "The second output string"
97+
* value: ${{ jobs.example_job.outputs.output2 }}
98+
*/
99+
OutputExpr getOutputExpr(string name) {
100+
this.(YamlMapping).lookup(name).(YamlMapping).lookup("value") = result
101+
}
102+
}
103+
104+
class OutputExpr extends Expression instanceof YamlString { }
105+
39106
/**
40107
* A Job is a collection of steps that run in an execution environment.
41108
*/
@@ -71,6 +138,16 @@ class JobStmt extends Statement instanceof Actions::Job {
71138
* out2: ${steps.foo.baz}
72139
*/
73140
JobOutputStmt getOutputStmt() { result = this.(Actions::Job).lookup("outputs") }
141+
142+
/**
143+
* Reusable workflow jobs may have Uses children
144+
* eg:
145+
* call-job:
146+
* uses: ./.github/workflows/reusable_workflow.yml
147+
* with:
148+
* arg1: value1
149+
*/
150+
JobUsesExpr getUsesExpr() { result.getJob() = this }
74151
}
75152

76153
/**
@@ -104,26 +181,85 @@ class StepStmt extends Statement instanceof Actions::Step {
104181
JobStmt getJob() { result = super.getJob() }
105182
}
106183

184+
/**
185+
* Abstract class representing a call to a 3rd party action or reusable workflow.
186+
*/
187+
abstract class UsesExpr extends Expression {
188+
abstract string getCallee();
189+
190+
abstract string getVersion();
191+
192+
abstract Expression getArgument(string key);
193+
}
194+
107195
/**
108196
* A Uses step represents a call to an action that is defined in a GitHub repository.
109197
*/
110-
class UsesExpr extends StepStmt, Expression {
198+
class StepUsesExpr extends StepStmt, UsesExpr {
111199
Actions::Uses uses;
112200

113-
UsesExpr() { uses.getStep() = this }
201+
StepUsesExpr() { uses.getStep() = this }
114202

115-
string getTarget() { result = uses.getGitHubRepository() }
203+
override string getCallee() { result = uses.getGitHubRepository() }
116204

117-
string getVersion() { result = uses.getVersion() }
205+
override string getVersion() { result = uses.getVersion() }
118206

119-
Expression getArgument(string key) {
207+
override Expression getArgument(string key) {
120208
exists(Actions::With with |
121209
with.getStep() = this and
122210
result = with.lookup(key)
123211
)
124212
}
125213
}
126214

215+
/**
216+
* A Uses step represents a call to an action that is defined in a GitHub repository.
217+
*/
218+
class JobUsesExpr extends UsesExpr instanceof YamlMapping {
219+
JobUsesExpr() {
220+
this instanceof JobStmt and this.maps(any(YamlString s | s.getValue() = "uses"), _)
221+
}
222+
223+
JobStmt getJob() { result = this }
224+
225+
/**
226+
* Gets a regular expression that parses an `owner/repo@version` reference within a `uses` field in an Actions job step.
227+
* local repo: octo-org/this-repo/.github/workflows/workflow-1.yml@172239021f7ba04fe7327647b213799853a9eb89
228+
* local repo: ./.github/workflows/workflow-2.yml
229+
* remote repo: octo-org/another-repo/.github/workflows/workflow.yml@v1
230+
*/
231+
private string repoUsesParser() { result = "([^/]+)/([^/]+)/([^@]+)@(.+)" }
232+
233+
private string pathUsesParser() { result = "\\./(.+)" }
234+
235+
override string getCallee() {
236+
exists(YamlString name |
237+
this.(YamlMapping).lookup("uses") = name and
238+
if name.getValue().matches("./%")
239+
then result = name.getValue().regexpCapture(this.pathUsesParser(), 1)
240+
else
241+
result =
242+
name.getValue().regexpCapture(this.repoUsesParser(), 1) + "/" +
243+
name.getValue().regexpCapture(this.repoUsesParser(), 2) + "/" +
244+
name.getValue().regexpCapture(this.repoUsesParser(), 3)
245+
)
246+
}
247+
248+
/** Gets the version reference used when checking out the Action, e.g. `v2` in `actions/checkout@v2`. */
249+
override string getVersion() {
250+
exists(YamlString name |
251+
this.(YamlMapping).lookup("uses") = name and
252+
if not name.getValue().matches("\\.%")
253+
then result = name.getValue().regexpCapture(this.repoUsesParser(), 4)
254+
else none()
255+
)
256+
}
257+
258+
override Expression getArgument(string key) {
259+
this.(YamlMapping).lookup("with").(YamlMapping).lookup(key) = result
260+
}
261+
}
262+
127263
/**
128264
* A Run step represents the evaluation of a provided script
129265
*/
@@ -183,16 +319,19 @@ class StepOutputAccessExpr extends ExprAccessExpr {
183319
/**
184320
* A ExprAccessExpr where the expression evaluated is a job output read.
185321
* eg: `${{ needs.job1.outputs.foo}}`
322+
* eg: `${{ jobs.job1.outputs.foo}}` (for reusable workflows)
186323
*/
187324
class JobOutputAccessExpr extends ExprAccessExpr {
188325
string jobId;
189326
string varName;
190327

191328
JobOutputAccessExpr() {
192329
jobId =
193-
this.getExpression().regexpCapture("needs\\.([A-Za-z0-9_-]+)\\.outputs\\.[A-Za-z0-9_-]+", 1) and
330+
this.getExpression()
331+
.regexpCapture("(needs|jobs)\\.([A-Za-z0-9_-]+)\\.outputs\\.[A-Za-z0-9_-]+", 2) and
194332
varName =
195-
this.getExpression().regexpCapture("needs\\.[A-Za-z0-9_-]+\\.outputs\\.([A-Za-z0-9_-]+)", 1)
333+
this.getExpression()
334+
.regexpCapture("(needs|jobs)\\.[A-Za-z0-9_-]+\\.outputs\\.([A-Za-z0-9_-]+)", 2)
196335
}
197336

198337
string getVarName() { result = varName }
@@ -201,7 +340,35 @@ class JobOutputAccessExpr extends ExprAccessExpr {
201340
exists(JobStmt job |
202341
job.getId() = jobId and
203342
job.getLocation().getFile() = this.getLocation().getFile() and
204-
job.getOutputStmt().getOutputExpr(varName) = result
343+
(
344+
// A Job can have multiple outputs, so we need to check both
345+
// jobs.<job_id>.outputs.<output_name>
346+
job.getOutputStmt().getOutputExpr(varName) = result
347+
or
348+
// jobs.<job_id>.uses (variables returned from the reusable workflow
349+
job.getUsesExpr() = result
350+
)
351+
)
352+
}
353+
}
354+
355+
/**
356+
* A ExprAccessExpr where the expression evaluated is a reusable workflow input read.
357+
* eg: `${{ inputs.foo}}`
358+
*/
359+
class ReusableWorkflowInputAccessExpr extends ExprAccessExpr {
360+
string paramName;
361+
362+
ReusableWorkflowInputAccessExpr() {
363+
paramName = this.getExpression().regexpCapture("inputs\\.([A-Za-z0-9_-]+)", 1)
364+
}
365+
366+
string getParamName() { result = paramName }
367+
368+
Expression getInputExpr() {
369+
exists(ReusableWorkflowStmt w |
370+
w.getLocation().getFile() = this.getLocation().getFile() and
371+
w.getInputs().getInputExpr(paramName) = result
205372
)
206373
}
207374
}

‎ql/lib/codeql/actions/Consistency.ql

+3
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
import DataFlow::DataFlow::Consistency
2+
3+

‎ql/lib/codeql/actions/DataFlow.qll

+8
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,12 @@ module DataFlow {
77
private import codeql.actions.dataflow.internal.DataFlowImplSpecific
88
import DataFlowMake<ActionsDataFlow>
99
import codeql.actions.dataflow.internal.DataFlowPublic
10+
11+
/** debug */
12+
private import codeql.actions.dataflow.internal.TaintTrackingImplSpecific
13+
import codeql.dataflow.internal.DataFlowImplConsistency as DFIC
14+
module ActionsConsistency implements DFIC::InputSig<ActionsDataFlow> { }
15+
module Consistency {
16+
import DFIC::MakeConsistency<ActionsDataFlow, ActionsTaintTracking, ActionsConsistency>
17+
}
1018
}

‎ql/lib/codeql/actions/controlflow/internal/Cfg.qll

+70-5
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,9 @@ module Completion {
8787
module CfgScope {
8888
abstract class CfgScope extends AstNode { }
8989

90-
private class JobScope extends CfgScope instanceof JobStmt { }
90+
class ReusableWorkflowScope extends CfgScope instanceof ReusableWorkflowStmt { }
91+
92+
class JobScope extends CfgScope instanceof JobStmt { }
9193
}
9294

9395
private module Implementation implements CfgShared::InputSig<Location> {
@@ -120,9 +122,15 @@ private module Implementation implements CfgShared::InputSig<Location> {
120122

121123
int maxSplits() { result = 0 }
122124

123-
predicate scopeFirst(CfgScope scope, AstNode e) { first(scope.(JobStmt), e) }
125+
predicate scopeFirst(CfgScope scope, AstNode e) {
126+
first(scope.(ReusableWorkflowStmt).getInputs(), e) or
127+
first(scope.(JobStmt), e)
128+
}
124129

125-
predicate scopeLast(CfgScope scope, AstNode e, Completion c) { last(scope.(JobStmt), e, c) }
130+
predicate scopeLast(CfgScope scope, AstNode e, Completion c) {
131+
last(scope.(ReusableWorkflowStmt), e, c) or
132+
last(scope.(JobStmt), e, c)
133+
}
126134

127135
predicate successorTypeIsSimple(SuccessorType t) { t instanceof NormalSuccessor }
128136

@@ -139,11 +147,55 @@ private import CfgImpl
139147
private import Completion
140148
private import CfgScope
141149

150+
private class ReusableWorkflowTree extends StandardPreOrderTree instanceof ReusableWorkflowStmt {
151+
override ControlFlowTree getChildNode(int i) {
152+
result =
153+
rank[i](Expression child, Location l |
154+
(child = super.getInputs() or child = super.getOutputs()) and
155+
l = child.getLocation()
156+
|
157+
child
158+
order by
159+
l.getStartLine(), l.getStartColumn(), l.getEndColumn(), l.getEndLine(), child.toString()
160+
)
161+
}
162+
}
163+
164+
private class ReusableWorkflowInputsTree extends StandardPreOrderTree instanceof InputsStmt {
165+
override ControlFlowTree getChildNode(int i) {
166+
result =
167+
rank[i](Expression child, Location l |
168+
child = super.getInputExpr(_) and l = child.getLocation()
169+
|
170+
child
171+
order by
172+
l.getStartLine(), l.getStartColumn(), l.getEndColumn(), l.getEndLine(), child.toString()
173+
)
174+
}
175+
}
176+
177+
private class InputExprTree extends LeafTree instanceof InputExpr { }
178+
179+
private class ReusableWorkflowOutputsTree extends StandardPreOrderTree instanceof OutputsStmt {
180+
override ControlFlowTree getChildNode(int i) {
181+
result =
182+
rank[i](Expression child, Location l |
183+
child = super.getOutputExpr(_) and l = child.getLocation()
184+
|
185+
child
186+
order by
187+
l.getStartLine(), l.getStartColumn(), l.getEndColumn(), l.getEndLine(), child.toString()
188+
)
189+
}
190+
}
191+
192+
private class OutputExprTree extends LeafTree instanceof OutputExpr { }
193+
142194
private class JobTree extends StandardPreOrderTree instanceof JobStmt {
143195
override ControlFlowTree getChildNode(int i) {
144196
result =
145197
rank[i](Expression child, Location l |
146-
(child = super.getAStep() or child = super.getOutputStmt()) and
198+
(child = super.getAStep() or child = super.getOutputStmt() or child = super.getUsesExpr()) and
147199
l = child.getLocation()
148200
|
149201
child
@@ -157,7 +209,20 @@ private class JobOutputTree extends StandardPreOrderTree instanceof JobOutputStm
157209
override ControlFlowTree getChildNode(int i) { result = super.asYamlMapping().getValueNode(i) }
158210
}
159211

160-
private class UsesTree extends StandardPreOrderTree instanceof UsesExpr {
212+
private class StepUsesTree extends StandardPreOrderTree instanceof StepUsesExpr {
213+
override ControlFlowTree getChildNode(int i) {
214+
result =
215+
rank[i](Expression child, Location l |
216+
child = super.getArgument(_) and l = child.getLocation()
217+
|
218+
child
219+
order by
220+
l.getStartLine(), l.getStartColumn(), l.getEndColumn(), l.getEndLine(), child.toString()
221+
)
222+
}
223+
}
224+
225+
private class JobUsesTree extends StandardPreOrderTree instanceof JobUsesExpr {
161226
override ControlFlowTree getChildNode(int i) {
162227
result =
163228
rank[i](Expression child, Location l |

‎ql/lib/codeql/actions/dataflow/FlowSources.qll

+1-1
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,7 @@ private class EventSource extends RemoteFlowSource {
127127
private class ChangedFilesSource extends RemoteFlowSource {
128128
ChangedFilesSource() {
129129
exists(UsesExpr uses |
130-
uses.getTarget() = "tj-actions/changed-files" and
130+
uses.getCallee() = "tj-actions/changed-files" and
131131
uses.getVersion() = ["v10", "v20", "v30", "v40"] and
132132
uses = this.asExpr()
133133
)

‎ql/lib/codeql/actions/dataflow/FlowSteps.qll

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class AdditionalTaintStep extends Unit {
2626
private class ActionsFindAndReplaceStringStep extends AdditionalTaintStep {
2727
override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
2828
exists(UsesExpr u |
29-
u.getTarget() = "mad9000/actions-find-and-replace-string" and
29+
u.getCallee() = "mad9000/actions-find-and-replace-string" and
3030
pred.asExpr() = u.getArgument(["source", "replace"]) and
3131
succ.asExpr() = u
3232
)

0 commit comments

Comments
 (0)