Skip to content

Commit e0d147f

Browse files
author
Alvaro Muñoz
committed
Add On and Event AST nodes
Capture information about trigger events on the new On and Event classes
1 parent 8590a0b commit e0d147f

File tree

3 files changed

+96
-23
lines changed

3 files changed

+96
-23
lines changed

ql/lib/codeql/actions/Ast.qll

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -254,13 +254,13 @@ class Workflow extends AstNode instanceof WorkflowImpl {
254254

255255
Job getJob(string jobId) { result = super.getJob(jobId) }
256256

257-
predicate hasTriggerEvent(string trigger) { super.hasTriggerEvent(trigger) }
258-
259-
string getATriggerEvent() { result = super.getATriggerEvent() }
257+
Event getATriggerEvent() { result = super.getATriggerEvent() }
260258

261259
Permissions getPermissions() { result = super.getPermissions() }
262260

263261
Strategy getStrategy() { result = super.getStrategy() }
262+
263+
On getOn() { result = super.getOn() }
264264
}
265265

266266
class ReusableWorkflow extends Workflow instanceof ReusableWorkflowImpl {
@@ -305,6 +305,20 @@ class Needs extends AstNode instanceof NeedsImpl {
305305
Job getANeededJob() { result = super.getANeededJob() }
306306
}
307307

308+
class On extends AstNode instanceof OnImpl {
309+
Event getAnEvent() { result = super.getAnEvent() }
310+
}
311+
312+
class Event extends AstNode instanceof EventImpl {
313+
string getName() { result = super.getName() }
314+
315+
string getAnActivityType() { result = super.getAnActivityType() }
316+
317+
string getAPropertyValue(string prop) { result = super.getAPropertyValue(prop) }
318+
319+
predicate hasProperty(string prop) { super.hasProperty(prop) }
320+
}
321+
308322
/**
309323
* An Actions job within a workflow.
310324
* See https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobs.
@@ -328,9 +342,7 @@ abstract class Job extends AstNode instanceof JobImpl {
328342

329343
Permissions getPermissions() { result = super.getPermissions() }
330344

331-
predicate hasTriggerEvent(string trigger) { super.hasTriggerEvent(trigger) }
332-
333-
string getATriggerEvent() { result = super.getATriggerEvent() }
345+
Event getATriggerEvent() { result = super.getATriggerEvent() }
334346

335347
Strategy getStrategy() { result = super.getStrategy() }
336348

ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 77 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,16 @@ private newtype TAstNode =
6868
TStrategyNode(YamlMapping n) { exists(YamlMapping m | m.lookup("strategy") = n) } or
6969
TNeedsNode(YamlMappingLikeNode n) { exists(YamlMapping m | m.lookup("needs") = n) } or
7070
TJobNode(YamlMapping n) { exists(YamlMapping w | w.lookup("jobs").(YamlMapping).lookup(_) = n) } or
71+
TOnNode(YamlMappingLikeNode n) { exists(YamlMapping w | w.lookup("on") = n) } or
72+
TEventNode(YamlScalar event, YamlMappingLikeNode n) {
73+
exists(OnImpl o |
74+
o.getNode().(YamlMapping).maps(event, n)
75+
or
76+
o.getNode().(YamlSequence).getAChildNode() = event and event = n
77+
or
78+
o.getNode().(YamlScalar) = n and event = n
79+
)
80+
} or
7181
TStepNode(YamlMapping n) {
7282
exists(YamlMapping m | m.lookup("steps").(YamlSequence).getElementNode(_) = n)
7383
} or
@@ -308,6 +318,9 @@ class WorkflowImpl extends AstNodeImpl, TWorkflowNode {
308318

309319
override YamlMapping getNode() { result = n }
310320

321+
/** Gets the `on` trigger events for this workflow. */
322+
OnImpl getOn() { result.getNode() = n.lookup("on") }
323+
311324
/** Gets the 'global' `env` mapping in this workflow. */
312325
EnvImpl getEnv() { result.getNode() = n.lookup("env") }
313326

@@ -323,15 +336,8 @@ class WorkflowImpl extends AstNodeImpl, TWorkflowNode {
323336
/** Gets the permissions granted to this workflow. */
324337
PermissionsImpl getPermissions() { result.getNode() = n.lookup("permissions") }
325338

326-
/** Workflow is triggered by given trigger event */
327-
predicate hasTriggerEvent(string trigger) {
328-
exists(YamlNode y | y = n.lookup("on").(YamlMappingLikeNode).getNode(trigger))
329-
}
330-
331339
/** Gets the trigger event that starts this workflow. */
332-
string getATriggerEvent() {
333-
exists(YamlNode y | y = n.lookup("on").(YamlMappingLikeNode).getNode(result))
334-
}
340+
EventImpl getATriggerEvent() { this.getOn().getAnEvent() = result }
335341

336342
/** Gets the strategy for this workflow. */
337343
StrategyImpl getStrategy() { result.getNode() = n.lookup("strategy") }
@@ -573,6 +579,66 @@ class NeedsImpl extends AstNodeImpl, TNeedsNode {
573579
}
574580
}
575581

582+
class OnImpl extends AstNodeImpl, TOnNode {
583+
YamlMappingLikeNode n;
584+
585+
OnImpl() { this = TOnNode(n) }
586+
587+
override string toString() { result = n.toString() }
588+
589+
override AstNodeImpl getAChildNode() { result.getNode() = n.getAChildNode*() }
590+
591+
override WorkflowImpl getParentNode() { result.getAChildNode() = this }
592+
593+
override string getAPrimaryQlClass() { result = "OnImpl" }
594+
595+
override Location getLocation() { result = n.getLocation() }
596+
597+
override YamlMappingLikeNode getNode() { result = n }
598+
599+
/** Gets an event that triggers the workflow. */
600+
EventImpl getAnEvent() { result.getParentNode() = this }
601+
}
602+
603+
class EventImpl extends AstNodeImpl, TEventNode {
604+
YamlScalar e;
605+
YamlMappingLikeNode n;
606+
607+
EventImpl() { this = TEventNode(e, n) }
608+
609+
override string toString() { result = e.getValue() }
610+
611+
override AstNodeImpl getAChildNode() { result.getNode() = n.getAChildNode*() }
612+
613+
override OnImpl getParentNode() { result.getAChildNode() = this }
614+
615+
override string getAPrimaryQlClass() { result = "EventImpl" }
616+
617+
override Location getLocation() { result = e.getLocation() }
618+
619+
override YamlScalar getNode() { result = e }
620+
621+
/** Gets the name of the event that triggers the workflow. */
622+
string getName() { result = e.getValue() }
623+
624+
/** Gets the Yaml Node associated with the event if any */
625+
YamlMappingLikeNode getValueNode() { result = n }
626+
627+
/** Gets an activity type */
628+
string getAnActivityType() {
629+
result =
630+
n.(YamlMapping).lookup("types").(YamlMappingLikeNode).getNode(_).(YamlScalar).getValue()
631+
}
632+
633+
/** Gets a string value for any property (eg: branches, branches-ignore, etc.) */
634+
string getAPropertyValue(string prop) {
635+
result = n.(YamlMapping).lookup(prop).(YamlMappingLikeNode).getNode(_).(YamlScalar).getValue()
636+
}
637+
638+
/** Holds if the event has a property with the given name */
639+
predicate hasProperty(string prop) { exists(this.getAPropertyValue(prop)) }
640+
}
641+
576642
class JobImpl extends AstNodeImpl, TJobNode {
577643
YamlMapping n;
578644
string jobId;
@@ -686,7 +752,7 @@ class JobImpl extends AstNodeImpl, TJobNode {
686752
// For workflows that are triggered by the pull_request_target event, the GITHUB_TOKEN is granted read/write repository permission unless the permissions key is specified and the workflow can access secrets, even when it is triggered from a fork.
687753
// The Job is triggered by an event other than `pull_request`
688754
count(this.getATriggerEvent()) = 1 and
689-
not this.getATriggerEvent() = ["pull_request", "workflow_call"]
755+
not this.getATriggerEvent().getName() = ["pull_request", "workflow_call"]
690756
or
691757
// The Workflow is only triggered by `workflow_call` and there is
692758
// a caller workflow triggered by an event other than `pull_request`
@@ -701,16 +767,11 @@ class JobImpl extends AstNodeImpl, TJobNode {
701767
count(this.getATriggerEvent()) > 1
702768
}
703769

704-
/** Workflow is triggered by given trigger event */
705-
predicate hasTriggerEvent(string trigger) {
706-
exists(YamlNode y | y = n.lookup("on").(YamlMappingLikeNode).getNode(trigger))
707-
}
708-
709770
/** Gets the trigger event that starts this workflow. */
710-
string getATriggerEvent() { result = this.getEnclosingWorkflow().getATriggerEvent() }
771+
EventImpl getATriggerEvent() { result = this.getEnclosingWorkflow().getATriggerEvent() }
711772

712773
private predicate hasSingleTrigger(string trigger) {
713-
this.getATriggerEvent() = trigger and
774+
this.getATriggerEvent().getName() = trigger and
714775
count(this.getATriggerEvent()) = 1
715776
}
716777

ql/src/Security/CWE-284/CodeExecutionOnSelfHostedRunner.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import codeql.actions.dataflow.ExternalFlow
2121
*/
2222
predicate staticallyIdentifiedSelfHostedRunner(Job job) {
2323
exists(string label |
24-
job.getEnclosingWorkflow().getATriggerEvent() =
24+
job.getEnclosingWorkflow().getATriggerEvent().getName() =
2525
["pull_request", "pull_request_review", "pull_request_review_comment", "pull_request_target"] and
2626
label = job.getARunsOnLabel() and
2727
// source: https://github.com/boostsecurityio/poutine/blob/main/opa/rego/poutine/utils.rego#L49C3-L49C136

0 commit comments

Comments
 (0)