Skip to content

fix(plugins): update code signing EKU OID encoding and improve Fulciointermediate handling#86

Merged
yacosta738 merged 1 commit into
mainfrom
plugins
Feb 25, 2026
Merged

fix(plugins): update code signing EKU OID encoding and improve Fulciointermediate handling#86
yacosta738 merged 1 commit into
mainfrom
plugins

Conversation

@yacosta738
Copy link
Copy Markdown
Contributor

@yacosta738 yacosta738 commented Feb 25, 2026

  • Correct DER encoding for CODE_SIGNING_EKU_OID to match webpki expectations
  • Add embedded Fulcio intermediate certificate and use as fallback if none provided
  • Clarify certificate validity checks and error handling for expired certs

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced certificate chain validation to include fallback intermediate certificate support, improving validation robustness
    • Refined certificate validation error handling to prioritize chain integrity verification while reducing false positives from time-based validation issues

… intermediate handling

- Correct DER encoding for CODE_SIGNING_EKU_OID to match webpki expectations
- Add embedded Fulcio intermediate certificate and use as fallback if none provided
- Clarify certificate validity checks and error handling for expired certs
@cloudflare-workers-and-pages
Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Updated (UTC)
✅ Deployment successful!
View logs
corvus-plugins-edge 1ded432 Feb 25 2026, 09:42 PM

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 25, 2026

📝 Walkthrough

Walkthrough

Modifies certificate verification logic by replacing OID byte encoding, introducing embedded Fulcio intermediate certificate as fallback, updating certificate chain validation to handle intermediates explicitly, and reworking runtime verification to selectively ignore time-based validation errors.

Changes

Cohort / File(s) Summary
Certificate Verification Logic
clients/agent-runtime/src/plugins/mod.rs
Updated OID encoding for id-kp-codeSigning from DER arc to explicit DER content bytes; added embedded Fulcio intermediate certificate constant and implemented fallback injection when intermediates are absent; refactored certificate chain validation to treat intermediates as mutable collection and pass them to trust-anchored verification; reworked runtime verification flow to explicitly ignore time-based errors while enforcing non-time checks with lowercase error matching; expanded validity checks to be conditional on check_validity flag with updated comments for install-time vs runtime behavior.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

  • PR #81: Modifies the same certificate verification logic in clients/agent-runtime/src/plugins/mod.rs to rework Sigstore/OID handling and certificate chain building validation.
  • PR #77: Alters certificate and signature verification logic in clients/agent-runtime/src/plugins/mod.rs affecting normalization and validation behavior.
🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description is largely incomplete; it lacks required sections like Related Issues, Tested Information, and only provides bullet points instead of structured content matching the template. Fill in all required sections from the template including Related Issues, Summary, Tested Information, and Breaking Changes sections with appropriate details.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main changes: fixing OID encoding and improving Fulcio intermediate handling, which directly align with the changeset.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch plugins

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 25, 2026

✅ Contributor Report

User: @yacosta738
Status: Passed (12/13 metrics passed)

Metric Description Value Threshold Status
PR Merge Rate PRs merged vs closed 89% >= 30%
Repo Quality Repos with ≥100 stars 0 >= 0
Positive Reactions Positive reactions received 9 >= 1
Negative Reactions Negative reactions received 0 <= 5
Account Age GitHub account age 3043 days >= 30 days
Activity Consistency Regular activity over time 108% >= 0%
Issue Engagement Issues with community engagement 0 >= 0
Code Reviews Code reviews given to others 370 >= 0
Merger Diversity Unique maintainers who merged PRs 3 >= 0
Repo History Merge Rate Merge rate in this repo 89% >= 0%
Repo History Min PRs Previous PRs in this repo 66 >= 0
Profile Completeness Profile richness (bio, followers) 90 >= 0
Suspicious Patterns Spam-like activity detection 1 N/A

Contributor Report evaluates based on public GitHub activity. Analysis period: 2025-02-25 to 2026-02-25

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
clients/agent-runtime/src/plugins/mod.rs (1)

1840-1853: ⚠️ Potential issue | 🟠 Major

Avoid string-matching rustls-webpki errors for trust decisions.

Line 1847 uses contains("validity"), which is too broad and can suppress non-time-based chain failures (e.g., ExtensionValueInvalid), weakening runtime verification guarantees. In rustls-webpki 0.103, use explicit enum matching instead:

🔧 Proposed fix
         if let Err(e) = result {
-            let error_str = e.to_string();
-            let error_lower = error_str.to_ascii_lowercase();
-            if !error_lower.contains("expired")
-                && !error_lower.contains("not yet valid")
-                && !error_lower.contains("validity")
-            {
-                return Err(e)
-                    .context("Certificate chain verification against Fulcio roots failed");
-            }
-            // Log the time-based issue at debug level for observability
-            tracing::debug!("Runtime certificate time validation skipped: {}", error_str);
+            match e {
+                webpki::Error::CertExpired { .. }
+                | webpki::Error::CertNotValidYet { .. }
+                | webpki::Error::InvalidCertValidity => {
+                    tracing::debug!("Runtime certificate time validation skipped: {e}");
+                }
+                other => {
+                    return Err(other)
+                        .context("Certificate chain verification against Fulcio roots failed");
+                }
+            }
         }

(Note: rustls-webpki 0.103 struct variants require { .. } destructuring; the enum is #[non_exhaustive].)

🤖 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 1840 - 1853, The code
currently string-matches the verification error (variables result / e /
error_str / error_lower) to detect time-based failures; replace that with an
explicit match on the concrete rustls-webpki error enum instead of
contains("validity") etc. Cast or downcast the error to the rustls_webpki error
type used by your verifier and match its time-related variants (e.g., the expiry
/ not-yet-valid variants using the enum variant patterns with { .. } as required
by rustls-webpki 0.103); if the error is one of those time-based variants log at
debug and continue, otherwise return Err(e). Update the tracing::debug message
to include the original error after matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@clients/agent-runtime/src/plugins/mod.rs`:
- Around line 1840-1853: The code currently string-matches the verification
error (variables result / e / error_str / error_lower) to detect time-based
failures; replace that with an explicit match on the concrete rustls-webpki
error enum instead of contains("validity") etc. Cast or downcast the error to
the rustls_webpki error type used by your verifier and match its time-related
variants (e.g., the expiry / not-yet-valid variants using the enum variant
patterns with { .. } as required by rustls-webpki 0.103); if the error is one of
those time-based variants log at debug and continue, otherwise return Err(e).
Update the tracing::debug message to include the original error after matching.

ℹ️ 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 5dfff1f and 1ded432.

📒 Files selected for processing (1)
  • clients/agent-runtime/src/plugins/mod.rs

@yacosta738 yacosta738 merged commit f22ede4 into main Feb 25, 2026
16 checks passed
@yacosta738 yacosta738 deleted the plugins branch February 25, 2026 22:04
@yacosta738 yacosta738 mentioned this pull request Mar 16, 2026
This was referenced Apr 19, 2026
This was referenced Apr 29, 2026
@dallay-bot dallay-bot Bot mentioned this pull request May 3, 2026
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