Add Xml default resolver parameter for XslCompiledTransform.Transform #8655

JaynieBai · 2023-04-12T03:42:48Z

Fixes #8570 part 1

Context

UseTrustedSettings parameter for XslTransformation task is ignored when using dotnet build since the resolver is not setting.

Changes Made

Use an XslCompiledTransform.Transform overload that takes an XmlResolver parameter.
Add the info log message when the UseTrustedSettings is opted-in on the Task

Testing

Enable the earlier disabled test XslDocumentFunctionWorks() on net7.0

Notes

src/Tasks.UnitTests/XslTransformation_Tests.cs

JanKrivanek

Looks good, security approved offline.
I have few minor things for consideration

FEATURE_COMPILED_XSL removal
Do we actually need to explictly opt-in in our code via?

AppContext.SetSwitch("Switch.System.Xml.AllowDefaultResolver", true);

Additionaly - it was advised to us offline that it'd be nice to log a message pointing to the doc (https://aka.ms/msbuild/xsltransformation-task) in case the UseTrustedSettings is opted in (so that users are pointed to the further details, informing about security implications and considerations).

…i/issue8570

…g log or binlog. And the recursively (concatenated) error in the error message.

src/Tasks/XslTransformation.cs

src/Shared/TaskLoggingHelper.cs

JanKrivanek · 2023-04-26T10:24:36Z

Thanks @JaynieBai for updating this PR.

Can you please as well add the info log message when the UseTrustedSettings is opted-in on the Task?

This was suggested by security during the approval for this. The formulation can be something like: "You are enabling relaxed XSLT security settings via UseTrustedSettings parameter. For more details on security implications of this settings please see https://aka.ms/msbuild/xsltransformation-task". It should be localized.

…i/issue8570

JaynieBai · 2023-04-27T08:26:03Z

Thanks @JaynieBai for updating this PR.

Can you please as well add the info log message when the UseTrustedSettings is opted-in on the Task?

This was suggested by security during the approval for this. The formulation can be something like: "You are enabling relaxed XSLT security settings via UseTrustedSettings parameter. For more details on security implications of this settings please see https://aka.ms/msbuild/xsltransformation-task". It should be localized.

Have added the log before the transform. Here is output

src/Tasks/XslTransformation.cs

rainersigwald · 2023-04-28T21:47:09Z

src/Tasks/Resources/Strings.resx

@@ -2329,6 +2329,9 @@
  </data>
  <data name="XslTransform.UseTrustedSettings" xml:space="preserve">
    <value>The usage of the document() method and embedded scripts is prohibited by default, due to risks of foreign code execution.  If "{0}" is a trusted source that requires those constructs, please set the "UseTrustedSettings" parameter to "true" to allow their execution.</value>
+  </data>
+  <data name="XslTransform.SecuritySettingsViaUseTrustedSettings" xml:space="preserve">
+    <value>You are enabling relaxed XSLT security settings via UseTrustedSettings parameter. For more details on security implications of this settings please see https://aka.ms/msbuild/xsltransformation-task</value>


Suggested change

<value>You are enabling relaxed XSLT security settings via UseTrustedSettings parameter. For more details on security implications of this settings please see https://aka.ms/msbuild/xsltransformation-task</value>

<value>You are enabling relaxed XSLT security settings via the UseTrustedSettings parameter. For more details on security implications of this setting please see https://aka.ms/msbuild/xsltransformation-task</value>

rainersigwald · 2023-05-01T14:08:51Z

