Skip to content

fix(security): Sanitize the SSE library's log output - #875

Closed
keelerm84 wants to merge 1 commit into
v8from
mk/SDK-3117/sanitize-stream-log
Closed

keelerm84 wants to merge 1 commit into
v8from
mk/SDK-3117/sanitize-stream-log

Conversation

@keelerm84

Copy link
Copy Markdown
Member

Summary

Relay passes its own logger to the eventsource library for the auto-configuration stream. When a connection attempt fails, the library logs Connection failed (%s), retrying in %0.4f secs with the error text, and for a non-200 response that error text embeds the entire response body -- read with io.ReadAll into SubscriptionError.Message.

The body comes from whatever answered the request, which may be an intermediary rather than LaunchDarkly. That has two consequences:

  • It can forge log entries. The body can contain newlines, so it ends Relay's line and starts lines of its own. Unwiring the fix in TestResponseBodyCannotForgeALogLine produces four copies of an attacker-chosen FAKE Error: forged log line, each indistinguishable from a real Relay line.
  • It can fill the log. A proxy's HTML error page is reproduced in full, once per retry.

The logger handed to the library now collapses every control character except tab to a space, so the text cannot span lines, and truncates at 200 characters. Nothing else about the line changes.

This is shipped behavior, not new

The error handler returns CloseNow: true only for a 401 or 403. Every other non-200 returns CloseNow: false, and the library then logs the retry line, so any 404 or 5xx on the auto-configuration stream reaches it today. The log line is the same in eventsource v1.11.1 (stream.go:148, which 8.21.0 ships) and v1.14.0 (stream.go:188).

The test uses a 500 for that reason: a rejected key closes the stream before the library logs, so a 401 would not exercise the path on this branch.

Origin

Split out of #866 on review. It was bundled there on the mistaken belief that the retry change made the line reachable, which it does not. That change does make it matter more -- a rejected key will log this line every five minutes indefinitely instead of once before shutdown -- but the fix stands on its own and belongs ahead of it.

Relay passes its own logger to the eventsource library. When a connection
attempt fails, the library logs the error text, and for a non-200 response
that text embeds the entire response body.

The body comes from whatever answered the request, which may be an
intermediary rather than LaunchDarkly. It can contain newlines, so it can
end Relay's log line and start lines of its own that look genuine. An HTML
error page is also reproduced in full, once per retry.

The logger passed to the library now collapses every control character
except tab, so the text cannot span lines, and truncates the line.
@keelerm84

Copy link
Copy Markdown
Member Author

Closing: wrong layer. The defect is entirely in eventsource -- it does the unbounded io.ReadAll of the body, puts it in SubscriptionError.Message, interpolates it into Error(), and logs that string. Relay only supplies the logger.

Worse, this PR fixed one of three consumers in the same process. internal/relayenv/env_context_impl.go:349 hands envLoggers to the SDK, and both go-server-sdk data sources pass that logger to eventsource, so a 5xx on a flag stream already logs the body verbatim and this change did nothing about it.

Fixed upstream instead: launchdarkly/eventsource#75, same ticket (SDK-3117). Relay picks it up with the next eventsource bump, and every Go SDK user gets it too.

@keelerm84 keelerm84 closed this Sep 16, 2026
@keelerm84
keelerm84 deleted the mk/SDK-3117/sanitize-stream-log branch September 16, 2026 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant