Move cert utilities to shared package and fix OpenSSL hash canonicalization by dnegstad · Pull Request #87 · dnegstad/devcontainer-dev-certs

dnegstad · 2026-08-28T01:45:47Z

Summary

A correctness review of the whole project. The headline is that container-side OpenSSL trust never worked — the subject hash was computed wrong, so the SSL_CERT_DIR symlinks pointed at a name nothing looks up. Fixing that surfaced several more silent failures in the same area, plus a set of dead-code and validation issues.

Every claim below was verified against the real tool (openssl x509 -hash, openssl verify -CApath, certutil -V) rather than reasoned from source, because the OpenSSL source is misleading in at least two places.

Correctness fixes

OpenSSL subject hash was wrong (container trust silently dead)

X509_NAME_hash — what by_dir (SSL_CERT_DIR / -CApath) searches for — is not a hash of the subject's DER. It hashes X509_NAME_canon output. Three things that requires:

Non-UTF8 string types are transcoded to UTF-8 first — T61String from Latin-1, BMPString from UTF-16BE, UniversalString from UTF-32BE. (A T61String holding UTF-8 bytes of 日本語 hashes to 02c4fa54; merely re-tagging those bytes gives e2c402e4.)
Values are then ASCII-lowercased and whitespace-folded, where "whitespace" is every ASCII whitespace byte, not just 0x20 — "a\tb", "a\nb", "a b", "A B" all hash alike, and leading trim behaves the same way.
The result is the bare concatenation of the RDN SET OF encodings, without the Name's outer SEQUENCE.

For CN=localhost the correct hash is ce275665; the old code produced ae2f22a0. openssl verify -CApath fails with the old name and passes with the new one.

Hash slots ran out after ten certificates

Every ASP.NET dev cert is CN=localhost, so slots are consumed by certificate count, not by genuine collision. The old bound of ten was reachable in ordinary use — a container that mints a fresh cert per rebuild (the default for most dotnet devcontainer base images) fills it in ten rebuilds — after which ensureHashSymlink returned silently, leaving ten dead certs linked and the live one unreachable.

Raised to 256, with a warning past ten and a thrown error at the bound: reporting a successful install for a certificate OpenSSL cannot find is the same failure merely delayed.

Upgrades did not repair either fix

The corrected hash is only written by the install/trust paths, and all of them were gated behind checks that only looked for files. An existing container or host has the PEM, the PFX, and a symlink under the wrong hash — so those checks reported health, the repair was skipped, and trust stayed dead for exactly the users the fix targets. isCertInstalled and Linux isTrusted now require a symlink that actually resolves.

NSS trust flags were wrong for Chromium

We sent a blanket -t "CT,," to every NSS database. The dev cert is a self-signed end entity (generateCertificate emits cA=FALSE), so C — CERTDB_TRUSTED_CA, consulted only in an issuer position — is never looked at for it. certutil -V -u V rejects such an entry outright with "Issuer certificate is invalid".

Now P,, (CERTDB_TRUSTED, "trusted peer") for Chromium-family databases and C,, for Firefox, which empirically ignores P for server certs. This is the same split dotnet dev-certs https --trust makes — UnixCertificateManager.TryAddCertificateToNssDb: usage = nssDb.IsFirefox ? "C" : "P" — and the same asymmetry explains why dotnet verifies Chromium with -V -u V but Firefox with only -L. Existing entries migrate through the delete-then-add already there for idempotency, since certutil -A won't rewrite an existing nickname's trust string.

ReDoS in PEM extraction (CodeQL, high)

/-----BEGIN CERTIFICATE-----\s*([\s\S]*?)\s*-----END CERTIFICATE-----/ wrapped a lazy quantifier in two \s*, giving the engine an ambiguous split. Measured: 5,000 trailing spaces took 40.7 s; 10,000 did not finish in 300 s. Reachable, since rehashDirectory feeds this every *.pem in the trust directory. Replaced with linear indexOf + slice.

Other silent failures

NSS used one shared nickname, so trusting a second dev cert evicted the first — host-generated and container-pushed certs could never both be browser-trusted, contradicting the deliberately-additive OpenSSL trust dir. Now per-thumbprint, with a one-time delete of the legacy name.
The .NET-store opt-out sweep was unreachable: isCertInstalled ignored the store path for opted-out certs, so a passwordless (plain-text-key) copy survived turning installUserCertsToDotNetStore off.
A cleared defaultKestrelCertificate could never take effect — the sweep was skipped on an empty bundle, and environmentVariableCollection persists across reloads.
Linux root-store certs were unremovable (strict loadPfx on a public-cert-only store).
1-in-256 test flake: generateSerialNumber could leave a 0x00 leading byte, which DER keeps as sign padding but every textual readback drops.

Security

validateLeafTrustShape gates the SAN check on container-pushed certs. The SAN-local restriction only constrains a cert that can authenticate only itself — a CA's own SANs place no limit on what it may issue, so a CA with localhost SANs previously passed validation and was installed into CurrentUser\Root / the login keychain / the OpenSSL CApath / the browser NSS databases. Now requires basicConstraints present with cA=FALSE, and EKU present, including serverAuth, excluding anyExtendedKeyUsage.
scanSanEntries replaces collectSanEntries and reports why a SAN set is unusable. Missing, undecodable, empty, or carrying a GeneralName type other than dNSName/iPAddress now reject as malformed-sans, which allowNonLocalContainerCertSans deliberately does not override — that setting is about scope, which is meaningless for a cert whose names could not be read.
Container-cert consent is a reversible tri-state. The boolean could only ever record yes: accepts persisted forever, declines persisted nothing, so the prompt returned on every activation. Now granted/denied/unset, with Trust / Never / Cancel as distinct outcomes and a resetContainerCertConsent command so Never isn't a ratchet in the other direction.

A review comment argued that cA=FALSE is not sufficient once the cert is a trust anchor, since anchor constraints are processed inconsistently. The premise is true in general — RFC 5280 §6.1 starts path validation from the anchor and never checks its own cA bit — but the attack requires those constraints checked in an issuer position, which is a separate check that both testable surfaces do perform. A self-signed CN=localhost with CA:FALSE and keyCertSign, used to sign a leaf for evil.example.com, is rejected by OpenSSL (error 79 at 1 depth lookup: invalid CA certificate) and by NSS (Issuer certificate is invalid). Details on the thread.

