Fix VS Code extension self-signed certificate serial flake - #19176
Draft
Adam Ratzman (adamint) wants to merge 2 commits into
Draft
Fix VS Code extension self-signed certificate serial flake#19176Adam Ratzman (adamint) wants to merge 2 commits into
Adam Ratzman (adamint) wants to merge 2 commits into
Conversation
Ensure generated X.509 serial numbers do not start with a DER-invalid leading zero after masking random bytes for positivity. Add a deterministic regression test for the bad draw that previously produced 00-prefixed serials. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19176Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19176" |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes intermittent malformed certificate serials in the VS Code extension.
Changes:
- Ensures generated serials begin with a positive, non-zero byte.
- Adds deterministic OpenSSL regression coverage.
Show a summary per file
| File | Description |
|---|---|
extension/src/utils/security.ts |
Normalizes certificate serial bytes safely. |
extension/src/test/security.test.ts |
Tests malformed DER regression and normalization. |
Review details
Tip
Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Contributor
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
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
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.
Description
The VS Code extension unit test that repeatedly creates a self-signed certificate has been intermittently failing in
new X509Certificate(...)with an OpenSSL ASN.1INVALID_INTEGER/ illegal-padding error.The serial generator was already masking off the high bit so the value stayed positive, but it could still hand
node-forgea serial with leading zero bytes.node-forgestrips at most one redundant leading00while DER-encoding INTEGERs, so a draw with multiple leading zeroes can still leave invalid padding in the certificate serial.The earlier
0.392%number was a leading-zero proxy, not the actual bad-DER rate. A direct probe of thenode-forgeencoder path saw 7 invalid encodings in 200,000 generated serials, which is in the expected ~1/65,536 range per generated certificate.This keeps generated serials positive and non-zero in the first byte before passing them to
node-forge, and adds a deterministic regression test for the malformed DER INTEGER case (80 00 01 ...) instead of relying on the probabilistic ten-certificate loop.No dedicated issue yet; this was observed while investigating the VS Code extension unit-test failure in PR #19069 run
31222085874.Validation:
Mutation check:
Checklist
<remarks />and<code />elements on your triple slash comments?