feat(lsp): default-on C# restore + filter for NuGet audit advisories#203
Open
pacaya wants to merge 1 commit into
Open
feat(lsp): default-on C# restore + filter for NuGet audit advisories#203pacaya wants to merge 1 commit into
pacaya wants to merge 1 commit into
Conversation
## Problem `csharp-ls` / OmniSharp build a Roslyn `MSBuildWorkspace` that escalates a NuGet **audit warning** (the `NU19xx` family — e.g. a transitively vulnerable package such as `log4net`) into a **fatal** project-load failure, dropping every project that references the flagged package. Those projects then have no compilation, so the server reports false `CS####` "unresolved type / namespace" diagnostics — even though `dotnet build` / `dotnet test` keep `NU19xx` a non-fatal warning and succeed. ## Fix Two complementary, C#-scoped fixes, **both on by default**: - **Pre-spawn restore (root cause)** — before spawning the C# server, run `dotnet restore -p:NuGetAudit=false` in the workspace so the MSBuild workspace loads every project. Best-effort: a failure logs and falls through to a normal spawn; skipped on passive IDE attach (the IDE owns restore) and when `dotnet` is not on `PATH`. - **Advisory diagnostic filter (symptom)** — the `publishDiagnostics` handler strips diagnostics whose code is the `NU####` NuGet family before storing / fanning out. Deliberately narrow: real `CS####` compiler diagnostics always pass through, so genuine errors are never hidden. Both are confined to providers that serve C# (`servesCSharp()`), so no other language's provider is affected. ## Defaults & opt-out | Env var | Default | Disable with | | --- | --- | --- | | `GORTEX_LSP_CSHARP_RESTORE` | on | `0` / `off` / `false` / `none` | | `GORTEX_LSP_CSHARP_DIAG_FILTER` | on | `0` / `off` / `false` / `none` | ## Why default-on - gortex only indexes repositories the user **explicitly adds** (it never auto-discovers), so "you pointed gortex at this repo" is the trust boundary — like opening a project in an IDE. - Spawning the C# server already evaluates the project's MSBuild graph, so the restore adds no execution surface beyond the workspace load it precedes. Under PackageReference, `dotnet restore` does not run NuGet package install scripts. - C# semantic resolution needs dependencies resolved to type-check at all, so restore is what makes analysis work out of the box instead of drowning in false `CS####` errors. - The off-switch covers offline / air-gapped indexing and network-egress policy. ## Docs `docs/lsp.md` gains a "Language-specific behaviour" section documenting both C# knobs and — finally — the existing `GORTEX_LSP_JDTLS_TRUST_BUILD` opt-in (which was source-only until now). ## Testing - `go build ./...` clean (CGO). - `go test -race ./internal/semantic/lsp/` green; new `csharp_diag_test.go` covers the NU-vs-CS predicate, code-string normalisation, the filter (drops NU, keeps CS, order-preserving, no-op fast path), language scoping, the filter toggle, and the restore-eligibility gate (on by default; off on falsey env / non-C# / passive attach).
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.
Problem
csharp-ls/ OmniSharp build a RoslynMSBuildWorkspacethat escalates a NuGet audit warning (theNU19xxfamily — e.g. a transitively vulnerable package such aslog4net) into a fatal project-load failure, dropping every project that references the flagged package. Those projects then have no compilation, so the server reports falseCS####"unresolved type / namespace" diagnostics — even thoughdotnet build/dotnet testkeepNU19xxa non-fatal warning and succeed.Fix
Two complementary, C#-scoped fixes, both on by default:
dotnet restore -p:NuGetAudit=falsein the workspace so the MSBuild workspace loads every project. Best-effort: a failure logs and falls through to a normal spawn; skipped on passive IDE attach (the IDE owns restore) and whendotnetis not onPATH.publishDiagnosticshandler strips diagnostics whose code is theNU####NuGet family before storing / fanning out. Deliberately narrow: realCS####compiler diagnostics always pass through, so genuine errors are never hidden.Both are confined to providers that serve C# (
servesCSharp()), so no other language's provider is affected.Defaults & opt-out
GORTEX_LSP_CSHARP_RESTORE0/off/false/noneWhy default-on
dotnet restoredoes not run NuGet package install scripts.CS####errors.Docs
docs/lsp.mdgains a "Language-specific behaviour" section documenting both C# knobs and — finally — the existingGORTEX_LSP_JDTLS_TRUST_BUILDopt-in (which was source-only until now).Testing
go build ./...clean (CGO).go test -race ./internal/semantic/lsp/green; newcsharp_diag_test.gocovers the NU-vs-CS predicate, code-string normalisation, the filter (drops NU, keeps CS, order-preserving, no-op fast path), language scoping, the filter toggle, and the restore-eligibility gate (on by default; off on falsey env / non-C# / passive attach).Summary
Brief description of what this PR does.
Changes
Testing
go test -race ./...)Checklist
Meta["methods"]for interfaces (if applicable)EdgeMemberOfedges to their containing type (if applicable)