src/Tasks/XslTransformation.cs

                for (int i = 0; i < xmlinput.Count; i++)
                {
                    using (XmlWriter xmlWriter = XmlWriter.Create(_outputPaths[i].ItemSpec, xslct.OutputSettings))
                    {
                        using (XmlReader xr = xmlinput.CreateReader(i))
                        {
-                            xslct.Transform(xr, arguments, xmlWriter);
+                            xslct.Transform(xr, arguments, xmlWriter, new XmlUrlResolver());


It feels weird to me to always pass the resolver, even when not trusted. I see though that it's also always passed to the constructors of the XslCompiledTransform.

Should we pass it only when UseTrustedSettings is true?

Since .NET Core does not allow resolving external URIs for XML by default. So, add that default resolver explicitly. If the UseTrustedSettings is false, it won't allow the XSLT document() function. The XmlUrlResolver that resolves the document function won't work.
If the UseTrustedSettings is false and there is document, it will report the following error before resolve the document.

…into jennybai/issue8570

edstegman · 2023-07-13T14:42:21Z

@JaynieBai

I'm sorry to bother you, but how and when does this fix get delivered? I see the fix was merged almost 2 months ago, but the Azure DevOps build is still failing, as is my local build.

The ADO build and my local build report using MSBuild version 17.6.8+c70978d4d for .NET

rainersigwald · 2023-07-13T14:57:11Z

@edstegman the fix will be in MSBuild 17.7, which will release with Visual Studio 17.7 "soon".

Add xml default resolver for xslct.Transform

3089684

JanKrivanek reviewed Apr 12, 2023

View reviewed changes

src/Tasks.UnitTests/XslTransformation_Tests.cs Show resolved Hide resolved

JanKrivanek suggested changes Apr 12, 2023

View reviewed changes

Merge branch 'main' of https://github.com/dotnet/msbuild into jennyba…

d053b00

…i/issue8570

JaynieBai marked this pull request as draft April 25, 2023 10:11

Log the ex.ToString() in the low-prio message, so it shows up in -dia…

0df29b6

…g log or binlog. And the recursively (concatenated) error in the error message.

JanKrivanek suggested changes Apr 25, 2023

View reviewed changes

src/Tasks/XslTransformation.cs Show resolved Hide resolved

wrap the flattened exception messages

b642163

JanKrivanek reviewed Apr 26, 2023

View reviewed changes

src/Shared/TaskLoggingHelper.cs Outdated Show resolved Hide resolved

make the method static as suggested

8aa7767

JaynieBai added 3 commits April 26, 2023 20:15

Merge branch 'main' of https://github.com/dotnet/msbuild into jennyba…

a2b4f8f

…i/issue8570

Add the info log message when the UseTrustedSettings is true

ac6cd52

format

1d59a85

JanKrivanek reviewed Apr 27, 2023

View reviewed changes

src/Tasks/XslTransformation.cs Outdated Show resolved Hide resolved

Move the log out of the loop

cf87ef1

JanKrivanek approved these changes Apr 27, 2023

View reviewed changes

JaynieBai marked this pull request as ready for review April 28, 2023 01:50

rainersigwald reviewed May 1, 2023

View reviewed changes

JaynieBai added 2 commits May 4, 2023 23:42

Merge branch 'jennybai/issue8570' of https://github.com/dotnet/msbuild …

b2803ab

…into jennybai/issue8570

Refactor resource string

0883df0

rainersigwald approved these changes May 15, 2023

View reviewed changes

JaynieBai merged commit 8d8421c into main May 16, 2023

JaynieBai deleted the jennybai/issue8570 branch May 16, 2023 10:38

KalleOlaviNiemitalo mentioned this pull request May 16, 2023

[Bug]: UseTrustedSettings parameter for XslTransformation task is ignored when using dotnet build #8570

Closed

JaynieBai mentioned this pull request Jun 10, 2025

Logs an error with code extraction and details #11931

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add Xml default resolver parameter for XslCompiledTransform.Transform #8655

Add Xml default resolver parameter for XslCompiledTransform.Transform #8655

Uh oh!

JaynieBai commented Apr 12, 2023 •

edited

Loading

Uh oh!

Uh oh!

JanKrivanek left a comment •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

JanKrivanek commented Apr 26, 2023

Uh oh!

JaynieBai commented Apr 27, 2023

Uh oh!

Uh oh!

rainersigwald Apr 28, 2023

Uh oh!

rainersigwald May 1, 2023

Uh oh!

JaynieBai May 5, 2023

Uh oh!

edstegman commented Jul 13, 2023

Uh oh!

rainersigwald commented Jul 13, 2023

Uh oh!

Uh oh!

	<value>You are enabling relaxed XSLT security settings via UseTrustedSettings parameter. For more details on security implications of this settings please see https://aka.ms/msbuild/xsltransformation-task</value>
	<value>You are enabling relaxed XSLT security settings via the UseTrustedSettings parameter. For more details on security implications of this setting please see https://aka.ms/msbuild/xsltransformation-task</value>

Add Xml default resolver parameter for XslCompiledTransform.Transform #8655

Add Xml default resolver parameter for XslCompiledTransform.Transform #8655

Uh oh!

Conversation

JaynieBai commented Apr 12, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Changes Made

Testing

Notes

Uh oh!

Uh oh!

JanKrivanek left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

JanKrivanek commented Apr 26, 2023

Uh oh!

JaynieBai commented Apr 27, 2023

Uh oh!

Uh oh!

rainersigwald Apr 28, 2023

Choose a reason for hiding this comment

Uh oh!

rainersigwald May 1, 2023

Choose a reason for hiding this comment

Uh oh!

JaynieBai May 5, 2023

Choose a reason for hiding this comment

Uh oh!

edstegman commented Jul 13, 2023

Uh oh!

rainersigwald commented Jul 13, 2023

Uh oh!

Uh oh!

JaynieBai commented Apr 12, 2023 •

edited

Loading

JanKrivanek left a comment •

edited

Loading