Skip to content

Sign Radar Hub OCI charts with keyless Cosign - #67

Merged
hisco merged 2 commits into
mainfrom
hub-chart-keyless-signing
Nov 12, 2026
Merged

hisco merged 2 commits into
mainfrom
hub-chart-keyless-signing

Conversation

@hisco

@hisco hisco commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Radar Hub OCI charts are currently published without signatures. The publisher also skips existing OCI versions, so retrying after an upload cannot complete a failed signing step.

This pilot signs only ghcr.io/skyhook-io/charts/radar-hub with GitHub OIDC and pinned Cosign, then verifies the exact digest against the release workflow identity and GitHub issuer before the job succeeds. All actions in the OIDC-capable job are pinned to immutable commits. The classic Helm repository, Radar, and connector stay unsigned. The README shows how to verify a pulled Hub digest and install that same archive.

Retries use the original GitHub Release archives, even when chart-releaser reports no changed charts or leaves no local packages. Existing releases are skipped by chart-releaser, but their OCI digests still go through Hub signing and verification. Review the trust checks in .github/scripts/publish-oci.sh: drafts are excluded, actual tags must resolve to the run SHA, and the Hub archive's file names and contents must match the checked-out source chart before it can be pushed or signed. Tar/gzip metadata can differ; the original published archive stays the upload input. An existing Hub OCI archive must also match those published bytes. Registry lookup failures cannot become replacement pushes.

Validation: 16 process-boundary tests run the actual publisher with real Helm 3.14 packages, including fresh publication, existing-digest retries, mixed batches, signing/verification failures, wrong identity/issuer, replaced release assets, and malformed archives. A real published Hub 1.15.0 archive also matched the checked-out chart's packaged file contents. Bash syntax, ShellCheck, actionlint, and whitespace checks pass. A read-only CI job runs the publisher tests. No rendered UI changed, so visual testing does not apply.

Actual GitHub OIDC issuance and GHCR signature storage still need an approved Hub publish. This PR does not publish or merge a release. Related to skyhook-io/radar#1377; this is the Hub OCI pilot, not full Helm provenance coverage.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

PR Summary by Qodo

Sign Radar Hub OCI charts with keyless Cosign

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Grey Divider

AI Description

• Sign and verify Radar Hub OCI digests using the release workflow’s GitHub OIDC identity.
• Recover release archives on retries so failed signing can complete without replacing existing
 chart versions.
• Add publisher tests and instructions for verifying a digest before installing the pulled archive.
Diagram

graph TD
  W["Release workflow"] --> R["Release admission"] --> P["Release archive"] --> Q{"OCI exists?"}
  Q -->|existing Hub| M["Compare archive"] --> D["OCI digest"] -->|Hub only| S["Cosign sign verify"]
  Q -->|missing| U["Push chart"] --> D
  Q -->|existing other charts| D
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Dedicated release-triggered signing workflow
  • ➕ Separates signing retries from chart publishing.
  • ➕ Allows independent signing permissions and execution.
  • ➖ Requires coordination between publishing and signing jobs.
  • ➖ Makes release-to-digest admission and workflow identity more complex.

Recommendation: Keep signing in the release workflow for this Hub-only pilot. It can fail the publishing run when verification fails and reuse the same admitted release archive on retries; a separate workflow would add coordination without a clear benefit at this scope.

Files changed (5) +498 / -25

Enhancement (2) +114 / -25
publish-oci.shPublish release archives and sign admitted Hub digests +101/-0

Publish release archives and sign admitted Hub digests

• Downloads non-draft release assets whose tags resolve to the run commit, then pushes missing OCI versions or retains existing digests. It checks existing Hub archives against release assets and signs and verifies Hub digests with Cosign.

.github/scripts/publish-oci.sh

release.ymlEnable keyless signing and retryable OCI publication +13/-25

Enable keyless signing and retryable OCI publication

• Grants the release job OIDC permission, installs pinned Cosign, and tells chart-releaser to skip existing releases. Replaces the changed-chart-gated OCI loop with the publisher script so retries can sign existing Hub digests.

.github/workflows/release.yml

Tests (2) +359 / -0
publish-oci-test.pyExercise OCI publishing and signing across retry and failure paths +319/-0

Exercise OCI publishing and signing across retry and failure paths

• Adds 13 process-boundary tests using real Helm packages and isolated CLI shims. They cover fresh and existing versions, partial retries, release admission, archive mismatches, registry errors, and signing or verification failures.

