Skip to content

feat(lsp): default-on C# restore + filter for NuGet audit advisories#203

Open
pacaya wants to merge 1 commit into
zzet:mainfrom
pacaya:csharp-restore-default
Open

feat(lsp): default-on C# restore + filter for NuGet audit advisories#203
pacaya wants to merge 1 commit into
zzet:mainfrom
pacaya:csharp-restore-default

Conversation

@pacaya

@pacaya pacaya commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

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

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).

Summary

Brief description of what this PR does.

Changes

Testing

  • All tests pass (go test -race ./...)
  • New tests added for new functionality
  • Benchmarks run if performance-relevant

Checklist

  • Code follows existing patterns in the codebase
  • No unnecessary abstractions added
  • Language extractor includes Meta["methods"] for interfaces (if applicable)
  • Methods have EdgeMemberOf edges to their containing type (if applicable)

## 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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant