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.

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

chore(ci): improvements to gha workflows #7089

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

sgammon
Copy link
Contributor

@sgammon sgammon commented Mar 8, 2024

Summary

Applies updates to GHA CI, and refactors into Reusable Workflows. Hardens supply chain security for Guava both in CI and release. Fixes and closes #7088.

Related PRs

Security hardening

  • CI flows now have an enumerated set of allowed internet endpoints. Other endpoint access is blocked.
  • CI flows can now publish SLSA provenance metadata
  • Maven will now reject dependencies which experience hash mismatches
  • Maven dependencies for Guava are now reported to the GitHub Dependency Graph

Enclosed features

Changelog

Expand for full changelog
- chore(ci): apply hardening to ci jobs
- chore: apply 'Harden Runner' auditing to all ci tasks
- chore: apply `persist-credentials: false` to checkout tasks
- chore: publish dependency graph and add dependency review check
- chore: add codeql scan job (temp)
- chore: bump `actions/checkout` → `4.1.1`
- chore: bump `actions/dependency-review-action` → `4.1.3`
- chore(ci): refactor into reusable workflows
- chore: move ci jobs to `workflow_call` trigger
- chore: add entrypoint jobs for PR and Push events
- chore: cleanup permissions and dispatch checks/tests
- chore(ci): switch to enforced hardening mode
- chore: gather and apply network endpoints for each job
- chore: move to `block` mode for `egress-policy` in `step-security/harden-runner`
- feat(ci): slsa provenance support
- feat: add slsa support to build workflow
- chore: split `test` into `build` and `test` workflows
- chore: use new workflows (build/test) from push/pr triggers
- chore(build): parameterize deploy repositories
- chore: add sigstore plugin to build
- chore: add `--strict-checksums` flag to `mvnw` calls in ci

Action updates

Bumps actions/checkout from 3.6.0 to 4.1.1.

Release notes

Sourced from actions/checkout's releases.

v4.1.1

What's Changed

New Contributors

Full Changelog: actions/checkout@v4.1.0...v4.1.1

v4.1.0

What's Changed

New Contributors

Full Changelog: actions/checkout@v4.0.0...v4.1.0

v4.0.0

What's Changed

New Contributors

Full Changelog: actions/checkout@v3...v4.0.0

Changelog

Sourced from actions/checkout's changelog.

Changelog

v4.1.0

v4.0.0

v3.6.0

v3.5.3

v3.5.2

v3.5.1

v3.5.0

v3.4.0

v3.3.0

v3.2.0

v3.1.0

v3.0.2

... (truncated)

Commits

StepSecurity PR

The PR content below describes the hardening applied by StepSecurity.

Harden Runner

Harden-Runner is an open-source security agent for the GitHub-hosted runner to prevent software supply chain attacks. It prevents exfiltration of credentials, detects tampering of source code during build, and enables running jobs without sudo access.

Harden runner usage

You can find link to view insights and policy recommendation in the build log

Please refer to documentation to find more details.

Detect Vulnerabilities with SAST Workflow

Static Code Analysis (also known as Source Code Analysis) is usually performed as part of a Code Review (also known as clear-box testing) and is carried out at the Implementation phase of a Security Development Lifecycle (SDL). Static Code Analysis commonly refers to the running of Static Code Analysis tools that attempt to highlight possible vulnerabilities within ‘static’ (non-running) source code by using techniques such as Taint Analysis and Data Flow Analysis.

Add Dependency Review Workflow

The Dependency Review Workflow enforces dependency reviews on your pull requests. The action scans for vulnerable versions of dependencies introduced by package version changes in pull requests, and warns you about the associated security vulnerabilities. This gives you better visibility of what's changing in a pull request, and helps prevent vulnerabilities being added to your repository.

Copy link
Contributor Author

@sgammon sgammon left a comment

Choose a reason for hiding this comment

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

Why propose this

By adopting this re-usable workflows pattern, the build gets much easier to understand and follow, because there is just one workflow driving all the jobs for a given PR or push.

Here is a side-by-side comparison of what it feels like in the UI. Left is before, right is after:

Screenshot 2024-03-08 at 10 41 25 AM

The job graph on the Summary page is now accurate:

Screenshot 2024-03-08 at 10 42 11 AM

Dependency Graph

Additionally, this PR helps Guava support the GitHub Dependency Graph. Guava's own dependencies will show up in the GitHub repo, and people who depend on Guava can see it in the graph. By supporting the Dependency Graph, we can also use Dependency Review:

Screenshot 2024-03-08 at 10 43 04 AM

PRs

Duplicate work was eliminated between the previous push and pull_request events. From a PR, things now all gather under one job:

Screenshot 2024-03-08 at 10 48 56 AM

