Skip to content

Commit d80c541

Browse files
committed
Encapsulate composite actions
1 parent 9406576 commit d80c541

File tree

5 files changed

+31
-7
lines changed

5 files changed

+31
-7
lines changed

javascript/ql/lib/semmle/javascript/Actions.qll

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@ module Actions {
2828
* A custom composite action. This is a mapping at the top level of an Actions YAML action file.
2929
* See https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions.
3030
*/
31-
class Action extends Node, YamlDocument, YamlMapping {
31+
class CompositeAction extends Node, YamlDocument, YamlMapping {
32+
CompositeAction() {
33+
this.getFile().getBaseName() = "action.yml" and
34+
this.lookup("runs").(YamlMapping).lookup("using").(YamlScalar).getValue() = "composite"
35+
}
36+
3237
/** Gets the `runs` mapping. */
3338
Runs getRuns() { result = this.lookup("runs") }
3439
}
@@ -38,12 +43,15 @@ module Actions {
3843
* See https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runs
3944
*/
4045
class Runs extends StepsContainer {
41-
Action action;
46+
CompositeAction action;
4247

4348
Runs() { action.lookup("runs") = this }
4449

4550
/** Gets the action that this `runs` mapping is in. */
46-
Action getAction() { result = action }
51+
CompositeAction getAction() { result = action }
52+
53+
/** Gets the `using` mapping. */
54+
Using getUsing() { result = this.lookup("using") }
4755
}
4856

4957
/**

javascript/ql/src/Security/CWE-094/ExpressionInjection.ql

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,8 @@ predicate isScriptInjectable(Actions::Script script, string injection, string co
149149

150150
from YamlNode node, string injection, string context
151151
where
152-
exists(Actions::Using u, Actions::Runs runs |
153-
u.getValue() = "composite" and
154-
u.getRuns() = runs and
152+
exists(Actions::CompositeAction action, Actions::Runs runs |
153+
action.getRuns() = runs and
155154
(
156155
exists(Actions::Run run |
157156
isRunInjectable(run, injection, context) and

javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,4 +62,4 @@
6262
| .github/workflows/workflow_run.yml:14:12:14:77 | echo '$ ... ame }}' | Potential injection from the ${ github.event.workflow_run.head_commit.committer.name }, which may be controlled by an external user. |
6363
| .github/workflows/workflow_run.yml:15:12:15:62 | echo '$ ... nch }}' | Potential injection from the ${ github.event.workflow_run.head_branch }, which may be controlled by an external user. |
6464
| .github/workflows/workflow_run.yml:16:12:16:78 | echo '$ ... ion }}' | Potential injection from the ${ github.event.workflow_run.head_repository.description }, which may be controlled by an external user. |
65-
| action.yml:14:12:14:50 | echo '$ ... ody }}' | Potential injection from the ${ github.event.comment.body }, which may be controlled by an external user. |
65+
| action1/action.yml:14:12:14:50 | echo '$ ... ody }}' | Potential injection from the ${ github.event.comment.body }, which may be controlled by an external user. |
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
name: 'Hello World'
2+
description: 'Greet someone and record the time'
3+
inputs:
4+
who-to-greet: # id of input
5+
description: 'Who to greet'
6+
required: true
7+
default: 'World'
8+
outputs:
9+
time: # id of output
10+
description: 'The time we greeted you'
11+
runs:
12+
using: 'docker'
13+
steps: # this is actually invalid, used to test we correctly identify composite actions
14+
- run: echo '${{ github.event.comment.body }}'
15+
image: 'Dockerfile'
16+
args:
17+
- ${{ inputs.who-to-greet }}

0 commit comments

Comments
 (0)