Redact credential headers from machines API errors - #90
Open
simoncocking wants to merge 1 commit into
Open
Conversation
`http_post!/3` interpolates the request headers into the errors it raises,
and those headers carry `{"Authorization", "Bearer " <> token}`. Every
failed machine create therefore writes the Fly API token into logs and
error trackers:
failed POST https://api.machines.dev/v1/apps/app/machines with
{:failed_connect, ...} [{~c"Content-Type", "application/json"},
{~c"Authorization", "Bearer <FLY_API_TOKEN>"}]
This defeats the `@derive {Inspect, only: [...]}` on the struct, which
already keeps `:token` out of inspected state.
Keep the header names, which are what you want when debugging a failed
request, and drop the values of the ones that carry credentials.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
FLAME.FlyBackend.http_post!/3interpolates its request headers into the errors it raises, and those headers carry the Fly API token as a bearer credential:So every failed machine create raises a message containing the token, which then lands in logs and error trackers:
Both raise clauses are affected — the non-2xx one and the transport-error one. This is easy to hit in practice: a rate limit, a capacity failure past the retry budget, or a TLS/DNS problem is enough. The 0.5.4 Let's Encrypt chain-depth issue was one such case, where every pool boot failed the handshake and raised.
This also defeats an existing intent in the module — the struct already carries
which deliberately keeps
:tokenout of inspected state. The raise path routes around it.The change
Redact the values of headers that carry credentials before interpolating them, keeping the header names, since those are the useful part when debugging a failed request:
proxy-authorizationis covered too, so a future header addition doesn't reintroduce the leak.Happy to adjust the approach — dropping the headers from the message entirely is also reasonable if you'd rather not carry the helper.
Test
test/fly_backend_test.exsgains a hermetic case that points:hostat a closed local port, so the POST fails andremote_boot/1raises, then asserts the token is absent and the header name is still present. It fails onmainwith the leaked token in the message and passes with the fix.