.github/ci/publish-oci-test.py

release-validation.ymlRun publisher checks in read-only CI +40/-0

Run publisher checks in read-only CI

• Adds path-filtered pull-request and main-branch validation with Helm 3.14, Bash syntax checks, ShellCheck, and the publisher test suite.

.github/workflows/release-validation.yml

Documentation (1) +25 / -0
README.mdDocument Radar Hub OCI verification and installation +25/-0

Document Radar Hub OCI verification and installation

• Shows how to pull a Hub chart, verify its digest against the expected issuer and workflow identity, and install that pulled archive. Clarifies that older versions and the other chart distribution paths may remain unsigned.

README.md

@qodo-free-for-open-source-projects

qodo-free-for-open-source-projects Bot commented Nov 11, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (3) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Valid Hub releases remain unsigned 🐞 Bug ≡ Correctness
Description
publish-oci.sh discards a release unless its target_commitish metadata equals GITHUB_SHA,
before resolving the actual tag. When that metadata names a branch rather than the commit, the
script never downloads the archive or reaches signing and can exit successfully as though there were
no releases.
Code

.github/scripts/publish-oci.sh[24]

+  [[ "$release_sha" == "$GITHUB_SHA" ]] || continue
Evidence
The workflow does not set a release target commit; the script itself notes that target_commitish is
only metadata for an existing tag. Nonetheless, it filters on that field before its authoritative
tag lookup, and treats an empty package set as a successful no-op.

.github/workflows/release.yml[46-50]
.github/scripts/publish-oci.sh[19-43]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The publisher filters releases by `target_commitish`, which can be branch metadata even when the release tag points to the run commit. Such releases are skipped before their tags are checked.
## Fix Focus Areas
- .github/scripts/publish-oci.sh[19-35]
- .github/ci/publish-oci-test.py[179-184]
## Recommended Fix
Use the release tag's resolved commit to decide whether to process a matching, non-draft release. Add a test where `target_commitish` is `main` but the tag resolves to `GITHUB_SHA`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Edited release assets can be signed 🐞 Bug ⛨ Security
Description
publish-oci.sh authenticates the release tag but checks the downloaded Hub archive only for its
filename, chart name, and version before pushing and signing it. If someone with release-asset
editing access replaces the archive while that OCI version is absent, a later publishing attempt can
sign the replacement under the trusted workflow identity.
Code

.github/scripts/publish-oci.sh[R34-35]

+  gh release download "$tag" --repo "$GITHUB_REPOSITORY" \
+    --pattern "${tag}.tgz" --dir "$work_dir/packages"
Evidence
The tag check establishes a commit for the tag, not the bytes of its downloadable asset. The script
validates only archive metadata, then pushes an absent version and signs the resulting digest; its
byte comparison applies only when an OCI version already exists.