Cleanup

Dropped removeCertificates (all three platform implementations plus ~130 lines of tests) — a complete feature with no production entry point; its only callers were CertManager.clean(), used solely by a test, and generate(force), never invoked with force. Also describeAutoBackend and CertProvider.clearCache, both unreferenced.
Retired the 14 re-export shims under vscode-ui-extension/src/{cert,platform}/ in favour of a rename.
Dropped the host's openssl binary dependency. LinuxCertificateStore shelled out for the hash and silently skipped the symlink when the binary was absent. It now shares the pure-TypeScript implementation with the container installer. openssl was the only host binary that was neither an OS built-in nor opt-in.

Process handling

runProcess capped captured output at Node's 1 MiB execFile default, which security find-certificate -a on macOS can exceed. Node reports the overflow with a string error.code, so it collapsed into exitCode: 1 with empty stderr — indistinguishable from a real failure. isCertInKeychain read that as "not present", force-skipped the cert as orphaned, and sent CertManager.trust() down the regenerate branch: a new cert and a keychain password prompt on every request, each one making the next truncation likelier.

Cap raised to 32 MiB, and ProcessResult.truncated now distinguishes overflow from failure. isCertInKeychain deliberately fails open on truncation, since the closed direction is the destructive one.

Deferred

Nothing prunes superseded certificates on any platform. Under rebuild-rotation the host accrues one trusted CN=localhost leaf per rebuild across every trust surface. The 365-day validity window bounds the security exposure but not the clutter. Closing it needs a per-platform untrustCertificate plus a supersede policy — and the policy is the hard part, since "untrust the previous cert on accept" ping-pongs with two containers open. Tracked in AGENTS.md; intended for a separate PR.

https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Findings from a correctness/dead-code review of the whole repo. **c_rehash symlinks never matched what OpenSSL looks up.** `computeSubjectHash` hashed the raw subject DER. OpenSSL's `X509_NAME_hash` — the value `by_dir` (SSL_CERT_DIR / -CApath) searches for — hashes `X509_NAME_canon` output instead: attribute values re-tagged UTF8String, ASCII-lowercased, space runs collapsed, and the RDN SET OF encodings concatenated *without* the Name's outer SEQUENCE. For a CN=localhost dev cert that is ce275665, not the ae2f22a0 we were writing, so `{hash}.0` pointed at a name nothing ever opened and container-side OpenSSL trust silently did nothing (`openssl verify -CApath` fails with the old name, passes with the new one). The existing tests only asserted symlink *shape* — "the actual hash value doesn't matter" — so this was invisible. Implemented the canonical form and pinned it against `openssl x509 -hash` output, including a multi-RDN fixture that exercises the normalization rules, plus an opportunistic cross-check against the local openssl binary. **The .NET-store opt-out sweep was unreachable.** Activation only calls `installUserCert` when `isCertInstalled` returns false, but for a cert with `installToDotNetStore: false` that check ignored the store path entirely — so a passwordless (plain-text-key) copy written under a previous opt-in survived the user flipping `installUserCertsToDotNetStore` off, permanently. `isCertInstalled` now reports "not installed" when an opted-out cert still has a store PFX on disk, which lets the existing sweep run. **Stale Kestrel default-cert selection could never be cleared.** `injectCertificate` returned before `applyDefaultKestrelCert` when the bundle came back empty, so with `environmentVariableCollection` persisted across reloads a cleared `defaultKestrelCertificate` kept applying its old `__Path`/`__Password`. The sweep now runs on the empty-bundle path. **NSS browser trust used one shared nickname.** Nicknames are unique per database, so each `trustInNss` call evicted the previously-trusted cert — host-generated and container-pushed certs could never both be trusted in browsers, contradicting the deliberately-additive OpenSSL trust dir. Nicknames are now per-thumbprint, with a one-time delete of the old shared name so upgrades don't strand a cert we no longer manage. **Linux root-store certs were unremovable.** `removeDevCertsFromDir` used the strict `loadPfx`, which requires a private key; Root-store entries are public-cert-only by construction, so they never matched. Switched to `loadPfxLenient`. Dead code: dropped `CertProvider.clearCache` (no callers) and the `classifyPlatformCandidate` / `selectBestPlatformDevCert` / `PlatformClassifyOptions` barrel aliases (zero consumers; the submodule path is what callers actually use). `upmapV1ToV3` now uses `DOTNET_DEV_CERT_NAME` instead of repeating the literal. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

