Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable redacting user-code exception messages #790

Merged
merged 4 commits into from
Sep 14, 2022

Conversation

cgillum
Copy link
Member

@cgillum cgillum commented Aug 24, 2022

Context

DTFx code is often hosted in Azure App Service and Azure Functions. When running on these platforms, certain framework traces are captured into an internal log capturing service. One of the things logged by DTFx is exception messages for activity failures. These exception messages are often generated by user code, which may contain sensitive information (PII, secrets, etc.).

Change

As a safety mechanism, this PR will redact exception messages that come from activity failures. This redaction is enabled by default when hosted in an Azure App Service environment, which we infer by checking for the WEBSITE_SITE_NAME environment variable. This policy of redaction can be overridden by setting a DTFX_REDACT_USER_CODE_EXCEPTIONS environment variable to 1 or true.

This PR also adds additional #nullable enable compiler directives, which is something we routinely do as we touch the code to help make it safe from unexpected null-ref exceptions.

Testing

I tested the exception redaction logic using the following LINQPad script:

try
{
	Foo();
}
catch (Exception e)
{
	Console.WriteLine(GetSafeExceptionDetails(e, false));
	Console.WriteLine();
	Console.WriteLine(GetSafeExceptionDetails(e, true));
}

static string GetSafeExceptionDetails(Exception exception, bool redact)
{
	if (redact)
	{
		// Redact the exception message since its possible to contain sensitive information (PII, secrets, etc.)
		// Exception.ToString() code: https://referencesource.microsoft.com/#mscorlib/system/exception.cs,e2e19f4ed8da81aa
		var sb = new StringBuilder(capacity: 1024);
		sb.Append(exception.GetType().FullName).Append(": ").Append("[Redacted]");
		if (exception.InnerException != null)
		{
			// Recursive
			sb.AppendLine().Append(" ---> ").AppendLine(GetSafeExceptionDetails(exception.InnerException, redact));
			sb.Append("   --- End of inner exception stack trace ---");
		}

		sb.AppendLine().Append(exception.StackTrace);
		return sb.ToString();
	}
	else
	{
		return exception.ToString();
	}
}

static void Foo()
{
	try
	{
		Bar();
	}
	catch (Exception e)
	{
		throw new ApplicationException("Oh no!", e);
	}
}

static void Bar()
{
	try
	{
		Baz();
	}
	catch (Exception e)
	{
		throw new Exception("Umm...", e);
	}
}

static void Baz()
{
	throw new InvalidOperationException("Kah-BOOOOM!!!");
}

The results when redaction IS NOT enabled:

System.ApplicationException: Oh no!
 ---> System.Exception: Umm...
 ---> System.InvalidOperationException: Kah-BOOOOM!!!
   at UserQuery.<Main>g__Baz|4_3() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 63
   at UserQuery.<Main>g__Bar|4_2() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 53
   --- End of inner exception stack trace ---
   at UserQuery.<Main>g__Bar|4_2() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 57
   at UserQuery.<Main>g__Foo|4_1() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 41
   --- End of inner exception stack trace ---
   at UserQuery.<Main>g__Foo|4_1() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 45
   at UserQuery.Main() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 4

This is what would be seen if redaction IS enabled:

System.ApplicationException: [Redacted]
 ---> System.Exception: [Redacted]
 ---> System.InvalidOperationException: [Redacted]
   at UserQuery.<Main>g__Baz|4_3() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 63
   at UserQuery.<Main>g__Bar|4_2() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 53
   --- End of inner exception stack trace ---
   at UserQuery.<Main>g__Bar|4_2() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 57
   at UserQuery.<Main>g__Foo|4_1() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 41
   --- End of inner exception stack trace ---
   at UserQuery.<Main>g__Foo|4_1() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 45
   at UserQuery.Main() in C:\Users\cgillum\AppData\Local\Temp\LINQPad7\_wrmpjfpn\otarjw\LINQPadQuery:line 4

@cgillum cgillum requested a review from jviau August 24, 2022 23:40
Copy link
Collaborator

@jviau jviau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any concerns on this being a breaking behavior change for AppService users?

@cgillum cgillum merged commit f79aab2 into main Sep 14, 2022
@cgillum cgillum deleted the cgillum/redact-exceptions branch September 14, 2022 21:40
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.

2 participants