Skip to content

Commit

Permalink
Fix security warnings from zizmor (#1115)
Browse files Browse the repository at this point in the history
Experimenting with the new [zizmor
tool](https://github.com/woodruffw/zizmor). There's still a number of
false-positives so probably not yet worth integrating into our CI run,
but have audited the current feedback.

Related to:
- #1114

## Only persist git credentials where we need to use them

- Don't leave these around when we don't need to.
- Explicitly set to true where we need them, with a comment highlighting
why we're keeping them.
- Fix a few places we weren't using the centrally managed checkout
version.
- Tweak the conditionals for submodules so the `with:` is always there
now.

## Use of fundamentally insecure workflow trigger -
`pull_request_target`

These appear ok because we're just using this to comment on community
PRs. These don't run builds

```
error[dangerous-triggers]: use of fundamentally insecure workflow trigger
  --> .github/workflows/community-moderation.yml:38:1
   |
38 | / on:
39 | |   pull_request_target:
...  |
42 | |     types:
43 | |     - opened
   | |_____________^ pull_request_target is almost always used insecurely
   |
```

```
error[dangerous-triggers]: use of fundamentally insecure workflow trigger
  --> .github/workflows/pull-request.yml:44:1
   |
44 | / on:
45 | |   pull_request_target: {}
   | |__________________________^ pull_request_target is almost always used insecurely
   |
```


## Code injection via template expansion

```
.github/workflows/master.yml
  env.COVERAGE_OUTPUT_DIR may expand into attacker-controllable code
```

This is not inputtable by a third party user. 

```
.github/workflows/prerequisites.yml
  inputs.default_branch may expand into attacker-controllable code
```

This is a workflow call (reusable workflow) and the input is always set
as `github.event.repository.default_branch`.

```
.github/workflows/upgrade-provider.yml
  github.event.inputs.version may expand into attacker-controllable code
  steps.upstream_version.outputs.latest_version may expand into attacker-controllable code
  github.repository may expand into attacker-controllable code
  steps.target_version.outputs.version may expand into attacker-controllable code
```

This can only be triggered by internal users.
  • Loading branch information
danielrbradley authored Nov 1, 2024
1 parent 0125ed1 commit f3712fc
Show file tree
Hide file tree
Showing 89 changed files with 236 additions and 47 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Cache examples generation
uses: actions/cache@v4
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,11 @@ jobs:
swap-storage: false
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Configure AWS Credentials
uses: #{{ .Config.actionVersions.configureAwsCredentials }}#
with:
Expand Down Expand Up @@ -140,10 +141,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,10 +80,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- uses: pulumi/provider-version-action@v1
id: provider-version
with:
Expand Down Expand Up @@ -77,7 +78,7 @@ jobs:
EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64)
{
echo "SCHEMA_CHANGES<<$EOF";
schema-tools compare -r github://api.github.com/#{{ .Config.organization }}# -p #{{ .Config.provider }}# -o ${{ inputs.default_branch }} -n --local-path=provider/cmd/pulumi-resource-#{{ .Config.provider }}#/schema.json;
schema-tools compare -r github://api.github.com/#{{ .Config.organization }}# -p #{{ .Config.provider }}# -o "${{ inputs.default_branch }}" -n --local-path=provider/cmd/pulumi-resource-#{{ .Config.provider }}#/schema.json;
echo "$EOF";
} >> "$GITHUB_ENV"
- if: inputs.is_pr && inputs.is_automated == false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,11 @@ jobs:
run: echo "Can't skip Go SDK for stable releases. This is likely a bug in the calling workflow." && exit 1
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down Expand Up @@ -63,7 +64,7 @@ jobs:
merge-multiple: true
- name: Calculate checksums
working-directory: dist
run: shasum ./*.tar.gz > pulumi-#{{ .Config.provider }}#_${{ inputs.version }}_checksums.txt
run: shasum ./*.tar.gz > "pulumi-#{{ .Config.provider }}#_${{ inputs.version }}_checksums.txt"
- name: Get Schema Change Summary
id: schema-summary
shell: bash
Expand Down Expand Up @@ -102,10 +103,12 @@ jobs:
steps:
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
# Persist credentials so we can push back to the repo
persist-credentials: true
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down Expand Up @@ -168,7 +171,9 @@ jobs:
runs-on: #{{ .Config.runner.default }}#
steps:
- name: Checkout Repo
uses: actions/checkout@v4
uses: #{{ .Config.actionVersions.checkout }}#
with:
persist-credentials: false
- name: Clean up release labels
uses: pulumi/action-release-by-pr-label@main
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@ jobs:
steps:
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
# Persist credentials so we can push a new branch.
persist-credentials: true
- name: Checkout repo
uses: #{{ .Config.actionVersions.checkout }}#
with:
path: ci-mgmt
repository: pulumi/ci-mgmt
persist-credentials: false
- id: run-url
name: Create URL to the run output
run: echo "run-url=https://github.com/$GITHUB_REPOSITORY/actions/runs/$GITHUB_RUN_ID" >> "$GITHUB_OUTPUT"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ jobs:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
persist-credentials: false
- name: Checkout p/examples
if: matrix.testTarget == 'pulumiExamples'
uses: #{{ .Config.actionVersions.checkout }}#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,11 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,12 @@ jobs:
#{{- end }}#
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
# Persist credentials so upgrade-provider can push a new branch.
persist-credentials: true
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@ jobs:
runs-on: #{{ .Config.runner.default }}#
steps:
- name: Checkout Repo
uses: actions/checkout@v4
uses: #{{ .Config.actionVersions.checkout }}#
with:
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ jobs:
steps:
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Install go
uses: actions/setup-go@v5
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ jobs:
steps:
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- name: Comment PR
uses: #{{ .Config.actionVersions.prComment }}#
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@ jobs:
runs-on: ${{ matrix.runner }}
steps:
- name: Checkout Repo
uses: actions/checkout@v4
uses: #{{ .Config.actionVersions.checkout }}#
with:
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ jobs:
steps:
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- uses: peter-evans/slash-command-dispatch@v4
with:
commands: |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@ jobs:
steps:
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
#{{- if .Config.checkoutSubmodules }}#
with:
#{{- if .Config.checkoutSubmodules }}#
submodules: #{{ .Config.checkoutSubmodules }}#
#{{- end }}#
#{{- end }}#
persist-credentials: false
- id: schema_changed
name: Check for diff in schema
uses: #{{ .Config.actionVersions.pathsFilter }}#
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ jobs:
steps:
- name: Checkout Repo
uses: #{{ .Config.actionVersions.checkout }}#
with:
persist-credentials: false
- name: Should release PR
uses: pulumi/action-release-by-pr-label@main
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Cache examples generation
uses: actions/cache@v4
with:
Expand Down
2 changes: 2 additions & 0 deletions provider-ci/test-providers/acme/.github/workflows/license.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
2 changes: 2 additions & 0 deletions provider-ci/test-providers/acme/.github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Install go
uses: actions/setup-go@v5
with:
Expand Down
4 changes: 4 additions & 0 deletions provider-ci/test-providers/acme/.github/workflows/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ jobs:
swap-storage: false
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Configure AWS Credentials
uses: aws-actions/configure-aws-credentials@v4
with:
Expand Down Expand Up @@ -136,6 +138,8 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- name: Setup tools
uses: ./.github/actions/setup-tools
with:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ jobs:
steps:
- name: Checkout Repo
uses: actions/checkout@v4
with:
persist-credentials: false
- uses: pulumi/provider-version-action@v1
id: provider-version
with:
Expand Down Expand Up @@ -76,7 +78,7 @@ jobs:
EOF=$(dd if=/dev/urandom bs=15 count=1 status=none | base64)
{
echo "SCHEMA_CHANGES<<$EOF";
schema-tools compare -r github://api.github.com/pulumiverse -p acme -o ${{ inputs.default_branch }} -n --local-path=provider/cmd/pulumi-resource-acme/schema.json;
schema-tools compare -r github://api.github.com/pulumiverse -p acme -o "${{ inputs.default_branch }}" -n --local-path=provider/cmd/pulumi-resource-acme/schema.json;
echo "$EOF";
} >> "$GITHUB_ENV"
- if: inputs.is_pr && inputs.is_automated == false
Expand Down
Loading

0 comments on commit f3712fc

Please sign in to comment.