Skip to content

feat: OpenSSF Scorecard and dependency review workflows#33

Open
ms280690 wants to merge 2 commits into
mainfrom
issue-29-governance-observability
Open

feat: OpenSSF Scorecard and dependency review workflows#33
ms280690 wants to merge 2 commits into
mainfrom
issue-29-governance-observability

Conversation

@ms280690
Copy link
Copy Markdown
Collaborator

Summary

Implements the code-deliverable items from issue #29 (enterprise governance and observability).

  • scorecard.yml — weekly OpenSSF Scorecard analysis; publishes results to the OpenSSF database and uploads SARIF to the GitHub Security tab
  • dependency-review.yml — blocks PRs that introduce dependencies with known vulnerabilities or denied licenses; reusable via workflow_call

Notes

Org-level items remaining in #29 (not implementable as code)

  • Org rulesets (required workflows enforcement) — GitHub UI
  • Audit log streaming to Wazuh — GitHub org settings
  • Actor rules for fork PRs — GitHub org settings

Closes #29

Test plan

  • Scorecard workflow runs on push to branch and completes without error
  • Scorecard results appear in the Security tab → Code scanning
  • Dependency review workflow triggers on PRs touching lockfiles
  • All action references are SHA-pinned (actionlint + zizmor pass)

scorecard.yml:
- Runs weekly (Monday 06:00 UTC), on push to main, and on workflow_dispatch
- Publishes results to the OpenSSF database (public repo, OIDC-signed)
- Uploads SARIF to GitHub Security tab and as a retained artifact
- Target score >= 8.0/10 per issue #29 acceptance criteria

dependency-review.yml:
- Triggers on PRs touching any supported lockfile format
- Reusable via workflow_call with fail-on-severity and deny-licenses inputs
- Posts a summary comment on the PR via comment-summary-in-pr: always
- Default deny list: GPL-2.0, GPL-3.0, AGPL-3.0

Also brings .pre-commit-config.yaml forward from issue-25 branch so
local hooks work while that PR is pending merge.

All action references SHA-pinned per #25 authoring standards.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds organization-governance workflows to improve supply-chain security posture for this repository and to provide a reusable dependency gate for consuming repositories.

Changes:

  • Introduces an OpenSSF Scorecard workflow that runs on schedule/push and uploads SARIF results to GitHub Security.
  • Adds a reusable Dependency Review workflow (also runnable on PRs) with severity/license policy inputs.
  • Adds a local .pre-commit-config.yaml to run actionlint and zizmor consistently.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
.pre-commit-config.yaml Adds pre-commit hooks for action/workflow linting and security checks.
.github/workflows/scorecard.yml Runs OpenSSF Scorecard and uploads SARIF/artifacts for Security tab visibility.
.github/workflows/dependency-review.yml Adds dependency review gate (PR + reusable workflow) with policy inputs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/dependency-review.yml Outdated
Comment thread .github/workflows/dependency-review.yml
Comment thread .github/workflows/scorecard.yml Outdated
@ms280690
Copy link
Copy Markdown
Collaborator Author

Code Review

Strengths

  • All action refs SHA-pinned with version comments — consistent with the standard being established in PR Add workflow authoring standards, actionlint/zizmor gate, and CONTRIB… #32.
  • persist-credentials: false on all checkout steps.
  • permissions: read-all at workflow level with job-level minimum overrides on Scorecard — the comment explaining why read-all is needed is a good call given it looks like an over-permission at first glance.
  • workflow_call inputs are well-designed on dependency-review.yml — exposed the two most likely customisation points (fail-on-severity, deny-licenses) with sensible defaults and descriptions.
  • || 'default' fallback pattern for workflow_call inputs is correct — handles the case where the workflow is triggered by pull_request (no inputs provided).
  • Artifact upload of SARIF before the CodeQL upload is a nice belt-and-suspenders approach.
  • retention-days: 5 on the artifact is appropriately short for ephemeral scan results.

Issues

1. upload-artifact action SHA looks incorrect

uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a  # v7.0.1

actions/upload-artifact is currently on v4 (latest stable). v7 does not exist — this SHA may resolve to an unrelated commit or fail. Verify against:

gh api repos/actions/upload-artifact/commits/v4 --jq '.sha'

2. No if: always() on the SARIF upload in scorecard.yml

- uses: github/codeql-action/upload-sarif@...
  with:
    sarif_file: scorecard-results.sarif

If the Scorecard action exits non-zero (rate-limited, transient failure), the SARIF upload will be skipped. Adding if: always() ensures results are uploaded even on partial failures — consistent with the pattern used in PR #32's zizmor upload.

3. Duplicate defaults for workflow_call inputs

fail-on-severity and deny-licenses have defaults defined twice: once in the inputs: block and once in the || fallback expressions. If these drift apart silently (e.g., someone updates one but not the other), behaviour will differ between pull_request and workflow_call triggers. GitHub resolves inputs.default even when triggered by pull_request, so the || fallback expressions can be dropped to keep defaults in one place.

4. fail-on-severity default is critical only

Blocking only on critical means high severity findings (CVSS 8.x) pass silently. For a security-focused org library, high is a more defensible default. Worth a conscious policy decision either way.

5. pull_request path filter omits some lockfiles

The list is comprehensive but misses a few patterns for completeness as a general-purpose org template:

  • **/pdm.lock (PDM, Python)
  • **/mix.lock (Elixir)
  • **/Package.resolved (Swift)

Not critical for Sparkgeo's current stack — flagging for awareness.

6. .pre-commit-config.yaml merge order dependency

As noted in the PR description, this file is identical to the one in PR #32. Whichever PR merges second will need a no-op conflict resolution. Merge PR #32 first to avoid the conflict landing here.


Priority before merging

  • (1) Verify the upload-artifact SHA — v7 does not exist, this is likely pointing at the wrong commit.
  • (2) Add if: always() to the Scorecard SARIF upload step.

Items 3 and 4 are policy/cleanup calls; items 5 and 6 are low priority.

scorecard.yml:
- Remove redundant workflow-level permissions: read-all; job-level block
  is definitive (job permissions override, not merge, workflow-level)
- Add if: always() to SARIF upload so results are captured even on
  transient Scorecard failures

dependency-review.yml:
- Raise fail-on-severity default from critical to high — CVSS 8.x
  findings should not pass silently in a security-focused org library
- Add pdm.lock, mix.lock, Package.resolved to lockfile path filter
- Expose comment-summary-in-pr as a workflow_call input (default:
  on-failure) so callers can control PR comment verbosity; avoids
  requiring pull-requests: write for callers that don't want comments

Note: upload-artifact v7.0.1 SHA (043fb46d) is confirmed correct —
v7.0.1 is the current latest release of actions/upload-artifact.
The || fallback pattern on inputs.* is intentional and necessary:
inputs.default is only applied on workflow_call; pull_request-triggered
runs receive an empty string from the inputs context, requiring the
fallback to supply the default value.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: enterprise governance, rulesets, and observability

2 participants