diff --git a/pkg/gofr/constants.go b/pkg/gofr/constants.go index 10a6974bc9..65ac705072 100644 --- a/pkg/gofr/constants.go +++ b/pkg/gofr/constants.go @@ -6,6 +6,8 @@ const ( defaultPublicStaticDir = "static" shutDownTimeout = 30 * time.Second gofrTraceExporter = "gofr" + otlpTraceExporter = "otlp" + jaegerTraceExporter = "jaeger" gofrTracerURL = "https://tracer.gofr.dev" checkPortTimeout = 2 * time.Second gofrHost = "https://gofr.dev" diff --git a/pkg/gofr/gofr_test.go b/pkg/gofr/gofr_test.go index 17f3b2f174..ca87b339b9 100644 --- a/pkg/gofr/gofr_test.go +++ b/pkg/gofr/gofr_test.go @@ -836,7 +836,7 @@ func Test_initTracer_invalidConfig(t *testing.T) { config config.Config expectedLogMessage string }{ - {"unsupported trace_exporter", mockConfig1, "unsupported TRACE_EXPORTER: abc"}, + {"unsupported trace_exporter", mockConfig1, "unsupported TRACE_EXPORTER=abc: expected one of otlp, jaeger, zipkin or gofr"}, {"missing trace_exporter", mockConfig2, "missing TRACE_EXPORTER config, should be provided with TRACER_URL to enable tracing"}, {"miss tracer_url ", mockConfig3, "missing TRACER_URL config, should be provided with TRACE_EXPORTER to enable tracing"}, diff --git a/pkg/gofr/otel.go b/pkg/gofr/otel.go index 542295e8c1..accc638f8a 100644 --- a/pkg/gofr/otel.go +++ b/pkg/gofr/otel.go @@ -3,8 +3,10 @@ package gofr import ( "context" "fmt" + "net/url" "strconv" "strings" + "unicode" "go.opentelemetry.io/otel" "go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracegrpc" @@ -170,8 +172,10 @@ func (a *App) tracerInsecure() (insecure, set bool) { b, err := strconv.ParseBool(v) if err != nil { - a.Logger().Warnf("invalid TRACER_INSECURE %q: expected a boolean; defaulting to plaintext for a "+ - "schemeless TRACER_URL", v) + // The raw value is not echoed. Tracer config reaches a log only through + // redactURL, redactExporterName or as a matched constant. + a.Logger().Warn("invalid TRACER_INSECURE: expected true or false; defaulting to plaintext for a " + + "schemeless TRACER_URL") return true, false } @@ -188,13 +192,10 @@ func (a *App) getExporter(name, host, port, url string) (sdktrace.SpanExporter, headers := a.getTracerHeaders() switch strings.ToLower(name) { - case "otlp", "jaeger": - // Resolved inside the case, not above the switch: TRACER_INSECURE means - // nothing to zipkin or gofr, and evaluating it for them would warn about a - // malformed value under an exporter that never reads it. - insecure, insecureSet := a.tracerInsecure() - - exporter, err = buildOtlpExporter(a.Logger(), name, url, host, port, headers, insecure, insecureSet) + case otlpTraceExporter: + exporter, err = a.buildOtlpFamilyExporter(otlpTraceExporter, url, host, port, headers) + case jaegerTraceExporter: + exporter, err = a.buildOtlpFamilyExporter(jaegerTraceExporter, url, host, port, headers) case "zipkin": a.Logger().Warn("TRACE_EXPORTER=zipkin is deprecated and will be removed in a future release. " + "Zipkin supports OTLP natively (v2.24+) — to migrate, switch to TRACE_EXPORTER=otlp " + @@ -204,12 +205,25 @@ func (a *App) getExporter(name, host, port, url string) (sdktrace.SpanExporter, case gofrTraceExporter: exporter = buildGoFrExporter(a.Logger(), url) default: - a.container.Errorf("unsupported TRACE_EXPORTER: %s", name) + a.container.Errorf("unsupported TRACE_EXPORTER=%s: expected one of otlp, jaeger, zipkin or gofr", + redactExporterName(name)) } return exporter, err } +// buildOtlpFamilyExporter builds the OTLP exporter for otlp and jaeger. The +// exporter name is passed as the matched constant rather than the configured +// string, so only a known name is ever logged. +func (a *App) buildOtlpFamilyExporter(name, endpoint, host, port string, headers map[string]string) (sdktrace.SpanExporter, error) { + // Resolved here, not in getExporter above the switch: TRACER_INSECURE means + // nothing to zipkin or gofr, and evaluating it for them would warn about a + // malformed value under an exporter that never reads it. + insecure, insecureSet := a.tracerInsecure() + + return buildOtlpExporter(a.Logger(), name, endpoint, host, port, headers, insecure, insecureSet) +} + // otlpTransport is the resolved transport-security decision for the OTLP trace // exporter: which endpoint option to use, whether to apply WithInsecure, and // whether the resulting connection carries traffic in the clear. @@ -266,7 +280,7 @@ func resolveOtlpTransport(logger logging.Logger, url string, insecure, insecureS if strings.HasPrefix(scheme, schemeHTTP) || strings.HasPrefix(scheme, schemeHTTPS) { if insecureSet { logger.Warnf("TRACER_INSECURE is ignored for TRACER_URL=%q: transport security is derived "+ - "from the URL scheme", url) + "from the URL scheme", redactURL(url)) } return otlpTransport{useEndpointURL: true, plaintext: strings.HasPrefix(scheme, schemeHTTP)} @@ -287,10 +301,10 @@ func buildOtlpExporter(logger logging.Logger, name, url, host, port string, head if transport.plaintext && len(headers) > 0 { logger.Warnf("traces are exported to %s over plaintext with headers configured: "+ - "headers (including auth credentials) will be sent in the clear", url) + "headers (including auth credentials) will be sent in the clear", redactURL(url)) } - logger.Infof("Exporting traces to %s at %s", strings.ToLower(name), url) + logger.Infof("Exporting traces to %s at %s", name, redactURL(url)) var opts []otlptracegrpc.Option @@ -316,7 +330,7 @@ func buildZipkinExporter(logger logging.Logger, url, host, port string, headers url = fmt.Sprintf("http://%s:%s/api/v2/spans", host, port) } - logger.Infof("Exporting traces to zipkin at %s", url) + logger.Infof("Exporting traces to zipkin at %s", redactURL(url)) var opts []zipkin.Option if len(headers) > 0 { @@ -331,11 +345,118 @@ func buildGoFrExporter(logger logging.Logger, url string) sdktrace.SpanExporter url = "https://tracer-api.gofr.dev/api/spans" } - logger.Infof("Exporting traces to GoFr at %s", url) + logger.Infof("Exporting traces to GoFr at %s", redactURL(url)) return NewExporter(url, logging.NewLogger(logging.INFO)) } +// redactedPlaceholder stands in for credentials when a tracer endpoint is logged. +const redactedPlaceholder = "REDACTED" + +// redactURL returns a tracer endpoint that is safe to write to a log. +// +// A scheme-bearing TRACER_URL is a real URL, so it can carry credentials in its +// userinfo (https://user:token@collector:4317) or its query +// (https://collector/api/v2/spans?api-key=...). Both are replaced rather than +// dropped, so the log still shows that something was configured there. +// +// Secrets placed in path segments (https://host/v1/TOKEN/spans) are not +// redacted: no tracer backend GoFr supports takes credentials there, and +// hiding the path would hide the part of the endpoint an operator debugs. +// +// Control characters are escaped on the unparsed path only: url.Parse rejects +// them, so such a value always lands there, and a raw newline would otherwise +// forge a second log line in the terminal's pretty output. +func redactURL(raw string) string { + u, err := url.Parse(raw) + if err != nil || u.Host == "" { + return escapeControlChars(redactUnparsedURL(raw)) + } + + if u.User != nil { + u.User = url.User(redactedPlaceholder) + } + + if u.RawQuery != "" { + u.RawQuery = redactedPlaceholder + } + + u.Fragment = "" + + return u.String() +} + +// escapeControlChars replaces every control character with its \x or \u +// escape, so a logged value always stays on one line. +func escapeControlChars(s string) string { + if strings.IndexFunc(s, unicode.IsControl) < 0 { + return s + } + + var b strings.Builder + + for _, r := range s { + switch { + case !unicode.IsControl(r): + b.WriteRune(r) + case r <= unicode.MaxLatin1: + fmt.Fprintf(&b, `\x%02x`, r) + default: + fmt.Fprintf(&b, `\u%04x`, r) + } + } + + return b.String() +} + +// maxEchoedExporterNameLen bounds how much of an unsupported TRACE_EXPORTER is +// echoed back; every supported name is at most 6 characters. +const maxEchoedExporterNameLen = 16 + +// redactExporterName returns an unsupported TRACE_EXPORTER value in a form safe +// to log. A typo of a real exporter name -- short, letters and '-' or '_' only +// -- is echoed so it can be spotted; anything else, which is more likely a +// value pasted into the wrong variable, is replaced. +func redactExporterName(name string) string { + if name == "" || len(name) > maxEchoedExporterNameLen { + return redactedPlaceholder + } + + for _, r := range name { + if (r < 'a' || r > 'z') && (r < 'A' || r > 'Z') && r != '-' && r != '_' { + return redactedPlaceholder + } + } + + return name +} + +// redactUnparsedURL redacts an endpoint that url.Parse cannot split into a host +// and the rest: a schemeless host:port, host/path or user:pass@host, or input +// that does not parse at all. It works on the raw string and over-redacts +// rather than risk a leak. +// +// Userinfo is handled first, up to the last '@'. A '?' in a password, or an +// '@' in a query value, then falls inside the replaced prefix instead of +// splitting the credential. The query and fragment are handled after that. +func redactUnparsedURL(raw string) string { + redacted := raw + + if i := strings.LastIndex(redacted, "@"); i >= 0 { + redacted = redactedPlaceholder + redacted[i:] + } + + if i := strings.IndexByte(redacted, '#'); i >= 0 { + redacted = redacted[:i] + } + + if i := strings.IndexByte(redacted, '?'); i >= 0 { + redacted = redacted[:i+1] + redactedPlaceholder + } + + return redacted +} + type otelErrorHandler struct { logger logging.Logger } diff --git a/pkg/gofr/otel_test.go b/pkg/gofr/otel_test.go index 4016a33e64..1c84fc1faa 100644 --- a/pkg/gofr/otel_test.go +++ b/pkg/gofr/otel_test.go @@ -614,3 +614,120 @@ func BenchmarkSpanStart_NeverSampleSDK(b *testing.B) { span.End() } } + +func Test_redactExporterName(t *testing.T) { + tests := []struct { + name string + value string + expected string + }{ + {name: "typo of a real exporter is echoed", value: "otpl", expected: "otpl"}, + {name: "mixed case and separators are echoed", value: "Open_Telemetry-x", expected: "Open_Telemetry-x"}, + {name: "empty is replaced", value: "", expected: "REDACTED"}, + {name: "longer than 16 is replaced", value: "abcdefghijklmnopq", expected: "REDACTED"}, + {name: "digits look like a pasted secret", value: "ab12cd34", expected: "REDACTED"}, + {name: "URL pasted into the wrong variable", value: "https://x", expected: "REDACTED"}, + {name: "control character is replaced", value: "otlp\n", expected: "REDACTED"}, + {name: "non-ASCII letter is replaced", value: "ötlp", expected: "REDACTED"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expected, redactExporterName(tt.value)) + }) + } +} + +func Test_redactURL(t *testing.T) { + tests := []struct { + name string + raw string + expected string + }{ + {name: "schemeless host:port is unchanged", raw: "collector:4317", expected: "collector:4317"}, + {name: "plain https URL is unchanged", raw: "https://collector:4317", expected: "https://collector:4317"}, + {name: "path is kept", raw: "http://localhost:2005/api/v2/spans", expected: "http://localhost:2005/api/v2/spans"}, + {name: "empty is unchanged", raw: "", expected: ""}, + {name: "userinfo with password", raw: "https://user:s3cret@collector:4317", expected: "https://REDACTED@collector:4317"}, + {name: "userinfo token only", raw: "https://t0ken@collector:4317", expected: "https://REDACTED@collector:4317"}, + {name: "query credentials", raw: "https://zipkin/api/v2/spans?api-key=s3cret", expected: "https://zipkin/api/v2/spans?REDACTED"}, + {name: "fragment is dropped", raw: "https://collector:4317#s3cret", expected: "https://collector:4317"}, + {name: "schemeless userinfo", raw: "user:s3cret@collector:4317", expected: "REDACTED@collector:4317"}, + {name: "unparsable with userinfo", raw: "https://user:s3cret@collector:43%17", expected: "REDACTED@collector:43%17"}, + {name: "schemeless query credentials", raw: "localhost:9411/api/v2/spans?api-key=s3cret", + expected: "localhost:9411/api/v2/spans?REDACTED"}, + {name: "schemeless userinfo and query", raw: "user:s3cret@collector:4317/p?k=s3cret", expected: "REDACTED@collector:4317/p?REDACTED"}, + {name: "schemeless password containing '?'", raw: "user:s3?cret@collector:4317", expected: "REDACTED@collector:4317"}, + {name: "schemeless '@' inside query value", raw: "collector:4317/p?k=s3cret@x", expected: "REDACTED@x"}, + {name: "schemeless fragment is dropped", raw: "collector:4317#s3cret", expected: "collector:4317"}, + {name: "newline cannot forge a log line", raw: "collector:4317\n{\"level\":\"INFO\"}", + expected: `collector:4317\x0a{"level":"INFO"}`}, + {name: "carriage return and tab are escaped", raw: "host\r:4317\t", expected: `host\x0d:4317\x09`}, + {name: "C1 control is escaped", raw: "host" + string(rune(0x85)) + ":4317", expected: `host\x85:4317`}, + {name: "control char with userinfo is escaped after redaction", raw: "https://user:s3cret@collector\n:4317", + expected: `REDACTED@collector\x0a:4317`}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + require.Equal(t, tt.expected, redactURL(tt.raw)) + }) + } +} + +// Test_initTracer_doesNotLogCredentials drives the real startup path for every +// exporter that logs its endpoint, and asserts the credentials in TRACER_URL and +// the raw TRACER_INSECURE value never reach the log. +func Test_initTracer_doesNotLogCredentials(t *testing.T) { + const secret = "s3cret-value" + + tests := []struct { + name string + exporter string + url string + insecure string + expected string + }{ + {name: "otlp userinfo", exporter: "otlp", url: "https://user:" + secret + "@localhost:4317", + expected: "Exporting traces to otlp at https://REDACTED@localhost:4317"}, + {name: "jaeger ignored TRACER_INSECURE", exporter: "JAEGER", url: "http://user:" + secret + "@localhost:4317", + insecure: "true", expected: "Exporting traces to jaeger at http://REDACTED@localhost:4317"}, + {name: "otlp invalid TRACER_INSECURE", exporter: "otlp", url: "localhost:4317", insecure: secret, + expected: "invalid TRACER_INSECURE"}, + {name: "zipkin query key", exporter: "zipkin", url: "http://localhost:2005/api/v2/spans?api-key=" + secret, + expected: "Exporting traces to zipkin at http://localhost:2005/api/v2/spans?REDACTED"}, + {name: "zipkin schemeless query key", exporter: "zipkin", url: "localhost:9411/api/v2/spans?api-key=" + secret, + expected: "Exporting traces to zipkin at localhost:9411/api/v2/spans?REDACTED"}, + {name: "gofr userinfo", exporter: "gofr", url: "https://user:" + secret + "@tracer.example.com/api/spans", + expected: "Exporting traces to GoFr at https://REDACTED@tracer.example.com/api/spans"}, + {name: "secret pasted into TRACE_EXPORTER", exporter: secret, url: "localhost:4317", + expected: "unsupported TRACE_EXPORTER=REDACTED"}, + {name: "typo in TRACE_EXPORTER is echoed", exporter: "otpl", url: "localhost:4317", + expected: "unsupported TRACE_EXPORTER=otpl"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := map[string]string{"TRACE_EXPORTER": tt.exporter, "TRACER_URL": tt.url, "TRACER_AUTH_KEY": "Bearer x"} + if tt.insecure != "" { + cfg["TRACER_INSECURE"] = tt.insecure + } + + var stderr string + + stdout := testutil.StdoutOutputForFunc(func() { + stderr = testutil.StderrOutputForFunc(func() { + mockContainer, _ := container.NewMockContainer(t) + + a := App{Config: config.NewMockConfig(cfg), container: mockContainer} + a.initTracer() + }) + }) + + out := stdout + stderr + + require.Contains(t, out, tt.expected) + require.NotContains(t, out, secret) + }) + } +}