From ec34e824a669280272ab25e0f43ae7e3452554ad Mon Sep 17 00:00:00 2001 From: lucor Date: Thu, 30 Apr 2026 20:28:33 +0200 Subject: [PATCH] fix: do not keep raw inspect tokens in memory The inspect flow is a temporary debugging utility with a 10-minute TTL. It lets an authenticated user capture one payload from a third-party service so they can configure webhook parsing correctly. Even though this token is short-lived and scoped to a single user, we were keeping the raw token in memory alongside its hash. That is unnecessary exposure. This change removes the Token field from InspectSession so only the hash is stored in memory. The raw token is still returned once to the caller (so the user gets their inspect URL), but after that it only exists as a hash. This matches how we already handle webhook tokens, and it is just good memory hygiene. --- internal/webhook/inspect.go | 8 ++-- internal/webhook/service.go | 6 +-- internal/webhook/service_test.go | 67 ++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/internal/webhook/inspect.go b/internal/webhook/inspect.go index d6693c7..ad7e697 100644 --- a/internal/webhook/inspect.go +++ b/internal/webhook/inspect.go @@ -22,7 +22,6 @@ const ( type InspectSession struct { UserID string TokenHash string - Token string Name string Description string Priority string @@ -46,10 +45,10 @@ func NewInspectStore() *InspectStore { } // Create generates a new inspect session for the given user, replacing any previous one. -func (s *InspectStore) Create(userID, name, description, priority string, topics []string) (*InspectSession, error) { +func (s *InspectStore) Create(userID, name, description, priority string, topics []string) (string, *InspectSession, error) { rawToken, err := secure.NewInspectToken() if err != nil { - return nil, err + return "", nil, err } tokenHash := secure.Hash(rawToken) @@ -57,7 +56,6 @@ func (s *InspectStore) Create(userID, name, description, priority string, topics session := &InspectSession{ UserID: userID, TokenHash: tokenHash, - Token: rawToken, Name: name, Description: description, Priority: priority, @@ -72,7 +70,7 @@ func (s *InspectStore) Create(userID, name, description, priority string, topics s.cleanupLocked() s.sessions[userID] = session - return session, nil + return rawToken, session, nil } // GetByUserID returns the inspect session for the given user, or nil if not found or expired. diff --git a/internal/webhook/service.go b/internal/webhook/service.go index 7158fec..367d7a6 100644 --- a/internal/webhook/service.go +++ b/internal/webhook/service.go @@ -240,14 +240,14 @@ func (s *Service) CreateInspectSession(ctx context.Context, userID, name, descri return nil, ErrInvalidTopicSelection } - session, err := s.inspectStore.Create(userID, name, description, priority, topics) + rawToken, session, err := s.inspectStore.Create(userID, name, description, priority, topics) if err != nil { return nil, err } return &InspectSessionResponse{ - Token: session.Token, - URL: hookURL + "/" + session.Token, + Token: rawToken, + URL: hookURL + "/" + rawToken, Status: session.Status, ExpiresAt: session.ExpiresAt, }, nil diff --git a/internal/webhook/service_test.go b/internal/webhook/service_test.go index b2184b1..ad6dff0 100644 --- a/internal/webhook/service_test.go +++ b/internal/webhook/service_test.go @@ -11,6 +11,7 @@ import ( "lucor.dev/beebuzz/internal/auth" "lucor.dev/beebuzz/internal/push" + "lucor.dev/beebuzz/internal/secure" "lucor.dev/beebuzz/internal/testutil" "lucor.dev/beebuzz/internal/topic" ) @@ -207,6 +208,72 @@ func TestExtractPayload_UnsupportedType(t *testing.T) { } } +func TestInspectStoreCreateReturnsRawTokenWithoutPersistingIt(t *testing.T) { + store := NewInspectStore() + + rawToken, session, err := store.Create("user-1", "inspect", "desc", push.PriorityNormal, []string{"topic-1"}) + if err != nil { + t.Fatalf("Create() error = %v, want nil", err) + } + if rawToken == "" { + t.Fatal("Create() rawToken = empty, want token") + } + if session == nil { + t.Fatal("Create() session = nil, want session") + } + if session.TokenHash != secure.Hash(rawToken) { + t.Fatalf("Create() tokenHash = %q, want hash of raw token", session.TokenHash) + } + + stored := store.GetByUserID("user-1") + if stored == nil { + t.Fatal("GetByUserID() = nil, want session") + } + if stored.TokenHash != secure.Hash(rawToken) { + t.Fatalf("GetByUserID() tokenHash = %q, want hash of raw token", stored.TokenHash) + } +} + +func TestCreateInspectSessionReturnsRawTokenAndStoresOnlyHash(t *testing.T) { + db := testutil.NewDB(t) + ctx := context.Background() + + authRepo := auth.NewRepository(db) + topicRepo := topic.NewRepository(db) + repo := NewRepository(db) + topicSvc := topic.NewService(topicRepo, slog.New(slog.NewTextHandler(io.Discard, nil))) + inspectStore := NewInspectStore() + svc := NewService(repo, inspectStore, noopDispatcher{}, topicSvc, slog.New(slog.NewTextHandler(io.Discard, nil))) + + user, _, err := authRepo.GetOrCreateUser(ctx, "inspect-create@example.com") + if err != nil { + t.Fatalf("GetOrCreateUser: %v", err) + } + tp, err := topicRepo.Create(ctx, user.ID, "alerts", "") + if err != nil { + t.Fatalf("topic.Create: %v", err) + } + + response, err := svc.CreateInspectSession(ctx, user.ID, "inspect", "desc", push.PriorityNormal, []string{tp.ID}, "https://hook.example.com") + if err != nil { + t.Fatalf("CreateInspectSession() error = %v, want nil", err) + } + if response.Token == "" { + t.Fatal("CreateInspectSession() token = empty, want token") + } + if response.URL != "https://hook.example.com/"+response.Token { + t.Fatalf("CreateInspectSession() url = %q, want token-based url", response.URL) + } + + session := inspectStore.GetByUserID(user.ID) + if session == nil { + t.Fatal("GetByUserID() = nil, want session") + } + if session.TokenHash != secure.Hash(response.Token) { + t.Fatalf("stored tokenHash = %q, want hash of response token", session.TokenHash) + } +} + // noopDispatcher is a test adapter that accepts all dispatches without sending. type noopDispatcher struct{}