docs: add ADR for sigstore handler#2300
Merged
jakobmoellerdev merged 4 commits intoApr 16, 2026
Merged
Conversation
✅ Deploy Preview for ocm-website canceled.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughAdded an ADR documenting Sigstore/Cosign keyless signing integration for the OCM CLI (choosing delegation to the external Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant OCM as OCM CLI
participant Exec as Executor/Downloader
participant Cache as Cache (~/.cache/ocm/cosign)
participant Cosign as cosign (binary)
participant Sigstore as Sigstore Services (Fulcio/Rekor)
User->>OCM: ocm sign / ocm verify (sigstore algorithm)
OCM->>Exec: request cosign execution
Exec->>Cache: check for pinned cosign version
alt cached
Cache-->>Exec: return binary path
else not cached
Exec->>Cosign: download pinned cosign -> Cache
Cache-->>Exec: store binary path
end
Exec->>Cosign: invoke cosign with args
Cosign->>Sigstore: perform keyless signing/verification (Fulcio/Rekor)
Sigstore-->>Cosign: return proof/result
Cosign-->>Exec: return exit code & output
Exec-->>OCM: relay result
OCM-->>User: display success/failure
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.github/config/wordlist.txt (1)
796-796: Consider adding a trailing newline at EOF.The file ends without a trailing newline character. Many coding standards and POSIX conventions recommend that text files end with a newline character. This can prevent issues with certain tools and version control systems.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/config/wordlist.txt at line 796, The file ends with the token "ux" and lacks a trailing newline; add a single newline character at the end of the file so the last line ("ux") is terminated by a newline (ensuring POSIX-compliant EOF newline and avoiding tooling/version-control issues).docs/adr/0016_sigstore_integration.md (3)
38-38: Consider clarifying what SHA256 verification applies to.The phrase "SHA256 verification" would be clearer if it explicitly states that it's verifying the integrity of the downloaded
cosignbinary. This helps readers understand the security mechanism.📝 Suggested clarification
-The main trade-off (dependency on an external binary) is fully mitigated by the transparent auto-download and caching mechanism with SHA256 verification. +The main trade-off (dependency on an external binary) is fully mitigated by the transparent auto-download and caching mechanism with SHA256 verification of the downloaded binary.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/0016_sigstore_integration.md` at line 38, Clarify that "SHA256 verification" refers specifically to verifying the integrity of the downloaded cosign binary: update the sentence that currently reads "SHA256 verification" to explicitly state something like "SHA256 verification of the downloaded cosign binary (the auto-downloaded release artifact)" so readers know which artifact is being checksummed and verified.
77-77: Consider clarifying "built-in OCM plugin" terminology.The phrase "built-in OCM plugin" might be slightly confusing to readers—is it a built-in handler or a dynamically loaded plugin? Since the code shows handlers are retrieved via
pluginManager.SigningRegistry.GetPlugin(), consider clarifying whether this will be:
- A built-in handler registered at startup, or
- A plugin that ships with the OCM binary, or
- Something else
This clarification would help readers better understand the deployment model.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/0016_sigstore_integration.md` at line 77, Clarify the "built-in OCM plugin" wording in the ADR by stating exactly how the Sigstore handler is provided: specify whether the handler is registered at startup as a built-in handler, shipped as a plugin bundled with the OCM binary, or dynamically loadable via pluginManager.SigningRegistry.GetPlugin(); update the sentence about implementation to explicitly say e.g. "implemented as a built-in handler registered at startup (not dynamically loaded), retrieved via pluginManager.SigningRegistry.GetPlugin()" or the alternative deployment model chosen so readers understand the registration and loading semantics.
75-78: Consider expanding security considerations for binary download and execution.This ADR documents a security-critical feature (signing/verification) that will download and execute an external binary. While SHA256 verification is mentioned on Line 38, the ADR would benefit from a dedicated section on security considerations covering:
- Binary integrity verification (checksums/signatures)
- Source verification (download URL validation)
- Secure storage and permission handling of the cached binary
- Mitigation of supply chain risks
- Handling of compromised or unavailable binaries
This would strengthen the ADR and provide clear security guidance for implementation.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/adr/0016_sigstore_integration.md` around lines 75 - 78, Add a dedicated "Security Considerations" subsection to the ADR addressing binary download and execution for the cosign integration: cover explicit binary integrity verification (e.g., SHA256 and signed checksums referenced at Line 38), strict source validation of the cosign download URL, secure storage and file permissions for the cached binary under ~/.cache/ocm/cosign, supply-chain mitigation strategies (pinning versions managed by Renovate, fallback/rollback plans, and vendor attestations), and operational guidance for handling compromised or unavailable binaries; reference the user-facing commands ('ocm sign' and 'ocm verify') and the cosign binary name so implementers know where to apply these controls.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/config/wordlist.txt:
- Line 796: The word "ux" is out of alphabetical order at the end of the
wordlist; remove the trailing "ux" entry and insert "ux" immediately after the
existing "utils" entry (i.e., place it between "utils" and "uwe") so the list
remains alphabetically sorted.
---
Nitpick comments:
In @.github/config/wordlist.txt:
- Line 796: The file ends with the token "ux" and lacks a trailing newline; add
a single newline character at the end of the file so the last line ("ux") is
terminated by a newline (ensuring POSIX-compliant EOF newline and avoiding
tooling/version-control issues).
In `@docs/adr/0016_sigstore_integration.md`:
- Line 38: Clarify that "SHA256 verification" refers specifically to verifying
the integrity of the downloaded cosign binary: update the sentence that
currently reads "SHA256 verification" to explicitly state something like "SHA256
verification of the downloaded cosign binary (the auto-downloaded release
artifact)" so readers know which artifact is being checksummed and verified.
- Line 77: Clarify the "built-in OCM plugin" wording in the ADR by stating
exactly how the Sigstore handler is provided: specify whether the handler is
registered at startup as a built-in handler, shipped as a plugin bundled with
the OCM binary, or dynamically loadable via
pluginManager.SigningRegistry.GetPlugin(); update the sentence about
implementation to explicitly say e.g. "implemented as a built-in handler
registered at startup (not dynamically loaded), retrieved via
pluginManager.SigningRegistry.GetPlugin()" or the alternative deployment model
chosen so readers understand the registration and loading semantics.
- Around line 75-78: Add a dedicated "Security Considerations" subsection to the
ADR addressing binary download and execution for the cosign integration: cover
explicit binary integrity verification (e.g., SHA256 and signed checksums
referenced at Line 38), strict source validation of the cosign download URL,
secure storage and file permissions for the cached binary under
~/.cache/ocm/cosign, supply-chain mitigation strategies (pinning versions
managed by Renovate, fallback/rollback plans, and vendor attestations), and
operational guidance for handling compromised or unavailable binaries; reference
the user-facing commands ('ocm sign' and 'ocm verify') and the cosign binary
name so implementers know where to apply these controls.
🪄 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: CHILL
Plan: Pro
Run ID: cff585cc-3716-43f0-95a4-8c86f03bf1fa
📒 Files selected for processing (2)
.github/config/wordlist.txtdocs/adr/0016_sigstore_integration.md
Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
ed276c5 to
a09a1c2
Compare
Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
389037f to
2ba1b73
Compare
jakobmoellerdev
approved these changes
Apr 16, 2026
matthiasbruns
approved these changes
Apr 16, 2026
This was referenced Apr 21, 2026
4 tasks
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
Create an ADR for the sigstore signing handler implementation. This ADR has been created from the comparison doc created in the spike in open-component-model/ocm-project#995