graphql-dotnet: Option to skip serialization of exception data to prevent leaking sensitive information
Summary
Similary to ExposeExceptions allow to disable serialization of Exception.Data that can contain sensitive data.
Basic example
services.AddGraphQL(options =>
{
options.ExposeExceptions = false;
options.ExposeExceptionExtensions = false; // Something like this
})
Motivation
Certain exceptions can possible leak sensitive information through Exception.Data.
In the example response below, we are leaking confidential data about our database server through Microsoft.Data.SqlClient.SqlException and there is no simple way to prevent this from happening globally (according to ExecutionResultJsonConverter)
{
"data": ...,
"errors": [
{
"message": "Error trying to resolve...",
"locations": [],
"path": [],
"extensions": {
"code": "SQL",
"data": {
"HelpLink.ProdName": "Microsoft SQL Server",
"HelpLink.ProdVer": "12.00.2000",
"HelpLink.EvtSrc": "MSSQLServer",
"HelpLink.EvtID": "208",
"HelpLink.BaseHelpUrl": "http://go.microsoft.com/fwlink",
"HelpLink.LinkId": "20476",
"OriginalClientConnectionId": "<CENSORED>",
"RoutingDestination": "<CENSORED>"
}
}
}
]
}
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Comments: 17 (11 by maintainers)
I am considering an option to make this a serializer property, not
ExecutionOptions. I also think that in fact,ExposeExceptionsis also a serializer property. These properties are used only during serialization, and before that they are only transmitted along the call chain. In addition, the presence of theExposeExceptionsproperty insideExecutionResultlooks rather strange. It is located there only because the serializer works with theExecutionResultobject and there is nowhere else to get this information from. Therefore, it was placed there. This is another argument for moving a property to a serializer(s).The second question is how to configure these settings for the serializer(s). We can use “standard”
Microsoft.Extensions.IOptionsbut I do not really like to add new dependencies.@joemcbride @benmccallum @GrangerAts @jsantha @Shane32 @SimonCropp What do you think?