Skip to content

Commit 2e616bb

Browse files
owen-mcCopilot
owen-mc
and
Copilot
committed
Actions: model environment expressions at declarations
Attach environment expressions once to their declaring workflow, job, or step so shared CFG nodes do not acquire multiple parents or form cycles. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent c081b4e commit 2e616bb

8 files changed

Lines changed: 59 additions & 180 deletions

File tree

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

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ private import codeql.util.Void
66
private class ActionsAstNode = AstNode;
77

88
module CfgImpl {
9+
private predicate isDeclaredEnvExpr(AstNode parent, AstNode child) {
10+
exists(Workflow workflow | parent = workflow and child = workflow.getEnv().getAnEnvVarExpr())
11+
or
12+
exists(Job job | parent = job and child = job.getEnv().getAnEnvVarExpr())
13+
or
14+
exists(Step step | parent = step and child = step.getEnv().getAnEnvVarExpr())
15+
}
16+
917
private predicate isCfgChild(AstNode parent, AstNode child) {
18+
isDeclaredEnvExpr(parent, child)
19+
or
1020
exists(CompositeAction action |
1121
parent = action and
1222
(child = action.getAnInput() or child = action.getOutputs() or child = action.getRuns())
@@ -43,24 +53,16 @@ module CfgImpl {
4353
parent = job and
4454
(
4555
child = job.getArgumentExpr(_) or
46-
child = job.getInScopeEnvVarExpr(_) or
4756
child = job.getOutputs() or
4857
child = job.getStrategy()
4958
)
5059
)
5160
or
52-
exists(UsesStep uses |
53-
parent = uses and
54-
(child = uses.getArgumentExpr(_) or child = uses.getInScopeEnvVarExpr(_))
55-
)
61+
exists(UsesStep uses | parent = uses and child = uses.getArgumentExpr(_))
5662
or
5763
exists(Run run |
5864
parent = run and
59-
(
60-
child = run.getInScopeEnvVarExpr(_) or
61-
child = run.getAnScriptExpr() or
62-
child = run.getScript()
63-
)
65+
(child = run.getAnScriptExpr() or child = run.getScript())
6466
)
6567
}
6668

‎actions/ql/test/library-tests/cfg-environment/.github/workflows/test.yml‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
on: issues
2+
3+
env:
4+
GLOBAL_VALUE: ${{ github.event.issue.title }}
5+
6+
jobs:
7+
local:
8+
runs-on: ubuntu-latest
9+
env:
10+
JOB_VALUE: ${{ github.event.issue.body }}
11+
steps:
12+
- uses: actions/checkout@v4
13+
- run: echo "$GLOBAL_VALUE $JOB_VALUE"
14+
- run: echo "$GLOBAL_VALUE $JOB_VALUE $STEP_VALUE"
15+
env:
16+
STEP_VALUE: ${{ github.actor }}
17+
18+
external:
19+
uses: octo/example/.github/workflows/reusable.yml@main
20+
with:
21+
value: ${{ github.event.issue.number }}

‎actions/ql/test/library-tests/cfg-environment/test.expected‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
envCfgNodes
2+
| .github/workflows/test.yml:4:18:4:48 | github.event.issue.title |
3+
| .github/workflows/test.yml:10:19:10:48 | github.event.issue.body |
4+
| .github/workflows/test.yml:16:24:16:42 | github.actor |
5+
cfgCycles
6+
cfgDeadEnds
7+
cfgConsistency

‎actions/ql/test/library-tests/cfg-environment/test.ql‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import codeql.actions.Ast
2+
import codeql.actions.Cfg as Cfg
3+
4+
query predicate envCfgNodes(Expression expression) {
5+
expression = any(Env env).getAnEnvVarExpr() and
6+
exists(Cfg::AstCfgNode node | node.getAstNode() = expression)
7+
}
8+
9+
query predicate cfgCycles(Cfg::Node node) { node.getASuccessor+() = node }
10+
11+
query predicate cfgDeadEnds(Cfg::Node node) {
12+
not node instanceof Cfg::ExitNode and
13+
not exists(node.getASuccessor())
14+
}
15+
16+
query predicate cfgConsistency(string query, int results) {
17+
Cfg::Consistency::consistencyOverview(query, results) and
18+
results != 0
19+
}

‎actions/ql/test/query-tests/Security/CWE-078/CONSISTENCY/CfgConsistency.expected‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +0,0 @@
1-
consistencyOverview
2-
| multipleSuccessors | 4 |
3-
multipleSuccessors
4-
| .github/workflows/test1.yml:20:16:20:130 | github.event_name == 'workflow_dispatch' && github.event.inputs.action \|\| github.event.client_payload.action | successor | .github/workflows/test1.yml:23:14:27:13 | if [[ "${ACTION}" != "promote" && "${ACTION}" != "rollback" ]]; then\n echo "Invalid action: ${ACTION}"\n exit 1\nfi\n |
5-
| .github/workflows/test1.yml:20:16:20:130 | github.event_name == 'workflow_dispatch' && github.event.inputs.action \|\| github.event.client_payload.action | successor | .github/workflows/test1.yml:29:9:31:6 | After Uses Step |
6-
| .github/workflows/test1.yml:20:16:20:130 | github.event_name == 'workflow_dispatch' && github.event.inputs.action \|\| github.event.client_payload.action | successor | .github/workflows/test1.yml:37:32:37:66 | secrets.DAGGER_CLOUD_TOKEN_2 |
7-
| .github/workflows/test1.yml:20:16:20:130 | github.event_name == 'workflow_dispatch' && github.event.inputs.action \|\| github.event.client_payload.action | successor | .github/workflows/test1.yml:53:32:53:66 | secrets.DAGGER_CLOUD_TOKEN_2 |

‎actions/ql/test/query-tests/Security/CWE-088/CONSISTENCY/CfgConsistency.expected‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +0,0 @@
1-
consistencyOverview
2-
| multipleSuccessors | 17 |
3-
multipleSuccessors
4-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:15:17:15:57 | github.event.pull_request.head.ref |
5-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:16:14:18:32 | # NOT VULNERABLE\necho "s/FOO/$TITLE/g"\n |
6-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:19:14:21:31 | # VULNERABLE\nsed "s/FOO/$TITLE/g"\n |
7-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:22:14:24:50 | # VULNERABLE\necho "foo" \| sed "s/FOO/$TITLE/g" > bar\n |
8-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:25:14:27:58 | # VULNERABLE\necho $(echo "foo" \| sed "s/FOO/$TITLE/g" > bar)\n |
9-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:28:14:30:31 | # VULNERABLE\nawk "BEGIN {$TITLE}"\n |
10-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:31:14:33:84 | # VULNERABLE\nsed -i "s/git_branch = .*/git_branch = \\"$GITHUB_HEAD_REF\\"/" config.json\n |
11-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:34:14:36:84 | # VULNERABLE\nsed -i "s\|git_branch = .*\|git_branch = \\"$GITHUB_HEAD_REF\\"\|" config.json\n |
12-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:37:14:42:111 | # VULNERABLE\nsed -e 's#<branch_to_sync>#${TITLE}#' \\\n -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n -e 's#<destination_repo>#TITLE#' \\\n .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n |
13-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:43:14:48:111 | # VULNERABLE\nsed -e 's#<branch_to_sync>#TITLE#' \\\n -e 's#<sot_repo>#${{ env.sot_repo }}#' \\\n -e 's#<destination_repo>#${TITLE}#' \\\n .github/workflows/common-copybara.bara.sky.template > .github/workflows/common-copybara.bara.sky\n |
14-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:49:14:52:41 | # VULNERABLE\nBODY=$(git log --format=%s)\nsed "s/FOO/$BODY/g" > /tmp/foo\n |
15-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:53:14:56:41 | # VULNERABLE\nBODY=$(git diff --name-only HEAD)\nsed "s/FOO/$BODY/g" > /tmp/foo\n |
16-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:57:14:60:41 | # VULNERABLE\nBODY=$(git diff --name-only HEAD )\nsed "s/FOO/$BODY/g" > /tmp/foo\n |
17-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:61:14:64:41 | # VULNERABLE\nBODY=$(git diff --name-only HEAD^ \| xargs)\nsed "s/FOO/$BODY/g" > /tmp/foo\n |
18-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:65:14:67:67 | # NOT VULNERABLE\necho "value=$(git log -1 --pretty=%s)" >> $GITHUB_OUTPUT\n |
19-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:68:14:70:33 | # NOT VULNERABLE\ngit log -1 --pretty=%s\n |
20-
| .github/workflows/arg_injection.yml:10:15:10:50 | github.event.pull_request.title | successor | .github/workflows/arg_injection.yml:71:14:74:41 | # NOT VULNERABLE\nBODY=$(git log --format=%s)\nsed -E 's/\\s+/\\n/g' <<<"$BODY"\n |

0 commit comments

Comments
 (0)