Skip to content

Commit bcf3081

Browse files
author
Alvaro Muñoz
committed
Refactor Input/Outpts
1 parent 0eabdd9 commit bcf3081

12 files changed

Lines changed: 108 additions & 107 deletions

File tree

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

Lines changed: 73 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ private import codeql.actions.ast.internal.Actions
22
private import codeql.Locations
33

44
/**
5-
* Base class for thejAST tree. Based on YamlNode from the Yaml library.
5+
* Base class for the AST tree. Based on YamlNode from the Yaml library.
66
*/
77
class AstNode instanceof YamlNode {
88
AstNode getParentNode() { result = super.getParentNode() }
@@ -23,7 +23,7 @@ class AstNode instanceof YamlNode {
2323
/**
2424
* Gets a environment variable expression by name in the scope of the current node.
2525
*/
26-
EnvExpr getEnvExpr(string name) {
26+
Expression getEnvExpr(string name) {
2727
exists(Actions::Env env |
2828
env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
2929
|
@@ -42,9 +42,18 @@ class AstNode instanceof YamlNode {
4242
class CompositeAction extends AstNode instanceof Actions::CompositeAction {
4343
Runs getRuns() { result = super.getRuns() }
4444

45-
Inputs getInputs() { result = this.(YamlMapping).lookup("inputs") }
46-
4745
Outputs getOutputs() { result = this.(YamlMapping).lookup("outputs") }
46+
47+
Expression getAnOutputExpr() { result = this.getOutputs().getAnOutputExpr() }
48+
49+
Expression getOutputExpr(string name) { result = this.getOutputs().getOutputExpr(name) }
50+
51+
Input getAnInput() { this.(YamlMapping).lookup("inputs").(YamlMapping).maps(result, _) }
52+
53+
Input getInput(string name) {
54+
this.(YamlMapping).lookup("inputs").(YamlMapping).maps(result, _) and
55+
result.(YamlString).getValue() = name
56+
}
4857
}
4958

5059
class Runs extends AstNode instanceof Actions::Runs {
@@ -81,34 +90,43 @@ class ReusableWorkflow extends Workflow {
8190

8291
ReusableWorkflow() { this.(Actions::Workflow).getOn().getNode("workflow_call") = workflow_call }
8392

84-
Inputs getInputs() { result = workflow_call.(YamlMapping).lookup("inputs") }
85-
8693
Outputs getOutputs() { result = workflow_call.(YamlMapping).lookup("outputs") }
87-
}
8894

89-
class Inputs extends AstNode instanceof YamlMapping {
90-
YamlMapping parent;
95+
Expression getAnOutputExpr() { result = this.getOutputs().getAnOutputExpr() }
9196

92-
Inputs() { parent.lookup("inputs") = this }
97+
Expression getOutputExpr(string name) { result = this.getOutputs().getOutputExpr(name) }
9398

94-
/**
95-
* Gets a specific input expression (YamlMapping) by name.
96-
*/
97-
InputExpr getInputExpr(string name) {
98-
result.(YamlString).getValue() = name and
99-
this.(YamlMapping).maps(result, _)
99+
Input getAnInput() { workflow_call.(YamlMapping).lookup("inputs").(YamlMapping).maps(result, _) }
100+
101+
Input getInput(string name) {
102+
workflow_call.(YamlMapping).lookup("inputs").(YamlMapping).maps(result, _) and
103+
result.(YamlString).getValue() = name
100104
}
101105
}
102106

107+
class Input extends AstNode {
108+
YamlMapping parent;
109+
110+
Input() { parent.lookup("inputs").(YamlMapping).maps(this, _) }
111+
}
112+
103113
class Outputs extends AstNode instanceof YamlMapping {
104114
YamlMapping parent;
105115

106116
Outputs() { parent.lookup("outputs") = this }
107117

108118
/**
109-
* Gets a specific output expression (YamlMapping) by name.
119+
* Gets an output expression.
120+
*/
121+
Expression getAnOutputExpr() {
122+
this.(YamlMapping).lookup(_).(YamlMapping).lookup("value") = result or
123+
this.(YamlMapping).lookup(_) = result
124+
}
125+
126+
/**
127+
* Gets a specific output expression by name.
110128
*/
111-
OutputExpr getOutputExpr(string name) {
129+
Expression getOutputExpr(string name) {
112130
this.(YamlMapping).lookup(name).(YamlMapping).lookup("value") = result or
113131
this.(YamlMapping).lookup(name) = result
114132
}
@@ -130,7 +148,7 @@ class Strategy extends AstNode instanceof YamlMapping {
130148
/**
131149
* Gets a specific matric expression (YamlMapping) by name.
132150
*/
133-
MatrixVariableExpr getMatrixVariableExpr(string name) {
151+
Expression getMatrixVariableExpr(string name) {
134152
this.(YamlMapping).lookup("matrix").(YamlMapping).lookup(name) = result
135153
}
136154

@@ -318,41 +336,40 @@ class Run extends Step {
318336
string getScript() { result = scriptExpr.getValue() }
319337
}
320338

321-
/**
322-
* An AST node associated with a Reusable Workflow input.
323-
*/
324-
class InputExpr extends AstNode {
325-
InputExpr() { exists(Inputs inputs | inputs.(YamlMapping).maps(this, _)) }
326-
}
327-
328-
/**
329-
* An AST node holding an Env var value.
330-
*/
331-
class EnvExpr extends AstNode {
332-
EnvExpr() { exists(Actions::Env env | env.(YamlMapping).lookup(_) = this) }
333-
}
334-
335-
/**
336-
* An AST node holding a job or workflow output var.
337-
*/
338-
class OutputExpr extends AstNode {
339-
OutputExpr() {
340-
exists(Outputs outputs |
341-
outputs.(YamlMapping).lookup(_).(YamlMapping).lookup("value") = this or
342-
outputs.(YamlMapping).lookup(_) = this
343-
)
344-
}
345-
}
346-
347-
/**
348-
* An AST node holding a matrix var.
349-
*/
350-
class MatrixVariableExpr extends AstNode {
351-
MatrixVariableExpr() {
352-
exists(Strategy outputs | outputs.(YamlMapping).lookup("matrix").(YamlMapping).lookup(_) = this)
353-
}
354-
}
355-
339+
// /**
340+
// * An AST node associated with a Reusable Workflow input.
341+
// */
342+
// class InputExpr extends AstNode {
343+
// InputExpr() { exists(Inputs inputs | inputs.(YamlMapping).maps(this, _)) }
344+
// }
345+
//
346+
// /**
347+
// * An AST node holding an Env var value.
348+
// */
349+
// class EnvExpr extends AstNode {
350+
// EnvExpr() { exists(Actions::Env env | env.(YamlMapping).lookup(_) = this) }
351+
// }
352+
//
353+
// /**
354+
// * An AST node holding a job or workflow output var.
355+
// */
356+
// class OutputExpr extends AstNode {
357+
// OutputExpr() {
358+
// exists(Outputs outputs |
359+
// outputs.(YamlMapping).lookup(_).(YamlMapping).lookup("value") = this or
360+
// outputs.(YamlMapping).lookup(_) = this
361+
// )
362+
// }
363+
// }
364+
//
365+
// /**
366+
// * An AST node holding a matrix var.
367+
// */
368+
// class MatrixVariableExpr extends AstNode {
369+
// MatrixVariableExpr() {
370+
// exists(Strategy outputs | outputs.(YamlMapping).lookup("matrix").(YamlMapping).lookup(_) = this)
371+
// }
372+
// }
356373
/**
357374
* Evaluation of a workflow expression ${{}}.
358375
*/
@@ -508,9 +525,9 @@ class InputsExpression extends ContextExpression {
508525
override AstNode getTarget() {
509526
result.getLocation().getFile() = this.getLocation().getFile() and
510527
(
511-
exists(ReusableWorkflow w | w.getInputs().getInputExpr(fieldName) = result)
528+
exists(ReusableWorkflow w | w.getInput(fieldName) = result)
512529
or
513-
exists(CompositeAction a | a.getInputs().getInputExpr(fieldName) = result)
530+
exists(CompositeAction a | a.getInput(fieldName) = result)
514531
)
515532
}
516533
}

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

Lines changed: 12 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ private class CompositeActionTree extends StandardPreOrderTree instanceof Compos
148148
result =
149149
rank[i](AstNode child, Location l |
150150
(
151-
child = this.(CompositeAction).getInputs() or
152-
child = this.(CompositeAction).getOutputs() or
151+
child = this.(CompositeAction).getAnInput() or
152+
child = this.(CompositeAction).getAnOutputExpr() or
153153
child = this.(CompositeAction).getRuns()
154154
) and
155155
l = child.getLocation()
@@ -172,10 +172,10 @@ private class WorkflowTree extends StandardPreOrderTree instanceof Workflow {
172172
result =
173173
rank[i](AstNode child, Location l |
174174
(
175-
child = this.(ReusableWorkflow).getInputs() or
176-
child = this.(ReusableWorkflow).getOutputs() or
177-
child = this.(ReusableWorkflow).getStrategy() or
178-
child = this.(ReusableWorkflow).getAJob()
175+
child = this.(ReusableWorkflow).getAJob() or
176+
child = this.(ReusableWorkflow).getAnInput() or
177+
child = this.(ReusableWorkflow).getAnOutputExpr() or
178+
child = this.(ReusableWorkflow).getStrategy()
179179
) and
180180
l = child.getLocation()
181181
|
@@ -199,19 +199,6 @@ private class WorkflowTree extends StandardPreOrderTree instanceof Workflow {
199199
}
200200
}
201201

202-
private class InputsTree extends StandardPreOrderTree instanceof Inputs {
203-
override ControlFlowTree getChildNode(int i) {
204-
result =
205-
rank[i](AstNode child, Location l |
206-
child = super.getInputExpr(_) and l = child.getLocation()
207-
|
208-
child
209-
order by
210-
l.getStartLine(), l.getStartColumn(), l.getEndColumn(), l.getEndLine(), child.toString()
211-
)
212-
}
213-
}
214-
215202
private class OutputsTree extends StandardPreOrderTree instanceof Outputs {
216203
override ControlFlowTree getChildNode(int i) {
217204
result =
@@ -287,14 +274,13 @@ private class RunTree extends StandardPreOrderTree instanceof Run {
287274

288275
private class UsesLeaf extends LeafTree instanceof Uses { }
289276

290-
private class InputExprTree extends LeafTree instanceof InputExpr { }
291-
292-
private class OutputExprTree extends LeafTree instanceof OutputExpr { }
293-
294-
private class MatrixVariableExprTree extends LeafTree instanceof MatrixVariableExpr { }
295-
296-
private class EnvExprTree extends LeafTree instanceof EnvExpr { }
277+
private class InputTree extends LeafTree instanceof Input { }
297278

279+
// private class OutputExprTree extends LeafTree instanceof OutputExpr { }
280+
//
281+
// private class MatrixVariableExprTree extends LeafTree instanceof MatrixVariableExpr { }
282+
//
283+
// private class EnvExprTree extends LeafTree instanceof EnvExpr { }
298284
private class ExprAccessTree extends LeafTree instanceof ContextExpression { }
299285

300286
private class AstNodeLeaf extends LeafTree instanceof Expression { }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ private class ExternallyDefinedSource extends RemoteFlowSource {
160160
private class CompositeActionInputSource extends RemoteFlowSource {
161161
CompositeAction c;
162162

163-
CompositeActionInputSource() { c.getInputs().getInputExpr(_) = this.asExpr() }
163+
CompositeActionInputSource() { c.getAnInput() = this.asExpr() }
164164

165165
override string getSourceType() { result = "Composite action input" }
166166

‎ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -54,8 +54,13 @@ DataFlowType getNodeType(Node node) { any() }
5454
predicate nodeIsHidden(Node node) { none() }
5555

5656
class DataFlowExpr extends Cfg::Node {
57-
DataFlowExpr() { any() }
58-
//DataFlowExpr() { this.getAstNode() instanceof Expression }
57+
DataFlowExpr() {
58+
this.getAstNode() instanceof Expression or
59+
this.getAstNode() instanceof Uses or
60+
this.getAstNode() instanceof Run or
61+
this.getAstNode() instanceof Outputs or
62+
this.getAstNode() instanceof Input
63+
}
5964
}
6065

6166
/**
@@ -150,7 +155,7 @@ ContentApprox getContentApprox(Content c) { result = c }
150155
* Made a string to match the ArgumentPosition type.
151156
*/
152157
class ParameterPosition extends string {
153-
ParameterPosition() { exists(any(ReusableWorkflow w).getInputs().getInputExpr(this)) }
158+
ParameterPosition() { exists(any(ReusableWorkflow w).getInput(this)) }
154159
}
155160

156161
/**

‎ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll‎

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,22 +48,19 @@ class ExprNode extends Node, TExprNode {
4848
* Reusable workflow input nodes
4949
*/
5050
class ParameterNode extends ExprNode {
51-
private InputExpr input;
51+
private Input input;
5252

53-
ParameterNode() {
54-
this.asExpr() = input and
55-
input = any(Inputs s).getInputExpr(_)
56-
}
53+
ParameterNode() { this.asExpr() = input }
5754

5855
predicate isParameterOf(DataFlowCallable c, ParameterPosition pos) {
59-
input = c.(ReusableWorkflow).getInputs().getInputExpr(pos)
56+
input = c.(ReusableWorkflow).getInput(pos)
6057
}
6158

6259
override string toString() { result = "input " + input.toString() }
6360

6461
override Location getLocation() { result = input.getLocation() }
6562

66-
InputExpr getInputExpr() { result = input }
63+
Input getInput() { result = input }
6764
}
6865

6966
/**

‎ql/src/Security/CWE-020/CompositeActionsSinks.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ private class ExpressionInjectionSink extends DataFlow::Node {
2525

2626
private module MyConfig implements DataFlow::ConfigSig {
2727
predicate isSource(DataFlow::Node source) {
28-
exists(CompositeAction c | c.getInputs().getInputExpr(_) = source.asExpr())
28+
exists(CompositeAction c | c.getAnInput() = source.asExpr())
2929
}
3030

3131
predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionInjectionSink }

‎ql/src/Security/CWE-020/CompositeActionsSources.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ private module MyConfig implements DataFlow::ConfigSig {
2424
}
2525

2626
predicate isSink(DataFlow::Node sink) {
27-
exists(CompositeAction c | c.getOutputs().getOutputExpr(_) = sink.asExpr())
27+
exists(CompositeAction c | c.getAnOutputExpr() = sink.asExpr())
2828
}
2929

3030
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet set) {

‎ql/src/Security/CWE-020/CompositeActionsSummaries.ql‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,11 @@ import codeql.actions.dataflow.ExternalFlow
1818

1919
private module MyConfig implements DataFlow::ConfigSig {
2020
predicate isSource(DataFlow::Node source) {
21-
exists(CompositeAction c | c.getInputs().getInputExpr(_) = source.asExpr())
21+
exists(CompositeAction c | c.getAnInput() = source.asExpr())
2222
}
2323

2424
predicate isSink(DataFlow::Node sink) {
25-
exists(CompositeAction c | c.getOutputs().getOutputExpr(_) = sink.asExpr())
25+
exists(CompositeAction c | c.getAnOutputExpr() = sink.asExpr())
2626
}
2727
}
2828

‎ql/src/Security/CWE-020/ReusableWorkflowsSinks.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ private class ExpressionInjectionSink extends DataFlow::Node {
2525

2626
private module MyConfig implements DataFlow::ConfigSig {
2727
predicate isSource(DataFlow::Node source) {
28-
exists(ReusableWorkflow w | w.getInputs().getInputExpr(_) = source.asExpr())
28+
exists(ReusableWorkflow w | w.getAnInput() = source.asExpr())
2929
}
3030

3131
predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionInjectionSink }

‎ql/src/Security/CWE-020/ReusableWorkflowsSources.ql‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ private module MyConfig implements DataFlow::ConfigSig {
2424
}
2525

2626
predicate isSink(DataFlow::Node sink) {
27-
exists(ReusableWorkflow w | w.getOutputs().getOutputExpr(_) = sink.asExpr())
27+
exists(ReusableWorkflow w | w.getAnOutputExpr() = sink.asExpr())
2828
}
2929

3030
predicate allowImplicitRead(DataFlow::Node node, DataFlow::ContentSet set) {

0 commit comments

Comments
 (0)