docs: restructure READMEs so the happy path leads and advanced usage is findable - #80
Merged
Conversation
…is findable Main README: - Reorder sections so usage leads: Getting Started -> How It Works -> Configuration reference -> workflows -> Troubleshooting -> reference -> Development -> Provenance. Release provenance and Repository Layout were sitting at positions 3 and 4, ahead of any configuration docs. - Add a Contents list; the file has 25+ top-level and nested sections with no navigation outside GitHub's outline sidebar. - Add "Verify it worked" with the three store/trust checks and a curl call, so the happy path ends in confirmation rather than an assumption. - Add a Troubleshooting section: output channel names for both extensions, plus the common failure modes (nothing happened, browser still warns, curl fails, Kestrel picks the wrong cert, container push rejected). - Add a Commands reference. All three commands were previously mentioned only inside settings-table cells, and the host-side browser trust command not at all. - Add a Linux browser trust section. NSS/certutil was documented only in the host extension README, though without it forwarded ports still warn in the browser on Linux hosts. - Add the per-entry field table for userCertificates. The host settings table pointed at a "per-entry schema" that did not exist, leaving trustInContainer used in an example but never defined. - Disambiguate generateDotNetCert: the feature option and the host setting share a name and had near-identical descriptions in adjacent tables. The feature row now describes pulling the host cert into the container. - Document setup-cert.sh: both invocation forms, the bundle JSON shape, the openssl/jq prerequisites, and the fact that the feature does not copy the script into the container. - Add an environment variable reference covering SSL_CERT_DIR, the DEVCONTAINER_DEV_CERTS_* vars, and the DOTNET_GENERATE_ASPNET_CERTIFICATE suppression, and move the suppression rationale out of How It Works. - Fix a broken sentence in the opening paragraph; normalize "Dev Container" capitalization in prose; link Prerequisites to Supported Platforms. Extension READMEs: - Host: add a Commands table; promote browser trust from a fourth-level subsection to a top-level section next to Quick Start; add a Troubleshooting pointer with the output channel name. - Remote: drop a duplicated install-prompt sentence; add "Verify it worked" and a Troubleshooting pointer. - Both: trim the repeated 7-row feature options table to the options that affect that extension, linking to the main reference for the rest, and replace the duplicated dotnet suppression prose with a summary plus link. - Both: link back to the repository, which neither marketplace page did. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
The script isn't installed into the container by the feature and isn't reachable in a usable form today, so documenting it as a non-VS Code path promised support that doesn't currently exist. Removes the section, its Contents entry, and the Repository Layout line. Limitations now states plainly that VS Code is the only supported editor. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
There was a problem hiding this comment.
Pull request overview
Restructures documentation around setup, configuration, verification, and troubleshooting.
Changes:
- Reorders the main README and adds navigation and references.
- Expands verification, Linux browser trust, and troubleshooting guidance.
- Aligns extension READMEs with the central documentation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
README.md |
Reorganizes and expands project documentation. |
src/vscode-ui-extension/README.md |
Updates host extension usage and reference material. |
src/vscode-workspace-extension/README.md |
Updates remote extension verification and troubleshooting. |
Comments suppressed due to low confidence (3)
README.md:212
- This value is not always formed with a colon. When pruning removes every system CA directory,
install.shdeliberately emits the trust directory alone to avoid a trailing empty path element (install.sh:280-289,333-339). Document the conditional join so users do not infer that a trailing colon is expected.
| `SSL_CERT_DIR` | `$HOME/.aspnet/dev-certs/trust:` + the resolved `sslCertDirs` | Always |
README.md:401
echo $SSL_CERT_DIRprints an expanded absolute home path (for example,/home/vscode/.aspnet/...), never a path beginning with the literal~. As written, a correctly installed feature fails this troubleshooting criterion.
4. **Did the feature actually install?** Check `echo $SSL_CERT_DIR` in a container terminal — it should start with `~/.aspnet/dev-certs/trust`. If it's empty, the feature didn't run; rebuild the container.
README.md:506
- This repository-layout entry references a nonexistent
test/hydrate.mjs. The actual hydration script used by the package command istest/sample-project/hydrate-sample-project.mjs, so the current path sends contributors to the wrong location.
hydrate.mjs Assembles a runnable test project from the template + feature
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
From PR #80 review: - The env var section claimed the feature's variables reach `docker exec` generally. install.sh uses three sinks — /etc/profile.d (login shells), /etc/environment (pam_env: sshd, console), and a system-bashrc bridge for interactive non-login shells. `docker exec -it ... bash` is covered; `docker exec ... env` and directly exec'd binaries are not. Say so. - SSL_CERT_DIR is only colon-joined when a system CA directory survives pruning; install.sh emits the trust directory alone otherwise. The table implied an unconditional trailing colon. - Two troubleshooting steps told users `echo $SSL_CERT_DIR` should start with a literal `~`. $HOME expands when profile.d is sourced, so a healthy container prints /home/vscode/... and the check failed on working setups. - Repository Layout listed util/paths.ts (lives in shared/) and test/hydrate.mjs (is test/sample-project/hydrate-sample-project.mjs), and omitted the shared package holding the cert primitives. Rebuilt the block against the tree; all 41 entries verified to exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
Comments suppressed due to low confidence (4)
README.md:108
- This section describes the browser-trust command as mandatory, but host provisioning already calls
trustInNssBrowsers()throughLinuxStore.trustCertificate(). Document the command as the retry path after the automatic import fails so users do not repeat a successful trust operation.
On Linux, Firefox and Chromium use [NSS](https://firefox-source-docs.mozilla.org/security/nss/) for certificate trust and do **not** read OpenSSL trust directories or the .NET root store. Without this step, forwarded ports still show a certificate warning in the browser even though `curl` works fine.
Run **Dev Certs: Trust Certificate in Browsers** from the Command Palette on the host. It:
README.md:410
- Windows cannot follow this troubleshooting step:
WindowsStore.trustCertificate()deliberately usescertutil.exe, which skips the CryptoAPI confirmation dialog. Split the guidance so Windows users check the host log for import errors instead of looking for a dialog that never appears.
- **Windows / macOS:** confirm you accepted the OS trust dialog on first use. If you dismissed it, the cert exists but isn't trusted — reload the window to re-trigger the flow.
src/vscode-ui-extension/README.md:200
- The Windows implementation uses
certutil.exespecifically to avoid the CryptoAPI confirmation dialog, so the parenthetical is factually incorrect. Document the non-interactive trust mechanism instead.
| Windows | Added to `CurrentUser\Root` (triggers system dialog on first use) |
README.md:449
- Browser trust is a separate NSS trust surface on Linux, but it is not always a separate user step because provisioning already attempts the NSS import automatically. Qualify this limitation so it only directs users to manual action when the automatic attempt cannot complete.
- **Linux browser trust is a separate step.** Firefox and Chromium on Linux don't read the OS/OpenSSL trust stores; see "[Linux hosts: browser trust](#linux-hosts-browser-trust)".
From the second PR #80 review round: - Windows trust does not show an OS dialog. WindowsStore.trustCertificate() uses `certutil.exe -addstore Root` specifically because it goes through CryptoAPI directly and skips the confirmation dialog that the PowerShell Import-Certificate path raised. Removed the "system dialog" / "MMC dialog" claims from first-run setup, the container-push flow, troubleshooting, the limitations list, and the host trust table. - Container-push consent is one global flag (CONTAINER_CERT_CONSENT_KEY), not per-thumbprint. Corrected the claim that trust prompts accumulate per cert; what accumulates is trust-store entries, plus macOS keychain authorization for each distinct cert. - Linux NSS trust is already automatic: LinuxStore.trustCertificate() calls trustInNssBrowsers(), and the reporter is wired up in extension.ts. The docs presented "Dev Certs: Trust Certificate in Browsers" as a required step for every Linux user. It's now described as what it is — automatic, best-effort, with the command as the retry path when certutil or browser profiles are missing. Updated the browser-trust sections, the limitation, the troubleshooting bullet, and the command description. - Split the abbreviated Kestrel env var row into two rows with complete variable names, since that table is meant to be copyable. Also folded NSS into the host README's Linux trust layer list, which listed three layers and omitted the browser one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
…ution From the third PR #80 review round: - `trustInContainer: false` was described as syncing the cert files without trusting the issuer. installUserCert gates the root-store PFX and the trust directory PEM on that flag, and the .NET my/ write on installToDotNetStore, so with trust off and neither installUserCertsToDotNetStore nor extraCertDestinations configured, nothing is written for that entry. Both field tables now say so. - The host README attributed SSL_CERT_DIR to trustCertificate(). On Linux that method writes the .NET root store, the OpenSSL trust directory, and NSS; SSL_CERT_DIR is handled by ensureTerminalSslCertDir(), which runs only when the provider serves a dotnet-dev cert and never on the container-accept path. Listing it as a trust layer implied container-pushed certs configure host terminals. Split into three trust-store writes plus a separate note about the terminal environment. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
From the fourth PR #80 review round: - The remote README described the three canonical writes as if every bundled cert got them. Only the dotnet dev cert does. A user cert's my/ PFX needs installUserCertsToDotNetStore (minus excludeFromDotNetStore, and a private key), and its trust PEM + root-store PFX need trustInContainer. Split the bullet by cert kind. - "Dev Certs: Trust Certificate in Browsers" was documented as the recovery path for any Linux browser-trust failure. It resolves the cert via certManager.check() -> checkStatus() -> findExistingDevCert(), which reads the host my/ store; the syncContainerCert accept path goes through trustExternalCertificate() -> trustCertificate() and deliberately never writes there. So for a reverse-synced cert the command reports no cert found, or re-imports an unrelated host-generated one. Qualified the command in both READMEs and in both command tables, and documented the manual path: trustViaOpenSsl does write the pushed cert's PEM to the trust directory as aspnetcore-localhost-{thumbprint}.pem (additively, so host-generated and pushed certs coexist), so a manual import works if you match the thumbprint. Automatic NSS import is unaffected — trustCertificate() runs it for pushed certs too. Only the manual retry can't target them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
From the fifth PR #80 review round: - Both architecture summaries said the remote extension writes two locations. installDotNetDevCert writes three: my/{thumbprint}.pfx, root/{thumbprint}.pfx (public-cert-only, which is what makes .NET clients report the cert trusted), and the trust-dir PEM + hash symlink. The detailed per-kind description added last round already listed all three, so the docs contradicted themselves. Added the root store to both summaries and to the verification checks. - installUserCertsToDotNetStore was described as copying "every entry". The provider computes dotNetStorePfxBase64 from passwordlessPfxBase64, which is undefined without a private key, and the installer's write is gated on that field — so CA-only entries are skipped. Qualified in both settings tables. - `dotnet dev-certs https --check` only confirms a valid cert in the my/ store, which the preceding `ls` already covers. Switched the verification snippets to `--check --trust` so they exercise the root-store trust artifact too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (2)
README.md:141
- This conflates generated and user-managed key support. The production generation path calls
generateCertificate(now, expiry)with its RSA-2048 default; ECDSA/Ed25519 keys are accepted when loading user-managed certificates, not generated by the extension. Split those capabilities so users are not promised configurable non-RSA generation.
2. **Host Extension** (`extensionKind: ["ui"]`) runs on your local machine. It generates certificates identical to `dotnet dev-certs https` (same OID marker, same SAN entries, same key parameters) using Node's built-in `crypto` plus `@peculiar/x509` and `pkijs` for X.509 / PKCS#12 — supporting RSA, ECDSA, and Ed25519 keys. On first use, it generates a cert and trusts it in the host OS certificate store. It then serves the certificate material to the remote side via VS Code's cross-host command routing.
README.md:279
- Setting
trustInContainertofalsedoes not remove trust artifacts from an earlier installation.installUserCertskips this block when false but only removes themy/PFX; the existing root-store PFX and named PEM remain trusted. Qualify the claim or add cleanup for those two paths.
| `trustInContainer` | optional, default `true` | Plant the cert in the container's OpenSSL trust directory and .NET root store, so tools inside the container trust the issuer. Setting it to `false` suppresses both of those writes — it does **not** mean "copy the files without trusting them". With trust off, the cert reaches the container only via the .NET `my/` store (when `installUserCertsToDotNetStore` applies to the entry) or [`extraCertDestinations`](#extra-destinations); with neither of those configured, nothing is written for that entry at all. |
From the sixth PR #80 review round: - trustNss was described as providing Chromium/Firefox trust inside the container. install.sh only installs the libnss3-tools/nss-tools package — it never invokes certutil — and the workspace extension has no NSS code at all (only the OpenSSL trust dir and rehash). The option supplies tooling, not trust. Reworded the feature-option row and the in-container browser note, and said what the user has to run themselves. - Turning trustInContainer off is not retroactive. installUserCert sweeps the my/ PFX when an entry loses its opt-in, but the root-store PFX and trust PEM are written under `if (material.trustInContainer)` with no else branch, so files from an earlier sync survive and the cert stays trusted. Documented the manual cleanup in all three field tables, and noted that cleanupStaleDevCerts doesn't help — it only matches dev cert PFXes in my/. - `curl https://localhost:<port>` isn't runnable: bash parses <port> as a redirect. Replaced with a concrete port plus a substitution note. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
#82 removed the trustNss feature option. The three READMEs on this branch had been restructured heavily, so git mis-anchored its one-line deletion against them — resolved by keeping this branch's content and applying the removal by hand: - Main README: dropped the option row, switched the feature-options example from trustNss to syncContainerCert, and rewrote the in-container browser trust note to point at the Dockerfile / postCreateCommand instead of the removed option. - Both extension READMEs: dropped trustNss from the "feature also accepts" sentences. Net change vs main is docs-only; the feature and test files match main exactly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH
dnegstad
pushed a commit
that referenced
this pull request
Jul 28, 2026
The merge from main re-integrated Manual / non-VS Code documentation (section, ToC entry, installFallbackTools option row, path-hint env-var rows) into the restructured README. That worked against #80's explicit decision to document non-VS Code usage as unsupported — a future PR will add it back if that support is taken on. The shipped artifacts (setup-cert.sh delivery, bundle schema, examples/manual-setup) stay in the tree; the Repository Layout inventory lists them factually with an "unsupported" note but the README no longer documents the workflow. The hostCertGenerator setting rows stay — that's VS Code extension surface.
dnegstad
pushed a commit
that referenced
this pull request
Jul 28, 2026
PR #80 explicitly removed setup-cert.sh documentation from the README ('docs: drop the setup-cert.sh fallback section'); the layout entries added during the merge reintroduced references to it. The README's only remaining deltas vs main are the hostCertGenerator setting row and the layout lines describing this PR's shared-package code moves.
dnegstad
pushed a commit
that referenced
this pull request
Jul 28, 2026
Correctness: - CertManager.trust(): the reload from the store only ran when NOTHING was cached, so with cert A in memory and the store entry externally replaced by untrusted cert B, trust() re-trusted the orphaned A while B stayed untrusted — and CertProvider's staleness check would then export untrusted B. The reload now also fires whenever the cached thumbprint differs from the store's, making the store authoritative. Regression test drives the A→B replacement sequence. - resolveSafeExecPath: "has an extension" was derived from PATHEXT membership, so a user whose PATHEXT omits .EXE got `certutil.exe` probed as `certutil.exe.cmd`/`.bat` and exit 127 despite the binary being on PATH. Any filename extension now counts as explicit (path.win32.extname); PATHEXT only governs extensionless names. - macStore.isCertInKeychain: `security find-certificate` has no filter-by-hash option — `-Z` is an output modifier taking no argument, so the thumbprint was being parsed as a keychain path and the check could never work as intended. Now enumerates with `find-certificate -a -Z <keychain>` and matches the `SHA-1 hash:` lines (modern macOS also prints SHA-256 lines; the SHA-1 line is matched specifically). The test mock modeled the same fictional filter and was corrected to emit realistic enumeration output. Robustness: - install.sh installFallbackTools: added microdnf, yum, and zypper branches so RPM/SUSE-family images that lack dnf don't fall through to the warn-and-skip path with an unusable installer. Docs consistency: - examples/manual-setup README banner now states the non-VS Code path is unsupported (reference-only, no support commitments), matching #80's stance and the PR description instead of claiming "minimally supported".
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.
Restructures the three READMEs so the happy path leads and advanced usage is findable, then fixes factual errors found in review.
Main README
Reordered so usage leads: Getting Started → How It Works → Configuration reference → workflows → Troubleshooting → reference → Development → Provenance. Release provenance and Repository Layout were sitting at positions 3 and 4, ahead of any configuration docs.
Added:
curlcall, so the happy path ends in confirmation rather than an assumption.userCertificates. The host settings table pointed at a "per-entry schema" that did not exist, leavingtrustInContainerused in an example but never defined.SSL_CERT_DIR, theDEVCONTAINER_DEV_CERTS_*vars, and theDOTNET_GENERATE_ASPNET_CERTIFICATEsuppression, with the suppression rationale moved out of How It Works.Fixed: disambiguated
generateDotNetCert, which exists as both a feature option and a host setting and had near-identical descriptions in adjacent tables; repaired a broken sentence in the opening paragraph; normalized "Dev Container" capitalization in prose; linked Prerequisites to Supported Platforms.Extension READMEs
Not included: non-VS Code /
setup-cert.shAn earlier commit documented
setup-cert.shas a manual fallback for JetBrains and other editors. That was removed:install.shnever copies the script into the container, so it isn't reachable in a usable form today and documenting it would promise support that doesn't exist. The Limitations section now states plainly that VS Code is the only supported editor.Review fixes
docker exec -it … bashis covered by the system-bashrc bridge, butdocker exec … envand directly exec'd binaries are not.SSL_CERT_DIRis only colon-joined when a system CA directory survives pruning; the table implied an unconditional trailing colon.echo $SSL_CERT_DIRshould start with a literal~.$HOMEexpands when profile.d is sourced, so the check failed on healthy installs.util/paths.ts(lives inshared/) andtest/hydrate.mjs(istest/sample-project/hydrate-sample-project.mjs), and omitted theshared/package entirely. All 41 entries are now verified to exist.Verification
Docs-only — no source changes. Every setting, default, command ID, output channel name, and environment variable in the new docs was checked against
package.json,devcontainer-feature.json, andinstall.sh. A link checker over all four READMEs reports every internal anchor and relative path resolving, and a path checker confirms all 41 Repository Layout entries exist.🤖 Generated with Claude Code
https://claude.ai/code/session_01KjJpJFDYwZTq7oYF7zLAkH