Clicking Details to the right on any job brings you to the same unified summary screen.

Builds, tests, and checks, self-organize because they are all under one job, so they sort hierarchically:

Screenshot 2024-03-08 at 10 49 32 AM

Comment on lines +1 to +8
# Guava GitHub CI
# ---------------------------------------------------------------------------------------------------------------------
# This is the main CI build on GitHub for the Google Guava project. This workflow is not invoked directly; instead, the
# `on.pr.yml` and `on.push.yml` workflows kick in on PR and push events, respectively, and call this workflow as a
# Reusable Workflow.
#
# This workflow can be tested independently of the entrypoint flow through the `workflow_dispatch` hook, which adds a
# button within the UI of the GitHub repository. You can trigger the workflow from here:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Docs at the top of each workflow.

Comment on lines +25 to +30
workflow_call:
inputs:
provenance:
type: boolean
description: "Provenance"
default: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

workflow_call allows reuse of the build and test workflows.

Comment on lines +115 to +131
run: |
echo "Building SLSA provenance material..."
ls guava/target/*.jar guava-gwt/target/*.jar guava-testlib/target/*.jar
echo "hashes=$(sha256sum guava/target/*.jar guava-gwt/target/*.jar guava-testlib/target/*.jar | base64 -w0)" >> ./provenance-hashes.txt
cat ./provenance-hashes.txt >> "$GITHUB_OUTPUT"
echo "Gathered provenance hashes:"
cat ./provenance-hashes.txt
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Builds SLSA hashes

.github/workflows/ci.test.yml Outdated Show resolved Hide resolved
.github/workflows/codeql.yml Outdated Show resolved Hide resolved
Comment on lines +52 to +56
- name: Maven Dependency Tree Dependency Submission
continue-on-error: true
uses: advanced-security/maven-dependency-submission-action@bfd2106013da0957cdede0b6c39fb5ca25ae375e # v4.0.2
- name: 'Dependency Review'
uses: actions/dependency-review-action@9129d7d40b8c12c1ed0f60400d00c92d437adcce # v4.1.3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This workflow covers Github Dependency Graph support; the dependency tree is gathered from Maven, published to Github, and then dependency-review-action is run, which reviews the dependency graph for licensing violations and vulnerabilities.

Both of these actions come directly from the Github team so they are typically very reliable.

Comment on lines 20 to 35
## Build the library and provenance material, but don't publish
build:
name: "Build"
uses: ./.github/workflows/ci.build.yml
permissions:
actions: write
contents: write
id-token: write
with:
provenance: true
provenance_publish: false
snapshot: false
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Entrypoint workflow at on.pr.yml runs on pull_request events. Build/test steps are invoked by calling into the shared workflows at ci.build.yml and ci.test.yml.

guava/pom.xml Outdated Show resolved Hide resolved
@sgammon
Copy link
Contributor Author

sgammon commented Mar 8, 2024

cc / @cpovirk I realize this one is going to look big, but I promise it's worth it if you dig in. Totally happy to close and not merge but I think it will make the build much smoother, and adds some dependency security as well.

@sgammon
Copy link
Contributor Author

sgammon commented Mar 8, 2024

The StepSecurity PR (before cleanup):
sgammon#1

Here is a sample run:
https://github.com/sgammon/guava/actions/runs/8207681690

@sgammon sgammon force-pushed the chore/ci-security branch 3 times, most recently from 4b46092 to 1d1d6dd Compare March 11, 2024 00:17
@sgammon sgammon marked this pull request as ready for review March 11, 2024 00:17
copybara-service bot pushed a commit that referenced this pull request Mar 11, 2024
Drops the JDK source steps involved with Javadoc builds, as suggested in #7089. There will be some build warnings related to Javadoc until subsequent PRs are merged.

Signed-off-by: Sam Gammon <[email protected]>

Gives up on the "inherting" half of #6790.

Fixes #7092

RELNOTES=n/a
PiperOrigin-RevId: 614681928
copybara-service bot pushed a commit that referenced this pull request Mar 11, 2024
Drops the JDK source steps involved with Javadoc builds, as suggested in #7089. There will be some build warnings related to Javadoc until subsequent PRs are merged.

Signed-off-by: Sam Gammon <[email protected]>

Gives up on the "inherting" half of #6790.

Fixes #7092

RELNOTES=n/a
PiperOrigin-RevId: 614693592
@sgammon
Copy link
Contributor Author

sgammon commented Mar 11, 2024

This needs a little bit of cleanup -- mostly an autorebase -- but otherwise it is ready for review @cpovirk

@sgammon
Copy link
Contributor Author

sgammon commented Mar 11, 2024

Okay, excellent, thank you for approving CI workflows by the way. That helps a bunch because I can test running these on an unauthorized fork.

Consequently, the two failures there (SLSA Provenance and CodeQL) are happening because the fork (properly) does not have permissions to id-token: write, security-events: write, etc. Those should be made conditional so that outside fork PRs don't fail but the publishing still works. I'll hunt that down.

@sgammon
Copy link
Contributor Author

sgammon commented Mar 12, 2024

Rebasing this shortly since #7087 was merged

- chore: apply 'Harden Runner' auditing to all ci tasks
- chore: apply `persist-credentials: false` to checkout tasks
- chore: publish dependency graph and add dependency review check
- chore: add codeql scan job (temp)

Bumps [actions/checkout](https://github.com/actions/checkout) from 3.6.0 to 4.1.1.
- [Release notes](https://github.com/actions/checkout/releases)
- [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md)
- [Commits](actions/checkout@v3.6.0...b4ffde6)

Bumps [actions/dependency-review-action](https://github.com/actions/dependency-review-action) from 2.5.1 to 4.1.3.
- [Release notes](https://github.com/actions/dependency-review-action/releases)
- [Commits](actions/dependency-review-action@0efb1d1...9129d7d)

---
updated-dependencies:
- dependency-name: actions/checkout
  dependency-type: direct:production
  update-type: version-update:semver-major
- dependency-name: actions/dependency-review-action
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: StepSecurity Bot <[email protected]>
Signed-off-by: Sam Gammon <[email protected]>
Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@sgammon
Copy link
Contributor Author

sgammon commented Mar 12, 2024

Provenance/Security stuff

  • Hardening is now strict and denies sudo and network traffic outside of allowlists for each job
  • SLSA provenance is generated on push to master, or on non-fork PRs, but not otherwise
  • SLSA provenance is not published in releases yet, or embedded in the JAR; it can go in a GitHub release, for example
  • SPDX is not published yet (as above for SLSA), but is embedded within the JAR, see JAR tree below (coming soon)
  • Sigstore plugin is added but does not run yet; I don't want to publish signatures of my fork
  • CodeQL is suppressed because "simple" mode must be disabled in the repo before "advanced" mode can be run

The Sigstore plugin is made by Google and Googlers (the infamous @loosebazooka) and it has worked well in the past purely with a GitHub token in CI. I am happy to test it if desired.

Screenshot 2024-03-12 at 3 50 36 PM Screenshot 2024-03-12 at 3 50 57 PM Screenshot 2024-03-12 at 3 53 25 PM

Notes on naming:
This SPDX document name and prefix (at META-INF/spdx/) are designed to work well with fat JAR/shadow merging. I'm not aware of standards yet about where to place this document (cc / @goneall @loosebazooka ?)

For instance, maybe this should be put in META-INF/sbom/* so that CycloneDX can be supported side-by-side someday, or other formats, if desired?

SPDX document sample:
com.google.guava_guava-HEAD-jre-SNAPSHOT.spdx.json

Multi-JVM testing

Subject to advice on #7087, the build currently maintains the same JVM matrix, and on a subsequent PR I will add multi-JVM testing in one job with Maven toolchains.

Any of these can be rolled back and of course this entire PR is optional for the other changes in flight in #7093 and #7094.

This change refactors the main CI workflow into two new workflows, `on.pr.yml` and
`on.push.yml`, which each call into the exiting CI job as a reusable workflow.

This has the nice benefit of putting all tests, checks, builds, etc., on one screen
during development on GitHub, allows customization of the PR vs. push flow, and yet
keeps behavior fully consistent between the two.

- chore: move ci jobs to `workflow_call` trigger
- chore: add entrypoint jobs for PR and Push events
- chore: cleanup permissions and dispatch checks/tests

Signed-off-by: Sam Gammon <[email protected]>
This changeset switches the StepSecurity hardening action to enforced mode, where
previously it was running in `audit` mode. Now, audit logs have been gathered and
it is time to seal off the list of accessible network endpoints for a given job.

- chore: gather and apply network endpoints for each job
- chore: move to `block` mode for `egress-policy` in `step-security/harden-runner`

Signed-off-by: Sam Gammon <[email protected]>
This changeset adds SLSA 3+ provenance support to the workflow. The main CI run has now been
split into two: `ci.build.yml`, which only builds the library and is provenance-capable, and
`ci.test.yml`, which is the previous CI logic.

The regular build logic is applied only on push, and can be applied on PRs too, with publish
of provenance material turned off. The test suite is invoked from PRs.

The workflows have been split into build/test phases to avoid publishing provenance data and
GitHub artifacts for build matrix outputs. JARs are uniform across OS targets, so there is no
need to gather and publish for more than Ubuntu.

- feat: add slsa support to build workflow
- chore: split `test` into `build` and `test` workflows
- chore: use new workflows (build/test) from push/pr triggers

Signed-off-by: Sam Gammon <[email protected]>
Fails the build if any downloaded dependencies fail their checksum
verification.

- chore: add `--strict-checksums` flag to `mvnw` calls in ci
- chore: don't rebuild javadoc during tests in ci
- chore: don't run with gpg enabled in ci

Signed-off-by: Sam Gammon <[email protected]>
Adds two build parameters
- `publishing.repository.snapshots`: Snapshot repo to deploy to
- `publishing.repository.releases`: Releases repo to deploy to

Both default to their current values, Sonatype. This small inert
change allows a fork to easily publish to a different repository
without resorting to a code change.

Signed-off-by: Sam Gammon <[email protected]>
This changeset adds the Maven Sigstore plugin for use during
publishing to Sonatype and other public repositories.

- chore: add sigstore plugin to build

Signed-off-by: Sam Gammon <[email protected]>
Signed-off-by: Sam Gammon <[email protected]>
@goneall
Copy link

goneall commented Mar 13, 2024

This SPDX document name and prefix (at META-INF/spdx/) are designed to work well with fat JAR/shadow merging. I'm not aware of standards yet about where to place this document (cc / @goneall @loosebazooka ?)

I recall a discussion in the OpenSSF SBOM Everywhere on creating a best practice guide on where to place SBOMs, but I couldn't find a link to the document/discussion - @joshbressers may be able to provide a pointer.

My personal advice is that the SPDX documents be placed in the directory adjacent to any release artifacts so that it can be picked up by any downstream consumers. If the SPDX document is uploaded to Maven central next to the deployed .jar file, the SPDX maven plugin should recognize it when guava is used in any downstream projects that use the SPDX maven plugin.

BTW - Thanks for adding the SPDX documents to the builds! I use guava in some of my projects, and this will improve the fidelity of the SPDX data I produce for those projects.

env:
CI_DEPLOY_USERNAME: ${{ secrets.CI_DEPLOY_USERNAME }}
CI_DEPLOY_PASSWORD: ${{ secrets.CI_DEPLOY_PASSWORD }}
run: ./util/deploy_snapshot.sh

Choose a reason for hiding this comment

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

@sgammon Thanks for opening this PR. It would be great to harden the build pipelines for Guava artifacts. I think one issue with the current workflow though is that only the snapshot artifacts are automatically published and not the main artifacts. So, the SLSA provenances can only attest to the snapshot artifacts. We also need to automate the publishing of main artifacts to Maven Central, and generate SLSA provenances for them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@behnazh-w Yes, you're right. I wanted to get things started and churning, and then, with good PR feedback, I am happy to extend the SLSA provenance changes to other parts of the workflow. I believe there are Maven and Bazel generators that could be put to good use.

@loosebazooka
Copy link

Just to clarify here, while I am a maintainer of sigstore. It's an openssf/linuxfoundation project. Not a Google project.

@loosebazooka
Copy link

loosebazooka commented May 9, 2024

Maven central will accept spdx documents as long as they follow the naming convention, and include the necessary signatures/checksums. I don't have any experience with embedding the sbom doc in the jar. I think maven central is a reasonable place to publish sboms (next to the artifact).

Either way I think it's important to make an effort to differentiate dependencies that are shaded in and external dependencies that may or may not change at resolution time. I'm of the opinion that libraries should only include their shaded dependencies in the sbom when publishing to maven central. But I think the jury's still out on this one (@lumjjb, @goneall)

@goneall
Copy link

goneall commented May 9, 2024

Maven central will accept spdx documents as long as they follow the naming convention, and include the necessary signatures/checksums. I don't have any experience with embedding the sbom doc in the jar. I think maven central is a reasonable place to publish sboms (next to the artifact).

Either way I think it's important to make an effort to differentiate dependencies that are shaded in and external dependencies that may or may not change at resolution time. I'm of the opinion that libraries should only include their shaded dependencies in the sbom when publishing to maven central. But I think the jury's still out on this one (@lumjjb, @goneall)

Dependencies that are included with the distributed artifact(s) can use the CONTAINS relationship. The CONTAINS relationships should not be used for dependencies that are not "contained" in the distributed artifacts. The more general DEPENDENCY_OF relationship can be used for dependencies that may or may not be contained (note: these are not mutually exclusive, you can have both a CONTAINS and DEPENDENCY_OF relationship between the same packages). There are additional dependency relationship types that loosely map to the the Maven scopes.

@sgammon
Copy link
Contributor Author

sgammon commented May 9, 2024

Gotcha. It sounds like it doesn't make sense to include SBOMs within the JAR itself, so I'll unwind that part and hold it alongside the JAR as an output artifact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Supply Chain Security
7 participants