chore(ci): improvements to gha workflows #7089
Conversation
There was a problem hiding this comment.
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:
The job graph on the Summary page is now accurate:
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:
PRs
Duplicate work was eliminated between the previous push and pull_request events. From a PR, things now all gather under one job:
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:
| # 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: |
There was a problem hiding this comment.
Docs at the top of each workflow.
| workflow_call: | ||
| inputs: | ||
| provenance: | ||
| type: boolean | ||
| description: "Provenance" | ||
| default: false |
There was a problem hiding this comment.
workflow_call allows reuse of the build and test workflows.
| 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 |
| - 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 |
There was a problem hiding this comment.
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.
.github/workflows/on.pr.yml
Outdated
| ## 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 |
There was a problem hiding this comment.
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.
|
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. |
|
The StepSecurity PR (before cleanup): Here is a sample run: |
sgammon
force-pushed
the
chore/ci-security
branch
3 times, most recently
from
4b46092
to
1d1d6dd
Compare
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
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
force-pushed
the
chore/ci-security
branch
from
bb90f37
to
85ed7db
Compare
|
This needs a little bit of cleanup -- mostly an autorebase -- but otherwise it is ready for review @cpovirk |
|
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 |
sgammon
force-pushed
the
chore/ci-security
branch
2 times, most recently
from
cafe44e
to
56bb0f0
Compare
sgammon
force-pushed
the
chore/ci-security
branch
from
e3e2cd9
to
0403cf7
Compare
|
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
force-pushed
the
chore/ci-security
branch
from
0403cf7
to
5bde323
Compare
Provenance/Security stuff
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. 
Notes on naming: For instance, maybe this should be put in SPDX document sample: Multi-JVM testingSubject 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]>
Signed-off-by: Sam Gammon <[email protected]>
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. |
sgammon
force-pushed
the
chore/ci-security
branch
2 times, most recently
from
73794d5
to
c5846e1
Compare
| env: | ||
| CI_DEPLOY_USERNAME: ${{ secrets.CI_DEPLOY_USERNAME }} | ||
| CI_DEPLOY_PASSWORD: ${{ secrets.CI_DEPLOY_PASSWORD }} | ||
| run: ./util/deploy_snapshot.sh |
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
@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.
|
Just to clarify here, while I am a maintainer of sigstore. It's an openssf/linuxfoundation project. Not a Google project. |
|
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 |
|
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. |
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
Enclosed features
Changelog
Expand for full changelog
Action updates
Bumps actions/checkout from 3.6.0 to 4.1.1.
Release notes
Sourced from actions/checkout's releases.
Changelog
Sourced from actions/checkout's changelog.
... (truncated)
Commits
b4ffde6Link to release page from what's new section (#1514)8530928Correct link to GitHub Docs (#1511)7cdaf2fUpdate CODEOWNERS to Launch team (#1510)8ade135Prepare 4.1.0 release (#1496)c533a0aAdd support for partial checkout filters (#1396)72f2cecUpdate README.md for V4 (#1452)3df4ab1Release 4.0.0 (#1447)8b5e8b7Support fetching without the --progress option (#1067)97a652bUpdate default runtime to node20 (#1436)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
sudoaccess.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.