Harden HTML export, repro script header, and CLI encryption

[erikdarlingdata]

What does this PR do?

A private security report found three problems. This PR fixes all three. Each fix is small, and each has regression tests.

HTML export wrote severity into class attributes

HtmlExporter wrote each warning's severity, in lowercase, into two class attributes. The web app can export the analysis of a shared plan. That analysis is JSON from the person who shared the plan. A crafted severity closed the attribute and added a script element to the exported file.

The exporter now maps severity to one of the three class names in its stylesheet: critical, warning or info. Any other value gets info. The visible severity text was already HTML-encoded, and it still is. A null severity also threw an exception. Now it exports as info.

The web app itself was not affected. It has no MarkupString, so Blazor encodes every value that it shows.

Repro script header took text from the plan

ReproScriptBuilder writes the plan's database name into the block comment at the top of the script. Plan XML can be crafted. A database name that contained */ ended the comment early. The text after it became T-SQL, and it ran when someone executed the script.

A new helper, CommentSafe, now handles every value in the header. It puts a space inside each */ and each /*. T-SQL block comments nest, so a /* in a value opened a comment that swallowed the rest of the script. The helper also changes line breaks to spaces. So each value stays on one line, and it cannot put GO on a line of its own.

The USE statement did not change. It already doubles ] in the name, and that is the correct escape for a bracketed name. go-sqlcmd v1.9.0 and ODBC sqlcmd 15.0 do not split a batch at a GO line inside a bracketed name or inside an N'...' string. Neither does the batch parser of the SSMS 21 and 22 query editor, in normal or SQLCMD mode. A client that splits at every GO line is out of scope, because the statement text itself can hold such a line.

A second commit changes the header's first line from "generated by SQL Server Performance Monitor" to "generated by Performance Studio". That text came with the code when it was ported from Performance Monitor.

CLI --trust-cert made encryption optional

CliConnectionResolver set encryption to Optional whenever --trust-cert was set. It builds the connection for analyze and querystore when you use a stored credential or Windows authentication. The direct login path in ConnectionHelper already kept encryption Mandatory with --trust-cert.

Now both paths keep encryption Mandatory. --trust-cert still skips certificate validation, so a server with a self-signed certificate still connects. This PR does not add a CLI switch to make encryption optional.

Which component(s) does this affect?

• Desktop App (PlanViewer.App)
• Core Library (PlanViewer.Core)
• CLI Tool (PlanViewer.Cli)
• SSMS Extension (PlanViewer.Ssms)
• Tests
• Documentation

How was this tested?

• New tests, 19 cases:
  • ReproScriptBuilderSafetyTests: seven crafted database names and one crafted source. The names include overlapping delimiters and the other line breaks: VT, FF, NEL, U+2028 and U+2029. ScriptDom parses each script and finds one batch and no PRINT. The header ends at its own closing line and has no line that is only GO.
  • HtmlExporterTests: the three known severities keep their classes. A crafted severity adds no script element and gets the info class. So does a severity that adds an attribute with no markup, and a crafted severity on an operator's warning. A null severity exports as info.
  • CliConnectionResolverTests: SQL and Windows authentication, each with and without --trust-cert. Encryption is Mandatory in all four cases. TrustServerCertificate follows the flag, and encryption matches ConnectionHelper.
• Against the old code, 14 of the new cases fail. They are the cases that the fixes change. The three known severities and the two CLI cases without trust pass on both.
• A SqlClient 7.1.0 probe used the CLI's settings against the test servers for SQL Server 2016, 2017, 2019, 2022 and 2025. With Mandatory encryption and TrustServerCertificate=true, each server connected, and sys.dm_exec_connections showed encrypt_option TRUE. Without trust, each connection failed the certificate check.
• Full suite: 918 passed, 0 failed, 1 skipped, before the review's extra cases. The Release build has 0 warnings. Windows only.
• Security review round 1 found that all three fixes hold. It asked for the extra test cases, and this PR has them now.

Checklist

• I have read the contributing guide
• My code builds with zero warnings (Release build, --no-incremental)
• All tests pass (dotnet test)
• I have not introduced any hardcoded credentials or server names

🤖 Generated with Claude Code

https://claude.ai/code/session_01G625JBNh45iTR1hpT4CxNR

[claude]

Reviewed. All three fixes are correct and narrowly scoped:

• CommentSafe (ReproScriptBuilder): hand-verified the two-pass Replace("*/", "* /") then Replace("/*", "/ *"). Since neither 2-char pattern can self-overlap, pass 1 provably removes every */ in the input and cannot introduce a new one (its replacement "* /" starts/ends with characters that can't complete a */ at either boundary). Pass 2 then operates on a string already free of */, so the only way it could reintroduce one would require an adjacent *+/ that provably can't exist post-pass-1. No injection path found (checked overlapping cases like "*//*", "/*/" by hand). The USE [...] line correctly leaves ]]-doubling as the only necessary escape for a bracketed identifier — newlines/;/-- inside it stay lexically part of the identifier, not separate statements.
• SeverityClass (HtmlExporter): fixed allowlist mapping to the three known class names, single call site, null-safe. Confirmed no other path writes Severity into an attribute.
• CliConnectionResolver: EncryptMode is now unconditionally "Mandatory", matching ConnectionHelper's direct-login path. TrustServerCertificate still honors --trust-cert for self-signed certs without weakening transport encryption.

No repo-convention issues (no new warnings, no NoWarn, no version-file changes needed since PlanViewer.Ssms isn't touched, no TRY_CONVERT). Tests are well-targeted at the actual attack surface (crafted severities, hostile database names/sources with ScriptDom-verified single-batch output, encryption-mode parity assertions). Nothing further to flag.