Default error-handling behaviour for AspNetCoreServer seems insecure

[duncanbrown]

Description

The default behaviour of Amazon.Lambda.AspNetCoreServer when an unhandled exception is thrown during a Request seems to be to add a response header containing the type-name of that exception - see, .e.g. https://github.com/aws/aws-lambda-dotnet/blob/087590ce99274e16e26d37e1dfd73b0b71d1230a/Libraries/src/Amazon.Lambda.AspNetCoreServer/APIGatewayProxyFunction.cs#L91-L94

Appreciate that this is overridable (though not easily, as the relevant method is private protected, so you'd need to override the whole ProcessRequest method that calls it?), and could be useful in some scenarios, but it seems somewhat insecure as a default, as it's leaking details about the internals of the application?

Reproduction Steps

In an ASP.Net Core application using AspNetCoreServer and configured to use Controllers but not any exception handling middleware (or similar), throw an Exception in a Controller method.

The response returned to the consumer contains an errortype header with the name of the exception thrown.

For example, if in a Controller method I try to access a non-existant object using the S3 SDK, the response returned to the client contains the header, errortype: AmazonS3Exception

Environment

Amazon.Lambda.AspNetCoreServer version 6.0.0, using Amazon.Lambda.AspNetCoreServer.APIGatewayProxyFunction

Resolution

• [ ] :wave: I can/would-like-to implement a fix for this problem myself

---

This is a :bug: bug-report

[normj]

I agree we should take it out. We aren't doing that for API Gateway HTTP APIs or Application Load Balancers functions. I see no reason we should do this special in API Gateway REST APIs.

[duncanbrown]

This actually is being done on other subtypes of AbstractAspNetCoreFunction. e.g. for HTTP APIs: https://github.com/aws/aws-lambda-dotnet/blob/087590ce99274e16e26d37e1dfd73b0b71d1230a/Libraries/src/Amazon.Lambda.AspNetCoreServer/APIGatewayHttpApiV2ProxyFunction.cs#L44-L47

PR linked above makes it opt-in rather than the default across the board, if that's a change you'd be happy with?

[github-actions[bot]]

We have noticed this issue has not received attention in 1 year. We will close this issue for now. If you think this is in error, please feel free to comment and reopen the issue.

[duncanbrown]

This is still an open issue as far as I know. The PR with a suggested fix is still open and awaiting review.

[normj]

The PR with this change has been released as part of version 8.0.0 of Amazon.Lambda.AspNetCoreServer. It was a major version bump due to the slight breaking change behavior.

[github-actions[bot]]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.