Skip to content

Commit 712c599

Browse files
authored
fix: remediate latest zizmor findings, fix supplying zizmor config (#1101)
The current version of Zizmor finds some potential template injection issues. We can fix these by indirecting via the `env`, or in a couple of cases by adding ignore comments where we can't really fix the issue. Something happened to break our config discovery. What we do is download a default config file from this repo, write it to a temporary file, set that file's path as `ZIZMOR_CONFIG`, and then pass this as `--config` if it's set. Possibly as the result of a version bump, Zizmor started handling `ZIZMOR_CONFIG` differently. An empty string here is treated as a file to search for, which doesn't work, and so we get errors. A fix for this last one is to use a different variable name that doesn't collide with the one Zizmor itself is using. Rename to `ZIZMOR_CONFIG_PATH` accordingly.
1 parent c6e5e2e commit 712c599

File tree

9 files changed

+76
-31
lines changed

9 files changed

+76
-31
lines changed

.github/workflows/reusable-zizmor.yml

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -355,9 +355,9 @@ jobs:
355355
echo "always-use-default-config is set. Using default config."
356356
fi
357357
358-
ZIZMOR_CONFIG="${{ runner.temp }}/zizmor.yml"
358+
ZIZMOR_CONFIG_PATH="${{ runner.temp }}/zizmor.yml"
359359
if [ -n "${DEFAULT_ZIZMOR_CONFIG_DOWNLOADED}" ]; then
360-
echo "zizmor-config=${ZIZMOR_CONFIG}" | tee -a "${GITHUB_OUTPUT}"
360+
echo "zizmor-config=${ZIZMOR_CONFIG_PATH}" | tee -a "${GITHUB_OUTPUT}"
361361
fi
362362
363363
- name: Setup UV
@@ -376,7 +376,7 @@ jobs:
376376

377377
- name: Run zizmor
378378
env:
379-
ZIZMOR_CONFIG: ${{ steps.setup-config.outputs.zizmor-config }}
379+
ZIZMOR_CONFIG_PATH: ${{ steps.setup-config.outputs.zizmor-config }}
380380
ZIZMOR_CACHE_DIR: ${{ runner.temp }}/.cache/zizmor
381381
shell: sh
382382
run: >-
@@ -385,7 +385,7 @@ jobs:
385385
--min-severity "${MIN_SEVERITY}"
386386
--min-confidence "${MIN_CONFIDENCE}"
387387
--cache-dir "${ZIZMOR_CACHE_DIR}"
388-
${ZIZMOR_CONFIG:+--config "${ZIZMOR_CONFIG}"}
388+
${ZIZMOR_CONFIG_PATH:+--config "${ZIZMOR_CONFIG_PATH}"}
389389
${RUNNER_DEBUG:+"--verbose"}
390390
${ZIZMOR_EXTRA_ARGS:+${ZIZMOR_EXTRA_ARGS}}
391391
.
@@ -410,7 +410,7 @@ jobs:
410410
id: zizmor-plain
411411
shell: bash
412412
env:
413-
ZIZMOR_CONFIG: ${{ steps.setup-config.outputs.zizmor-config }}
413+
ZIZMOR_CONFIG_PATH: ${{ steps.setup-config.outputs.zizmor-config }}
414414
ZIZMOR_CACHE_DIR: ${{ runner.temp }}/.cache/zizmor
415415
run: |
416416
set -o pipefail
@@ -425,7 +425,7 @@ jobs:
425425
--min-confidence "${MIN_CONFIDENCE}" \
426426
--cache-dir "${ZIZMOR_CACHE_DIR}" \
427427
${RUNNER_DEBUG:+"--verbose"} \
428-
${ZIZMOR_CONFIG:+--config "${ZIZMOR_CONFIG}"} \
428+
${ZIZMOR_CONFIG_PATH:+--config "${ZIZMOR_CONFIG_PATH}"} \
429429
${ZIZMOR_EXTRA_ARGS:+${ZIZMOR_EXTRA_ARGS}} \
430430
. \
431431
| tee -a "${GITHUB_OUTPUT}"
@@ -443,9 +443,9 @@ jobs:
443443
444444
- name: Remove zizmor config
445445
env:
446-
ZIZMOR_CONFIG: ${{ steps.setup-config.outputs.zizmor-config }}
446+
ZIZMOR_CONFIG_PATH: ${{ steps.setup-config.outputs.zizmor-config }}
447447
if: steps.setup-config.outputs.zizmor-config
448-
run: rm "${ZIZMOR_CONFIG}"
448+
run: rm "${ZIZMOR_CONFIG_PATH}"
449449

450450
- name: Hide any previous comments
451451
if: ${{ !cancelled() && github.event.pull_request.head.repo.full_name == github.repository }}

.github/workflows/test-get-vault-secrets.yaml

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,12 @@ jobs:
6767
- name: Check secret value is ${{ matrix.instance }}
6868
if: matrix.instance != 'invalid'
6969
run: |
70-
if [[ "${{ env.INSTANCE }}" != "${{ matrix.instance }}" ]]; then
70+
if [[ "${INSTANCE}" != "${{ matrix.instance }}" ]]; then
7171
echo "Test failed: secret value does not match vault_instance input"
7272
exit 1
7373
fi
74+
env:
75+
INSTANCE: ${{ env.INSTANCE }}
7476

7577
- name: Ensure 'invalid' errored
7678
if: matrix.instance == 'invalid' && steps.test-vault-action.outcome != 'failure'

.github/workflows/test-login-to-gar.yaml

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,19 @@ on:
2020

2121
merge_group:
2222

23-
permissions:
24-
contents: read
25-
id-token: write
23+
permissions: {}
2624

2725
jobs:
2826
test:
27+
permissions:
28+
contents: read
29+
id-token: write
30+
2931
runs-on: ubuntu-latest
32+
3033
# Don't run for forks - they don't have access to secrets
3134
if: github.event.pull_request.head.repo.full_name == github.repository
35+
3236
steps:
3337
- name: Harden the runner (Audit all outbound calls)
3438
uses: step-security/harden-runner@6c439dc8bdf85cadbbce9ed30d1c7b959517bc49 # v2.12.2

actions/build-push-to-dockerhub/README.md

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,13 @@
33
> [!NOTE]
44
> If you are at Grafana Labs:
55
>
6-
> - A docker mirror is available on our self-hosted runners, see [the internal documentation](https://enghub.grafana-ops.net/docs/default/component/deployment-tools/platform/continuous-integration/#docker-caching-in-github-actions) for more info.
6+
> - A docker mirror is available on our self-hosted runners, see [the internal
7+
> documentation](https://enghub.grafana-ops.net/docs/default/component/deployment-tools/platform/continuous-integration/#docker-caching-in-github-actions)
8+
> for more info.
79
8-
This is a composite GitHub Action, used to build Docker images and push them to DockerHub.
9-
It uses `get-vault-secrets` action to get the DockerHub username and password from Vault.
10+
This is a composite GitHub Action, used to build Docker images and push them to
11+
DockerHub. It uses `get-vault-secrets` action to get the DockerHub username and
12+
password from Vault.
1013

1114
Example of how to use this action in a repository:
1215

@@ -64,4 +67,8 @@ jobs:
6467

6568
- If you specify `platforms` then the action will use buildx to build the image.
6669
- You must create a Dockerhub repo before you are able to push to it.
67-
- Most projects should be using Google Artifact Registry (instead of Dockerhub) to store their images. You can see more about that in the push-to-gar-docker shared workflow.
70+
- Most projects at Grafana Labs should be using Google Artifact Registry instead
71+
of Dockerhub to store their images. You can see more about that in the
72+
[push-to-gar-docker] shared workflow.
73+
74+
[push-to-gar-docker]: ../push-to-gar-docker/README.md

actions/build-push-to-dockerhub/action.yaml

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,16 @@ runs:
103103
images: ${{ inputs.repository }}
104104
tags: ${{ inputs.tags }}
105105

106-
- name: Build and push Docker image
106+
# The `context` input is flagged by Zizmor as a [sink]. This means that with
107+
# the upstream action the user's input to the input ends up in an output,
108+
# and so if it's not handled properly, it could lead to a template injection
109+
# attack. In this action, we pass through the inputs, but we don't then pass
110+
# back the outputs, so we should be fine. Even if we did pass back the
111+
# outputs, we consider ourselves a proxy, so in that case our job would be
112+
# to warn users but not to take any action.
113+
#
114+
# [sink]: https://github.blog/security/application-security/how-to-secure-your-github-actions-workflows-with-codeql/#models
115+
- name: Build and push Docker image # zizmor: ignore[template-injection]
107116
uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0
108117
with:
109118
context: ${{ inputs.context }}

actions/login-to-gcs/action.yaml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,11 @@ runs:
7878
if: ${{ inputs.delete_credentials_file == 'true' && env.GOOGLE_APPLICATION_CREDENTIALS != '' }}
7979
shell: sh
8080
run: |
81-
if [ -f "${{ env.GOOGLE_APPLICATION_CREDENTIALS }}" ]; then
82-
rm -f "${{ env.GOOGLE_APPLICATION_CREDENTIALS }}"
81+
if [ -f "${GOOGLE_APPLICATION_CREDENTIALS}" ]; then
82+
rm -f "${GOOGLE_APPLICATION_CREDENTIALS}"
8383
echo "::notice::Successfully deleted credentials file"
8484
else
85-
echo "::warning::Credentials file not found at ${{ env.GOOGLE_APPLICATION_CREDENTIALS }}"
85+
echo "::warning::Credentials file not found at ${GOOGLE_APPLICATION_CREDENTIALS}"
8686
fi
87+
env:
88+
GOOGLE_APPLICATION_CREDENTIALS: ${{ env.GOOGLE_APPLICATION_CREDENTIALS }}

actions/push-to-gar-docker/README.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,16 @@
33
> [!NOTE]
44
> If you are at Grafana Labs:
55
>
6-
> - Follow these steps in the [internal documentation](https://enghub.grafana-ops.net/docs/default/component/deployment-tools/platform/continuous-integration/google-artifact-registry/) to set up a repository before using this action.
7-
> - A docker mirror is available on our self-hosted runners, see [the internal documentation](https://enghub.grafana-ops.net/docs/default/component/deployment-tools/platform/continuous-integration/#docker-caching-in-github-actions) for more info.
6+
> - Follow these steps in the [internal
7+
> documentation](https://enghub.grafana-ops.net/docs/default/component/deployment-tools/platform/continuous-integration/google-artifact-registry/)
8+
> to set up a repository before using this action.
9+
> - A docker mirror is available on our self-hosted runners, see [the internal
10+
> documentation](https://enghub.grafana-ops.net/docs/default/component/deployment-tools/platform/continuous-integration/#docker-caching-in-github-actions)
11+
> for more info.
812
9-
This is a composite GitHub Action, used to push docker images to Google Artifact Registry (GAR).
10-
It uses [OIDC authentication](https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect)
13+
This is a composite GitHub Action, used to push docker images to Google Artifact
14+
Registry (GAR). It uses [OIDC
15+
authentication](https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect),
1116
which means that only workflows which get triggered based on certain rules can
1217
trigger these composite workflows.
1318

@@ -80,6 +85,10 @@ input.
8085

8186
## Outputs
8287

88+
> [!IMPORTANT]
89+
> Be careful when handling the `metadata` output. This contains user-supplied
90+
> information, and so it can be a vector for template injection.
91+
8392
The following outputs are exposed from [`docker/metadata-action`](https://github.com/docker/metadata-action?tab=readme-ov-file#outputs) and [`docker/build-push-action`](https://github.com/docker/build-push-action?tab=readme-ov-file#outputs):
8493

8594
| Name | Type | Description | From |

actions/push-to-gar-docker/action.yaml

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,15 @@ runs:
173173
version: latest # see https://github.com/docker/build-push-action/issues/1345#issuecomment-2770572479
174174
buildkitd-config: ${{ runner.environment == 'self-hosted' && '/etc/buildkitd.toml' || '' }}
175175

176-
- name: Build the container
176+
# The `context` input is flagged by Zizmor as a [sink]. This means that with
177+
# the upstream action the user's input to the input ends up in an output,
178+
# and so if it's not handled properly, it could lead to a template injection
179+
# attack. In this action, we do pass this back out via our `metadata`
180+
# output. However, we consider ourselves a proxy, so in that case our job is
181+
# to warn users but not to take any action.
182+
#
183+
# [sink]: https://github.blog/security/application-security/how-to-secure-your-github-actions-workflows-with-codeql/#models
184+
- name: Build the container # zizmor: ignore[template-injection]
177185
uses: docker/build-push-action@263435318d21b8e681c14492fe198d362a7d2c83 # v6.18.0
178186
id: build
179187
with:
@@ -207,9 +215,11 @@ runs:
207215
if: ${{ inputs.delete_credentials_file == 'true' && env.GOOGLE_APPLICATION_CREDENTIALS != '' }}
208216
shell: sh
209217
run: |
210-
if [ -f "${{ env.GOOGLE_APPLICATION_CREDENTIALS }}" ]; then
211-
rm -f "${{ env.GOOGLE_APPLICATION_CREDENTIALS }}"
218+
if [ -f "${GOOGLE_APPLICATION_CREDENTIALS}" ]; then
219+
rm -f "${GOOGLE_APPLICATION_CREDENTIALS}"
212220
echo "::notice::Successfully deleted credentials file"
213221
else
214-
echo "::warning::Credentials file not found at ${{ env.GOOGLE_APPLICATION_CREDENTIALS }}"
222+
echo "::warning::Credentials file not found at ${GOOGLE_APPLICATION_CREDENTIALS}"
215223
fi
224+
env:
225+
GOOGLE_APPLICATION_CREDENTIALS: ${{ env.GOOGLE_APPLICATION_CREDENTIALS }}

actions/push-to-gcs/action.yaml

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,9 +120,11 @@ runs:
120120
if: ${{ inputs.delete_credentials_file == 'true' && env.GOOGLE_APPLICATION_CREDENTIALS != '' }}
121121
shell: sh
122122
run: |
123-
if [ -f "${{ env.GOOGLE_APPLICATION_CREDENTIALS }}" ]; then
124-
rm -f "${{ env.GOOGLE_APPLICATION_CREDENTIALS }}"
123+
if [ -f "${GOOGLE_APPLICATION_CREDENTIALS}" ]; then
124+
rm -f "${GOOGLE_APPLICATION_CREDENTIALS}"
125125
echo "::notice::Successfully deleted credentials file"
126126
else
127-
echo "::warning::Credentials file not found at ${{ env.GOOGLE_APPLICATION_CREDENTIALS }}"
127+
echo "::warning::Credentials file not found at ${GOOGLE_APPLICATION_CREDENTIALS}"
128128
fi
129+
env:
130+
GOOGLE_APPLICATION_CREDENTIALS: ${{ env.GOOGLE_APPLICATION_CREDENTIALS }}

0 commit comments

Comments
 (0)