chore(ops): harden OpenSSF baseline, DCO, and release SBOMs - #855
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds DCO validation, conditional CI execution, signed CycloneDX SBOM release artifacts, dry-run reporting, direct fuzzing builds, and updates governance, security, contribution, architecture, release, and verification documentation. ChangesRelease assurance and CI governance
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ReleaseWorkflow
participant Syft
participant Cosign
participant GitHubRelease
ReleaseWorkflow->>Syft: Generate and checksum CycloneDX SBOM
Syft-->>ReleaseWorkflow: Return SBOM path and metadata
ReleaseWorkflow->>Cosign: Sign DMG and SBOM
Cosign-->>ReleaseWorkflow: Return Sigstore bundles
ReleaseWorkflow->>GitHubRelease: Publish release assets
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.bestpractices.json:
- Around line 22-23: Update the osps_br_02_02 status and justification in
.bestpractices.json to accurately reflect that Thaw.dmg and
Thaw.dmg.sigstore.json are not versioned with the release tag, or version those
assets in the release workflow and retain the compliant status. Ensure the
assurance declaration matches the actual published asset names.
In @.github/GOVERNANCE.md:
- Around line 226-231: Update the “Update feed / Pages” guidance to describe
legacy appcast continuity through the CI mirror of appcast.xml to the legacy
repository, rather than claiming existing installs rely on an HTTP redirect.
Align the wording with the documented recovery procedure in RELEASES.md while
preserving the ownership and project-controlled-domain guidance.
In @.github/workflows/pr-metadata.yml:
- Around line 295-301: Update the sign-off parsing in the commit-message
validation flow around signoffEmails so it examines only Git’s trailing trailer
block, not arbitrary lines in the message. Extract the contiguous trailer
section at the end of message before matching Signed-off-by entries, then
preserve the existing normalization and authorEmail comparison behavior.
- Around line 266-284: The PR metadata workflow must not bypass DCO checks based
on attacker-controlled PR titles or arbitrary login text. Remove the title-based
CrowdIn early return in the enclosing step, and update the commit loop around
dcoBotLogins so exemptions apply only to the explicitly allowlisted verified bot
identities; remove the login.toLowerCase().includes('crowdin') condition while
preserving DCO validation for ordinary commits.
In @.github/workflows/release.yml:
- Around line 144-160: Update the “Generate CycloneDX SBOM” step to avoid
executing the unsigned remote Syft installer via curl-pipe-to-sh. Install Syft
outside the signing job, or replace this installation with an authenticated
pinned action, checksum verification, or attestation before execution; preserve
the pinned SYFT_VERSION and existing SBOM flow.
In `@docs/VERIFYING_RELEASES.md`:
- Around line 89-93: Update the Sigstore verification commands for both the DMG
and SBOM in VERIFYING_RELEASES.md to use exact certificate identity/workflow
predicates instead of the broad --certificate-identity-regexp match. Constrain
each verification to the expected Thaw Release workflow identity and an
appropriate release/tag ref, preserving the existing verification inputs and
options.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a583a6d-0801-496e-b79f-d2079fcd3605
📒 Files selected for processing (9)
.bestpractices.json.github/CONTRIBUTING.md.github/GOVERNANCE.md.github/SECURITY.md.github/workflows/pr-metadata.yml.github/workflows/release.ymldocs/ARCHITECTURE.mddocs/RELEASES.mddocs/VERIFYING_RELEASES.md
b80ba07 to
1db273c
Compare
b71bfac to
033c56d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.bestpractices.json:
- Around line 82-83: Update the justification for osps_qa_02_02_justification to
accurately describe the published SBOM assets, or update the release workflow’s
SBOM upload step to publish ${SBOM_PATH}.sha256 alongside the SBOM and its
.sigstore.json bundle. Ensure the final statement does not claim a published
SHA-256 checksum unless the release asset list uploads it.
In @.github/GOVERNANCE.md:
- Around line 111-117: Update the release workflow policy implementation
associated with release.yml so secret-consuming jobs require the documented
authorization, preferably by assigning the required `release` Environment with
mandatory reviewers or enforcing equivalent actor/organization checks before
those jobs run. If the workflow cannot enforce this distinction, revise the
governance table to separately document who may dispatch releases and who may
consume release secrets.
In @.github/workflows/pr-metadata.yml:
- Around line 39-53: Deduplicate the Crowdin detection used by the “Apply
labels” and “Check PR conventions” steps: compute the predicate once in a shared
metadata step, expose its boolean result as a step output, and have both steps
consume that output for their early-return logic. Remove both local
isCrowdinSync predicate copies while preserving the existing trusted
head-branch, same-repository, and maintainer-association checks.
- Around line 280-340: Update the dcoBotLogins allowlist to include only the
repository’s verified Crowdin commit login; remove crowdin-bot unless the
integration is explicitly confirmed to use that exact Crowdin-controlled
account, while retaining crowdin[bot] when applicable.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 3e7e2e7d-02dd-4a3d-82e2-5dd616539ef4
📒 Files selected for processing (10)
.bestpractices.json.github/CONTRIBUTING.md.github/GOVERNANCE.md.github/SECURITY.md.github/workflows/fuzz.yml.github/workflows/pr-metadata.yml.github/workflows/release.ymldocs/ARCHITECTURE.mddocs/RELEASES.mddocs/VERIFYING_RELEASES.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 5-71: Gate the checkout step in the static_analysis job on
needs.changes.outputs.code == 'true', matching the existing conditions on its
downstream validation and lint steps. Leave the checkout action and fetch-depth
unchanged for code-related changes, while ensuring non-code PRs skip the
full-history clone.
- Around line 170-217: Unify coverage configuration between the SonarQube Scan
step and sonar-project.properties by removing the unconditional
sonar.coverageReportPaths setting from sonar-project.properties. Keep the
existing gated args expression in the SonarQube Scan step, so coverage.xml is
referenced only when needs.changes.outputs.code is true.
In @.github/workflows/fuzz.yml:
- Around line 71-74: Update the Linux fuzzer compile command in the workflow to
use the complete directory-based source set defined for the FuzzSettingsURI
target in Fuzzing/Package.swift, rather than listing individual Swift files.
Preserve the existing compiler flags and output while ensuring newly added
sources are included automatically.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: df961a3e-1d57-4a81-bd0c-d863a9384550
📒 Files selected for processing (3)
.github/workflows/ci.yml.github/workflows/fuzz.ymlFuzzing/Package.swift
Enforce DCO in PR Metadata, document support/SCA/governance leads, add Baseline proposals, and emit cosign-signed CycloneDX SBOMs on release. Signed-off-by: diazdesandi <diazdesandi@proton.me>
Follow-up to the OpenSSF baseline work, addressing PR review findings. Supply chain: - Install Syft from a release tarball pinned by version *and* sha256 instead of piping an install.sh fetched from a mutable git tag into sh. - Generate the SBOM from the tagged source tree rather than the exported .app. SwiftPM dependencies are statically linked, so only Package.resolved records them; the app-bundle scan produced zero components. Build output and CI workflows are excluded so the SBOM lists shipped dependencies rather than release tooling. Verified: 0 -> 5 components. - Publish Thaw.dmg.sha256 alongside the SBOM checksum. PR Metadata / DCO: - Detect Crowdin sync PRs by head branch, same-repo origin and maintainer association instead of PR title. A fork could previously skip every convention check, DCO included, by putting "crowdin" in the title. - Drop the login substring exemption so only allowlisted bot identities are exempt; any account named "...crowdin..." was skipping DCO. - Match Signed-off-by only within the trailing trailer block, so a sign-off quoted in the commit body no longer satisfies the check. Docs and assurance: - Anchor the cosign identity pattern to the release workflow so a bundle signed by any other workflow in the repo is no longer accepted. The ref component stays open because the tag is a workflow_dispatch input, not the dispatch ref. - Correct the legacy appcast continuity note: GitHub Pages does not redirect across an owner transfer, so continuity comes from the CI mirror. - Correct osps_br_02_02, which claimed all assets carry the version in their filename; the DMG keeps a stable name for Homebrew and Sparkle URLs. - Replace admin-only settings URLs with public evidence in three controls. Signed-off-by: diazdesandi <diazdesandi@proton.me>
The job runs in the swift:6.2-noble container, and Actions defaults run
steps inside a container to `sh -e {0}`. Ubuntu's sh is dash, which has no
`pipefail`, so both steps died on line 1:
/__w/_temp/....sh: 1: set: Illegal option -o pipefail
The workflow has never passed since the fuzz target was added; all three
runs failed this way at Build fuzzer, before any Swift compilation. Set a
job-level bash default, matching the convention in ci.yml. This also covers
the Fuzz step, which has the same latent bug but was never reached.
Signed-off-by: diazdesandi <diazdesandi@proton.me>
Rehearsing a release previously meant creating draft GitHub Releases on two repositories and writing permanent, public entries to the Sigstore transparency log, none of which can be fully undone. Add a `dry_run` workflow_dispatch input. Build, notarize, SBOM generation and Sparkle packaging run as normal; creating either GitHub Release, cosign signing, the appcast push and the legacy mirror are all skipped. The job summary then reports what a real run would have done: every asset that would be uploaded and where, with exact size and SHA-256, whether the release would be draft or published, the SBOM component inventory, and a unified diff of the generated appcast against both currently live feeds. The DMG checksum, SBOM and appcast are attached as a dry-run-<tag> artifact; the DMG itself is not, since it is large and the real run rebuilds it. Dry runs use their own concurrency group so they neither queue behind nor block a real release. Signed-off-by: diazdesandi <diazdesandi@proton.me>
SwiftPM rewrites the ELF entry via `--defsym main=<Module>_main`, but `-parse-as-library` never emits that symbol, so the fuzzer link fails once the bash/pipefail issue no longer masks it. Drive swiftc directly as the Swift libFuzzer docs do; keep swift build for the macOS replay.
Separate release dispatch from secret consumption in GOVERNANCE and enforce an org-owner actor allowlist in release.yml until a protected release Environment exists. Deduplicate Crowdin sync detection in PR metadata and drop the unverified crowdin-bot DCO exemption.
Skip the full-history clone when no code paths change, keep coverage.xml only on the Sonar args path so docs/chore scans do not reference a missing file, and compile the fuzzer from the target directory glob.
bbc3b82 to
9c87d96
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/release.yml (1)
546-557: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winFail dry runs when required artifacts are missing.
actions/upload-artifactwith multiplepathentries only fails when no files are found across all entries; it can succeed when required files are missing and warnings are logged. Add an explicit existence check for each required path before uploading, then useif-no-files-found: erroras the fallback.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release.yml around lines 546 - 557, Update the “Upload dry run artifacts” step to explicitly verify that each required artifact path exists before invoking actions/upload-artifact, failing the dry run when any file is missing. Keep the existing artifact paths, and change if-no-files-found from warn to error as a fallback.
♻️ Duplicate comments (1)
.github/workflows/pr-metadata.yml (1)
296-309: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire the Git trailer boundary.
Git trailers require an empty line before the trailing trailer group.
trailerBlockaccepts a finalSigned-off-by:line without that boundary. A non-trailer body line can therefore satisfy DCO validation. (git-scm.com)Proposed fix
const trailerBlock = (message) => { const lines = message.replace(/\s+$/, '').split('\n'); let start = lines.length; for (let i = lines.length - 1; i >= 0; i--) { const line = lines[i]; if (line.trim() === '') break; // A trailer is `Key: value`, or a folded continuation line. if (!/^[A-Za-z][A-Za-z0-9-]*:\s/.test(line) && !/^\s+\S/.test(line)) break; start = i; } + if (start === lines.length || start === 0 || lines[start - 1].trim() !== '') { + return []; + } return lines.slice(start); };#!/bin/bash set -euo pipefail without_separator=$( printf '%s\n' $'feat: valid\nSigned-off-by: Alice <alice@example.com>' | git interpret-trailers --parse ) test -z "$without_separator" with_separator=$( printf '%s\n' $'feat: valid\n\nSigned-off-by: Alice <alice@example.com>' | git interpret-trailers --parse ) printf '%s\n' "$with_separator" | grep -Fx 'Signed-off-by: Alice <alice@example.com>'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/pr-metadata.yml around lines 296 - 309, Update trailerBlock so it only returns a trailer group when the group is preceded by an empty line, matching Git trailer boundary rules. Ensure a message containing only a body line followed directly by Signed-off-by: yields no trailer block, while preserving valid separated trailer parsing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release.yml:
- Around line 68-81: Replace the string-based actor check in the “Authorize
release dispatcher” step with a protected GitHub environment requiring
designated reviewers, and associate that environment with the signing and
publication jobs or steps. Remove reliance on the ACTOR allowlist as the release
trust boundary while preserving the existing release flow after environment
approval.
---
Outside diff comments:
In @.github/workflows/release.yml:
- Around line 546-557: Update the “Upload dry run artifacts” step to explicitly
verify that each required artifact path exists before invoking
actions/upload-artifact, failing the dry run when any file is missing. Keep the
existing artifact paths, and change if-no-files-found from warn to error as a
fallback.
---
Duplicate comments:
In @.github/workflows/pr-metadata.yml:
- Around line 296-309: Update trailerBlock so it only returns a trailer group
when the group is preceded by an empty line, matching Git trailer boundary
rules. Ensure a message containing only a body line followed directly by
Signed-off-by: yields no trailer block, while preserving valid separated trailer
parsing.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8a577553-2a25-4caa-abc2-2d872091f25f
📒 Files selected for processing (3)
.github/GOVERNANCE.md.github/workflows/pr-metadata.yml.github/workflows/release.yml
Replace the actor allowlist with environment: release so signing and publication wait for designated reviewers (stonerl, diazdesandi).
Keep diazdesandi as the only required release reviewer so stonerl is not notified on every run; other owners retain admin bypass.
|



Summary
Harden OpenSSF Baseline readiness: enforce DCO on PRs, clarify governance/support/SCA docs, add BadgeApp proposal JSON, and ship cosign-signed CycloneDX SBOMs from the release workflow (needed for today’s release tag to include the new
release.yml).Scope: Ops / supply-chain only (workflows + docs). No app behavior change.
Closes: N/A
PR Type
Describe what this change does (not the linked issue’s request kind). Bug reports use
bugon issues; bug fixes usefixon PRs.If you tick Feature or Refactor and touch more than ~20 files, please mention why this can’t be split.
Area
Product surfaces (optional when the change is not about the app UI). Path-based labeling also applies.
Use PR Type for what changed (
CI/CD,Documentation,Other/ chore, etc.).Use
opsfor where when it is repo operations: CI, release, GitHub hygiene, scripts, lint/sonar config — not a product surface.menubar
icebar
layout
appearance
settings
onboarding
permissions
profiles
hotkeys
updates
ops
Does this PR introduce a breaking change?
What is the new behavior?
Signed-off-bymatching the author email (bots/Crowdin exempt). Documented in CONTRIBUTING..bestpractices.jsonproposes OSPS answers (SBOM left Unmet until a release actually attaches assets; VEX and SCA hard-gate still Unmet).Thaw_<tag>.cdx.jsonwith pinned Syft, basename.sha256, cosign-sign SBOM, upload with DMG on the Thaw GitHub Release. VERIFYING_RELEASES / RELEASES updated.PR Checklist
swiftformat .to keep the code style consistent.xcodebuild test …orswift test --package-path MenuBarModel.developmentbranch.Test commands run:
build/Export/Thaw.app).Known limitations / follow-ups
osps_qa_02_02Met in.bestpractices.json/ BadgeApp after the first SBOM-bearing release.osps_vm_04_02), SCA merge-blocking gate (osps_vm_05_03).release.yml.Other information
After merge: BadgeApp → Save (and continue) 🤖 to import
.bestpractices.json.Summary by CodeRabbit