fix(plugins): harden install preflight and signature verification#76
Merged
Conversation
Deploying with
|
| Status | Name | Latest Commit | Updated (UTC) |
|---|---|---|---|
| ✅ Deployment successful! View logs |
corvus-plugins-edge | 67454a7 | Feb 24 2026, 10:06 PM |
Deploying corvus with
|
| Latest commit: |
67454a7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://115e886a.corvus-42x.pages.dev |
| Branch Preview URL: | https://fix-install-preflight-plugin.corvus-42x.pages.dev |
Contributor
✅ Contributor ReportUser: @yacosta738
Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-24 to 2026-02-24 |
Deploying corvus-plugins with
|
| Latest commit: |
67454a7
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://888b5cff.corvus-plugins.pages.dev |
| Branch Preview URL: | https://fix-install-preflight-plugin.corvus-plugins.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. 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR replaces shell-based cosign verification in the publish workflow with a Python verifier (with PEM checks and timeout), adds normalization and validation for plugin signatures/certificates in agent-runtime, and adds platform detection plus preflight dependency management to the web app install script. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant InstallScript as Install Script
participant PlatformDetect as Platform Detection
participant DepCheck as Dependency Checker
participant PkgMgr as Package Manager
User->>InstallScript: run installer
InstallScript->>PlatformDetect: detect_platform()
PlatformDetect-->>InstallScript: OS_TYPE, LINUX_DISTRO
InstallScript->>PlatformDetect: detect_package_manager()
PlatformDetect-->>InstallScript: PKG_MGR
InstallScript->>InstallScript: print_preflight_notice()
InstallScript->>DepCheck: preflight_dependencies()
loop for each dependency (curl, tar, sha256, cosign)
DepCheck->>DepCheck: check existence
alt missing & auto-installable
DepCheck->>PkgMgr: install_dep_with_pkg_mgr(dep)
PkgMgr-->>DepCheck: result
else missing & not auto-installable
DepCheck-->>InstallScript: print_dependency_manual_help()
end
end
DepCheck-->>InstallScript: dependencies ready (or COSIGN_AVAILABLE=0)
InstallScript->>InstallScript: proceed with installation
sequenceDiagram
participant Workflow as CI Workflow
participant PyVerify as Python Verifier
participant FileOps as File Operations
participant PEMValidator as PEM Validator
participant Cosign
Workflow->>PyVerify: run verify step
PyVerify->>FileOps: build artifact/signature/cert paths (pathlib)
FileOps-->>PyVerify: read certificate file
PyVerify->>PEMValidator: check PEM delimiters
alt PEM invalid
PEMValidator-->>PyVerify: error -> exit
PyVerify-->>Workflow: fail
else PEM valid
PyVerify->>Cosign: subprocess("./cosign verify-blob ...", timeout=30s)
Cosign-->>PyVerify: verification result
PyVerify-->>Workflow: success/failure
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly Related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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: 7
🧹 Nitpick comments (5)
clients/web/apps/marketing/public/install (3)
129-131:require_dependenciesis now dead code.This function is no longer called from
main()(replaced bypreflight_dependencies). Remove it to avoid confusion.Proposed fix
-require_dependencies() { - has_cmd curl || die "curl is required. Please install curl and retry." -}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/marketing/public/install` around lines 129 - 131, Remove the dead function require_dependencies (which contains the has_cmd curl check) since main() now uses preflight_dependencies; delete the entire require_dependencies definition to avoid unused/dead code and any references to it, and verify preflight_dependencies implements the necessary dependency checks (e.g., curl) so behavior is unchanged.
341-419:detect_package_manageris re-invoked on everyensure_dependencycall (line 367).Since
preflight_dependenciesalready callsdetect_package_managerat line 423, and the result is stored in the globalPKG_MGR, the re-invocation insideensure_dependencyis redundant. You could guard withif [ -z "$PKG_MGR" ] && ! detect_package_manager; thento skip the redundant detection, or simply check the cached value.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/marketing/public/install` around lines 341 - 419, The ensure_dependency function redundantly re-runs detect_package_manager even though preflight_dependencies already sets the global PKG_MGR; modify ensure_dependency to check the cached PKG_MGR first (e.g., if PKG_MGR is empty) before calling detect_package_manager, so replace the unconditional detect_package_manager invocation with a guarded check like "if [ -z \"$PKG_MGR\" ] && ! detect_package_manager; then ..." ensuring subsequent logic (error, print_dependency_manual_help, return) remains the same and keeping references to ensure_dependency, detect_package_manager, PKG_MGR, and install_dep_with_pkg_mgr.
222-319: Heavy duplication of root/sudo logic across every package manager branch.The
if [ "$(id -u)" -eq 0 ]; then ... else sudo ...; fipattern is copy-pasted ~8 times. Consider extracting a helper likerun_privilegedto reduce this to a single call site per install action.Proposed refactor: extract a `run_privileged` helper
+run_privileged() { + if [ "$(id -u)" -eq 0 ]; then + "$@" + else + sudo "$@" + fi +} + install_dep_with_pkg_mgr() { local dep="$1" + local pkg="$dep" + + # Map logical dep names to actual package names + if [ "$dep" = "sha256" ]; then + pkg="coreutils" + fi case "$PKG_MGR" in brew) case "$dep" in curl) brew install curl ;; cosign) brew install cosign ;; *) return 1 ;; esac ;; - apt-get) - case "$dep" in - sha256) - if [ "$(id -u)" -eq 0 ]; then - apt-get update -qq - apt-get install -y -qq coreutils - else - sudo apt-get update -qq - sudo apt-get install -y -qq coreutils - fi - ;; - *) - if [ "$(id -u)" -eq 0 ]; then - apt-get update -qq - apt-get install -y -qq "$dep" - else - sudo apt-get update -qq - sudo apt-get install -y -qq "$dep" - fi - ;; - esac - ;; + apt-get) + run_privileged apt-get update -qq + run_privileged apt-get install -y -qq "$pkg" + ;; + dnf) + run_privileged dnf install -y -q "$pkg" + ;; + yum) + run_privileged yum install -y -q "$pkg" + ;; + pacman) + run_privileged pacman -S --noconfirm "$pkg" + ;; ...This eliminates ~70 lines of near-identical branches and makes future package-manager additions trivial.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/marketing/public/install` around lines 222 - 319, The install_dep_with_pkg_mgr function duplicates root vs sudo logic repeatedly; extract a helper (e.g., run_privileged) that accepts a command string/args, runs it directly when id -u == 0 and prefixes with sudo otherwise, then replace every duplicated if/else block inside install_dep_with_pkg_mgr with a single call to run_privileged for each install action (including the special-case mapping of dep "sha256" to installing coreutils per package manager). Ensure the helper is used for brew/apt-get/dnf/yum/pacman branches and that the case branches still select the correct install command before delegating to run_privileged..github/workflows/publish-plugins.yml (1)
507-508: Non-idiomaticwith_suffixusage for compound extensions
with_suffix(artifact_path.suffix + ".sig")produces the correct result (plugin.wasm.sig) by exploiting the "replace last suffix" semantics ofwith_suffix, but it reads as confusing — a reader must reason that.wasmis being replaced with.wasm.sig. The standard idiom for appending to a full path name is direct string concatenation on the path.♻️ Cleaner path construction
- signature_path = artifact_path.with_suffix(artifact_path.suffix + ".sig") - certificate_path = artifact_path.with_suffix(artifact_path.suffix + ".pem") + signature_path = artifact_path.parent / (artifact_path.name + ".sig") + certificate_path = artifact_path.parent / (artifact_path.name + ".pem")🤖 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 507 - 508, The current use of with_suffix to build signature_path and certificate_path is non-idiomatic because it replaces the last suffix (artifact_path.suffix) instead of appending; change construction to append the suffix to the filename portion: build signature_path and certificate_path from artifact_path by using the filename (artifact_path.name) plus ".sig" and ".pem" respectively (e.g., via artifact_path.with_name(artifact_path.name + ".sig") and artifact_path.with_name(artifact_path.name + ".pem")), keeping references to signature_path, certificate_path, artifact_path, and with_suffix as the places to update.clients/agent-runtime/src/plugins/mod.rs (1)
976-1016: Double normalization:verify_blob_with_cosignnormalizes again internally
normalize_signature_bundleis called here (line 976) and then called a second time insideverify_blob_with_cosign(line 1624). The second call is idempotent — PEM passes throughcontains_pem_certificate_markersunchanged, and a decoded cosign signature base64-decodes to binary that failsfrom_utf8, returning the original — so behaviour is correct. However the API contract is implicit: callers cannot tell whether they are responsible for normalizing before callingverify_blob_with_cosignor not.Consider one of:
- Remove the
normalize_signature_bundlecall fromverify_blob_with_cosignand normalise in all call sites (two: here andverify_locked_plugin_signature), or- Remove the normalization from this function and rely solely on the one inside
verify_blob_with_cosign, surfacing errors via the existing.with_contextwrapper.🤖 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 976 - 1016, The codebase currently double-normalizes signature bundles; fix this by removing the internal normalization from verify_blob_with_cosign and making normalization the caller's responsibility: delete the normalize_signature_bundle call and related branching inside verify_blob_with_cosign, update its docstring to state callers must pass a normalized (PEM/base64-handled) signature and certificate, and keep the existing normalize_signature_bundle + size checks in the two call sites (this function where VerifiedSignatureBundle is constructed and verify_locked_plugin_signature) so errors remain surfaced via their .with_context wrappers.
🤖 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/workflows/publish-plugins.yml:
- Around line 516-531: The subprocess.run call that invokes "./cosign
verify-blob" can hang indefinitely; update the subprocess.run invocation (the
call to subprocess.run that runs "./cosign", "verify-blob", ...) to include a
timeout argument (e.g., timeout=30) matching COSIGN_VERIFY_TIMEOUT semantics so
the step is bounded; ensure the timeout value is in seconds and add any
necessary handling for subprocess.TimeoutExpired where appropriate.
- Line 527: The f-string interpolates os.environ['GITHUB_REPOSITORY'] directly
into a regex which treats dots as wildcards; update the interpolation to escape
the repository name (e.g., use re.escape(os.environ['GITHUB_REPOSITORY']) or
explicitly replace '.' with '\\.') so the pattern matches literal dots,
mirroring how OFFICIAL_PLUGIN_IDENTITY_REGEX escapes dots; modify the f-string
containing "{os.environ['GITHUB_REPOSITORY']}" accordingly to produce a safe
literal regex.
In `@clients/agent-runtime/src/plugins/mod.rs`:
- Around line 1736-1742: The current contains_pem_certificate_markers(payload:
&[u8]) uses contains(...) which doesn’t enforce ordering; update it to decode
payload to text as before, then locate the positions of "-----BEGIN
CERTIFICATE-----" and "-----END CERTIFICATE-----" (e.g., with find or position
methods) and return true only when both are present and the BEGIN index is less
than the END index (and optionally ensure non-empty content between them), using
the same function name and the trimmed text variable to perform the checks.
- Around line 1682-1767: The new normalization functions
(normalize_signature_bundle, normalize_certificate_payload,
normalize_signature_payload, contains_pem_certificate_markers,
looks_like_cosign_signature_text, decode_base64_text) must be validated: run and
fix any issues from cargo fmt --all -- --check, cargo clippy --all-targets -- -D
warnings, and cargo test locally (or in CI) and either commit style/lint/test
fixes or add a brief PR note documenting which checks were intentionally skipped
and why; update the PR description with the validation results and include any
clippy lint exceptions if you cannot resolve them, referencing the above
function names so reviewers can find the changes.
In `@clients/web/apps/marketing/public/install`:
- Around line 297-313: The pacman invocations use "-Sy" which risks partial
database sync; update the pacman calls inside the pacman) case (the sha256
branch and the default branch that install "$dep") to remove the "-y" flag and
call "pacman -S --noconfirm coreutils" for the sha256 branch and "pacman -S
--noconfirm \"$dep\"" for the default branch, preserving the existing "$(id -u)"
root check and sudo usage so non-root runs still prefix with sudo.
- Around line 421-445: preflight_dependencies currently treats cosign as a hard
dependency which blocks installs; change the cosign check in
preflight_dependencies so it does not return failure: instead call
ensure_dependency for "cosign" but do not propagate its non-zero exit (remove
the "|| return 1"), log a warning via print_warn if cosign is missing, and set a
flag (e.g. COSIGN_AVAILABLE or SKIP_PLUGIN_VERIFICATION) that other code (the
plugin verification path) can check and only fail when plugin signature
verification is actually attempted (move the hard-fail behavior into the plugin
verification logic that uses cosign).
- Around line 200-220: The function can_auto_install_dependency incorrectly
reports cosign as auto-installable on non-brew managers; update
can_auto_install_dependency so that for the apt-get|dnf|yum|pacman branch it
returns failure for "cosign" (i.e., detect dep == "cosign" and return 1) while
still returning 0 for other deps, so install_dep_with_pkg_mgr will not attempt a
failing apt/yum/pacman install and will instead fall back to user-directed
installation instructions or the Sigstore docs.
---
Nitpick comments:
In @.github/workflows/publish-plugins.yml:
- Around line 507-508: The current use of with_suffix to build signature_path
and certificate_path is non-idiomatic because it replaces the last suffix
(artifact_path.suffix) instead of appending; change construction to append the
suffix to the filename portion: build signature_path and certificate_path from
artifact_path by using the filename (artifact_path.name) plus ".sig" and ".pem"
respectively (e.g., via artifact_path.with_name(artifact_path.name + ".sig") and
artifact_path.with_name(artifact_path.name + ".pem")), keeping references to
signature_path, certificate_path, artifact_path, and with_suffix as the places
to update.
In `@clients/agent-runtime/src/plugins/mod.rs`:
- Around line 976-1016: The codebase currently double-normalizes signature
bundles; fix this by removing the internal normalization from
verify_blob_with_cosign and making normalization the caller's responsibility:
delete the normalize_signature_bundle call and related branching inside
verify_blob_with_cosign, update its docstring to state callers must pass a
normalized (PEM/base64-handled) signature and certificate, and keep the existing
normalize_signature_bundle + size checks in the two call sites (this function
where VerifiedSignatureBundle is constructed and verify_locked_plugin_signature)
so errors remain surfaced via their .with_context wrappers.
In `@clients/web/apps/marketing/public/install`:
- Around line 129-131: Remove the dead function require_dependencies (which
contains the has_cmd curl check) since main() now uses preflight_dependencies;
delete the entire require_dependencies definition to avoid unused/dead code and
any references to it, and verify preflight_dependencies implements the necessary
dependency checks (e.g., curl) so behavior is unchanged.
- Around line 341-419: The ensure_dependency function redundantly re-runs
detect_package_manager even though preflight_dependencies already sets the
global PKG_MGR; modify ensure_dependency to check the cached PKG_MGR first
(e.g., if PKG_MGR is empty) before calling detect_package_manager, so replace
the unconditional detect_package_manager invocation with a guarded check like
"if [ -z \"$PKG_MGR\" ] && ! detect_package_manager; then ..." ensuring
subsequent logic (error, print_dependency_manual_help, return) remains the same
and keeping references to ensure_dependency, detect_package_manager, PKG_MGR,
and install_dep_with_pkg_mgr.
- Around line 222-319: The install_dep_with_pkg_mgr function duplicates root vs
sudo logic repeatedly; extract a helper (e.g., run_privileged) that accepts a
command string/args, runs it directly when id -u == 0 and prefixes with sudo
otherwise, then replace every duplicated if/else block inside
install_dep_with_pkg_mgr with a single call to run_privileged for each install
action (including the special-case mapping of dep "sha256" to installing
coreutils per package manager). Ensure the helper is used for
brew/apt-get/dnf/yum/pacman branches and that the case branches still select the
correct install command before delegating to run_privileged.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/publish-plugins.ymlclients/agent-runtime/src/plugins/mod.rsclients/web/apps/marketing/public/install
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (5)
clients/agent-runtime/src/plugins/mod.rs (3)
1783-1791: Minor:payload.lines().count()scans the entire string to verify single-line.For payloads within the 64 KB limit this is fine, but you could short-circuit with
payload.contains('\n')after trimming (orpayload.bytes().filter(|&b| b == b'\n').count() > 0) to avoid a full iteration. Not a real concern at current sizes.🤖 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 1783 - 1791, The check in looks_like_cosign_signature_text currently uses payload.lines().count() which iterates the whole string; change the single-line check to a short-circuit test such as trimming and then using payload.contains('\n') (or payload.bytes().any(|b| b == b'\n')) to detect multi-line content and return false early, keeping the existing is_empty() check and the final chars().all(...) validation unchanged.
1712-1734:normalize_certificate_payloadallocates even when the input is already valid PEM.Line 1714 returns
certificate.to_vec()on the happy path (valid PEM input), which allocates. Since this is called sparingly (once per plugin install/verify), the overhead is negligible, but if you want to avoid it in the future, the function could return aCow<[u8]>. Not worth changing now.🤖 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 1712 - 1734, The function normalize_certificate_payload currently always clones the input on the PEM happy path by returning certificate.to_vec(), causing an unnecessary allocation; change its signature to return a Cow<[u8]> (or otherwise return a borrowed slice type) and update callers accordingly so that when contains_pem_certificate_markers(certificate) is true you return Cow::Borrowed(certificate) and when you decode base64 you return Cow::Owned(decoded_vec); adjust any uses of normalize_certificate_payload to accept the Cow result (or obtain an owned Vec only when needed) and update error handling to preserve the same failure semantics.
1793-1806: AddURL_SAFE_NO_PADas a fallback to handle unpadded URL-safe base64 (common in JWTs).The current implementation tries
STANDARD(which requires padding) andURL_SAFE(which also requires canonical padding). Unpadded URL-safe base64 input—common in JWTs and other web contexts—will fail to decode. Addinggeneral_purpose::URL_SAFE_NO_PADas a third fallback makes this function more robust.Suggested improvement
general_purpose::STANDARD .decode(compact.as_bytes()) .ok() .or_else(|| general_purpose::URL_SAFE.decode(compact.as_bytes()).ok()) + .or_else(|| general_purpose::URL_SAFE_NO_PAD.decode(compact.as_bytes()).ok()) }🤖 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 1793 - 1806, The decode_base64_text function currently tries general_purpose::STANDARD then general_purpose::URL_SAFE and will fail on unpadded URL-safe inputs (common in JWTs); update decode_base64_text to also attempt decoding with general_purpose::URL_SAFE_NO_PAD as a third fallback after URL_SAFE so unpadded URL-safe base64 is accepted, keeping the same Option<Vec<u8>> return pattern and using .ok() chaining similar to the existing calls.clients/web/apps/marketing/public/install (1)
253-263:apt-get updateruns on every dependency install — minor inefficiency.If multiple dependencies are missing (e.g., both
curlandtar),apt-get update -qqwill be called once per dependency. Consider caching a flag or running the update once inpreflight_dependenciesbefore the individualensure_dependencycalls. This is a minor concern for an installer that typically installs at most one or two packages.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@clients/web/apps/marketing/public/install` around lines 253 - 263, The apt-get branch currently calls "run_privileged apt-get update -qq" for every dependency in the apt-get case, causing repeated updates; change the flow so the update runs only once before installing multiple packages—either by moving "apt-get update -qq" into preflight_dependencies (so it runs once for apt-based installs) or by adding a cached flag (e.g., UPDATE_RUN) checked by ensure_dependency before calling "run_privileged apt-get update -qq"; keep the per-dependency install calls using run_privileged apt-get install -y -qq "$dep" and ensure the flag is set after the first update to avoid duplicate updates..github/workflows/publish-plugins.yml (1)
500-537: Looks good overall — both past review findings are addressed.The
timeout=30onsubprocess.run,re.escape()forGITHUB_REPOSITORY, PEM pre-validation, andTimeoutExpiredhandling are all solid improvements.One remaining nit: the literal
github.comin the regex on line 530 has an unescaped dot (.matches any character). While practically unexploitable in this OIDC context, it's inconsistent with the care taken to escape the rest of the pattern.Optional fix
- f"https://github.com/{repository_pattern}/\\.github/workflows/publish-plugins\\.yml@.*", + f"https://github\\.com/{repository_pattern}/\\.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 500 - 537, Update the certificate identity regex passed to subprocess.run (the argument after "--certificate-identity-regexp") to escape the dot in "github.com" so the pattern uses "github\\.com" instead of "github.com"; adjust the f-string that builds the pattern (which currently uses repository_pattern) to include the escaped host portion (e.g., "https://github\\.com/{repository_pattern}/\\.github/workflows/publish-plugins\\.yml@.*") so the literal dot is not treated as a wildcard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In @.github/workflows/publish-plugins.yml:
- Around line 500-537: Update the certificate identity regex passed to
subprocess.run (the argument after "--certificate-identity-regexp") to escape
the dot in "github.com" so the pattern uses "github\\.com" instead of
"github.com"; adjust the f-string that builds the pattern (which currently uses
repository_pattern) to include the escaped host portion (e.g.,
"https://github\\.com/{repository_pattern}/\\.github/workflows/publish-plugins\\.yml@.*")
so the literal dot is not treated as a wildcard.
In `@clients/agent-runtime/src/plugins/mod.rs`:
- Around line 1783-1791: The check in looks_like_cosign_signature_text currently
uses payload.lines().count() which iterates the whole string; change the
single-line check to a short-circuit test such as trimming and then using
payload.contains('\n') (or payload.bytes().any(|b| b == b'\n')) to detect
multi-line content and return false early, keeping the existing is_empty() check
and the final chars().all(...) validation unchanged.
- Around line 1712-1734: The function normalize_certificate_payload currently
always clones the input on the PEM happy path by returning certificate.to_vec(),
causing an unnecessary allocation; change its signature to return a Cow<[u8]>
(or otherwise return a borrowed slice type) and update callers accordingly so
that when contains_pem_certificate_markers(certificate) is true you return
Cow::Borrowed(certificate) and when you decode base64 you return
Cow::Owned(decoded_vec); adjust any uses of normalize_certificate_payload to
accept the Cow result (or obtain an owned Vec only when needed) and update error
handling to preserve the same failure semantics.
- Around line 1793-1806: The decode_base64_text function currently tries
general_purpose::STANDARD then general_purpose::URL_SAFE and will fail on
unpadded URL-safe inputs (common in JWTs); update decode_base64_text to also
attempt decoding with general_purpose::URL_SAFE_NO_PAD as a third fallback after
URL_SAFE so unpadded URL-safe base64 is accepted, keeping the same
Option<Vec<u8>> return pattern and using .ok() chaining similar to the existing
calls.
In `@clients/web/apps/marketing/public/install`:
- Around line 253-263: The apt-get branch currently calls "run_privileged
apt-get update -qq" for every dependency in the apt-get case, causing repeated
updates; change the flow so the update runs only once before installing multiple
packages—either by moving "apt-get update -qq" into preflight_dependencies (so
it runs once for apt-based installs) or by adding a cached flag (e.g.,
UPDATE_RUN) checked by ensure_dependency before calling "run_privileged apt-get
update -qq"; keep the per-dependency install calls using run_privileged apt-get
install -y -qq "$dep" and ensure the flag is set after the first update to avoid
duplicate updates.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
.github/workflows/publish-plugins.ymlclients/agent-runtime/src/plugins/mod.rsclients/web/apps/marketing/public/install
Merged
This was referenced Apr 9, 2026
Merged
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 introduces significant improvements to the plugin signature verification process and enhances the installation script for better dependency management and user experience. The main changes include robust normalization and validation of plugin signatures and certificates on the Rust backend, as well as smarter, more user-friendly dependency checks and auto-installation in the shell installer.
Plugin signature verification improvements:
clients/agent-runtime/src/plugins/mod.rs) [1] [2] [3] [4]clients/agent-runtime/src/plugins/mod.rs)Installer script enhancements:
curl,tar,sha256sum/shasum, andcosign) on macOS and various Linux distributions. This makes the installation process smoother and more robust for users. (clients/web/apps/marketing/public/install) [1] [2]clients/web/apps/marketing/public/install) [1] [2]Workflow adjustment:
.github/workflows/publish-plugins.yml)Summary by CodeRabbit
New Features
Improvements
Tests