Cover analyzer-invisible path consumers in MSBuildTask0003

[Copilot]

Context

MSBuildTask0003 decides whether a call needs an absolute path by matching the containing type against an allowlist. It missed what the issue calls analyzer-invisible path consumers: APIs on types unrelated to System.IO that still take a path string and hit the file system. AssemblyName.GetAssemblyName(string) was the most-repeated defect found in a manual audit after ~150 task migrations across dotnet/arcade, dotnet/source-build-assets and the dotnet/dotnet VMR — where the analyzer had reported zero warnings.

One correction to the issue's premise: the "fixed set of eight types" it quotes came from README.md, which was stale. XDocument/XElement.Load, XmlReader/XmlWriter.Create, ZipFile.* and X509Certificate2(string) were already covered; the doc was misreporting real coverage.

Changes Made

• Sinks added to SharedAnalyzerHelpers.ResolveFilePathTypes (shared with the transitive rule, so MSBuildTask0005 picks them up automatically):

  Type	Path members covered
  System.Reflection.AssemblyName	GetAssemblyName(assemblyFile)
  System.Xml.XmlDocument	Load(filename), Save(filename)
  System.Xml.XPath.XPathDocument	XPathDocument(uri)
  System.IO.Compression.ZipFileExtensions	ExtractToFile, ExtractToDirectory, CreateEntryFromFile (+ async)
  X509CertificateLoader	LoadCertificateFromFile, LoadPkcs12FromFile, LoadPkcs12CollectionFromFile
  X509Certificate2Collection	Import(fileName), ImportFromPemFile

• XmlDocument un-excluded. A comment asserted it had to stay out because CreateElement/CreateAttribute would false-positive. That reasoning was wrong — arguments are filtered by SpecialType.System_String and IsPathParameterName, so CreateElement, SelectNodes, LoadXml, LoadPkcs12(byte[], password) and every Stream/TextReader overload are skipped. Comment replaced with why the filter makes whole-type monitoring safe.

• Code fix correctness. ZipFileExtensions members are extension methods; in static form the first argument is a ZipArchive. The fixer wrapped the first syntactically unwrapped argument, which would emit code that does not compile. It now binds arguments to parameters and wraps the first unwrapped string path parameter — what the README already claimed it did.

  // before: wrapped the receiver
  ZipFileExtensions.CreateEntryFromFile(TaskEnvironment.GetAbsolutePath(archive), sourceFileName, entryName);
  // after
  ZipFileExtensions.CreateEntryFromFile(archive, TaskEnvironment.GetAbsolutePath(sourceFileName), entryName);

• README.md: stale eight-type line replaced with the full set plus a note on the parameter-name filter; fixer description corrected.

Testing

10 analyzer tests (5 positive, 5 guarding stream overloads and non-path strings) and 1 code-fix test. Each positive test was confirmed to fail without its corresponding source change — one initially passed spuriously because its ZipFile.OpenRead setup line was itself producing the diagnostic, and was reworked to assert a single diagnostic naming ExtractToFile.

False-positive risk was checked empirically rather than by inspection: a reflection probe enumerated every string-parameter member of each added type against the real IsPathParameterName filter. No matches outside the intended path overloads.

Measured against src/Tasks/Microsoft.Build.Tasks.csproj -p:BuildAnalyzer=true -t:Rebuild (the configuration CI's "Linux Core Multithreaded Mode" job uses): 0 errors, +12 MSBuildTask0003, +8 MSBuildTask0005, none lost. New warnings are real unrooted consumers — 5 × AssemblyName.GetAssemblyName in GetAssemblyIdentity/ResolveComReference, plus XmlDocument.Save(string) reached from SignFile/UpdateManifest. Both codes are already in that project's WarningsNotAsErrors.

Notes

Two judgement calls worth reviewer attention:

• URI-capable overloads. XmlDocument.Load(string filename) and new XPathDocument(uri) also accept URLs, and wrapping one would corrupt it. This is pre-existing and deliberate rather than introduced here: IsPathParameterName has always matched uri/url, and XDocument.Load(string uri) / XmlReader.Create(string inputUri) were already sinks. Narrowing it now would silently drop warnings from rules this change does not touch, so behavior is left as-is — but it is a reasonable thing to revisit deliberately.

• The "invert the rule shape" proposal is not attempted. Taint-tracking from task input properties is the stronger design and the only thing that catches the InstallDotNetTool case, where paths flow through IFileSystem/ICommandFactory and no BCL type appears. It is a different rule with its own false-positive surface (new AbsolutePath(path), Path.Combine(path1, path2) would trip a naive version) and warrants its own change rather than riding along with a sink-list fix.

• Fixes TaskAnalyzer: MSBuildTask0003 misses analyzer-invisible path consumers such as AssemblyName.GetAssemblyName #14791

[dotnet-policy-service]

Hello @copilot, I noticed that you’re changing an .swr file or any file under src/Package/MSBuild.VSSetup.. Please make sure to validate this change by an experimental VS insertion. This is accomplished by pushing to an exp/* branch, which requires write permissions to this repo.