… dep **Removed `describeAutoBackend`.** No references anywhere, tests included — a status-surface helper written for a status surface that doesn't exist. **Removed the `removeCertificates` surface.** Audit of test-only code found this is a complete, well-tested feature with no production entry point: `CertManager.clean()` is called only from `manager.test.ts`, and `generate(force)` is only ever invoked as `generate()` (manager.ts:142). Those two are the sole callers of `store.removeCertificates`, so the three platform implementations behind it — including macOS's untrust-then-drain loop with its 100-iteration bound and temp-DER dance — have never run outside a test. The tell: the Linux one passed a public-cert-only Root store through the key-requiring `loadPfx`, so it could never have deleted a root-store cert; its tests were green throughout. Deleted the interface member, the abstract member, all three implementations, and the ~130 lines of tests asserting behavior nothing reaches. `generate()` is now documented as additive, which is what it has always been in practice: selection by version-then-expiry retires a superseded cert without anyone deleting it, and nothing can revoke a cert another flow deliberately trusted. If a "reset dev certs" command lands later, this comes back with an entry point attached. Kept the other test-only exports (`resolveSafeExecPath`, `computeSubjectHash`, `resolveDotnetProvisioning`, `formatCleanupSummary`, `isValidCertName`, `pkcs12Kdf`): each has a production caller inside its own module and is exported so tests can reach a pure function without driving a vscode-heavy entry point. That's testability, not dead weight. **Retired the 14 re-export shims** under `vscode-ui-extension/src/{cert, platform}/` in favor of a rename. Every `./cert/*` / `./platform/*` import across the extension and its suite now names `@devcontainer-dev-certs/ shared` directly (or the `platform/baseStore` submodule for the localized classifier wrappers). No `vi.mock` target moved — they all already pointed at real shared modules, which is what made the shims pure indirection. The integration suite's `await import("../src/platform/linuxStore.js")` became a static import: the dynamic form existed to defer loading until `DOTNET_DEV_CERTS_OPENSSL_CERTIFICATE_DIRECTORY` was set, but `getOpenSslTrustDir()` reads that at call time, so the deferral bought nothing. **Dropped the host's `openssl` binary dependency.** `LinuxCertificateStore` shelled out to `openssl x509 -hash` for the trust-dir symlink name and silently skipped the symlink when the binary was absent — OpenSSL trust quietly not working on a host we don't control. The pure-TypeScript implementation moved from `vscode-workspace-extension/src/util/rehash.ts` to `shared/src/cert/rehash.ts`, and the host now calls the same `ensureHashSymlink` the container installer uses. One implementation, one set of tests, both ends of the sync. `openssl` was the only host binary that was neither an OS built-in (`security`, `pwsh`, `certutil.exe`) nor opt-in (`dotnet` under `hostCertGenerator`, `certutil` for NSS), so the host now needs nothing installed. The linuxStore unit tests that asserted the openssl mechanism now assert the outcome instead: the symlink is named with the canonical subject hash, and no `openssl` process is spawned. `linuxStore.integration.test.ts` already proved the result with `openssl verify -CApath`; because the host and container now share one implementation, that check covers both. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

