Relax Windows Trusted Signing issuer check#589
Conversation
|
Warning Review limit reached
More reviews will be available in 36 minutes and 36 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughWindows Authenticode verification now passes an optional issuer override and a default issuer regex into the embedded PowerShell check. The signer loop uses exact issuer matching when the override is set, otherwise it matches the issuer against the pattern. ChangesWindows Authenticode issuer policy
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Deploying maple with
|
| Latest commit: |
01efd9b
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c090b1a9.maple-ca8.pages.dev |
| Branch Preview URL: | https://fix-windows-authenticode-iss.maple-ca8.pages.dev |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
a73ea20 to
a6cf6af
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
scripts/ci/_common.sh (1)
3663-3663: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the default issuer pattern tolerant of CA-number width and RDN spacing.
The default pattern bakes in two brittle assumptions that can reproduce the exact failure this PR addresses:
[0-9][0-9]matches exactly two digits. It works forCA 03/CA 04today, but a single- or triple-digit rotation (CA 5,CA 100) would fail verification again.- The literal
,separator requires one space after each comma. Note the CN check on Line 3700 deliberately tolerates spacing with(^|,\s*)…(\s*,|$). IfX509Certificate2.Issuerrenders the DN with different spacing on the runner, the issuer pattern fails while the CN check would not.♻️ Proposed more tolerant default pattern
- MAPLE_WINDOWS_AUTHENTICODE_EXPECTED_ISSUER_PATTERN="${MAPLE_WINDOWS_AUTHENTICODE_EXPECTED_ISSUER_PATTERN:-^CN=Microsoft ID Verified CS AOC CA [0-9][0-9], O=Microsoft Corporation, C=US$}" \ + MAPLE_WINDOWS_AUTHENTICODE_EXPECTED_ISSUER_PATTERN="${MAPLE_WINDOWS_AUTHENTICODE_EXPECTED_ISSUER_PATTERN:-^CN=Microsoft ID Verified CS AOC CA [0-9]+,\s*O=Microsoft Corporation,\s*C=US$}" \You may also want case-insensitive matching (
[regex]::IsMatch($issuer, $expectedIssuerPattern, 'IgnoreCase')) if the DN casing isn't guaranteed.Worth confirming the exact DN string
Get-AuthenticodeSignature(...).SignerCertificate.Issueremits on the CI runner (separator spacing, component order, and casing) so the default pattern matches what Microsoft Trusted Signing actually produces.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/_common.sh` at line 3663, The default issuer regex in MAPLE_WINDOWS_AUTHENTICODE_EXPECTED_ISSUER_PATTERN is too strict; update the pattern to tolerate variable CA-number width and flexible RDN spacing. In scripts/ci/_common.sh, adjust the default used by the Authenticode issuer check so the Microsoft ID Verified CS AOC CA segment matches one or more digits instead of exactly two, and make comma/space handling consistent with the more permissive CN matching logic used elsewhere in the signing verification flow.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@scripts/ci/_common.sh`:
- Line 3663: The default issuer regex in
MAPLE_WINDOWS_AUTHENTICODE_EXPECTED_ISSUER_PATTERN is too strict; update the
pattern to tolerate variable CA-number width and flexible RDN spacing. In
scripts/ci/_common.sh, adjust the default used by the Authenticode issuer check
so the Microsoft ID Verified CS AOC CA segment matches one or more digits
instead of exactly two, and make comma/space handling consistent with the more
permissive CN matching logic used elsewhere in the signing verification flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5c2c8b0-c02b-4462-a5ee-3fba35d50e3a
📒 Files selected for processing (1)
scripts/ci/_common.sh
a6cf6af to
01efd9b
Compare
1 participant
Summary
Why
The master signed Windows build reached successful Azure signing, then failed our post-sign Authenticode verification because Microsoft issued from CA 04 while the verifier default pinned CA 03.
Validation
Summary by CodeRabbit