api-problem-bundle
api-problem-bundle copied to clipboard
Exceptions are too public
I am concerned with random exception messages being revealed in the details field by default.
For example, today I've met Symfony's exception was thrown by internal logic of Translations engine with the text Unable to write to the "/var/task/var/cache/lambda/translations" directory. which reveals the internal structure of the project (and the fact project uses Symfony) which could be considered as a security breach.
My proposition is to not show exception's message in details field by default in production environment.
Yeah, that would make sense ....
The best option IMO is to put this logic inside the exception apiproblem: https://github.com/phpro/api-problem/blob/master/src/Http/ExceptionApiProblem.php
An other option is to add an exception transformation that does this job.
Care to create a PR?
@veewee Prepared PR but I don't feel good about it, looks like a big change in core functionality people can rely on. Maybe better create default Transformer for Symfony where we will not reveal exception message?
Sorry for the delay @chekalsky and thanks for picking up the PR.
I feel the same way about the PR and am going to decline it in a moment. After some thinking, it turns out that it is already possible to achieve what you want with the tools we currently provide. Let me share you my vision:
You can create a new transformer and specify a good priority for it. This transformer takes a list of exception classes it either makes visible or otherwise obfuscates. Something along these lines:
<?php
use Phpro\ApiProblem\ApiProblemInterface;
use Phpro\ApiProblem\Http\ExceptionApiProblem;
use Phpro\ApiProblemBundle\Transformer\ExceptionTransformerInterface;
class PublicExceptionTransformer implements ExceptionTransformerInterface
{
public function __constuct(
private string ... $publicExceptionClasses
) {
}
public function transform(\Throwable $exception): ApiProblemInterface
{
$exceptionClass = get_class($exception);
if (in_array($exceptionClass, $this->publicExceptionClasses, true)) {
return new ExceptionApiProblem($exception);
}
// We could add a constructor variable to overwrite the message or add a withDetails() immutable method.
return new ExceptionApiProblem($exception, $exceptionClass);
}
public function accepts(\Throwable $exception): bool
{
return true;
}
}
(You could make the code even smarter and specify a PublicExceptionInterface inside your own codebase. That interface could then be used to determine wether the exception is public or not.)
We could accept a PR for a default implementation of this listener.
What do you think?
i think this feature must depend with env - dev (more detailed) prod (hide trace and other sensitive information)