Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update pipeline to use reusable witness workflow #78

Merged
merged 3 commits into from
Oct 27, 2023
Merged

Update pipeline to use reusable witness workflow #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ad49d5d
Update pipeline to use reusable witness workflow
jkjell Oct 27, 2023
0e61bab
Add release job to pipeline
jkjell Oct 27, 2023
ba742f0
Change release needs to e2e-test
jkjell Oct 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 94 additions & 0 deletions .github/workflows/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
# Copyright 2023 The Archivista Contributors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

permissions:
id-token: write # This is required for requesting the JWT
contents: read # This is required for actions/checkout
name: pipeline
on:
push:
tags:
- v*
branches:
- main
pull_request:
jobs:
fmt:
uses: ./.github/workflows/witness.yml
with:
pull_request: ${{ github.event_name == 'pull_request' }}
step: static-analysis
attestations: "github"
command: go fmt ./...

static_analysis:
uses: ./.github/workflows/witness.yml
with:
pull_request: ${{ github.event_name == 'pull_request' }}
step: static-analysis
attestations: "github"
command: go vet ./...

test:
needs: [fmt, static_analysis]
uses: ./.github/workflows/witness.yml
with:
pull_request: ${{ github.event_name == 'pull_request' }}
step: "test"
attestations: "github"
command: go test -v -coverprofile=profile.cov -covermode=atomic ./...

e2e-tests:
needs: test
uses: ./.github/workflows/witness.yml
with:
pull_request: ${{ github.event_name == 'pull_request' }}
step: "e2e"
attestations: "github"
command: ./test/test.sh

release:
needs: e2e-tests
permissions:
id-token: write
contents: write
packages: write
runs-on: ubuntu-latest
if: github.event_name == 'push' && contains(github.ref, 'refs/tags/')

steps:
- uses: actions/[email protected]
- uses: actions/[email protected]
with:
go-version: 1.21.x

- name: Login to GitHub Container Registry
uses: docker/login-action@v2
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Download GoReleaser
run: go install github.com/goreleaser/goreleaser@latest

- name: Run GoReleaser
uses: testifysec/witness-run-action@40aa4ef36fc431a37de7c3faebcb66513c03b934
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
GITHUB_REPOSITORY_OWNER: ${{ github.repository_owner }}
with:
step: "build"
attestations: "github"
command: goreleaser release --clean
127 changes: 0 additions & 127 deletions .github/workflows/release.yml
View file
Open in desktop

This file was deleted.

47 changes: 47 additions & 0 deletions .github/workflows/witness.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# Copyright 2023 The Archivista Contributors
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

on:
workflow_call:
inputs:
pull_request:
required: true
type: boolean
command:
required: true
type: string
step:
required: true
type: string
attestations:
required: true
type: string

jobs:
witness:
runs-on: ubuntu-latest
steps:
- uses: actions/[email protected]
- uses: actions/[email protected]
with:
go-version: 1.21.x

- if: ${{ inputs.pull_request == false }}
uses: testifysec/witness-run-action@40aa4ef36fc431a37de7c3faebcb66513c03b934
with:
step: ${{ inputs.step }}
attestations: $${ inputs.attestations }}
command: /bin/sh -c "$${ inputs.command }}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a potential for a script injection attack. https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions#example-of-a-script-injection-attack

Here is a suggestion

- if: ${{ inputs.pull_request == false }}
  uses: testifysec/witness-run-action@40aa4ef36fc431a37de7c3faebcb66513c03b934
  env:
    INPUT_COMMAND: ${{ inputs.command }}
  with:
    step: ${{ inputs.step }}
    attestations: $${ inputs.attestations }}
    command: /bin/sh -c "$INPUT_COMMAND"

Copy link

@BHunter2889 BHunter2889 Oct 27, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is something that in general caught my attention as I've been familiarizing myself with Witness and some of the pipeline examples.

I have thoughts on how this could be solved, though it's possible its over-engineering so it would be better to discuss it elsewhere. I can create a feature issue if that is what would be most appropriate for internal assessment || community discussion.

EDIT: As @mikhailswift pointed out though, it's likely most if not all scenarios like this may not be vulnerable in a meaningful way. That said, there may be something to be said about creating a more streamlined secure way of implementing pipeline job scripts in general for orgs/devs who may not have the same level of insight into issues such as this.

Comment on lines +44 to +45
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can I ask, does the $${ ... }} have special meaning here? A little thrown off by the imbalanced curly braces and double $$ as I'm unfamiliar with any special context they have in GitHub workflows and I was unable to find any documentation after a thorough search. I know GitLab uses it to escape another $ for when auto variable expansion is enabled. Also that shell itself uses it as a PID reference.

It seems to be getting interpolated as expected by the action so I assume either there's some special meaning, or its auto escaping these special characters through some means. Or maybe I'm wildly missing something... 😅

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, this was just wrong. 😅 And I deleted the branch too fast to fix it here. 😞

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#79

- if: ${{ inputs.pull_request == true }}
run: ${{ inputs.command }}