Skip to content

Commit b65dbc5

Browse files
owen-mcCopilot
owen-mc
and
Copilot
committed
Actions: preserve CFG API compatibility
Restore Actions-specific basic-block, scope, node, and traversal APIs on top of the shared CFG implementation, and document the intentional completion API removal. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 49ef164 commit b65dbc5

5 files changed

Lines changed: 316 additions & 2 deletions

File tree

‎actions/ql/lib/change-notes/2026-09-29-shared-cfg.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
category: breaking
3+
---
4+
* The GitHub Actions control flow graph (CFG) now uses the shared CFG library.
5+
The CFG includes explicit before and after nodes and uses the shared entry and
6+
exit node representations. Existing code that relies on specific CFG nodes,
7+
edges, textual representations, or basic block boundaries may need to be
8+
updated. The legacy `Completion`, `NormalCompletion`, `SimpleCompletion`,
9+
`BooleanCompletion`, and `ReturnCompletion` classes have been removed because
10+
completions are no longer part of the Actions CFG API. Code that inspected
11+
completions should inspect CFG edge labels such as `DirectSuccessor`,
12+
`BooleanSuccessor`, and `ReturnSuccessor` instead.

‎actions/ql/lib/codeql/actions/controlflow/BasicBlocks.qll‎

Lines changed: 107 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,111 @@
22

33
private import codeql.actions.Cfg as Cfg
44

5-
class BasicBlock = Cfg::BasicBlock;
5+
/**
6+
* A basic block, that is, a maximal straight-line sequence of control flow nodes
7+
* without branches or joins.
8+
*/
9+
class BasicBlock extends Cfg::BasicBlock {
10+
/** Gets an immediate successor of this basic block, if any. */
11+
BasicBlock getASuccessor() { result = super.getASuccessor() }
612

7-
class EntryBasicBlock = Cfg::EntryBasicBlock;
13+
/** Gets an immediate successor of this basic block of a given type, if any. */
14+
BasicBlock getASuccessor(Cfg::SuccessorType t) { result = super.getASuccessor(t) }
15+
16+
/** Gets an immediate predecessor of this basic block, if any. */
17+
BasicBlock getAPredecessor() { result = super.getAPredecessor() }
18+
19+
/** Gets an immediate predecessor of this basic block of a given type, if any. */
20+
BasicBlock getAPredecessor(Cfg::SuccessorType t) { result = super.getAPredecessor(t) }
21+
22+
/** Gets the control flow node at a specific (zero-indexed) position in this basic block. */
23+
Cfg::Node getNode(int pos) { result = super.getNode(pos) }
24+
25+
/** Gets a control flow node in this basic block. */
26+
Cfg::Node getANode() { result = super.getANode() }
27+
28+
/** Gets the first control flow node in this basic block. */
29+
Cfg::Node getFirstNode() { result = super.getFirstNode() }
30+
31+
/** Gets the last control flow node in this basic block. */
32+
Cfg::Node getLastNode() { result = super.getLastNode() }
33+
34+
predicate immediatelyDominates(BasicBlock bb) { super.immediatelyDominates(bb) }
35+
36+
predicate strictlyDominates(BasicBlock bb) { super.strictlyDominates(bb) }
37+
38+
predicate dominates(BasicBlock bb) { super.dominates(bb) }
39+
40+
predicate inDominanceFrontier(BasicBlock df) { super.inDominanceFrontier(df) }
41+
42+
BasicBlock getImmediateDominator() { result = super.getImmediateDominator() }
43+
44+
predicate strictlyPostDominates(BasicBlock bb) { super.strictlyPostDominates(bb) }
45+
46+
predicate postDominates(BasicBlock bb) { super.postDominates(bb) }
47+
}
48+
49+
/**
50+
* An entry basic block, that is, a basic block whose first node is
51+
* an entry node.
52+
*/
53+
class EntryBasicBlock extends BasicBlock, Cfg::EntryBasicBlock { }
54+
55+
/**
56+
* An annotated exit basic block, that is, a basic block that contains an
57+
* annotated exit node.
58+
*/
59+
class AnnotatedExitBasicBlock extends BasicBlock {
60+
AnnotatedExitBasicBlock() { this.getANode() instanceof Cfg::AnnotatedExitNode }
61+
62+
/** Holds if this block represents a normal exit. */
63+
final predicate isNormal() { this.getANode() instanceof Cfg::NormalExitNode }
64+
}
65+
66+
/**
67+
* An exit basic block, that is, a basic block whose last node is
68+
* an exit node.
69+
*/
70+
class ExitBasicBlock extends BasicBlock {
71+
ExitBasicBlock() { this.getLastNode() instanceof Cfg::ExitNode }
72+
}
73+
74+
/** A basic block with more than one predecessor. */
75+
class JoinBlock extends BasicBlock {
76+
JoinBlock() { strictcount(this.getFirstNode().getAPredecessor()) > 1 }
77+
78+
/**
79+
* Gets the `i`th predecessor of this join block, with respect to some
80+
* arbitrary order.
81+
*/
82+
JoinBlockPredecessor getJoinBlockPredecessor(int i) { none() }
83+
}
84+
85+
/** A basic block that is an immediate predecessor of a join block. */
86+
class JoinBlockPredecessor extends BasicBlock {
87+
JoinBlockPredecessor() { this.getASuccessor() instanceof JoinBlock }
88+
}
89+
90+
/** A basic block that terminates in a condition, splitting the subsequent control flow. */
91+
class ConditionBlock extends BasicBlock {
92+
ConditionBlock() {
93+
exists(this.getLastNode().getASuccessor(any(Cfg::BooleanSuccessor successor)))
94+
}
95+
96+
/**
97+
* Holds if basic block `succ` is immediately controlled by this basic
98+
* block with conditional value `s`.
99+
*/
100+
predicate immediatelyControls(BasicBlock succ, Cfg::BooleanSuccessor s) {
101+
succ = this.getASuccessor(s) and
102+
forall(BasicBlock pred | pred = succ.getAPredecessor() and pred != this | succ.dominates(pred))
103+
}
104+
105+
/**
106+
* Holds if basic block `controlled` is controlled by this basic block with
107+
* conditional value `s`.
108+
*/
109+
predicate controls(BasicBlock controlled, Cfg::BooleanSuccessor s) {
110+
exists(BasicBlock succ | this.immediatelyControls(succ, s) and succ.dominates(controlled))
111+
}
112+
}

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

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,16 @@ module CfgImpl {
7575
)
7676
}
7777

