Don't Expose Stack Traces in OperationOutcome

[alexanderkiel]

error-response* turns every exception that isn't already an anomaly into a fault carrying the fully formatted stack trace:

(instance? Throwable error)
(if (ba/anomaly? (ex-data error))
  ...
  (do
    (log/error error)
    (error-response*
     (ba/fault
      (ex-message error)
      :blaze/stacktrace (format-exception error))
     f)))

and operation-outcome-issue renders that stack trace into OperationOutcome.issue.diagnostics:

message
(assoc :diagnostics (type/string message))
stacktrace
(assoc :diagnostics (type/string stacktrace))

Because both clauses assign :diagnostics in the same cond->, the stack trace doesn't accompany the message, it replaces it.

Problem

Any unexpected exception from any handler — the class of error that carries the most internal detail — is sent to the client with internal class and namespace names, file names, line numbers, thread names and library internals. Depending on the deployment, that client can be unauthenticated. This is CWE-209 and it also makes error responses unnecessarily large.

It affects the FHIR RESTful API through error-response and, inside a Bundle entry response.outcome, through bundle-error-response. json-error-response isn't affected, because it only renders ::anom/message.

There is no configuration for this. :blaze/stacktrace has exactly two occurrences in the whole codebase: the one that sets it and the one that renders it.

Expected

Stack traces belong in the log, not in a response. error-response* already logs the exception with (log/error error) directly before it builds the anomaly, so nothing is lost operationally by leaving it out of the response.

Two ways to do it:

1. Drop :blaze/stacktrace and let diagnostics carry the exception message. Simple, and the message is usually the useful part.
2. Keep it behind a configuration option that defaults to off, for development.

Option 1 is preferable. It removes the mechanism instead of leaving it one environment variable away from being on in production.

Worth deciding in the same change: whether the exception message should reach the client either. For an unexpected exception it can carry internals as well (paths, host names, statements). Reporting a general message for a fault and keeping the details in the log is the more consistent counterpart to option 1.

Notes

• blaze.anomaly/-anomaly builds (fault (.getMessage e) :stacktrace (format-exception e)) for a plain Throwable. That's a different, unqualified key and nothing renders it today, but it means anomalies carry formatted stack traces around in memory. It should be looked at in the same change, so that a future consumer of that anomaly doesn't reintroduce the leak.
• The tests of the affected namespaces don't cover the stack trace in a response at all, so the change needs a test asserting that diagnostics doesn't contain one.