… cert The SAN-local restriction on the reverse-sync path only constrains a certificate that can authenticate ONLY itself. Nothing checked that. `isValidDevCert` gates on CN, validity window, and the ASP.NET OID version byte — none of which a container can't trivially satisfy, the OID included (it is just an extension you add). A container could therefore push a self-signed cert with `basicConstraints cA=TRUE`, `keyCertSign`, and SANs of `localhost` + `127.0.0.1`, sail through `validateLocalSans`, and have the host install it via `trustCertificate` — which is `certutil -addstore Root` on Windows, `add-trusted-cert -p ssl` on macOS, and the .NET Root store + OpenSSL CApath + browser NSS with the `C` "trusted CA" flag on Linux. All CA positions. The container would then hold a CA key the host trusts and could sign a leaf for any name at all; a CA's own SANs place no limit on what it issues, and nothing checks name constraints. Verified by probing the real validators: a CA=true cert returned `isValidDevCert: true` and `validateLocalSans: {ok: true}`. `validateLeafTrustShape` now gates the SAN check. basicConstraints must be present with cA=FALSE (absent leaves the question to each validator's historical quirks), and EKU must be present, include id-kp-serverAuth, and exclude anyExtendedKeyUsage (absent EKU reads as "any purpose", and Windows `-addstore Root` applies no policy constraint of its own). Extra concrete usages such as clientAuth are tolerated so the check isn't brittle. Every genuine dev cert carries both extensions in this shape, which a new test pins by driving the real `generateCertificate` through the accept path. Alongside that, `collectSanEntries` became `scanSanEntries` and now reports why a SAN set is unusable instead of quietly returning what it recognized: - No SAN extension, or an empty one, used to return `ok` — vouching that "SANs are local-only" for a cert whose scope was never established. - A GeneralName type other than dNSName / iPAddress (rfc822Name, uniformResourceIdentifier, directoryName…) used to be dropped. Those play no part in TLS server identity, so ignoring them was defensible, but it meant reporting on a cert we had only partially inspected. - Undecodable SAN DER: `@peculiar/x509` parses extensions lazily and throws from `getExtension`, which escaped into the accept handler's blanket catch and landed as a generic parse failure. Fail-closed by accident of the call site — adding a `try/catch` inside the scanner, the obvious defensive edit, would have silently inverted it. Now caught and named locally. All three surface as `malformed-sans`, which `allowNonLocalContainerCertSans` deliberately does NOT override: that setting lets a user say "yes, I mean to trust this cert for that name", which is meaningless for a cert whose names we could not read. It still overrides `non-local` exactly as before. Tests pin the non-override for both the structural SAN case and the CA case. Also fixes a pre-existing 1-in-256 flake found while re-running the suite: `generateSerialNumber` cleared the high bit of the leading byte, which can leave 0x00. DER retains that byte as sign padding (`02 10 00 b5 ...`, so the emitted certs were always conformant), but every textual readback drops it, making the serial look like a 15-byte value starting at or above 0x80 — and `generator.test.ts`'s "positive serial number" assertion then failed. The leading byte is now rejection-sampled into 0x01..0x7f, so serials are positive, non-zero, and minimally encoded with no padding byte to reason about. Pinned with 10k direct samples rather than 10k RSA keygens. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

`runProcess` used Node's `execFile` default `maxBuffer` of 1 MiB. `security find-certificate -a` on macOS can exceed it — it dumps every certificate in the login keychain, ~2 KB per entry for the attribute form and ~1.5 KB for the PEM form, so roughly 500-700 certs. Uncommon, but MDM-pushed user certs and years of accumulated dev certs get there. The failure was silent and self-feeding. Measured, not assumed: 2MB -> exitCode=1 stdoutLen=1048576 stderr="" raw: code="ERR_CHILD_PROCESS_STDIO_MAXBUFFER" (a string) Because Node reports overflow with a *string* `error.code`, `typeof error.code === "number" ? error.code : 1` folded it into `exitCode: 1`, and stderr came back empty — nothing in the returned `ProcessResult` distinguished "the command failed" from "the command was succeeding and we discarded everything past 1 MiB". `isCertInKeychain` read that as "not in the keychain", which force-skipped the on-disk PFX as an orphaned cache file, emptied `findExistingDevCert`, made `checkStatus()` report `exists: false`, and sent `CertManager.trust()` down the `generate()` branch — a fresh cert plus an `add-trusted-cert` keychain password prompt, on every provisioning request. Each new cert then landed in the same keychain, so the next call truncated sooner: a loop that fed itself with no way out. Two changes. The cap is now 32 MiB (~16,000 keychain entries; the buffer is only as big as the output actually produced), and `ProcessResult.truncated` distinguishes overflow from failure. `isCertInKeychain` deliberately fails OPEN on truncation — inverting the usual instinct, because here the closed direction is the destructive one. The open direction costs at most one redundant re-trust: `checkStatus` establishes trust separately through `security verify-cert`. `enumerateKeychainDevCerts` only drives a warning, so it logs the shortfall and carries on with the prefix it got. Tests drive real child processes rather than mocks, since the whole point is Node's own semantics: 2 MiB now arrives intact, `yes` still overflows and sets the flag, and an ordinary non-zero exit does not. Recorded as follow-ups in AGENTS.md: narrowing both keychain queries with `-c localhost` (the actual fix, but `security`'s `-c` semantics can't be verified off macOS and a false negative lands in the destructive direction), and evaluating `@azure/core-process` in place of hand-rolled `execFile` + PATH resolution — including what to confirm first (relative-PATH-entry skipping, preserved truncation semantics, VSIX bundle cost) and that it currently has exactly one published version, 1.0.0 from 2026-08-13. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

AGENTS.md claimed the UI extension "has no user-facing commands" and "exposes only the internal getCertMaterial command". Both halves were stale, and the second contradicted the Architecture section a few lines above, which already lists all four cross-host entry points. Actual surface: `contributes.commands` holds exactly one palette entry, `devcontainer-dev-certs.trustInBrowsers`, while `activate()` additionally registers four IPC commands (getCertMaterial, getAllCertMaterial, getAllCertMaterialV3, acceptContainerDevCert) that never appear in the palette. The "provisioning happens automatically, with no command for it" intent was correct and is kept, now stated as a decision rather than as a claim about the command count. Also recorded two properties of trustInBrowsers that the README documents for users but AGENTS.md did not explain for maintainers: it resolves its target via certManager.check(), so it can only re-import the host-generated cert — a cert accepted through syncContainerCert is public-cert-only and never lands in `my/`, so check() cannot see it — and it carries no `when` clause, so it stays visible in the palette on Windows and macOS where it no-ops with an informational message. Two references to the reverse-sync validation pair now also name validateLeafTrustShape, which gates them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

The consent boolean could only ever record *yes*. An accept was written to global state forever; a decline wrote nothing, so `pushContainerCertToHost` re-showed the modal on every container activation. The only durable state a security prompt could reach was the permissive one — a one-way ratchet — and the only ways to stop being asked were disproportionate: edit devcontainer.json and rebuild, or disable `generateDotNetCert`, which also kills host-side generation. `containerCertProvisionConsented` is now "granted" / "denied" / absent, read through `normalizeContainerCertConsent`, which maps the historical `true` to "granted" so anyone who already consented is never re-prompted. Unrecognized values fall back to "unset" — ask, rather than silently opt the user in or out. The modal has three outcomes, and separating the last two is the point: - Trust → record "granted", still AFTER the trust step, so a failed add-trusted-cert leaves consent unpersisted and the next push re-prompts (unchanged ordering). - Never → record "denied" immediately; there is no trust step to fail. - Cancel → decline this push, record NOTHING, so a stray Escape cannot disable the feature permanently. A standing "denied" short-circuits before the prompt, which is what makes declining actually stop the asking. Consent stays host-wide rather than per-thumbprint because container dev certs rotate on rebuild: unless the cert is baked into the image or `~/.dotnet/corefx/cryptography/x509stores/my/` is on a volume, a fresh one is minted each rebuild, so per-certificate consent would prompt every rebuild — the shape of consent people click through without reading. That assumption is now written down so it gets revisited before anyone "improves" the granularity. Adds `devcontainer-dev-certs.resetContainerCertConsent` ("Dev Certs: Reset Container Certificate Consent"). Without it, Never would be a one-way ratchet in the opposite direction — reachable only by hand-clearing extension state — which is the same defect pointing the other way. It clears either recorded answer and deliberately does not untrust certs already in the host store. Also documents a gap this surfaced rather than fixing it: because certs rotate per rebuild and every accepted one is trusted permanently, a developer who rebuilds regularly accrues one trusted CN=localhost leaf per rebuild across every trust surface, with nothing to prune them. Closing that needs a targeted per-platform untrustCertificate — acceptable to reintroduce, but only wired to a real entry point. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Rebuild-rotation is the default, not an edge case: most dotnet devcontainer base images mint a fresh dev cert on the container's first HTTPS build, and nothing lets this extension verify that a devcontainer.json setting syncContainerCert:true has persisted its cert store. So the design has to assume a new certificate arrives on every rebuild. Under that assumption the trust directory breaks at rebuild 11. Every ASP.NET dev cert is CN=localhost, so all of them share one OpenSSL subject hash and consume `{hash}.{n}` slots by certificate COUNT, not by genuine hash collision. `ensureHashSymlink` stopped at ten and then fell out of its loop without allocating, logging, or throwing. Measured on twelve rotations: rebuild 10: pems=10 symlinks=10 thisCertLinked=YES rebuild 11: pems=11 symlinks=10 thisCertLinked=** NO ** openssl verify newest via -CApath: FAILED (error 18, self-signed) openssl verify oldest via -CApath: OK The host went on trusting ten dead certificates while the live one was unreachable — the feature inverted, with nothing in the log to say so. The ten-slot cap was ours, not OpenSSL's: `by_dir` walks `{hash}.{n}` upward until a file is missing. Raised to 256, with a warning once a hash exceeds ten entries (so accumulation is visible rather than silent) and an explicit failure log at the bound, because a certificate with no reachable slot is indistinguishable from an untrusted one at the point of use. The old test asserted the broken behavior — that an 11th same-subject PEM was correctly refused a slot — so it is replaced with one that pins the opposite, plus a contiguity test: `by_dir` stops at the first gap, so any future pruning must re-densify via rehashDirectory rather than unlink in place. The regression is pinned end to end in linuxStore.integration.test.ts, which drives twelve rotations and asks real `openssl verify -CApath` whether the newest cert is reachable. AGENTS.md now records rebuild-rotation as the governing assumption rather than something to revisit, and restates the accumulation gap with what makes it hard: 365-day validity bounds the security exposure but not the clutter, and "untrust the previous cert on accept" ping-pongs when two containers are open — which is why trustViaOpenSsl was made additive to begin with. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Copilot

🟡 Changes recommended

OpenSSL canonicalization remains incorrect for several valid string encodings, and existing broken hash links are not repaired during upgrade.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Centralizes certificate utilities in the shared package while hardening Linux trust, container-certificate validation, consent, and process handling.

Changes:

Implements canonical OpenSSL subject hashing and expanded collision handling.
Moves certificate/platform utilities into the shared package and removes UI shims.
Strengthens certificate validation, consent state, NSS coexistence, and regression coverage.

File summaries

File	Description
`src/vscode-workspace-extension/tests/rehash.test.ts`	Expands hash and slot tests.
`src/vscode-workspace-extension/tests/installUserCert.test.ts`	Tests stale PFX cleanup.
`src/vscode-workspace-extension/src/util/upmap.ts`	Uses shared certificate name.
`src/vscode-workspace-extension/src/util/rehash.ts`	Removes relocated implementation.
`src/vscode-workspace-extension/src/extension.ts`	Clears stale Kestrel defaults.
`src/vscode-workspace-extension/src/containerCertPush.ts`	Handles new rejection reasons.
`src/vscode-workspace-extension/src/cleanupCerts.ts`	Uses shared rehash utility.
`src/vscode-workspace-extension/src/certInstaller.ts`	Uses shared hashing and handles opt-outs.
`src/vscode-ui-extension/tests/windowsStore.test.ts`	Updates shared imports and mocks.
`src/vscode-ui-extension/tests/windowsStore.integration.test.ts`	Uses shared Windows store APIs.
`src/vscode-ui-extension/tests/validateLocalSans.test.ts`	Tests structural SAN rejection.
`src/vscode-ui-extension/tests/selectBestDevCert.test.ts`	Uses shared classifier APIs.
`src/vscode-ui-extension/tests/resolveSafeExecPath.test.ts`	Tests process output limits.
`src/vscode-ui-extension/tests/pkcs12LegacyPbe.test.ts`	Updates shared imports.
`src/vscode-ui-extension/tests/nssTrust.test.ts`	Tests per-certificate NSS names.
`src/vscode-ui-extension/tests/nssTrust.integration.test.ts`	Uses shared NSS utilities.
`src/vscode-ui-extension/tests/nativeBackend.test.ts`	Reformats test imports.
`src/vscode-ui-extension/tests/manager.test.ts`	Updates manager tests for additive behavior.
`src/vscode-ui-extension/tests/macStore.test.ts`	Tests truncated keychain enumeration.
`src/vscode-ui-extension/tests/loader.test.ts`	Uses shared loading APIs.
`src/vscode-ui-extension/tests/linuxStore.test.ts`	Tests in-process canonical hashing.
`src/vscode-ui-extension/tests/linuxStore.integration.test.ts`	Verifies multi-rotation OpenSSL trust.
`src/vscode-ui-extension/tests/legacyPfxRejection.test.ts`	Uses shared PFX parser.
`src/vscode-ui-extension/tests/hostCertGenerator.test.ts`	Uses shared generation APIs.
`src/vscode-ui-extension/tests/generator.test.ts`	Tests minimally encoded serials.
`src/vscode-ui-extension/tests/exportLoadedCert.test.ts`	Uses shared export APIs.
`src/vscode-ui-extension/tests/exporter.test.ts`	Uses shared exporter APIs.
`src/vscode-ui-extension/tests/dotnetPfx.integration.test.ts`	Uses shared generation/export APIs.
`src/vscode-ui-extension/tests/dotnetMacosCache.integration.test.ts`	Uses shared PFX loader.
`src/vscode-ui-extension/tests/dotnetBackend.test.ts`	Updates process-result mocks.
`src/vscode-ui-extension/tests/containerCertAccept.test.ts`	Tests trust shape, SANs, and consent.
`src/vscode-ui-extension/tests/classifyCandidate.test.ts`	Uses shared classifier types.
`src/vscode-ui-extension/tests/certProvider.test.ts`	Uses shared certificate APIs.
`src/vscode-ui-extension/src/platform/windowsStore.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/platform/types.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/platform/processUtil.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/platform/nssTrust.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/platform/macStore.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/platform/linuxStore.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/platform/baseStore.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/extension.ts`	Adds tri-state consent and reset command.
`src/vscode-ui-extension/src/containerCertAccept.ts`	Enforces leaf, EKU, and SAN validation.
`src/vscode-ui-extension/src/certProvider.ts`	Imports directly from shared.
`src/vscode-ui-extension/src/cert/types.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/cert/properties.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/cert/pfx.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/cert/manager.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/cert/loader.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/cert/generator.ts`	Removes re-export shim.
`src/vscode-ui-extension/src/cert/exporter.ts`	Removes re-export shim.
`src/vscode-ui-extension/package.json`	Contributes consent-reset command.
`src/shared/src/platform/windowsStore.ts`	Removes broad certificate deletion.
`src/shared/src/platform/types.ts`	Removes removal API contract.
`src/shared/src/platform/processUtil.ts`	Adds larger buffers and truncation reporting.
`src/shared/src/platform/nssTrust.ts`	Adds thumbprint-based NSS nicknames.
`src/shared/src/platform/macStore.ts`	Handles truncated keychain output.
`src/shared/src/platform/linuxStore.ts`	Uses shared in-process rehashing.
`src/shared/src/platform/baseStore.ts`	Updates platform-store abstraction.
`src/shared/src/index.ts`	Exports consolidated shared APIs.
`src/shared/src/cert/validation.ts`	Adds structural SAN and leaf validation.
`src/shared/src/cert/rehash.ts`	Implements canonical hashing and slot allocation.
`src/shared/src/cert/manager.ts`	Makes generation additive.
`src/shared/src/cert/generator.ts`	Produces minimally encoded serial numbers.
`src/shared/src/backends/select.ts`	Removes unused backend description API.
`README.md`	Documents commands and shared architecture.
`AGENTS.md`	Records architecture and security decisions.

Review details

Files reviewed: 66/66 changed files
Comments generated: 6
Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

…edos) CodeQL flagged `src/shared/src/cert/rehash.ts:210` high on PR #87. The regex /-----BEGIN CERTIFICATE-----\s*([\s\S]*?)\s*-----END CERTIFICATE-----/ wraps a lazy `[\s\S]*?` in two `\s*` quantifiers. All three match whitespace, so input that opens with the BEGIN marker and continues with a run of spaces but never reaches an END marker leaves the engine an ambiguous split to backtrack over, and matching goes quadratic. Measured: 5000 spaces -> old regex 40727ms indexOf 0ms 10000 spaces -> old regex (did not finish in 300s) The regex predates this branch, but moving the file into the shared package brought it into the diff, so it is this PR's to fix. It is reachable rather than theoretical: `rehashDirectory` feeds this function every `*.pem` file it finds in the OpenSSL trust directory, and nothing guarantees those files are well-formed — the host's trust dir in particular accumulates files this extension did not write. Replaced with `indexOf` + `slice`, which scans linearly and cannot backtrack. The whitespace strip stays `/\s/g` — one character class, no quantifier, so also linear. Behaviour is otherwise identical: first BEGIN paired with the first following END, surrounding whitespace discarded, null when either marker is absent. Concatenated-PEM and missing-marker cases are now pinned explicitly since they were previously only implied by the regex. The regression test uses 100k spaces, which under the old regex is on the order of hours, so a reintroduction hangs the suite rather than slowing it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Addresses the Copilot review on #87. Four of its six findings were real; all four are fixed here. **Canonicalization was wrong for non-UTF8 string types and for whitespace.** I had reconstructed `asn1_string_canon` from the OpenSSL source, which reads as though it folds bytes in place. It does not. Settled against `openssl x509 -hash` on real certificates (3.0.13): - A T61String holding the UTF-8 bytes of 日本語 hashes to 02c4fa54. Re-tagging those bytes as UTF8String gives e2c402e4; only reading them as Latin-1 and re-encoding as UTF-8 reproduces OpenSSL. BMPString (UTF-16BE) and UniversalString (UTF-32BE) are transcoded the same way. - Folding covers every ASCII whitespace byte, not just 0x20: CN stored as "a\tb", "a\nb", "a b" and "A B" all hash to 49cdc5e0, and "a", " a", "\ta" all hash to 20b69a40, so trimming is not space-only either. Both produced plausible-looking {hash}.N links that OpenSSL never opens — the same class of failure this branch exists to fix, for any certificate whose subject is not plain ASCII. Fixtures for each case are pinned in tests/rehash.test.ts, built as minimal hand-rolled DER so a subject can use an encoding `openssl req` won't emit. **Upgraded installs kept their broken symlink.** The corrected hash was only written by installDotNetDevCert / installUserCert / trustCertificate, and all three are gated behind checks that only looked for files. A container or host set up before this branch has the PEM, the PFX and a symlink under the WRONG hash, so `isCertInstalled` and Linux `isTrusted` both reported health, the repair was skipped, and trust stayed dead for exactly the users the fix targets. Both predicates now require a symlink that actually resolves (`hasHashSymlink`). A PEM whose subject cannot be hashed is exempt: `ensureHashSymlink` is a no-op for it, so demanding a link would re-run the install on every activation and never converge. **Slot exhaustion still lied to the caller.** After 256 same-subject certificates `ensureHashSymlink` logged and returned, so the install reported success for a certificate OpenSSL could not find. Under the documented rebuild-rotation model with no pruning that bound is reachable, making it the ten-slot bug again with a bigger number. It now throws. **Stale doc comment** on ensureHashSymlink still advertised slots 0-9. The two findings not actioned as code: the PR description's "10 MiB" is stale against the implementation's 32 MiB — the constant is deliberate and documented, so the description is what needs correcting — and the review's claim that OpenSSL calls ASN1_STRING_to_UTF8 before folding is right in effect even though the source does not name that function; the transcode is what matters and is now implemented. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Copilot