78+
private AstNode getLastCfgAstNode(AstNode node) {
79+
not exists(getCfgChild(node, _)) and result = node
80+
or
81+
exists(AstNode child, int index |
82+
child = getCfgChild(node, index) and
83+
not exists(int later | later > index and exists(getCfgChild(node, later))) and
84+
result = getLastCfgAstNode(child)
85+
)
86+
}
87+
7888
private module CfgAst implements CfgShared::AstSig<Location> {
7989
class AstNode = ActionsAstNode;
8090

@@ -361,7 +371,99 @@ module CfgImpl {
361371

362372
class CfgScope = CfgAst::Callable;
363373

374+
/** A CFG scope for a workflow. */
375+
class WorkflowScope extends CfgScope instanceof Workflow { }
376+
377+
/** A CFG scope for a composite action. */
378+
class CompositeActionScope extends CfgScope instanceof CompositeAction { }
379+
380+
/**
381+
* A control flow node.
382+
*
383+
* Only nodes that can be reached from an entry point are included in the CFG.
384+
*/
364385
class Node extends ControlFlowNode {
386+
/** Gets the CFG scope containing this node. */
365387
CfgScope getScope() { result = this.getEnclosingCallable() }
388+
389+
Node getASuccessor(SuccessorType type) { result = super.getASuccessor(type) }
390+
391+
Node getASuccessor() { result = super.getASuccessor() }
392+
393+
/** Gets an immediate predecessor connected by an edge of type `type`, if any. */
394+
Node getAPredecessor(SuccessorType type) { result.getASuccessor(type) = this }
395+
396+
Node getAPredecessor() { result = super.getAPredecessor() }
397+
398+
/** Holds if this node has a conditional successor. */
399+
predicate isCondition() { exists(this.getASuccessor(any(ConditionalSuccessor successor))) }
400+
401+
/** Holds if this node has more than one predecessor. */
402+
predicate isJoin() { strictcount(this.getAPredecessor()) > 1 }
403+
404+
/** Holds if this node has more than one successor. */
405+
predicate isBranch() { strictcount(this.getASuccessor()) > 1 }
406+
}
407+
408+
/** The control flow node at the entry point of a scope. */
409+
class EntryNode extends Node, ControlFlow::EntryNode { }
410+
411+
/** A control flow node indicating normal or exceptional termination of a scope. */
412+
class AnnotatedExitNode extends Node, ControlFlow::AnnotatedExitNode {
413+
/** Holds if this node represents a normal exit. */
414+
predicate isNormal() { this instanceof NormalExitNode }
366415
}
416+
417+
/** A control flow node indicating normal termination of a scope. */
418+
class NormalExitNode extends AnnotatedExitNode, ControlFlow::NormalExitNode { }
419+
420+
/** A control flow node indicating exceptional termination of a scope. */
421+
class ExceptionalExitNode extends AnnotatedExitNode, ControlFlow::ExceptionalExitNode { }
422+
423+
/** A control flow node indicating the termination of a scope. */
424+
class ExitNode extends Node, ControlFlow::ExitNode { }
425+
426+
/** The empty split type retained for compatibility with the legacy Actions CFG. */
427+
class Split = Void;
428+
429+
/**
430+
* A node that uniquely represents an AST node.
431+
*
432+
* Unreachable AST nodes do not have an `AstCfgNode`.
433+
*/
434+
class AstCfgNode extends Node {
435+
AstCfgNode() { this.injects(_) }
436+
437+
AstNode getAstNode() { this.injects(result) }
438+
439+
/** Gets a comma-separated list of splits in this node, if any. */
440+
string getSplitsString() { none() }
441+
442+
/** Gets a split for this control flow node, if any. */
443+
Split getASplit() { none() }
444+
}
445+
446+
/**
447+
* If needed, call this predicate to force a stage dependency on the cached CFG stage.
448+
*/
449+
cached
450+
predicate forceCachingInSameStage() { CfgCachedStage::ref() }
451+
452+
/** Gets the first AST node executed within `node`. */
453+
cached
454+
AstNode getAControlFlowEntryNode(AstNode node) {
455+
result = node and
456+
exists(Node cfgNode | cfgNode.injects(node))
457+
}
458+
459+
/** Gets a potential last AST node executed within `node`. */
460+
cached
461+
AstNode getAControlFlowExitNode(AstNode node) {
462+
exists(Node cfgNode | cfgNode.injects(node)) and
463+
result = getLastCfgAstNode(node)
464+
}
465+
466+
/** Gets the CFG scope of `node`. */
467+
cached
468+
CfgScope getNodeCfgScope(Node node) { result = node.getScope() }
367469
}

‎actions/ql/test/library-tests/basic/test.expected‎

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1736,6 +1736,59 @@ scopes
17361736
| .github/workflows/poisonable_steps.yml:1:1:46:111 | on: push |
17371737
| .github/workflows/shell.yml:1:1:22:32 | on: push |
17381738
| .github/workflows/test.yml:1:1:40:53 | on: push |
1739+
workflowScopes
1740+
| .github/workflows/commands.yml:1:1:39:30 | on: push |
1741+
| .github/workflows/controlcheck.yml:1:1:16:25 | on: |
1742+
| .github/workflows/expression_nodes.yml:1:1:21:47 | on: issue_comment |
1743+
| .github/workflows/many_strings.yml:1:1:18:1211 | on: |
1744+
| .github/workflows/multiline2.yml:1:1:89:35 | on: |
1745+
| .github/workflows/multiline.yml:1:1:89:29 | on: |
1746+
| .github/workflows/poisonable_steps.yml:1:1:46:111 | on: push |
1747+
| .github/workflows/shell.yml:1:1:22:32 | on: push |
1748+
| .github/workflows/test.yml:1:1:40:53 | on: push |
1749+
compositeActionScopes
1750+
workflowCfgBounds
1751+
| .github/workflows/commands.yml:1:1:39:30 | on: push | 1 | 38 |
1752+
| .github/workflows/controlcheck.yml:1:1:16:25 | on: | 1 | 16 |
1753+
| .github/workflows/expression_nodes.yml:1:1:21:47 | on: issue_comment | 1 | 20 |
1754+
| .github/workflows/many_strings.yml:1:1:18:1211 | on: | 1 | 11 |
1755+
| .github/workflows/multiline2.yml:1:1:89:35 | on: | 1 | 86 |
1756+
| .github/workflows/multiline.yml:1:1:89:29 | on: | 1 | 86 |
1757+
| .github/workflows/poisonable_steps.yml:1:1:46:111 | on: push | 1 | 44 |
1758+
| .github/workflows/shell.yml:1:1:22:32 | on: push | 1 | 22 |
1759+
| .github/workflows/test.yml:1:1:40:53 | on: push | 1 | 40 |
1760+
workflowCfgNodes
1761+
| .github/workflows/commands.yml:1:1:39:30 | on: push |
1762+
| .github/workflows/controlcheck.yml:1:1:16:25 | on: |
1763+
| .github/workflows/expression_nodes.yml:1:1:21:47 | on: issue_comment |
1764+
| .github/workflows/many_strings.yml:1:1:18:1211 | on: |
1765+
| .github/workflows/multiline2.yml:1:1:89:35 | on: |
1766+
| .github/workflows/multiline.yml:1:1:89:29 | on: |
1767+
| .github/workflows/poisonable_steps.yml:1:1:46:111 | on: push |
1768+
| .github/workflows/shell.yml:1:1:22:32 | on: push |
1769+
| .github/workflows/test.yml:1:1:40:53 | on: push |
1770+
entryScopes
1771+
| .github/workflows/commands.yml:1:1:39:30 | Entry | .github/workflows/commands.yml:1:1:39:30 | on: push |
1772+
| .github/workflows/controlcheck.yml:1:1:16:25 | Entry | .github/workflows/controlcheck.yml:1:1:16:25 | on: |
1773+
| .github/workflows/expression_nodes.yml:1:1:21:47 | Entry | .github/workflows/expression_nodes.yml:1:1:21:47 | on: issue_comment |
1774+
| .github/workflows/many_strings.yml:1:1:18:1211 | Entry | .github/workflows/many_strings.yml:1:1:18:1211 | on: |
1775+
| .github/workflows/multiline2.yml:1:1:89:35 | Entry | .github/workflows/multiline2.yml:1:1:89:35 | on: |
1776+
| .github/workflows/multiline.yml:1:1:89:29 | Entry | .github/workflows/multiline.yml:1:1:89:29 | on: |
1777+
| .github/workflows/poisonable_steps.yml:1:1:46:111 | Entry | .github/workflows/poisonable_steps.yml:1:1:46:111 | on: push |
1778+
| .github/workflows/shell.yml:1:1:22:32 | Entry | .github/workflows/shell.yml:1:1:22:32 | on: push |
1779+
| .github/workflows/test.yml:1:1:40:53 | Entry | .github/workflows/test.yml:1:1:40:53 | on: push |
1780+
normalExitNodes
1781+
| .github/workflows/commands.yml:1:1:39:30 | Normal Exit |
1782+
| .github/workflows/controlcheck.yml:1:1:16:25 | Normal Exit |
1783+
| .github/workflows/expression_nodes.yml:1:1:21:47 | Normal Exit |
1784+
| .github/workflows/many_strings.yml:1:1:18:1211 | Normal Exit |
1785+
| .github/workflows/multiline2.yml:1:1:89:35 | Normal Exit |
1786+
| .github/workflows/multiline.yml:1:1:89:29 | Normal Exit |
1787+
| .github/workflows/poisonable_steps.yml:1:1:46:111 | Normal Exit |
1788+
| .github/workflows/shell.yml:1:1:22:32 | Normal Exit |
1789+
| .github/workflows/test.yml:1:1:40:53 | Normal Exit |
1790+
legacyNodeProperties
1791+
legacyCfgSplits
17391792
sources
17401793
| AvraamMavridis/files-changed-action | * | output.CHANGED_FILES | filename | manual |
17411794
| AvraamMavridis/files-changed-action | * | output.CHANGED_FILES_EXTENSIONS | filename | manual |

‎actions/ql/test/library-tests/basic/test.ql‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,48 @@ query predicate nodeLocations(DataFlow::Node n, Location l) { n.getLocation() =
5454

5555
query predicate scopes(Cfg::CfgScope c) { any() }
5656

57+
query predicate workflowScopes(Cfg::WorkflowScope c) { any() }
58+
59+
query predicate compositeActionScopes(Cfg::CompositeActionScope c) { any() }
60+
61+
query predicate workflowCfgBounds(Workflow workflow, int entryLine, int exitLine) {
62+
exists(AstNode entry, AstNode exit |
63+
entry = Cfg::getAControlFlowEntryNode(workflow) and
64+
exit = Cfg::getAControlFlowExitNode(workflow) and
65+
entryLine = entry.getLocation().getStartLine() and
66+
exitLine = exit.getLocation().getStartLine()
67+
)
68+
}
69+
70+
query predicate workflowCfgNodes(Cfg::AstCfgNode node) {
71+
Cfg::forceCachingInSameStage() and
72+
node.getAstNode() instanceof Workflow and
73+
Cfg::getNodeCfgScope(node) = node.getAstNode()
74+
}
75+
76+
query predicate entryScopes(Cfg::EntryNode entry, Cfg::CfgScope scope) { scope = entry.getScope() }
77+
78+
query predicate normalExitNodes(Cfg::AnnotatedExitNode exit) { exit.isNormal() }
79+
80+
query predicate legacyNodeProperties(
81+
Cfg::Node node, Cfg::SuccessorType successorType, string property
82+
) {
83+
node = node.getASuccessor(successorType) and property = "successor"
84+
or
85+
node = node.getAPredecessor(successorType) and property = "predecessor"
86+
or
87+
node.isCondition() and property = "condition"
88+
or
89+
node.isJoin() and property = "join"
90+
or
91+
node.isBranch() and property = "branch"
92+
}
93+
94+
query predicate legacyCfgSplits(Cfg::AstCfgNode node) {
95+
exists(node.getSplitsString()) or
96+
exists(node.getASplit())
97+
}
98+
5799
query predicate sources(string action, string version, string output, string kind, string provenance) {
58100
actionsSourceModel(action, version, output, kind, provenance)
59101
}

0 commit comments

Comments
 (0)