Skip to content
/ beebuzz Public

Commit 267ba98

Browse files
authored
fix: do not keep raw inspect tokens in memory (#13)
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.
1 parent c803233 commit 267ba98

3 files changed

Lines changed: 73 additions & 8 deletions

File tree

internal/webhook/inspect.go

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ const (
2222
type InspectSession struct {
2323
UserID string
2424
TokenHash string
25-
Token string
2625
Name string
2726
Description string
2827
Priority string
@@ -46,18 +45,17 @@ func NewInspectStore() *InspectStore {
4645
}
4746

4847
// Create generates a new inspect session for the given user, replacing any previous one.
49-
func (s *InspectStore) Create(userID, name, description, priority string, topics []string) (*InspectSession, error) {
48+
func (s *InspectStore) Create(userID, name, description, priority string, topics []string) (string, *InspectSession, error) {
5049
rawToken, err := secure.NewInspectToken()
5150
if err != nil {
52-
return nil, err
51+
return "", nil, err
5352
}
5453

5554
tokenHash := secure.Hash(rawToken)
5655

5756
session := &InspectSession{
5857
UserID: userID,
5958
TokenHash: tokenHash,
60-
Token: rawToken,
6159
Name: name,
6260
Description: description,
6361
Priority: priority,
@@ -72,7 +70,7 @@ func (s *InspectStore) Create(userID, name, description, priority string, topics
7270
s.cleanupLocked()
7371
s.sessions[userID] = session
7472

75-
return session, nil
73+
return rawToken, session, nil
7674
}
7775

7876
// GetByUserID returns the inspect session for the given user, or nil if not found or expired.

internal/webhook/service.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,14 +240,14 @@ func (s *Service) CreateInspectSession(ctx context.Context, userID, name, descri
240240
return nil, ErrInvalidTopicSelection
241241
}
242242

243-
session, err := s.inspectStore.Create(userID, name, description, priority, topics)
243+
rawToken, session, err := s.inspectStore.Create(userID, name, description, priority, topics)
244244
if err != nil {
245245
return nil, err
246246
}
247247

248248
return &InspectSessionResponse{
249-
Token: session.Token,
250-
URL: hookURL + "/" + session.Token,
249+
Token: rawToken,
250+
URL: hookURL + "/" + rawToken,
251251
Status: session.Status,
252252
ExpiresAt: session.ExpiresAt,
253253
}, nil

internal/webhook/service_test.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"lucor.dev/beebuzz/internal/auth"
1313
"lucor.dev/beebuzz/internal/push"
14+
"lucor.dev/beebuzz/internal/secure"
1415
"lucor.dev/beebuzz/internal/testutil"
1516
"lucor.dev/beebuzz/internal/topic"
1617
)
@@ -207,6 +208,72 @@ func TestExtractPayload_UnsupportedType(t *testing.T) {
207208
}
208209
}
209210

211+
func TestInspectStoreCreateReturnsRawTokenWithoutPersistingIt(t *testing.T) {
212+
store := NewInspectStore()
213+
214+
rawToken, session, err := store.Create("user-1", "inspect", "desc", push.PriorityNormal, []string{"topic-1"})
215+
if err != nil {
216+
t.Fatalf("Create() error = %v, want nil", err)
217+
}
218+
if rawToken == "" {
219+
t.Fatal("Create() rawToken = empty, want token")
220+
}
221+
if session == nil {
222+
t.Fatal("Create() session = nil, want session")
223+
}
224+
if session.TokenHash != secure.Hash(rawToken) {
225+
t.Fatalf("Create() tokenHash = %q, want hash of raw token", session.TokenHash)
226+
}
227+
228+
stored := store.GetByUserID("user-1")
229+
if stored == nil {
230+
t.Fatal("GetByUserID() = nil, want session")
231+
}
232+
if stored.TokenHash != secure.Hash(rawToken) {
233+
t.Fatalf("GetByUserID() tokenHash = %q, want hash of raw token", stored.TokenHash)
234+
}
235+
}
236+
237+
func TestCreateInspectSessionReturnsRawTokenAndStoresOnlyHash(t *testing.T) {
238+
db := testutil.NewDB(t)
239+
ctx := context.Background()
240+
241+
authRepo := auth.NewRepository(db)
242+
topicRepo := topic.NewRepository(db)
243+
repo := NewRepository(db)
244+
topicSvc := topic.NewService(topicRepo, slog.New(slog.NewTextHandler(io.Discard, nil)))
245+
inspectStore := NewInspectStore()
246+
svc := NewService(repo, inspectStore, noopDispatcher{}, topicSvc, slog.New(slog.NewTextHandler(io.Discard, nil)))
247+
248+
user, _, err := authRepo.GetOrCreateUser(ctx, "inspect-create@example.com")
249+
if err != nil {
250+
t.Fatalf("GetOrCreateUser: %v", err)
251+
}
252+
tp, err := topicRepo.Create(ctx, user.ID, "alerts", "")
253+
if err != nil {
254+
t.Fatalf("topic.Create: %v", err)
255+
}
256+
257+
response, err := svc.CreateInspectSession(ctx, user.ID, "inspect", "desc", push.PriorityNormal, []string{tp.ID}, "https://hook.example.com")
258+
if err != nil {
259+
t.Fatalf("CreateInspectSession() error = %v, want nil", err)
260+
}
261+
if response.Token == "" {
262+
t.Fatal("CreateInspectSession() token = empty, want token")
263+
}
264+
if response.URL != "https://hook.example.com/"+response.Token {
265+
t.Fatalf("CreateInspectSession() url = %q, want token-based url", response.URL)
266+
}
267+
268+
session := inspectStore.GetByUserID(user.ID)
269+
if session == nil {
270+
t.Fatal("GetByUserID() = nil, want session")
271+
}
272+
if session.TokenHash != secure.Hash(response.Token) {
273+
t.Fatalf("stored tokenHash = %q, want hash of response token", session.TokenHash)
274+
}
275+
}
276+
210277
// noopDispatcher is a test adapter that accepts all dispatches without sending.
211278
type noopDispatcher struct{}
212279

0 commit comments

Comments
 (0)