🟡 Changes recommended

Hash-link detection mishandles occupied regular slots, and same-name user certificate rotations can remain stale.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (4)

Previously missed (2) — in code that hasn't changed since the last review.

src/vscode-workspace-extension/src/containerCertPush.ts:368

This message is inaccurate for the missing-basic-constraints case, which maps here even when the certificate is not a CA. Tell the user that the certificate is a CA or does not explicitly declare itself as a leaf so the rejection matches the actual validation result.

This issue also appears on line 389 of the same file.
AGENTS.md:13

The idempotency description contradicts the implementation: CertManager.trustExternalCertificate calls store.isCertTrusted() and returns before trustCertificate, and macOS explicitly documents that add-trusted-cert is not a no-op. Update this architecture note so future changes do not rely on a nonexistent platform guarantee.

  - `devcontainer-dev-certs.acceptContainerDevCert({ thumbprint, pemCertBase64 })` — **reverse-sync push entry point** (issue #63). Takes a **public-cert-only** PEM pushed from a Dev Container that opted into `syncContainerCert`, independently re-validates (`isValidDevCert` + `validateLeafTrustShape` + `validateLocalSans`), prompts for one-time consent (`containerCertProvisionConsented` global state — distinct from the host-generation consent because the user is approving trust of a cert that came from a container they may or may not control), and **only trusts** the cert in the host's OS trust surfaces (Root store / OpenSSL trust dir / NSS / keychain trust). Does NOT save to `CurrentUser/My`, the keychain identity slot, or the .NET `my/` dir; the host doesn't need the private key (Kestrel runs in the container with its own copy). Gated on the SAME host settings as the generation flow: `devcontainerDevCerts.generateDotNetCert` and `devcontainer-dev-certs.autoProvision`. SAN-local restriction has an opt-out via `devcontainerDevCerts.allowNonLocalContainerCertSans`. Idempotent on repeat pushes — no `alreadyTrusted` short-circuit, each platform's `trustCertificate` is a no-op for an already-trusted cert.

src/vscode-workspace-extension/src/certInstaller.ts:212

This checks only the stable filename and its existing hash link, so rotating a user certificate while keeping the same name is treated as already installed when installToDotNetStore is false. The container then keeps serving the old PEM indefinitely. Compare the on-disk PEM with material.pemCertBase64 before accepting the link as settled.
src/vscode-workspace-extension/src/containerCertPush.ts:389
malformed-sans also covers a certificate that has a valid DNS/IP SAN plus an unsupported GeneralName, so saying it has “no usable host names” can be false. Describe the SAN extension as missing, unreadable, empty, or unsupported instead.

Files reviewed: 66/66 changed files
Comments generated: 1
Review effort level: Balanced

… messages Second Copilot review on #87. All four findings were real. **hasHashSymlink disagreed with ensureHashSymlink about what a slot is.** `ensureHashSymlink` treats a regular file at `{hash}.N` as OCCUPIED and steps over it to a later slot; OpenSSL's `by_dir` likewise processes whatever it finds and keeps walking. But `hasHashSymlink` called readlink and treated any error as end-of-search — and readlink on a regular file raises EINVAL, not ENOENT. So it stopped before reaching our link and reported "not linked" for a certificate that was in fact reachable. Now that both `isCertInstalled` and Linux `isTrusted` consult it, that meant a perpetual reinstall / re-trust on every activation. It now lstats first: absent is a gap and stops the scan, present-but-not-a-symlink is skipped. **A user cert rotated under the same name looked installed.** User certs are keyed by the user-chosen `name`, not by thumbprint, so a rotated certificate lands on the same `{name}.pem`. The stale file's own hash link resolves perfectly well, so the existence check passed and the container went on serving the superseded certificate indefinitely. `isCertInstalled` now compares the on-disk PEM against the material it was handed. (Pre-existing rather than introduced here, but in a function this PR rewrites.) **Two user-facing messages overstated their case.** `not-a-leaf-cert` also covers `missing-basic-constraints`, where the certificate is not necessarily a CA — the log line said so but the toast asserted flatly that it was one. And `malformed-sans` covers a SAN carrying a valid DNS/IP entry alongside an unsupported GeneralName, so "no usable host names" could be simply false. Both now describe the actual validation result. **AGENTS.md contradicted the implementation.** It claimed reverse-sync pushes are idempotent because "each platform's `trustCertificate` is a no-op for an already-trusted cert", with no `alreadyTrusted` short-circuit. There is one: `trustExternalCertificate` calls `store.isCertTrusted(cert)` and returns early, precisely because `security add-trusted-cert` is NOT a no-op on macOS and can re-prompt for the keychain password. Left as written, that note would have invited someone to delete the check on the strength of a platform guarantee that does not exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Copilot

🟡 Changes recommended

Container-controlled certificates are still installed into CA trust positions, so the new leaf-shape validation does not reliably prevent arbitrary certificate issuance.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/shared/src/cert/rehash.ts:522

The new UniversalString/UTF-32BE canonicalization path has no fixture, although the T61String and BMPString branches are pinned against openssl x509 -hash. Add a hand-built UniversalString subject (including a non-ASCII code point) with an OpenSSL-derived expected hash so byte order and code-point conversion regressions cannot silently create unusable links.

    case 0x1c: {
      // UniversalString — UTF-32BE, decoded a code point at a time.
      if (content.length % 4 !== 0) return null;
      let text = "";
      for (let i = 0; i < content.length; i += 4) {
        const codePoint = content.readUInt32BE(i);
        if (codePoint > 0x10ffff) return null;
        text += String.fromCodePoint(codePoint);
      }
      return Buffer.from(text, "utf8");

src/vscode-workspace-extension/src/certInstaller.ts:208

This can report the user cert fully installed when its .NET Root-store PFX was deleted or a previous install stopped after writing the PEM. Current V3 bundles provide rootPfxBase64 whenever container trust is requested, and installUserCert writes that artifact, but this branch only checks the OpenSSL link; activation then skips the reinstall and .NET clients remain untrusted. Check the Root PFX conditionally when rootPfxBase64 is present before returning true.

Files reviewed: 66/66 changed files
Comments generated: 1
Review effort level: Balanced

Two of the three findings from the third Copilot review on #87. The third is a design question about trust-anchor semantics and is raised on the PR rather than actioned here. **UniversalString had no fixture.** T61String and BMPString were pinned against `openssl x509 -hash`, but the UTF-32BE branch was only exercised by its own implementation. To fix that non-circularly the hand-built cert helper now emits a COMPLETE Certificate — signatureAlgorithm, dummy signatureValue, placeholder subjectPublicKeyInfo — so `openssl x509` can parse it, even though `computeSubjectHash` stops reading at the subject. That yields an OpenSSL-derived expected value (ba8aa3f2 for CN="Tëst"), plus a test that UniversalString and UTF8String of the same text hash identically, which is the property transcoding exists to provide. It also allowed a broader guard: a cross-check that re-derives all five encodings (UTF8, UniversalString, BMPString, T61String, PrintableString with whitespace) from the local openssl binary rather than trusting the recorded constants. **A user cert's .NET Root-store PFX was not part of "installed".** `installUserCert` writes it whenever the bundle carries `rootPfxBase64`, but `isCertInstalled` checked only the OpenSSL PEM and hash link — so deleting that file, or an install interrupted between the two writes, left a cert reported as fully installed while .NET clients in the container kept distrusting it. The dotnet-dev branch already checked its Root PFX; this makes the user branch symmetric. Gated on `rootPfxBase64` so a bundle that never supplied one isn't held to a file the install would not have written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

We sent a blanket `-t "CT,,"` to every NSS database. That is right for Firefox and wrong for Chromium. The dev cert is a self-signed end entity — `generateCertificate` emits `basicConstraints` with `cA=FALSE`. `C` is `CERTDB_TRUSTED_CA`, consulted only when a cert sits in an issuer position, which ours never does. The bit that applies to an end entity is `P` (`CERTDB_TRUSTED`, "trusted peer"). Sending `C` to a Chromium database produces an entry NSS refuses to validate: `certutil -V -u V` reports "Issuer certificate is invalid". Firefox is an empirical exception — it ignores `P` for server certs, so `C` is what actually produces trust there. This mirrors `dotnet dev-certs https --trust`, whose `UnixCertificateManager.TryAddCertificateToNssDb` makes the same split (`usage = nssDb.IsFirefox ? "C" : "P"`). Microsoft validated that against real browsers; we follow it rather than re-deriving it. The same asymmetry explains why dotnet verifies Chromium databases with `-V -u V` but Firefox with only `-L`: `-V` cannot pass under `C`. `getNssTargets` already tags every target with its family, so the flag is threaded through from the existing `kind` rather than re-detected. The stray `T` (trusted CA for client auth) is dropped — we never needed it. Existing entries migrate through the delete-then-add that was already there for idempotency: `certutil -A` does not rewrite the trust string of an existing nickname, so the delete is what moves a Chromium database off the old flags. Tests: unit coverage that each family gets its own flag, including one call spanning both. The integration suite drives real `certutil` against a real generated cert to pin the underlying reason — `P,,` validates for server auth, `CT,,` does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Copilot

🔵 Needs a closer look

Trust failures are misreported as parse errors, and Linux trusted-state checks can accept stale PEM contents.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

src/vscode-ui-extension/src/containerCertAccept.ts:393

Platform trust failures are still returned as parse-failed by the outer catch. Cancelling macOS keychain authorization or exhausting OpenSSL hash slots therefore makes the workspace report “could not parse the certificate,” even though validation succeeded. Add a distinct trust-failed wire reason and report it as a trust/install error; reserve parse-failed for malformed certificate data.

  // re-trying trust without UX.
  await deps.trustCertificate(parsed);

src/shared/src/platform/linuxStore.ts:149

This verifies the link name using cert.pem, but never verifies that the linked on-disk PEM still contains that certificate. If the PEM is truncated or replaced while the root PFX and link remain, isCertTrusted returns true and trustExternalCertificate skips repair, although OpenSSL loads different/invalid bytes. Compare the file with cert.pem and check the link against that on-disk content, as the workspace-side check does.

      fs.existsSync(pemPath) &&
        fs.existsSync(rootPfxPath) &&
        hasHashSymlink(trustDir, pemFileName, cert.pem)

Files reviewed: 66/66 changed files
Comments generated: 0 new
Review effort level: Balanced

Two findings from the suppressed comments on the fourth review round. Both real. **Trust failures were reported as parse failures.** `acceptContainerDevCert` wraps its whole body in a try/catch that maps every exception to `parse-failed`, and that catch also swallowed a throw from `trustCertificate`. A cancelled macOS keychain dialog, an unwritable NSS database, or an exhausted hash slot all told the user "the host could not parse the container's dev certificate" — false, and pointing at the wrong remedy, since the certificate had already parsed and passed every validation gate. This branch made it worse: `ensureHashSymlink` now throws at slot exhaustion where it previously returned silently, so a new failure mode was routed straight into the wrong message. The trust step now has its own catch and a distinct `trust-failed` wire reason, with a message saying the certificate is fine and the install is what failed. Consent still stays un-persisted on failure — that invariant is unchanged and still tested. **Linux `isTrusted` checked the PEM's name, not its contents.** It verified the file existed and that a hash link resolved to it, but never that the file still held the certificate in question. A truncated or externally-rewritten PEM kept both, so `isCertTrusted` returned true, `trustExternalCertificate` short-circuited, and OpenSSL went on loading bytes it cannot parse. The filename is thumbprint-derived, so this is narrower than the same-name user-cert rotation fixed in 4838133 — but it is the same class of bug, and the workspace extension's `pemInstalledAndLinked` already compares contents. The two sides of one trust directory should not disagree about what "installed" means. `isTrusted` now compares the on-disk PEM against `cert.pem` and resolves the link against what is actually on disk, with the same carve-out for a subject that cannot be hashed (`ensureHashSymlink` writes no link for one, so requiring a link would never converge). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

Latest stable (confirmed against the NuGet flat-container index; 13.5.3 is the newest non-prerelease of Aspire.AppHost.Sdk). This is the only Aspire version pin in the sample project. The CLI installer in .devcontainer/aspire-cli/install.sh tracks `--quality release` rather than a fixed version, so it needs no change. Not build-verified: no dotnet SDK in this environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EViCNJno9sPrnF6kaWJKwP

claude added 7 commits August 28, 2026 00:09

dnegstad requested a balanced review from Copilot August 28, 2026 01:45

Copilot started reviewing on behalf of dnegstad August 28, 2026 01:46 View session

github-advanced-security Bot found potential problems Aug 28, 2026

View reviewed changes

Comment thread src/shared/src/cert/rehash.ts Fixed

Copilot AI reviewed Aug 28, 2026

View reviewed changes

claude added 2 commits August 28, 2026 01:57

dnegstad requested a balanced review from Copilot August 28, 2026 02:14

Copilot started reviewing on behalf of dnegstad August 28, 2026 02:15 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread src/shared/src/cert/rehash.ts

dnegstad requested a balanced review from Copilot August 28, 2026 02:27

Copilot started reviewing on behalf of dnegstad August 28, 2026 02:28 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread src/shared/src/cert/validation.ts

claude added 2 commits August 28, 2026 02:38

dnegstad requested a balanced review from Copilot August 28, 2026 05:46

Copilot started reviewing on behalf of dnegstad August 28, 2026 05:46 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

claude added 2 commits August 28, 2026 06:00

Navigation Menu

Move cert utilities to shared package and fix OpenSSL hash canonicalization - #87

Move cert utilities to shared package and fix OpenSSL hash canonicalization#87
dnegstad wants to merge 14 commits into
mainfrom
claude/project-review-correctness-29djry

dnegstad commented Aug 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

dnegstad commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Correctness fixes

OpenSSL subject hash was wrong (container trust silently dead)

Hash slots ran out after ten certificates

Upgrades did not repair either fix

NSS trust flags were wrong for Chromium

ReDoS in PEM extraction (CodeQL, high)

Other silent failures

Security

Cleanup

Process handling

Deferred

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (4)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟡 Changes recommended

Suppressed comments (2)

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🔵 Needs a closer look

Suppressed comments (2)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dnegstad commented Aug 28, 2026 •

edited

Loading