Information Disclosure

Proof of Concept: 1) Run Grafana with GF_LOG_LEVEL set to info and enable the Pyroscope datasource. 2) Make a Pyroscope data source request with a body containing sensitive data (for example, a token). 3) Observe server logs on stderr. In the pre-fix version, you would see a DEBUG JSON log containing the Body payload, e.g. Body: "{token: 12345}"; the log line would be emitted even though the log level is info. 4) After applying this fix, the debug JSON line will no longer appear, or the Body will be redacted; logs will respect GF_LOG_LEVEL.

diff --git a/pkg/tsdb/grafana-pyroscope-datasource/instance.go b/pkg/tsdb/grafana-pyroscope-datasource/instance.go index a55bed732cb30..92f7ed4fd0a2b 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/instance.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/instance.go @@ -12,6 +12,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/httpclient" "github.com/grafana/grafana-plugin-sdk-go/backend/instancemgmt" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana-plugin-sdk-go/backend/tracing" "github.com/grafana/grafana-plugin-sdk-go/data" "github.com/prometheus/prometheus/model/labels" @@ -45,10 +46,11 @@ type PyroscopeDatasource struct { httpClient *http.Client client ProfilingClient settings backend.DataSourceInstanceSettings + logger log.Logger } // NewPyroscopeDatasource creates a new datasource instance. -func NewPyroscopeDatasource(ctx context.Context, httpClientProvider httpclient.Provider, settings backend.DataSourceInstanceSettings) (instancemgmt.Instance, error) { +func NewPyroscopeDatasource(ctx context.Context, httpClientProvider httpclient.Provider, settings backend.DataSourceInstanceSettings, logger log.Logger) (instancemgmt.Instance, error) { opt, err := settings.HTTPClientOptions(ctx) if err != nil { return nil, backend.DownstreamErrorf("failed to get HTTP client options: %w. function: %s", err, logEntrypoint()) @@ -62,11 +64,12 @@ func NewPyroscopeDatasource(ctx context.Context, httpClientProvider httpclient.P httpClient: httpClient, client: NewPyroscopeClient(httpClient, settings.URL), settings: settings, + logger: logger, }, nil } func (d *PyroscopeDatasource) CallResource(ctx context.Context, req *backend.CallResourceRequest, sender backend.CallResourceResponseSender) error { - ctxLogger := logger.FromContext(ctx) + ctxLogger := d.logger.FromContext(ctx) ctx, span := tracing.DefaultTracer().Start(ctx, "datasource.pyroscope.CallResource", trace.WithAttributes(attribute.String("path", req.Path), attribute.String("method", req.Method))) defer span.End() ctxLogger.Debug("CallResource", "Path", req.Path, "Method", req.Method, "Body", req.Body, "function", logEntrypoint()) @@ -231,7 +234,7 @@ func (d *PyroscopeDatasource) profileMetadata(ctx context.Context, _ *backend.Ca // The QueryDataResponse contains a map of RefID to the response for each query, and each response // contains Frames ([]*Frame). func (d *PyroscopeDatasource) QueryData(ctx context.Context, req *backend.QueryDataRequest) (*backend.QueryDataResponse, error) { - ctxLogger := logger.FromContext(ctx) + ctxLogger := d.logger.FromContext(ctx) ctxLogger.Debug("Processing queries", "queryLength", len(req.Queries), "function", logEntrypoint()) // create response struct @@ -256,7 +259,7 @@ func (d *PyroscopeDatasource) QueryData(ctx context.Context, req *backend.QueryD // datasource configuration page which allows users to verify that // a datasource is working as expected. func (d *PyroscopeDatasource) CheckHealth(ctx context.Context, _ *backend.CheckHealthRequest) (*backend.CheckHealthResult, error) { - logger.FromContext(ctx).Debug("CheckHealth called", "function", logEntrypoint()) + d.logger.FromContext(ctx).Debug("CheckHealth called", "function", logEntrypoint()) status := backend.HealthStatusOk message := "Data source is working" @@ -279,7 +282,7 @@ func (d *PyroscopeDatasource) CheckHealth(ctx context.Context, _ *backend.CheckH // SubscribeStream is called when a client wants to connect to a stream. // This callback allows sending the first message. func (d *PyroscopeDatasource) SubscribeStream(_ context.Context, req *backend.SubscribeStreamRequest) (*backend.SubscribeStreamResponse, error) { - logger.Debug("Subscribing stream called", "function", logEntrypoint()) + d.logger.Debug("Subscribing stream called", "function", logEntrypoint()) status := backend.SubscribeStreamStatusPermissionDenied if req.Path == "stream" { @@ -294,7 +297,7 @@ func (d *PyroscopeDatasource) SubscribeStream(_ context.Context, req *backend.Su // RunStream is called once for any open channel. // Results are shared with everyone subscribed to the same channel. func (d *PyroscopeDatasource) RunStream(ctx context.Context, req *backend.RunStreamRequest, sender *backend.StreamSender) error { - ctxLogger := logger.FromContext(ctx) + ctxLogger := d.logger.FromContext(ctx) ctxLogger.Debug("Running stream", "path", req.Path, "function", logEntrypoint()) // Create the same data frame as for query data. @@ -332,7 +335,7 @@ func (d *PyroscopeDatasource) RunStream(ctx context.Context, req *backend.RunStr // PublishStream is called when a client sends a message to the stream. func (d *PyroscopeDatasource) PublishStream(ctx context.Context, _ *backend.PublishStreamRequest) (*backend.PublishStreamResponse, error) { - logger.FromContext(ctx).Debug("Publishing stream", "function", logEntrypoint()) + d.logger.FromContext(ctx).Debug("Publishing stream", "function", logEntrypoint()) // Do not allow publishing at all. return &backend.PublishStreamResponse{ diff --git a/pkg/tsdb/grafana-pyroscope-datasource/instance_test.go b/pkg/tsdb/grafana-pyroscope-datasource/instance_test.go index 5d60d65b6eeff..4c736cac6dcd4 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/instance_test.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/instance_test.go @@ -5,12 +5,13 @@ import ( "testing" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/stretchr/testify/require" ) // This is where the tests for the datasource backend live. func Test_QueryData(t *testing.T) { - ds := PyroscopeDatasource{} + ds := PyroscopeDatasource{logger: log.NewNullLogger()} resp, err := ds.QueryData( context.Background(), @@ -32,6 +33,7 @@ func Test_QueryData(t *testing.T) { func Test_CallResource(t *testing.T) { ds := &PyroscopeDatasource{ client: &FakeClient{}, + logger: log.NewNullLogger(), } t.Run("series resource", func(t *testing.T) { diff --git a/pkg/tsdb/grafana-pyroscope-datasource/query.go b/pkg/tsdb/grafana-pyroscope-datasource/query.go index 494c9ffda6c21..95286cd32ead2 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/query.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/query.go @@ -16,6 +16,7 @@ import ( "github.com/grafana/grafana-plugin-sdk-go/backend" "github.com/grafana/grafana-plugin-sdk-go/backend/gtime" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana-plugin-sdk-go/backend/tracing" "github.com/grafana/grafana-plugin-sdk-go/config" "github.com/grafana/grafana-plugin-sdk-go/data" @@ -84,7 +85,7 @@ func (d *PyroscopeDatasource) query(ctx context.Context, pCtx backend.PluginCont parsedInterval, err = gtime.ParseDuration(dsJson.MinStep) if err != nil { parsedInterval = time.Second * 15 - logger.Error("Failed to parse the MinStep using default", "MinStep", dsJson.MinStep, "function", logEntrypoint()) + d.logger.Error("Failed to parse the MinStep using default", "MinStep", dsJson.MinStep, "function", logEntrypoint()) } } @@ -122,7 +123,7 @@ func (d *PyroscopeDatasource) query(ctx context.Context, pCtx backend.PluginCont if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - logger.Error("Querying SelectSeries()", "err", err, "function", logEntrypoint()) + d.logger.Error("Querying SelectSeries()", "err", err, "function", logEntrypoint()) return err } // add the frames to the response. @@ -134,7 +135,7 @@ func (d *PyroscopeDatasource) query(ctx context.Context, pCtx backend.PluginCont if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - logger.Error("Querying SelectSeries()", "err", err, "function", logEntrypoint()) + d.logger.Error("Querying SelectSeries()", "err", err, "function", logEntrypoint()) return err } response.Frames = append(response.Frames, frames...) @@ -147,22 +148,22 @@ func (d *PyroscopeDatasource) query(ctx context.Context, pCtx backend.PluginCont g.Go(func() error { var profileResp *ProfileResponse if len(qm.SpanSelector) > 0 { - logger.Debug("Calling GetSpanProfile", "queryModel", qm, "function", logEntrypoint()) + d.logger.Debug("Calling GetSpanProfile", "queryModel", qm, "function", logEntrypoint()) prof, err := d.client.GetSpanProfile(gCtx, profileTypeId, labelSelector, qm.SpanSelector, query.TimeRange.From.UnixMilli(), query.TimeRange.To.UnixMilli(), qm.MaxNodes) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - logger.Error("Error GetSpanProfile()", "err", err, "function", logEntrypoint()) + d.logger.Error("Error GetSpanProfile()", "err", err, "function", logEntrypoint()) return err } profileResp = prof } else { - logger.Debug("Calling GetProfile", "queryModel", qm, "function", logEntrypoint()) + d.logger.Debug("Calling GetProfile", "queryModel", qm, "function", logEntrypoint()) prof, err := d.client.GetProfile(gCtx, profileTypeId, labelSelector, query.TimeRange.From.UnixMilli(), query.TimeRange.To.UnixMilli(), qm.MaxNodes, qm.ProfileIdSelector) if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - logger.Error("Error GetProfile()", "err", err, "function", logEntrypoint()) + d.logger.Error("Error GetProfile()", "err", err, "function", logEntrypoint()) return err } profileResp = prof @@ -183,11 +184,11 @@ func (d *PyroscopeDatasource) query(ctx context.Context, pCtx backend.PluginCont parsedInterval, err = gtime.ParseDuration(dsJson.MinStep) if err != nil { parsedInterval = time.Second * 15 - logger.Error("Failed to parse the MinStep using default", "MinStep", dsJson.MinStep, "function", logEntrypoint()) + d.logger.Error("Failed to parse the MinStep using default", "MinStep", dsJson.MinStep, "function", logEntrypoint()) } } stepDuration := math.Max(query.Interval.Seconds(), parsedInterval.Seconds()) - frame = responseToDataFrames(profileResp, stepDuration, profileTypeId) + frame = responseToDataFrames(profileResp, stepDuration, profileTypeId, d.logger) // If query called with streaming on then return a channel // to subscribe on a client-side and consume updates from a plugin. @@ -245,7 +246,7 @@ func (d *PyroscopeDatasource) queryHeatmap(ctx context.Context, span trace.Span, if err != nil { span.RecordError(err) span.SetStatus(codes.Error, err.Error()) - logger.Error("Querying SelectHeatmap()", "err", err, "function", logEntrypoint()) + d.logger.Error("Querying SelectHeatmap()", "err", err, "function", logEntrypoint()) return nil, err } @@ -299,8 +300,8 @@ func (d *PyroscopeDatasource) queryHeatmap(ctx context.Context, span trace.Span, // responseToDataFrames turns Pyroscope response to data.Frame. We encode the data into a nested set format where we have // [level, value, label] columns and by ordering the items in a depth first traversal order we can recreate the whole // tree back. -func responseToDataFrames(resp *ProfileResponse, stepDurationSec float64, profileTypeID string) *data.Frame { - tree := levelsToTree(resp.Flamebearer.Levels, resp.Flamebearer.Names) +func responseToDataFrames(resp *ProfileResponse, stepDurationSec float64, profileTypeID string, logger log.Logger) *data.Frame { + tree := levelsToTree(resp.Flamebearer.Levels, resp.Flamebearer.Names, logger) return treeToNestedSetDataFrame(tree, resp.Units, stepDurationSec, profileTypeID) } @@ -330,7 +331,7 @@ type ProfileTree struct { // levelsToTree converts flamebearer format into a tree. This is needed to then convert it into nested set format // dataframe. This should be temporary, and ideally we should get some sort of tree struct directly from Pyroscope API. -func levelsToTree(levels []*Level, names []string) *ProfileTree { +func levelsToTree(levels []*Level, names []string, logger log.Logger) *ProfileTree { if len(levels) == 0 { return nil } diff --git a/pkg/tsdb/grafana-pyroscope-datasource/query_test.go b/pkg/tsdb/grafana-pyroscope-datasource/query_test.go index 6f97d0cb5451b..02fa2f548b769 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/query_test.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/query_test.go @@ -8,6 +8,7 @@ import ( "github.com/stretchr/testify/require" "github.com/grafana/grafana-plugin-sdk-go/backend" + "github.com/grafana/grafana-plugin-sdk-go/backend/log" "github.com/grafana/grafana-plugin-sdk-go/data" querierv1 "github.com/grafana/pyroscope/api/gen/proto/go/querier/v1" @@ -21,6 +22,7 @@ func Test_query(t *testing.T) { client := &FakeClient{} ds := &PyroscopeDatasource{ client: client, + logger: log.NewNullLogger(), } pCtx := backend.PluginContext{ @@ -135,7 +137,7 @@ func Test_profileToDataFrame(t *testing.T) { }, Units: "short", } - frame := responseToDataFrames(profile, 15.0, "goroutine:goroutine:count:goroutine:count") + frame := responseToDataFrames(profile, 15.0, "goroutine:goroutine:count:goroutine:count", log.NewNullLogger()) require.Equal(t, 4, len(frame.Fields)) require.Equal(t, data.NewField("level", nil, []int64{0, 1, 1}), frame.Fields[0]) require.Equal(t, data.NewField("value", nil, []int64{20, 10, 5}).SetConfig(&data.FieldConfig{Unit: "short"}), frame.Fields[1]) @@ -154,7 +156,7 @@ func Test_levelsToTree(t *testing.T) { {Values: []int64{0, 15, 0, 3}}, } - tree := levelsToTree(levels, []string{"root", "func1", "func2", "func1:func3"}) + tree := levelsToTree(levels, []string{"root", "func1", "func2", "func1:func3"}, log.NewNullLogger()) require.Equal(t, &ProfileTree{ Start: 0, Value: 100, Level: 0, Name: "root", Nodes: []*ProfileTree{ { @@ -174,7 +176,7 @@ func Test_levelsToTree(t *testing.T) { {Values: []int64{0, 20, 0, 4, 50, 10, 0, 5}}, } - tree := levelsToTree(levels, []string{"root", "func1", "func2", "func3", "func1:func4", "func3:func5"}) + tree := levelsToTree(levels, []string{"root", "func1", "func2", "func3", "func1:func4", "func3:func5"}, log.NewNullLogger()) require.Equal(t, &ProfileTree{ Start: 0, Value: 100, Level: 0, Name: "root", Nodes: []*ProfileTree{ { diff --git a/pkg/tsdb/grafana-pyroscope-datasource/service.go b/pkg/tsdb/grafana-pyroscope-datasource/service.go index 04a4c5240fcb6..81c874d509fb2 100644 --- a/pkg/tsdb/grafana-pyroscope-datasource/service.go +++ b/pkg/tsdb/grafana-pyroscope-datasource/service.go @@ -34,8 +34,6 @@ type Service struct { logger log.Logger } -var logger = backend.NewLoggerWith("logger", "tsdb.pyroscope") - // Return the file, line, and (full-path) function name of the caller func getRunContext() (string, int, string) { pc := make([]uintptr, 10) @@ -64,15 +62 ... [truncated]