OPS-157454: Restore the main build with warnings-as-errors kept on

[davidnmbond]

Fixes the broken main build for OPS-157454. TreatWarningsAsErrors stays on; each error class is fixed rather than suppressed, with one exception (CS1591, below).

Approach

Error class	Fix
Nullability (~2,650)	Vendored library and tests: <Nullable>annotations</Nullable>. The source already has 88 ? annotations, which disable turns into CS8632 errors. annotations turns the warnings off and keeps the nullability metadata consumers see the same as in 10.1.9. Samples: disable.
CA1416 (101)	[SupportedOSPlatform("windows")] on the System.Drawing-dependent members, not the assembly. Paths reachable from platform-neutral code are guarded (see below).
CA2022 (23)	ReadAtLeast(buffer, n, throwOnEndOfStream: false): short reads are now completed, full reads behave as before, and a stream that ends early still doesn't throw, as before.
SYSLIB0022/0060	WinZipAes: RijndaelManaged to Aes (128-bit block, ECB, no padding, same key) and the Rfc2898DeriveBytes constructor to Rfc2898DeriveBytes.Pbkdf2 (SHA1, same iterations, one output split into key / MAC key / verifier). Both checked byte-identical with a harness for 128/192/256-bit keys.
SYSLIB0057	X509CertificateLoader.LoadCertificate. The VBA signature's certificate-store entry 0x20 holds a DER X.509 certificate.
SYSLIB0013	The escaped string is written back as the relationship Target (OriginalString) on save, so EscapeDataString would change saved files (it escapes : and /). Replaced with a private helper that gives the same output as Uri.EscapeUriString, checked equal on 28,000+ inputs, including non-ASCII and lone surrogates.
CA2200 / CS0168 / CS8981	throw;, unused variables removed, util alias renamed to Utils.
CS8794	is not Equal or not Inside is always true (as was upstream's != || !=). Simplified to the same behaviour: the worksheet dimension is used whenever there is one.
CS1572/1573/1574	Stale <param> tags and unresolvable crefs corrected.
CS1591 (2,211)	Was hidden behind the compile errors. Suppressed for the library only (NoWarn), for the same reason as nullability: vendored members were never documented. Test and sample projects simply stop generating XML docs.

The same commit also restores three fixes from 070eab9 that 91f6bac reverted. All three block the build or tests:

• removed PackageLicenseExpression=MIT (NU5033 pack failure, and it misdeclared an LGPL package);
• removed the Microsoft.Testing.Platform runner from global.json (the test project uses VSTest);
• SampleWebApp ImplicitUsings disabled again, and the sample moved to WebApplication hosting, because its obsolete WebHostBuilder/IHostingEnvironment/UseMvcWithDefaultRoute are now errors. SampleApp moves from obsolete System.Data.SqlClient to Microsoft.Data.SqlClient 7.1.1 (note: that package defaults to Encrypt=true).

Consumer-visible change

These public APIs are now marked Windows-only. A consumer targeting net10.0 that calls them gets a CA1416 warning, which becomes an error if the consumer treats warnings as errors:
ExcelRangeBase.AutoFitColumns(*), ExcelColumn.AutoFit(*), ExcelDrawings.AddPicture(string, Image[, Uri]) / AddPicture(string, FileInfo[, Uri]), ExcelPicture.Image, ExcelPicture.ImageFormat, ExcelFont/ExcelFontXml/ExcelTextFont.SetFromFont, ExcelHeaderFooterText.InsertPicture(*), ExcelBackgroundImage.Image/SetFromFile, ExcelVmlDrawingPicture.Image.

All of them already failed off Windows, because System.Drawing.Common 10 throws there. The byte-array AddPicture path stays platform-neutral.

Guarded rather than annotated, because platform-neutral code reaches them: loading an existing System.Drawing picture and copying a worksheet with such a picture to another package now throw PlatformNotSupportedException off Windows. Previously the first threw System.Drawing's own exception and the second threw a NullReferenceException. ExcelPicture.SetSize/Dispose take the non-image path off Windows, where Image is always null.

Verification

• dotnet build -c Release (whole solution, --no-incremental): 0 errors on Windows. dotnet pack succeeds.
• CI does not run tests. Full suite run locally against both this branch and origin/main (built with TreatWarningsAsErrors=false), excluding DateFunctionsWorkWithDifferentCultureDateFormats_*, which crashes the test host on both: 1158 passed, 11 failed, 214 skipped on both, with the same 11 failures (missing Workbooks test data and R1C1 tests).
• Zip64, AddPictureFromBytes, CompoundDoc and VBAerror tests pass. The encryption and VBA-signing tests are all [Ignore]d upstream, and no test covers WinZipAes, so those changes were verified with the equivalence harness described above.

🤖 Generated with Claude Code

[codacy-production]

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 4 complexity · 0 duplication

Metric	Results
Complexity	4
Duplication	0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

[codacy-production]

Pull Request Overview

While this PR successfully addresses the build restoration and modernization of cryptographic APIs, it is currently not up to standards according to Codacy and contains high-risk logic flaws. The implementation of 'Stream.ReadAtLeast' to address CA2022 warnings is incorrect in multiple locations; by suppressing end-of-stream exceptions without verifying the number of bytes actually read, the code risks writing uninitialized data to output streams, leading to file corruption.

Several security concerns remain, including the use of insecure cipher modes and the exposure of developer exception pages in production environments. Furthermore, critical logic changes—such as the custom URI escaping helper and new cryptographic loaders—lack internal automated test coverage. Given that existing CI tests are failing or not running, these changes pose a significant regression risk.

About this PR

• Critical logic changes regarding cryptography and URI escaping were verified using external harnesses. These tests should be integrated into the project's test suite to prevent future regressions, especially as the current CI environment is reported to have existing test failures.

Test suggestions

• Found recommended test scenario: Verify AutoFitColumns correctly calculates widths on Windows.
• Found recommended test scenario: Verify ExcelPicture can be added, sized, and saved correctly using System.Drawing on Windows.
• Found recommended test scenario: Verify copying worksheets with pictures between packages correctly handles image parts.
• Missing recommended test scenario: Ensure ReadAtLeast correctly handles potential short reads when decrypting data blocks.
• Missing recommended test scenario: Verify WinZip AES encryption/decryption remains compatible with 128, 192, and 256-bit keys using the new Aes implementation.
• Missing recommended test scenario: Verify VBA signature loading using X509CertificateLoader produces identical results to the legacy implementation.
• Missing recommended test scenario: Verify the custom URI escaping helper produces identical output to the obsolete Uri.EscapeUriString across ASCII and non-ASCII inputs.
• Missing recommended test scenario: Confirm that Windows-only drawing APIs throw PlatformNotSupportedException when called on non-Windows platforms.

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Missing recommended test scenario: Ensure ReadAtLeast correctly handles potential short reads when decrypting data blocks.
2. Missing recommended test scenario: Verify WinZip AES encryption/decryption remains compatible with 128, 192, and 256-bit keys using the new Aes implementation.
3. Missing recommended test scenario: Verify VBA signature loading using X509CertificateLoader produces identical results to the legacy implementation.
4. Missing recommended test scenario: Verify the custom URI escaping helper produces identical output to the obsolete Uri.EscapeUriString across ASCII and non-ASCII inputs.
5. Missing recommended test scenario: Confirm that Windows-only drawing APIs throw PlatformNotSupportedException when called on non-Windows platforms.

Low confidence findings

• The custom 'EscapeUriString' helper is currently private to 'ZipPackageRelationshipBase'. Consider moving this to a shared utility class to avoid logic duplication if URI escaping is required elsewhere in the library.

_{TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback}

[codacy-production]

_{🔴 HIGH RISK}

The use of 'CipherMode.ECB' is insecure. While likely used to implement a custom AES-CTR stream for ZIP compatibility, please provide a security justification in the comments or evaluate if modern authenticated encryption APIs can be used.

See Issue in Codacy
See Issue in Codacy

[codacy-production]

_{🔴 HIGH RISK}

Wrap 'app.UseDeveloperExceptionPage()' in an environment check (e.g., 'if (app.Environment.IsDevelopment())') to prevent sensitive data disclosure in production environments.

See Issue in Codacy

[codacy-production]

_{🔴 HIGH RISK}

Ignoring the return value when throwOnEndOfStream is false can cause uninitialized bytes to be written to the output archive. Use the default behavior to ensure an EndOfStreamException is raised if the input descriptor is truncated.

[codacy-production]

_{🔴 HIGH RISK}

Using throwOnEndOfStream: false without verifying the returned byte count leads to silent corruption when the truncated buffer is written to the document. Use the default true to ensure an exception is thrown if the required data is missing.

[codacy-production]

_{🟡 MEDIUM RISK}

Failing to read the full 8 bytes of metadata will result in BitConverter producing an incorrect value. It is safer to use the default throwOnEndOfStream: true here so that the parser fails immediately on a truncated file.

[codacy-production]

_{⚪ LOW RISK}

Nitpick: Enclose the 'if' block statement in curly braces to satisfy code style requirements and prevent maintenance errors.

See Issue in Codacy

[davidnmbond]

Merged via #1: this branch was merged into fix/codeql-full-clone so the combined change got a fully green run (build, Analyze (csharp), CodeQL, Codacy). This PR's own Analyze could only fail on the shallow clone that #1 fixes. The CodeQL cs/ecb-encryption alert on WinZipAes was dismissed as a false positive (AES-CTR keystream).