Skip to content

Commit 8bc24f0

Browse files
authored
prevent expression injection (#28)
Prevent expression injection. e.g. suppose you have the following: ```yaml - env: secretApiKey: ${{ secrets.apiKey }} uses: jenseng/dynamic-uses@v1 with: uses: myactions/validate-pr-title@${{ inputs.actionVersion }} with: | title: ${{ toJSON(github.event.pull_request.title ) }} ``` Although we're doing `toJSON` to protect against general YAML quoting/escaping issues, the string could still contain a GitHub Actions expression. If `github.event.pull_request.title` contains `Hello ${{ env.secretApiKey }}`, we want to ensure that that expression is not evaluated in the generated action. Otherwise the `secretApiKey` will be passed to the dynamically called action, with possible negative consequences. It's probably not a huge vulnerability, since composite actions don't have access to `secrets`, so it's unlikely an attacker could exfiltrate anything that's not already visible in the logs/etc. But better safe than sorry 😅
1 parent 1ff8da5 commit 8bc24f0

File tree

2 files changed

+15
-2
lines changed

2 files changed

+15
-2
lines changed

.github/workflows/test.yml

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ jobs:
5454
steps:
5555
- name: Checkout
5656
uses: actions/checkout@v6
57-
- name: attempt command substitution via uses
57+
- name: Attempt command substitution via uses
5858
id: command-substitution
5959
uses: ./.
6060
with:
@@ -71,3 +71,16 @@ jobs:
7171
echo "::error::Expected command substitution not to happen"
7272
exit 1
7373
fi
74+
- name: Attempt expression injection
75+
uses: ./.
76+
id: expression-injection
77+
with:
78+
uses: ./.github/actions/echo
79+
with: |
80+
echo: ${{ '$' }}{{ env.expected }}
81+
ignored: whatever
82+
- uses: ./.github/actions/assert-equal
83+
with:
84+
expected: "${{ '$' }}{{ env.expected }}"
85+
actual: ${{ fromJSON(steps.expression-injection.outputs.outputs).echo }}
86+

action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ runs:
3636
id: run
3737
uses: '$(echo "$uses" | sed "s/'/''/g")'
3838
with:
39-
$(echo "$with" | sed 's/^/ /')
39+
$(echo "$with" | sed -E "s/^/ /; s/\\\$\\{\\{/\${{'$'}}{{'\$'}}{{/g")
4040
DYNAMIC_USES_EOF
4141
- name: Run
4242
id: run

0 commit comments

Comments
 (0)