Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions .claude/skills/beacon-data-privacy/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,12 @@ lines already include `chat=<chat_id>` for observability and that is fine. Do no
`@username`, message body content, or any other off-limits field. The access-log format
`middleware [200] GET /api/v1/me/subscriptions` intentionally omits the
`X-Telegram-Init-Data` header for the same reason.

**An outbound URL is a log field too.** Open-Meteo requests carry a user-selected city's
coordinates, and geocoding requests carry the search term the user typed. Both are
pre-approved data, so this is hygiene rather than policy — but neither belongs in a log line
by accident. `internal/infrastructure/weather/openmeteo.go` composes its status-code errors
from host and path only, and runs transport errors through `redactURLError`, because
`net/http` returns a `*url.Error` whose `Error()` embeds the request URL verbatim and any
caller printing it with `%v` prints the query string with it. A new outbound client wraps its
own transport errors the same way.
26 changes: 26 additions & 0 deletions internal/infrastructure/weather/openmeteo.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,31 @@ func (o *OpenMeteo) get(ctx context.Context, rawURL string) ([]byte, error) {
)
}

// redactURLError rebuilds a *url.Error with the query string stripped from its URL.
//
// The status-code branch below composes its own message from host and path deliberately, to
// keep coordinates and search terms out of the logs. A transport failure defeats that on its
// own: net/http returns a *url.Error whose Error() embeds the URL verbatim, so a plain
// `dial tcp: i/o timeout` arrives carrying every latitude, longitude and query term the
// request was built with, and any caller formatting it with %v prints them.
//
// It is called on the error http.Client.Do returns, where the *url.Error is the whole chain.
// Anything else is handed back untouched.
func redactURLError(err error) error {
var urlErr *url.Error
if !errors.As(err, &urlErr) {
return err
}

// A URL that will not parse yields the empty string rather than the original: the point
// is that nothing unexamined reaches the log.
redacted := ""
if parsed, parseErr := url.Parse(urlErr.URL); parseErr == nil {
redacted = parsed.Host + parsed.Path
}
return &url.Error{Op: urlErr.Op, URL: redacted, Err: urlErr.Err}
}

// attempt performs exactly one request. Its errors are classified by retryableError so
// get can tell an upstream hiccup from an answer that will not change.
func (o *OpenMeteo) attempt(ctx context.Context, rawURL string) ([]byte, error) {
Expand All @@ -286,6 +311,7 @@ func (o *OpenMeteo) attempt(ctx context.Context, rawURL string) ([]byte, error)
// Transport-level failures — timeout, reset, refused — are indistinguishable
// from a 5xx from here and just as transient. A cancelled context is not: the
// caller asked to stop, and re-sending would ignore that.
err = redactURLError(err)
if ctx.Err() != nil {
return nil, fmt.Errorf("open-meteo: do request: %w", err)
}
Expand Down
51 changes: 51 additions & 0 deletions internal/infrastructure/weather/openmeteo_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package weather

import (
"context"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -665,6 +666,56 @@ func loggedDuration(t *testing.T, text, pattern string) time.Duration {
return d
}

func TestOpenMeteoTransportErrorRedaction(t *testing.T) {
t.Parallel()

// A dead listener forces http.Client.Do to fail at the transport, which is the branch
// that returns a *url.Error carrying the whole request URL.
deadServer := func(t *testing.T) string {
t.Helper()
srv := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}))
addr := srv.URL
srv.Close()
return addr
}

t.Run("a forecast transport failure carries no coordinates", func(t *testing.T) {
t.Parallel()
addr := deadServer(t)
om := newTestOpenMeteo(t, addr, addr)

_, err := om.Forecast(t.Context(), 51.169392, 71.449074)
require.Error(t, err)
assert.NotContains(t, err.Error(), "51.169392")
assert.NotContains(t, err.Error(), "71.449074")
assert.NotContains(t, err.Error(), "latitude")
assert.Contains(t, err.Error(), "/v1/forecast", "the path still has to say what failed")
})

t.Run("a geocode transport failure carries no search term", func(t *testing.T) {
t.Parallel()
addr := deadServer(t)
om := newTestOpenMeteo(t, addr, addr)

_, err := om.Geocode(t.Context(), "Karagandy", 3)
require.Error(t, err)
assert.NotContains(t, err.Error(), "Karagandy")
assert.Contains(t, err.Error(), "/v1/search")
})

t.Run("an error that is not a *url.Error is untouched", func(t *testing.T) {
t.Parallel()
sentinel := errors.New("nothing to redact")
assert.Equal(t, sentinel, redactURLError(sentinel))
})

t.Run("a URL that will not parse redacts to nothing, not to itself", func(t *testing.T) {
t.Parallel()
got := redactURLError(&url.Error{Op: "Get", URL: "://%zz", Err: errors.New("boom")})
assert.NotContains(t, got.Error(), "%zz")
})
}

func TestOpenMeteo_ForecastRange(t *testing.T) {
t.Parallel()

Expand Down
Loading