feat: plugins doble sing#64
Merged
Merged
Conversation
Add tag-driven plugin release automation with immutable artifact paths, signing, and catalog updates so plugin releases deploy consistently without per-plugin workflow changes.
…d update manifest structure
Deploying corvus with
|
| Latest commit: |
19b89e8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://a15487b9.corvus-42x.pages.dev |
| Branch Preview URL: | https://feature-plugins-doble-sing.corvus-42x.pages.dev |
Contributor
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughRefactors the plugin publish workflow to enforce cosign keyless OIDC signing, adds plugin signature types and policy-driven verification in the agent runtime, updates configuration to accept optional identity regexes, adjusts docs, and removes signature blocks from some published plugin manifests. Changes
Sequence DiagramsequenceDiagram
participant Agent as Agent Runtime
participant Manifest as Manifest Source
participant Cosign as Cosign Verifier
participant Storage as Local Storage
Agent->>Manifest: Fetch plugin manifest & artifact
Manifest-->>Agent: Return manifest (may include signature metadata)
Agent->>Agent: Determine SignaturePolicy for source/plugin
alt Signature required (keyless OIDC)
Agent->>Manifest: Fetch signature bytes & certificate
Manifest-->>Agent: Signature + certificate
Agent->>Cosign: verify_blob_with_cosign(artifact, signature, certificate)
Cosign-->>Agent: Verification result
alt Verification succeeds
Agent->>Storage: Persist artifact, signature, certificate
Storage-->>Agent: Persisted
else Verification fails
Agent-->>Agent: Abort installation
end
else No signature required
Agent->>Storage: Persist artifact only
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
Deploying corvus-plugins with
|
| Latest commit: |
19b89e8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c4a8bbb2.corvus-plugins.pages.dev |
| Branch Preview URL: | https://feature-plugins-doble-sing.corvus-plugins.pages.dev |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-21 to 2026-02-21 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/publish-plugins.yml (1)
392-412:⚠️ Potential issue | 🟡 MinorUpdate cosign to v2.6.1 or later.
The installation is properly hardened with checksum verification. However, v2.4.1 (used in the code) is outdated; the latest v2.x release is v2.6.1 (Oct 2, 2025), with v3.0.3 now available as the latest stable release (Dec 10, 2025). Update to at least v2.6.1 for security fixes and improvements, or evaluate migration to v3.0.3.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-plugins.yml around lines 392 - 412, Update the hardcoded COSIGN_VERSION (and derived COSIGN_BASE_URL/COSIGN_BINARY) from "v2.4.1" to at least "v2.6.1" (or "v3.0.3" if you choose to migrate), and ensure the downloaded checksum file and grep target still match the new COSIGN_BINARY name; specifically, change COSIGN_VERSION to the desired release, verify COSIGN_BINARY equals the release asset name on GitHub for that version, and confirm the grep/sha256sum steps still locate and validate the correct checksum for the new release.
🧹 Nitpick comments (5)
.github/workflows/publish-plugins.yml (2)
426-448: Keyless OIDC signing and verification look correct.The
sign-blob+verify-blobpair with--certificate-oidc-issuerand--certificate-identity-regexpis the standard approach for GitHub Actions keyless signing. Theid-token: writepermission at line 39 is required and present.One minor note: the
.in.githubwithin the--certificate-identity-regexpis an unescaped regex metacharacter. In practice, exploiting this is infeasible given the specificity of the surrounding pattern (repo name + full path), so this is a cosmetic nit rather than a real risk.🔧 Optional: escape the dot for correctness
- --certificate-identity-regexp "https://github.com/${GITHUB_REPOSITORY}/.github/workflows/publish-plugins.yml@.*" \ + --certificate-identity-regexp "https://github\\.com/${GITHUB_REPOSITORY}/\\.github/workflows/publish-plugins\\.yml@.*" \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-plugins.yml around lines 426 - 448, The regex in the --certificate-identity-regexp argument used by the ./cosign verify-blob step contains an unescaped dot in ".github"; update the regexp string in the verify-blob invocation so the dot is escaped (i.e., use "\.github") to make the match precise and avoid accidental wildcard behavior, ensuring the change is applied to the verify-blob command's --certificate-identity-regexp argument in the publish-plugins.yml workflow.
494-497: Leftover conditional certificate checks from key-based flow.With keyless OIDC signing, the certificate is always produced. These
if [ -f ... ]guards (also at line 528) are now dead conditionals — they'll always be true. Removing them makes the intent clearer and ensures a loud failure if the certificate is unexpectedly missing (which would indicate a signing problem).♻️ Proposed fix for OCI push (lines 494–497)
- certificate_path="$DIST_DIR/${ARTIFACT_RELATIVE_PATH}.pem" - if [ -f "$certificate_path" ]; then - oras_args+=("$certificate_path:application/x-pem-file") - fi + certificate_path="$DIST_DIR/${ARTIFACT_RELATIVE_PATH}.pem" + oras_args+=("$certificate_path:application/x-pem-file")♻️ Proposed fix for staging step (lines 528–530)
- if [ -f "$DIST_DIR/$CERTIFICATE_RELATIVE_PATH" ]; then - cp "$DIST_DIR/$CERTIFICATE_RELATIVE_PATH" "$catalog_public/$CERTIFICATE_RELATIVE_PATH" - fi + cp "$DIST_DIR/$CERTIFICATE_RELATIVE_PATH" "$catalog_public/$CERTIFICATE_RELATIVE_PATH"Also applies to: 528-530
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/publish-plugins.yml around lines 494 - 497, Remove the dead conditional that wraps adding the certificate to oras_args; set certificate_path="$DIST_DIR/${ARTIFACT_RELATIVE_PATH}.pem" then explicitly assert the file exists and fail loudly if it does not (e.g. test -f "$certificate_path" || { echo "missing certificate: $certificate_path" >&2; exit 1; }) before unconditionally appending the entry to oras_args (oras_args+=("$certificate_path:application/x-pem-file")); apply the same change for the other occurrence that uses the same certificate_path/oras_args pattern.clients/agent-runtime/src/plugins/mod.rs (3)
1814-1841: Good baseline policy tests; consider expanding coverage.The two tests validate the main policy branches (official-remote → keyless required, local → not required). Consider adding tests for:
- Non-official remote sources (the
identity_regex: Nonebranch)signature_policy_for_locked_pluginwith and withoutsource_url- Loopback HTTP sources
These additional tests would help validate the full policy matrix and catch edge cases like the
identity_regex: Nonecosign behavior.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/plugins/mod.rs` around lines 1814 - 1841, Add unit tests to cover the missing branches of signature policy resolution: create tests using PluginSourceConfig and signature_policy_for_source that assert SignaturePolicy::RequiredKeyless with identity_regex: None for non-official remote sources, and SignaturePolicy::NotRequired for loopback HTTP sources; also add tests for signature_policy_for_locked_plugin that exercise both paths when a locked plugin has a source_url and when it does not (assert expected SignaturePolicy variants in each case). Ensure each test constructs the appropriate PluginSourceConfig or locked-plugin input and uses expect/assert with matches! against the SignaturePolicy variants to validate identity_regex presence/absence.
930-949: Consider streaming size limits on signature/certificate downloads.
fetch_bytesdownloads the full response body before the size check at lines 932 and 943. A malicious server could send an arbitrarily large response (bounded only by the 30s timeout). Since signature and certificate files should be small (64KB / 512KB), consider adding acontent-lengthpre-check or using a size-limited reader to avoid unnecessary memory consumption.This is a minor concern given the timeout, but it's defense-in-depth.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/plugins/mod.rs` around lines 930 - 949, The current fetch_bytes call reads the entire response for signature_url and certificate_url before checking size (used with MAX_SIGNATURE_BYTES and MAX_CERTIFICATE_BYTES), so change the download to enforce size limits during streaming: first perform a HEAD or a lightweight GET to inspect Content-Length and reject if > limit, and/or read the response body with a size-limited reader (e.g., read up to limit+1 bytes and bail if exceeded) when fetching in fetch_bytes for the signature/certificate paths; ensure the error messages still include candidate.manifest.id and the URL context used in the with_context calls.
1387-1403: Backward-compatible fallback for legacy lock entries.For locked plugins without
source_url, only the"official"source name triggers keyless verification. All other legacy entries silently skip verification. This is a reasonable migration trade-off, but consider logging a warning whensource_urlisNonefor a non-local source, so operators are aware that signature verification is being skipped.💡 Optional: log a diagnostic when signature verification is skipped due to missing source_url
fn signature_policy_for_locked_plugin(plugin: &LockedPlugin) -> Result<SignaturePolicy> { if let Some(source_url) = plugin.source_url.as_deref() { let source = PluginSourceConfig { name: plugin.source.clone(), url: source_url.to_string(), }; return signature_policy_for_source(&source); } if plugin.source == "official" { return Ok(SignaturePolicy::RequiredKeyless { identity_regex: Some(OFFICIAL_PLUGIN_IDENTITY_REGEX), }); } + tracing::debug!( + "Plugin '{}' locked entry has no source_url; skipping signature verification", + plugin.id + ); Ok(SignaturePolicy::NotRequired) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/agent-runtime/src/plugins/mod.rs` around lines 1387 - 1403, In signature_policy_for_locked_plugin, detect the legacy case where plugin.source_url is None and plugin.source is not "local" (and not "official") and emit a diagnostic warning that signature verification will be skipped for that plugin; reference the LockedPlugin instance via plugin (e.g., plugin.source and plugin.name/id) and use the project's logging facility (e.g., tracing::warn! or log::warn!) to record which plugin/source is affected before returning SignaturePolicy::NotRequired.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/plugins/mod.rs`:
- Around line 1483-1509: The cosign invocation currently uses
Command::new(...).output() with no timeout; change to spawn the child (use
command.spawn() to get a Child), then use a timeout mechanism (e.g.,
wait-timeout crate's ChildExt::wait_timeout or a thread + join with
Duration::from_secs(30)) to wait for completion, and on timeout kill the child
(Child.kill()/Child.wait()) and return an error; replace the current
command.output() error handling with this flow so verify-blob invocation
(references: Command::new("cosign"), .arg("verify-blob"), certificate_path,
signature_path, artifact_path, and the output variable) will be bounded (e.g.,
20–30s) and properly report timeout vs execution errors.
- Around line 1362-1385: signature_policy_for_source currently returns
SignaturePolicy::RequiredKeyless { identity_regex: None } for non-official
remote URLs which causes cosign to fail; update signature_policy_for_source (and
PluginSourceConfig) to provide a concrete identity_regex for non-official
remotes: either read a new optional field from PluginSourceConfig (e.g.,
plugin_identity_regex) and use it when constructing
SignaturePolicy::RequiredKeyless, or if that field is absent supply a permissive
fallback like ".*" (or alternatively change the logic to explicitly reject
non-official remotes with a clear application-level error); ensure
OFFICIAL_PLUGIN_CATALOG_HOST and OFFICIAL_PLUGIN_IDENTITY_REGEX logic remains
unchanged for official hosts and that the resulting SignaturePolicy always sets
identity_regex to Some(...) for any case passed to cosign verification.
---
Outside diff comments:
In @.github/workflows/publish-plugins.yml:
- Around line 392-412: Update the hardcoded COSIGN_VERSION (and derived
COSIGN_BASE_URL/COSIGN_BINARY) from "v2.4.1" to at least "v2.6.1" (or "v3.0.3"
if you choose to migrate), and ensure the downloaded checksum file and grep
target still match the new COSIGN_BINARY name; specifically, change
COSIGN_VERSION to the desired release, verify COSIGN_BINARY equals the release
asset name on GitHub for that version, and confirm the grep/sha256sum steps
still locate and validate the correct checksum for the new release.
---
Nitpick comments:
In @.github/workflows/publish-plugins.yml:
- Around line 426-448: The regex in the --certificate-identity-regexp argument
used by the ./cosign verify-blob step contains an unescaped dot in ".github";
update the regexp string in the verify-blob invocation so the dot is escaped
(i.e., use "\.github") to make the match precise and avoid accidental wildcard
behavior, ensuring the change is applied to the verify-blob command's
--certificate-identity-regexp argument in the publish-plugins.yml workflow.
- Around line 494-497: Remove the dead conditional that wraps adding the
certificate to oras_args; set
certificate_path="$DIST_DIR/${ARTIFACT_RELATIVE_PATH}.pem" then explicitly
assert the file exists and fail loudly if it does not (e.g. test -f
"$certificate_path" || { echo "missing certificate: $certificate_path" >&2; exit
1; }) before unconditionally appending the entry to oras_args
(oras_args+=("$certificate_path:application/x-pem-file")); apply the same change
for the other occurrence that uses the same certificate_path/oras_args pattern.
In `@clients/agent-runtime/src/plugins/mod.rs`:
- Around line 1814-1841: Add unit tests to cover the missing branches of
signature policy resolution: create tests using PluginSourceConfig and
signature_policy_for_source that assert SignaturePolicy::RequiredKeyless with
identity_regex: None for non-official remote sources, and
SignaturePolicy::NotRequired for loopback HTTP sources; also add tests for
signature_policy_for_locked_plugin that exercise both paths when a locked plugin
has a source_url and when it does not (assert expected SignaturePolicy variants
in each case). Ensure each test constructs the appropriate PluginSourceConfig or
locked-plugin input and uses expect/assert with matches! against the
SignaturePolicy variants to validate identity_regex presence/absence.
- Around line 930-949: The current fetch_bytes call reads the entire response
for signature_url and certificate_url before checking size (used with
MAX_SIGNATURE_BYTES and MAX_CERTIFICATE_BYTES), so change the download to
enforce size limits during streaming: first perform a HEAD or a lightweight GET
to inspect Content-Length and reject if > limit, and/or read the response body
with a size-limited reader (e.g., read up to limit+1 bytes and bail if exceeded)
when fetching in fetch_bytes for the signature/certificate paths; ensure the
error messages still include candidate.manifest.id and the URL context used in
the with_context calls.
- Around line 1387-1403: In signature_policy_for_locked_plugin, detect the
legacy case where plugin.source_url is None and plugin.source is not "local"
(and not "official") and emit a diagnostic warning that signature verification
will be skipped for that plugin; reference the LockedPlugin instance via plugin
(e.g., plugin.source and plugin.name/id) and use the project's logging facility
(e.g., tracing::warn! or log::warn!) to record which plugin/source is affected
before returning SignaturePolicy::NotRequired.
…ty regex for remote sources
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@clients/agent-runtime/src/plugins/mod.rs`:
- Around line 1488-1505: The current logic in signature_policy_for_locked_plugin
(handling LockedPlugin) silently returns SignaturePolicy::NotRequired for remote
ResolvedSource::Remote when plugin.source_identity_regex is None, which weakens
security; change this to fail fast or require explicit opt-in: replace the
tracing::warn + Ok(SignaturePolicy::NotRequired) path with an Err return (e.g.,
a descriptive error type indicating missing source_identity_regex for remote
legacy plugin) or guard it behind a clear feature/flag check (inspect a
config/opt_in field or environment) before allowing
SignaturePolicy::NotRequired; ensure callers of
signature_policy_for_locked_plugin handle the new Err case or the opt‑in flag is
validated earlier so remote plugins without identity regex cannot bypass
verification by default.
…ut identity regex
This was referenced Feb 22, 2026
Merged
This was referenced Apr 9, 2026
Merged
Merged
This was referenced Apr 19, 2026
This was referenced Apr 29, 2026
8 tasks
8 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
1 participant
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.
This pull request updates the plugin publishing workflow to exclusively use keyless OIDC-based signing with cosign, removing support for key-based signing. Documentation and code have been updated to reflect this change, and related secrets and logic for key-based signing have been removed. The manifest and catalog formats are also updated to match the new signing approach.
Workflow and Signing Process Updates:
publish-plugins.yml) now only supports keyless OIDC signing with cosign, removing all handling and secrets related to key-based signing. The signing and verification steps have been simplified accordingly. [1] [2] [3] [4]signatureentry in plugin manifests and the catalog is now always structured for keyless signing, with bothurlandcertificate_urlfields set unconditionally. [1] [2] [3]Documentation Updates:
Other Improvements:
Summary by CodeRabbit
New Features
Documentation
Chores