From 0e19be5e54748fe835c9c66bfb09297a8429c011 Mon Sep 17 00:00:00 2001 From: Konippi Date: Fri, 12 Jun 2026 01:02:12 +0900 Subject: [PATCH 1/2] fix(router): stop logging secrets in ext_proc request logs --- cmd/atenet/internal/router/extproc.go | 15 +++--- cmd/atenet/internal/router/extproc_in.go | 4 +- cmd/atenet/internal/router/extproc_in_test.go | 18 +++---- cmd/atenet/internal/router/extproc_test.go | 51 +++++++++++++++++++ 4 files changed, 69 insertions(+), 19 deletions(-) diff --git a/cmd/atenet/internal/router/extproc.go b/cmd/atenet/internal/router/extproc.go index 7c8c184e2..6e47aa66e 100644 --- a/cmd/atenet/internal/router/extproc.go +++ b/cmd/atenet/internal/router/extproc.go @@ -108,7 +108,7 @@ func (s *ExtProcServer) Process(stream extprocv3.ExternalProcessor_ProcessServer default: // No modification for other processing states, but log because this should // not be called. - slog.Error("Unexpected request type", slog.Any("reqType", reqType)) + slog.Error("Unexpected request type", slog.String("reqType", fmt.Sprintf("%T", reqType))) resp.Response = &extprocv3.ProcessingResponse_RequestHeaders{ RequestHeaders: &extprocv3.HeadersResponse{ Response: &extprocv3.CommonResponse{}, @@ -127,7 +127,7 @@ func (s *ExtProcServer) handleRequestHeaders( reqHeaders *extprocv3.HttpHeaders, ) (*extprocv3.HeadersResponse, *requestMetadata, string, string, string, error) { metadata := newRequestMetadata(reqHeaders.Headers.GetHeaders()) - slog.InfoContext(ctx, "Request", slog.String("metadata", metadata.String())) + slog.InfoContext(ctx, "Request", slog.String("host", metadata.host)) actorID, err := parseActorID(metadata.host) if err != nil { @@ -137,12 +137,6 @@ func (s *ExtProcServer) handleRequestHeaders( slog.InfoContext(ctx, "ResumeActor", slog.String("actorID", actorID)) actor, err := s.resumer.ResumeActor(ctx, actorID) - - slog.InfoContext(ctx, "ResumeActor result", - slog.String("actor", fmt.Sprintf("%+v", actor)), - slog.String("worker_ip", actor.GetAteomPodIp()), - slog.Any("err", err)) - if err != nil { return nil, metadata, "", "", "", mapResumeError(actorID, err) } @@ -153,6 +147,11 @@ func (s *ExtProcServer) handleRequestHeaders( tmplName := actor.GetActorTemplateName() workerIP := actor.GetAteomPodIp() + slog.InfoContext(ctx, "ResumeActor result", + slog.String("actorID", actorID), + slog.String("status", actor.GetStatus().String()), + slog.String("workerIP", workerIP)) + if ip := net.ParseIP(workerIP); ip == nil { return nil, metadata, "", tmplNs, tmplName, newReqError(envoy_type.StatusCode_InternalServerError, "actor %q routing failed", actorID) diff --git a/cmd/atenet/internal/router/extproc_in.go b/cmd/atenet/internal/router/extproc_in.go index a394f98da..d2efbf5df 100644 --- a/cmd/atenet/internal/router/extproc_in.go +++ b/cmd/atenet/internal/router/extproc_in.go @@ -30,7 +30,7 @@ type requestMetadata struct { } func (m *requestMetadata) String() string { - return fmt.Sprintf("%+v", *m) + return fmt.Sprintf("host=%s path=%s", m.host, m.path) } func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata { @@ -47,7 +47,7 @@ func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata { headersMap[k] = val if k == ":path" { - path = val + path, _, _ = strings.Cut(val, "?") } if k == ":authority" || k == "host" { host = val diff --git a/cmd/atenet/internal/router/extproc_in_test.go b/cmd/atenet/internal/router/extproc_in_test.go index 8fa9ea076..520407f5d 100644 --- a/cmd/atenet/internal/router/extproc_in_test.go +++ b/cmd/atenet/internal/router/extproc_in_test.go @@ -15,8 +15,8 @@ package router import ( - "fmt" "reflect" + "strings" "testing" corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -121,17 +121,17 @@ func TestExtractMetadata(t *testing.T) { } func TestRequestMetadata_String(t *testing.T) { - headers := []*corev3.HeaderValue{ + m := newRequestMetadata([]*corev3.HeaderValue{ {Key: ":path", Value: "/api/v1/test"}, {Key: ":authority", Value: "example.com"}, + {Key: "authorization", Value: "Bearer do-not-log-me"}, + }) + got := m.String() + if want := "host=example.com path=/api/v1/test"; got != want { + t.Errorf("String() = %q, want %q", got, want) } - m := newRequestMetadata(headers) - str := m.String() - if str == "" { - t.Errorf("expected non-empty string from String()") - } - if !reflect.DeepEqual(str, fmt.Sprintf("%+v", *m)) { - t.Errorf("String() = %q, want %q", str, fmt.Sprintf("%+v", *m)) + if strings.Contains(got, "do-not-log-me") { + t.Errorf("String() leaked header value: %q", got) } } diff --git a/cmd/atenet/internal/router/extproc_test.go b/cmd/atenet/internal/router/extproc_test.go index 005ef24d1..52f345035 100644 --- a/cmd/atenet/internal/router/extproc_test.go +++ b/cmd/atenet/internal/router/extproc_test.go @@ -15,8 +15,11 @@ package router import ( + "bytes" "context" + "encoding/json" "errors" + "log/slog" "strings" "testing" "time" @@ -39,6 +42,54 @@ func (m *mockClient) ResumeActor(ctx context.Context, in *ateapipb.ResumeActorRe return m.resumeFn(ctx, in, opts...) } +func TestHandleRequestHeadersDoesNotLogSensitiveData(t *testing.T) { + const testUUID = "123e4567-e89b-12d3-a456-426614174000" + const secret = "do-not-log-me" + + var buf bytes.Buffer + prev := slog.Default() + slog.SetDefault(slog.New(slog.NewJSONHandler(&buf, nil))) + t.Cleanup(func() { slog.SetDefault(prev) }) + + s := NewExtProcServer(50051, &mockClient{ + resumeFn: func(ctx context.Context, in *ateapipb.ResumeActorRequest, opts ...grpc.CallOption) (*ateapipb.ResumeActorResponse, error) { + return &ateapipb.ResumeActorResponse{Actor: &ateapipb.Actor{AteomPodIp: "10.0.0.52"}}, nil + }, + }, nil) + + reqHeaders := &extprocv3.HttpHeaders{ + Headers: &corev3.HeaderMap{ + Headers: []*corev3.HeaderValue{ + {Key: ":path", Value: "/api/v1/reset?token=" + secret}, + {Key: ":authority", Value: testUUID + ".actors.resources.substrate.ate.dev"}, + {Key: ":method", Value: "POST"}, + {Key: "authorization", Value: "Bearer " + secret}, + {Key: "cookie", Value: "session=" + secret}, + }, + }, + } + + _, metadata, target, _, _, err := s.handleRequestHeaders(context.Background(), reqHeaders) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + out := buf.String() + if strings.Contains(out, secret) { + t.Errorf("router log leaked sensitive value: %s", out) + } + if !strings.Contains(out, testUUID) { + t.Errorf("router log missing actor/host routing context: %s", out) + } + + s.recorder.AddRouterRequest(time.Now(), time.Millisecond, "Route ok", target, metadata) + for _, q := range s.recorder.Get() { + if blob, _ := json.Marshal(q); strings.Contains(string(blob), secret) { + t.Errorf("recorder/statusz retained sensitive value: %s", blob) + } + } +} + func TestExtProcHeadersEvaluation(t *testing.T) { const testUUID = "123e4567-e89b-12d3-a456-426614174000" From f35e75f3f694bcf221f66e8ac33d3f53d8a5837f Mon Sep 17 00:00:00 2001 From: Konippi Date: Sat, 13 Jun 2026 09:50:15 +0900 Subject: [PATCH 2/2] refactor(router): redact path query at statusz sink --- cmd/atenet/internal/router/extproc_in.go | 6 +----- cmd/atenet/internal/router/extproc_in_test.go | 16 ---------------- cmd/atenet/internal/router/status.go | 8 +++++++- 3 files changed, 8 insertions(+), 22 deletions(-) diff --git a/cmd/atenet/internal/router/extproc_in.go b/cmd/atenet/internal/router/extproc_in.go index d2efbf5df..ae3e1b4d2 100644 --- a/cmd/atenet/internal/router/extproc_in.go +++ b/cmd/atenet/internal/router/extproc_in.go @@ -29,10 +29,6 @@ type requestMetadata struct { host string } -func (m *requestMetadata) String() string { - return fmt.Sprintf("host=%s path=%s", m.host, m.path) -} - func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata { headersMap := make(map[string]string) var path string @@ -47,7 +43,7 @@ func newRequestMetadata(headers []*corev3.HeaderValue) *requestMetadata { headersMap[k] = val if k == ":path" { - path, _, _ = strings.Cut(val, "?") + path = val } if k == ":authority" || k == "host" { host = val diff --git a/cmd/atenet/internal/router/extproc_in_test.go b/cmd/atenet/internal/router/extproc_in_test.go index 520407f5d..901d98976 100644 --- a/cmd/atenet/internal/router/extproc_in_test.go +++ b/cmd/atenet/internal/router/extproc_in_test.go @@ -16,7 +16,6 @@ package router import ( "reflect" - "strings" "testing" corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" @@ -120,21 +119,6 @@ func TestExtractMetadata(t *testing.T) { } } -func TestRequestMetadata_String(t *testing.T) { - m := newRequestMetadata([]*corev3.HeaderValue{ - {Key: ":path", Value: "/api/v1/test"}, - {Key: ":authority", Value: "example.com"}, - {Key: "authorization", Value: "Bearer do-not-log-me"}, - }) - got := m.String() - if want := "host=example.com path=/api/v1/test"; got != want { - t.Errorf("String() = %q, want %q", got, want) - } - if strings.Contains(got, "do-not-log-me") { - t.Errorf("String() leaked header value: %q", got) - } -} - func TestParseActorID(t *testing.T) { tests := []struct { name string diff --git a/cmd/atenet/internal/router/status.go b/cmd/atenet/internal/router/status.go index 3d5f2dc74..2946b93a8 100644 --- a/cmd/atenet/internal/router/status.go +++ b/cmd/atenet/internal/router/status.go @@ -103,6 +103,12 @@ func (qr *QueryRecorder) Get() []RecordedQuery { return res } +// redactPath drops the query string, which may carry credentials (CWE-598). +func redactPath(path string) string { + p, _, _ := strings.Cut(path, "?") + return p +} + func (qr *QueryRecorder) AddRouterRequest( start time.Time, duration time.Duration, @@ -114,7 +120,7 @@ func (qr *QueryRecorder) AddRouterRequest( Timestamp: start, Client: m.headers[":authority"], Host: m.host, - Path: m.path, + Path: redactPath(m.path), Method: m.headers[":method"], Action: action, Target: target,