Skip to content

Commit f65587e

Browse files
author
Alvaro Muñoz
committed
feat(fieldflow): Refactor flow through Job outputs
Job output should flow to the “key” (YamlString) and be read from there from the JobOutputAccessExpr. - NeedsCtxAccessExpr.getRefExpr should point to the UsesExpr(RW calling Job) or to the OutputsStmt(Regular Job). - JobsCtxAccessExpr.getRefExpr should point to the OutputsStmt(Regular Job). - Create storeStep from OutputExpr to OutputStmt using output var name as the field name. - Create a readStep for CtxAccessExpr to read the referenced fields from the job outputs.
1 parent 90d1ae4 commit f65587e

File tree

1 file changed

+16
-7
lines changed

1 file changed

+16
-7
lines changed

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -133,9 +133,10 @@ predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() }
133133

134134
newtype TContent =
135135
TFieldContent(string name) {
136-
// We only use field flow for steps and jobs outputs, not for accessing other context fields such as jobs, env or inputs
136+
// We only use field flow for steps and jobs outputs, not for accessing other context fields such as env or inputs
137137
name = any(StepsCtxAccessExpr a).getFieldName() or
138-
name = any(NeedsCtxAccessExpr a).getFieldName()
138+
name = any(NeedsCtxAccessExpr a).getFieldName() or
139+
name = any(JobsCtxAccessExpr a).getFieldName()
139140
}
140141

141142
/**
@@ -196,9 +197,8 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos =
196197
* field name.
197198
*/
198199
predicate stepsCtxLocalStep(Node nodeFrom, Node nodeTo) {
199-
exists(StepStmt astFrom, StepsCtxAccessExpr astTo |
200+
exists(UsesExpr astFrom, StepsCtxAccessExpr astTo |
200201
externallyDefinedSource(nodeFrom, _, "output." + astTo.getFieldName()) and
201-
astFrom instanceof UsesExpr and
202202
astFrom = nodeFrom.asExpr() and
203203
astTo = nodeTo.asExpr() and
204204
astTo.getRefExpr() = astFrom
@@ -259,9 +259,16 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { localFlowStep(nodeFr
259259
*/
260260
predicate jumpStep(Node nodeFrom, Node nodeTo) { none() }
261261

262+
/**
263+
* Holds if a CtxAccessExpr reads a field from a job (needs/jobs), step (steps) output via a read of `c` (fieldname)
264+
*/
262265
predicate ctxFieldReadStep(Node node1, Node node2, ContentSet c) {
263266
exists(CtxAccessExpr access |
264-
(access instanceof NeedsCtxAccessExpr or access instanceof StepsCtxAccessExpr) and
267+
(
268+
access instanceof NeedsCtxAccessExpr or
269+
access instanceof StepsCtxAccessExpr or
270+
access instanceof JobsCtxAccessExpr
271+
) and
265272
c = any(FieldContent ct | ct.getName() = access.getFieldName()) and
266273
node1.asExpr() = access.getRefExpr() and
267274
node2.asExpr() = access
@@ -272,12 +279,13 @@ predicate ctxFieldReadStep(Node node1, Node node2, ContentSet c) {
272279
* Holds if data can flow from `node1` to `node2` via a read of `c`. Thus,
273280
* `node1` references an object with a content `c.getAReadContent()` whose
274281
* value ends up in `node2`.
282+
* Store steps without corresponding reads are pruned aggressively very early, since they can never contribute to a complete path.
275283
*/
276284
predicate readStep(Node node1, ContentSet c, Node node2) { ctxFieldReadStep(node1, node2, c) }
277285

278286
/**
279-
* A store step to store an output expression (node1) into its OutputsStm node (node2)
280-
* with a given access path (fieldName)
287+
* Stores an output expression (node1) into its OutputsStm node (node2)
288+
* using the output variable name as the access path
281289
*/
282290
predicate fieldStoreStep(Node node1, Node node2, ContentSet c) {
283291
exists(OutputsStmt out, string fieldName |
@@ -291,6 +299,7 @@ predicate fieldStoreStep(Node node1, Node node2, ContentSet c) {
291299
* Holds if data can flow from `node1` to `node2` via a store into `c`. Thus,
292300
* `node2` references an object with a content `c.getAStoreContent()` that
293301
* contains the value of `node1`.
302+
* Store steps without corresponding reads are pruned aggressively very early, since they can never contribute to a complete path.
294303
*/
295304
predicate storeStep(Node node1, ContentSet c, Node node2) {
296305
fieldStoreStep(node1, node2, c) or

0 commit comments

Comments
 (0)