.github/scripts/publish-oci.sh[19-35]
.github/scripts/publish-oci.sh[48-59]
.github/scripts/publish-oci.sh[65-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The signing input is a mutable GitHub Release asset whose contents are not authenticated by the release-tag check. A replacement asset with the same chart metadata can be signed on first OCI publication.
## Fix Focus Areas
- .github/scripts/publish-oci.sh[27-35]
- .github/scripts/publish-oci.sh[48-59]
- .github/scripts/publish-oci.sh[80-99]
## Recommended Fix
Require an authenticated, commit-bound digest or equivalent provenance for the release archive before pushing or signing it. Preserve that check on retries when local chart-releaser packages are unavailable.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Mutable actions gain signing access 🐞 Bug ⛨ Security
Description
The new job-level id-token: write permission also applies to checkout, Helm setup, and
chart-releaser, which run through mutable action tags before Cosign. If one of those tags is
replaced, its code can request an OIDC token in the release workflow rather than leaving that
capability confined to the signing step.
Code

.github/workflows/release.yml[24]

+      id-token: write
Evidence
The PR adds OIDC permission at job scope. The same job executes three tag-referenced actions before
invoking the script that signs using GitHub Actions OIDC.

.github/workflows/release.yml[19-50]
.github/workflows/release.yml[89-95]
.github/scripts/publish-oci.sh[93-99]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Granting OIDC permission to the whole release job exposes it to earlier actions referenced by mutable tags.
## Fix Focus Areas
- .github/workflows/release.yml[19-51]
- .github/workflows/release.yml[89-95]
## Recommended Fix
Pin the actions executing in the OIDC-capable job to immutable commits, or move signing into a narrowly scoped job that does not execute the mutable actions.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Tip of the day
💡 Did you know, you can open the rule or relationship behind a finding in one click

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

@hisco
hisco force-pushed the hub-chart-keyless-signing branch from cde59b8 to 756f344 Compare November 11, 2026 14:53
@hisco

hisco commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author
  1. The branch-valued target_commitish case is outside this workflow's retry path. The pinned chart-releaser action passes the exact checkout SHA to cr upload with -c "$(git rev-parse HEAD)" (source). Recovery deliberately admits only those releases, then independently verifies the actual tag commit.
  2. The Hub archive's regular-file names and contents are now compared with a package of the checked-out chart before either a fresh push or existing-digest signing. Replaced templates, extra/missing files, duplicate names, links and unsafe paths fail. Packaging metadata can differ, and the original published bytes remain the upload input.
  3. Checkout, Helm setup and chart-releaser are now pinned to immutable commit SHAs in the OIDC-capable job.

The publisher tests cover both absent-OCI and matching-existing-OCI replacement cases, plus packaging-only differences.

@cursor

cursor Bot commented Nov 11, 2026

Copy link
Copy Markdown

Bugbot needs on-demand usage enabled

Bugbot uses usage-based billing for this team and requires on-demand usage to be enabled.

A team admin can enable on-demand usage in the Cursor dashboard.

@hisco

hisco commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

/agentic_review

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 756f344

@hisco

hisco commented Nov 11, 2026

Copy link
Copy Markdown
Contributor Author

@qodo please close these three findings against final commit 756f344d6a24e52311d5232750da349e1a4fd815:

  1. Valid Hub releases remain unsigned: dismiss as a false positive for this workflow. The pinned chart-releaser action explicitly passes -c "$(git rev-parse HEAD)" (source). Its releases have SHA-valued target_commitish; manual branch-valued release backfill is outside this publisher's scope. The actual tag is checked independently.
  2. Edited release assets can be signed: fixed in this commit. The full archive member names and bytes are compared with packaged checked-out source before either OCI path (source check). The statement that only name/version are checked no longer matches the code. The tests reject replaced same-version templates for both absent and matching-existing OCI artifacts.
  3. Mutable actions gain signing access: fixed in this commit. All four actions in the OIDC-capable job now use immutable commit SHAs (workflow).

Please mark 2 and 3 resolved as implemented. Their current claims are stale after these changes. All 16 publisher tests and all 3 CI checks pass on this head.

@qodo-free-for-open-source-projects

Copy link
Copy Markdown

please close these three findings against final commit 756f344d6a24e52311d5232750da349e1a4fd815:

  1. Valid Hub releases remain unsigned: dismiss as a false positive for this workflow. The pinned chart-releaser action explicitly passes -c "$(git rev-parse HEAD)" (source). Its releases have SHA-valued target_commitish; manual branch-valued release backfill is outside this publisher's scope. The actual tag is checked independently.
  2. Edited release assets can be signed: fixed in this commit. The full archive member names and bytes are compared with packaged checked-out source before either OCI path (source check). The statement that only name/version are checked no longer matches the code. The tests reject replaced same-version templates for both absent and matching-existing OCI artifacts.
  3. Mutable actions gain signing access: fixed in this commit. All four actions in the OIDC-capable job now use immutable commit SHAs (workflow).

Please mark 2 and 3 resolved as implemented. Their current claims are stale after these changes. All 16 publisher tests and all 3 CI checks pass on this head.

I can't close these yet. This PR's findings index is empty, so I can't match findings 1, 2, and 3 from the published report to the stable IDs the dismiss tool needs. I also won't guess IDs.

Even with IDs, I can't mark findings as implemented. My only closing action is dismissal, with one of these reasons: false_positive, intentional, deferred, or rejected.

Here's how I'd handle each one:

  • Finding 1 (Valid Hub releases remain unsigned): Dismissal as false_positive fits your reasoning, since the pinned chart-releaser action sets target_commitish to the commit SHA. Dismissal is limited to the PR author, so I'll only apply it if you are the author.
  • Findings 2 and 3 (Edited release assets can be signed; Mutable actions gain signing access): I can dismiss these, but the implemented status has to come from the fix landing on the PR head. I can't set it.

If the findings index is populated for this head, reply with the finding numbers and I'll dismiss the ones you name. Otherwise, rerun the review on 756f344 so the findings are indexed, and then ask me again.

@hisco
hisco merged commit aea46d2 into main Nov 12, 2026
3 checks passed
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.

1 participant