From 308cc133abff7fbb750ba7cebc932b0a04860adf Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Thu, 5 Nov 2026 15:09:16 +0000 Subject: [PATCH 1/2] Remove connection details from stats response --- README.md | 4 +--- docs/swagger/docs.go | 6 ------ docs/swagger/swagger.json | 6 ------ internal/server/server.go | 4 ---- internal/server/server_test.go | 26 -------------------------- internal/server/stats_test.go | 2 +- 6 files changed, 2 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 2a08eaac..e47216eb 100644 --- a/README.md +++ b/README.md @@ -1515,9 +1515,7 @@ Response: { "cached_artifacts": 142, "total_size_bytes": 523456789, - "total_size": "499.2 MB", - "storage_url": "file:///path/to/cache/artifacts", - "database_path": "./cache/proxy.db" + "total_size": "499.2 MB" } ``` diff --git a/docs/swagger/docs.go b/docs/swagger/docs.go index 3ad61f7f..fe8723d8 100644 --- a/docs/swagger/docs.go +++ b/docs/swagger/docs.go @@ -1461,9 +1461,6 @@ const docTemplate = `{ "cached_artifacts": { "type": "integer" }, - "database_path": { - "type": "string" - }, "downloaded": { "type": "string" }, @@ -1484,9 +1481,6 @@ const docTemplate = `{ "description": "StatsUnavailable distinguishes a proxy that has served nothing from one\nwhose aggregation failed with no snapshot to fall back on. Without it\nboth report zeros and an empty array.", "type": "boolean" }, - "storage_url": { - "type": "string" - }, "total_size": { "type": "string" }, diff --git a/docs/swagger/swagger.json b/docs/swagger/swagger.json index d0f27c30..496aecde 100644 --- a/docs/swagger/swagger.json +++ b/docs/swagger/swagger.json @@ -1454,9 +1454,6 @@ "cached_artifacts": { "type": "integer" }, - "database_path": { - "type": "string" - }, "downloaded": { "type": "string" }, @@ -1477,9 +1474,6 @@ "description": "StatsUnavailable distinguishes a proxy that has served nothing from one\nwhose aggregation failed with no snapshot to fall back on. Without it\nboth report zeros and an empty array.", "type": "boolean" }, - "storage_url": { - "type": "string" - }, "total_size": { "type": "string" }, diff --git a/internal/server/server.go b/internal/server/server.go index 55fe597b..8599c489 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -1387,8 +1387,6 @@ type StatsResponse struct { CachedArtifacts int64 `json:"cached_artifacts"` TotalSize int64 `json:"total_size_bytes"` TotalSizeHuman string `json:"total_size"` - StorageURL string `json:"storage_url"` - DatabasePath string `json:"database_path"` // DownloadedBytes is the accumulated download volume across every // ecosystem: cache hits multiplied by the artifact size they served. DownloadedBytes int64 `json:"downloaded_bytes"` @@ -1452,8 +1450,6 @@ func (s *Server) handleStats(w http.ResponseWriter, r *http.Request) { TotalSizeHuman: formatSize(size), Ecosystems: make([]EcosystemStatsEntry, 0, len(ecosystems)), StatsUnavailable: statsErr != nil && len(ecosystems) == 0, - StorageURL: s.storage.URL(), - DatabasePath: s.cfg.Database.String(), } for _, e := range ecosystems { diff --git a/internal/server/server_test.go b/internal/server/server_test.go index e2d22648..5fa49e9b 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -534,10 +534,6 @@ func TestStatsEndpoint(t *testing.T) { if stats.CachedArtifacts != 0 { t.Errorf("expected 0 cached artifacts, got %d", stats.CachedArtifacts) } - - if !strings.HasPrefix(stats.StorageURL, "file://") { - t.Errorf("expected storage_url to start with file://, got %q", stats.StorageURL) - } } func TestDashboard(t *testing.T) { @@ -1676,28 +1672,6 @@ func TestNewServer_InvalidAccessLogFailsBeforeDatabaseInit(t *testing.T) { } } -func TestStatsEndpoint_StorageURL(t *testing.T) { - ts := newTestServer(t) - defer ts.close() - - req := httptest.NewRequest("GET", "/stats", nil) - w := httptest.NewRecorder() - ts.handler.ServeHTTP(w, req) - - if w.Code != http.StatusOK { - t.Fatalf("expected status 200, got %d", w.Code) - } - - // Verify the JSON response uses storage_url (not storage_path) - body := w.Body.String() - if !strings.Contains(body, `"storage_url"`) { - t.Errorf("expected JSON key storage_url in response, got: %s", body) - } - if strings.Contains(body, `"storage_path"`) { - t.Errorf("unexpected JSON key storage_path in response (should be storage_url)") - } -} - // TestUIRefusesFraming checks that other sites cannot frame UI pages, where // a framed Delete button could be clicked by a signed-in user. func TestUIRefusesFraming(t *testing.T) { diff --git a/internal/server/stats_test.go b/internal/server/stats_test.go index 467f3c16..e7777587 100644 --- a/internal/server/stats_test.go +++ b/internal/server/stats_test.go @@ -108,7 +108,7 @@ func TestStatsFlagsUnavailableFigures(t *testing.T) { if len(stats.Ecosystems) != 0 { t.Errorf("ecosystems = %v, want empty", stats.Ecosystems) } - if stats.StorageURL == "" { + if stats.TotalSizeHuman == "" { t.Error("the rest of the response must still be served") } } From 0ed91b5872ef9c1cfa6785b0bde4783a02b4de5d Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Fri, 6 Nov 2026 09:20:43 +0000 Subject: [PATCH 2/2] Redact storage passwords in startup logs --- internal/server/server.go | 6 ++- internal/server/server_test.go | 89 ++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) diff --git a/internal/server/server.go b/internal/server/server.go index 8599c489..b6c67e40 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -436,11 +436,15 @@ func (s *Server) serve(listener net.Listener) error { IdleTimeout: serverIdleTimeout, } + storageURL := "[redacted]" + if u, err := url.Parse(s.storage.URL()); err == nil { + storageURL = u.Redacted() + } s.logger.Info("starting server", "listen", s.cfg.Listen, "base_url", s.cfg.BaseURL, "ui_url", s.cfg.UIBaseURL, - "storage", s.storage.URL(), + "storage", storageURL, "database", s.cfg.Database.String()) for _, warning := range s.cfg.EnvWarnings() { s.logger.Warn(warning) diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 5fa49e9b..02d781d1 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -150,6 +150,95 @@ func (ts *testServer) close() { _ = os.RemoveAll(ts.tempDir) } +type storageWithURL struct { + storage.Storage + url string +} + +func (s storageWithURL) URL() string { return s.url } + +func TestStartRedactsStoragePassword(t *testing.T) { + for _, tc := range []struct { + name string + url string + want string + }{ + {"password", "s3://user:secret@bucket?region=us-east-1", "s3://user:xxxxx@bucket?region=us-east-1"}, + {"escaped password", "s3://user:secret%40word@bucket", "s3://user:xxxxx@bucket"}, + {"no password", "s3://bucket?region=us-east-1", "s3://bucket?region=us-east-1"}, + {"invalid URL", "s3://user:secret%zz@bucket", "[redacted]"}, + } { + t.Run(tc.name, func(t *testing.T) { + output := startStorageLog(t, tc.url) + if strings.Contains(output, "secret") { + t.Errorf("startup log contains storage password: %s", output) + } + decoder := json.NewDecoder(strings.NewReader(output)) + for decoder.More() { + var entry struct { + Message string `json:"msg"` + Storage string `json:"storage"` + } + if err := decoder.Decode(&entry); err != nil { + t.Fatal(err) + } + if entry.Message == "starting server" { + if entry.Storage != tc.want { + t.Errorf("logged storage = %q, want %q", entry.Storage, tc.want) + } + t.Logf("logged storage = %q", entry.Storage) + return + } + } + t.Fatal("startup log missing") + }) + } +} + +func startStorageLog(t *testing.T, storageURL string) string { + t.Helper() + listener, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = listener.Close() }) + + cfg := config.Default() + cfg.Listen = listener.Addr().String() + cfg.BaseURL = "http://" + cfg.Listen + cfg.Database.Path = filepath.Join(t.TempDir(), "proxy.db") + cfg.Storage.URL = "file://" + filepath.Join(t.TempDir(), "artifacts") + var logs syncBuffer + srv, err := New(cfg, slog.New(slog.NewJSONHandler(&logs, nil)), BuildInfo{}) + if err != nil { + t.Fatal(err) + } + srv.storage = storageWithURL{Storage: srv.storage, url: storageURL} + done := make(chan error, 1) + go func() { done <- srv.Start(listener) }() + t.Cleanup(func() { + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + if err := srv.Shutdown(ctx); err != nil { + t.Errorf("shutting down server: %v", err) + } + if err := <-done; !errors.Is(err, http.ErrServerClosed) { + t.Errorf("Start() = %v, want %v", err, http.ErrServerClosed) + } + }) + + client := &http.Client{Timeout: 5 * time.Second} + resp, err := client.Get(cfg.BaseURL + "/health") + if err != nil { + t.Fatal(err) + } + _ = resp.Body.Close() + if resp.StatusCode != http.StatusOK { + t.Fatalf("health status = %d, want 200", resp.StatusCode) + } + return logs.String() +} + func TestUpstreamSafeHTTPOptions(t *testing.T) { opts := upstreamSafeHTTPOptions(config.UpstreamConfig{ AllowPrivateHosts: []string{"registry.internal"},