diff --git a/.env.enterprise b/.env.enterprise index 81fa188ef45..4f1e87340e7 100644 --- a/.env.enterprise +++ b/.env.enterprise @@ -55,3 +55,7 @@ SHELLHUB_OBJECT_STORAGE_SECRET_KEY=password # NOTICE: Leave empty for license-less development; the API then starts # without a license and gated features stay disabled. SHELLHUB_LICENSE_FILE= + +# How long sessions and their events are kept after the session started (days). +# 0 keeps them indefinitely. +SHELLHUB_SESSION_RETENTION_DAYS=180 diff --git a/docker-compose.enterprise.yml b/docker-compose.enterprise.yml index c3188300f33..e790cbc12c2 100644 --- a/docker-compose.enterprise.yml +++ b/docker-compose.enterprise.yml @@ -27,6 +27,7 @@ services: # GeoIP (MaxMind) database source, read by the enterprise binary - MAXMIND_MIRROR=${SHELLHUB_MAXMIND_MIRROR:-} - MAXMIND_LICENSE=${SHELLHUB_MAXMIND_LICENSE:-} + - SHELLHUB_SESSION_RETENTION_DAYS=${SHELLHUB_SESSION_RETENTION_DAYS} secrets: - api_private_key - api_public_key diff --git a/server/api/services/service.go b/server/api/services/service.go index 5d62bac073c..386d5568f07 100644 --- a/server/api/services/service.go +++ b/server/api/services/service.go @@ -25,6 +25,7 @@ type service struct { billing BillingProvider licenseEvaluator LicenseEvaluator firewallEvaluator FirewallEvaluator + recordingPruner SessionRecordingPruner } type Service interface { @@ -94,6 +95,12 @@ func WithFirewallEvaluator(fe FirewallEvaluator) Option { } } +func WithSessionRecordingPruner(rp SessionRecordingPruner) Option { + return func(service *APIService) { + service.recordingPruner = rp + } +} + func NewService(store store.Store, privKey *rsa.PrivateKey, pubKey *rsa.PublicKey, cache cache.Cache, options ...Option) *APIService { if privKey == nil || pubKey == nil { var err error @@ -114,6 +121,7 @@ func NewService(store store.Store, privKey *rsa.PrivateKey, pubKey *rsa.PublicKe billing: nil, // injected via WithBilling option licenseEvaluator: nil, // injected via WithLicenseEvaluator option firewallEvaluator: nil, // injected via WithFirewallEvaluator option + recordingPruner: nil, // injected via WithSessionRecordingPruner option }, } diff --git a/server/api/services/session-recording.go b/server/api/services/session-recording.go new file mode 100644 index 00000000000..678e38218a4 --- /dev/null +++ b/server/api/services/session-recording.go @@ -0,0 +1,95 @@ +package services + +import ( + "context" + + "github.com/shellhub-io/shellhub/pkg/cache" + "github.com/shellhub-io/shellhub/server/api/store" +) + +// SessionRecordingPrunerFactoryFunc constructs a SessionRecordingPruner from the core store and +// cache. Enterprise packages register a factory via RegisterSessionRecordingPruner in their +// init() functions; it runs during server setup. +type SessionRecordingPrunerFactoryFunc func(ctx context.Context, store store.Store, cache cache.Cache) (SessionRecordingPruner, error) + +var sessionRecordingPrunerFactory SessionRecordingPrunerFactoryFunc + +// RegisterSessionRecordingPruner registers the factory that creates the recording pruner. +// It must be called before the server's Setup() runs. +func RegisterSessionRecordingPruner(f SessionRecordingPrunerFactoryFunc) { + sessionRecordingPrunerFactory = f +} + +// SessionRecordingPrunerFactory returns the registered factory, or nil in Community Edition +// builds. +func SessionRecordingPrunerFactory() SessionRecordingPrunerFactoryFunc { + return sessionRecordingPrunerFactory +} + +// SessionRecordingPruner discards the stored recordings of sessions that retention is about to +// delete. +// +// A recording is an object, not a row, and nothing in the schema points at it: it is found by +// composing a key from the session's UID. Deleting the session row therefore does not delete the +// recording — it destroys the only thing that could still name it. Everything else in this seam +// follows from that asymmetry. +type SessionRecordingPruner interface { + // DeleteRecordings removes the recordings of the given sessions, whatever seats they had, + // and returns the subset it managed to purge. + // + // Returning a subset rather than failing the batch is what keeps one unreachable object from + // halting retention: the caller deletes the rows it names and leaves the rest, so a session + // whose recording cannot be removed holds up nothing but itself. The error is reserved for a + // failure that makes the whole batch moot, such as a cancelled context. + DeleteRecordings(ctx context.Context, uids []string) ([]string, error) +} + +// pruneRecordings discards the recordings of the recorded sessions in the batch and returns the +// sessions whose rows may now be deleted. +// +// Without a pruner — Community Edition, or an enterprise instance with no object storage — no +// session owns anything outside the database, so the whole batch is deletable as it stands. +func (s *service) pruneRecordings(ctx context.Context, sessions []store.ExpiredSession) ([]string, error) { + uids := make([]string, 0, len(sessions)) + recorded := make([]string, 0, len(sessions)) + + for _, session := range sessions { + uids = append(uids, session.UID) + + if session.Recorded { + recorded = append(recorded, session.UID) + } + } + + if s.recordingPruner == nil || len(recorded) == 0 { + return uids, nil + } + + purged, err := s.recordingPruner.DeleteRecordings(ctx, recorded) + if err != nil { + return nil, err + } + + // Everything that was never recorded, plus the recordings actually purged. A session left + // out here keeps its row, so its object stays reachable for the next run to retry. + deletable := make([]string, 0, len(uids)) + purgedSet := make(map[string]struct{}, len(purged)) + + for _, uid := range purged { + purgedSet[uid] = struct{}{} + } + + for _, session := range sessions { + if !session.Recorded { + deletable = append(deletable, session.UID) + + continue + } + + if _, ok := purgedSet[session.UID]; ok { + deletable = append(deletable, session.UID) + } + } + + return deletable, nil +} diff --git a/server/api/services/task.go b/server/api/services/task.go index 31287d30c48..c4a73fedb18 100644 --- a/server/api/services/task.go +++ b/server/api/services/task.go @@ -21,6 +21,20 @@ const ( CronEphemeralCleanup = worker.CronSpec("*/5 * * * *") CronEnrollmentCallbackCleanup = worker.CronSpec("0 4 * * *") CronSSHApprovalCleanup = worker.CronSpec("*/10 * * * *") + CronSessionCleanup = worker.CronSpec("0 1 * * *") +) + +const ( + // A session cascades into its events, so one batch is already thousands of rows. + sessionCleanupBatchSize = 1000 + + // Together with the batch size this caps a run at 100k sessions. An instance adopting + // retention for the first time can have years to shed, and draining all of it in one night + // is the write storm the batching exists to avoid. + sessionCleanupMaxBatches = 100 + + // Leaves room between batches for live traffic and for autovacuum to follow behind. + sessionCleanupBatchPause = 200 * time.Millisecond ) func (s *service) DeviceCleanup() worker.CronHandler { @@ -57,6 +71,93 @@ func (s *service) SSHApprovalCleanup() worker.CronHandler { } } +// SessionCleanup enforces the instance's session retention window: sessions that started longer +// ago than retention are deleted, taking their events and recordings with them. +// +// A retention that is not positive means "keep forever" and prunes nothing. The guard matters +// more than it looks: read as a window, a zero would put the cutoff at now and delete every +// session on the instance. +func (s *service) SessionCleanup(retention time.Duration) worker.CronHandler { + return func(ctx context.Context) error { + return s.sessionCleanup(ctx, retention, sessionCleanupBatchPause) + } +} + +// sessionCleanup takes the pause as an argument so a test can drive the batching loop without +// waiting it out. +func (s *service) sessionCleanup(ctx context.Context, retention, pause time.Duration) error { + if retention <= 0 { + return nil + } + + cutoff := clock.Now().Add(-retention) + + total := int64(0) + batches := 0 + + for batches < sessionCleanupMaxBatches { + sessions, err := s.store.SessionListExpired(ctx, cutoff, sessionCleanupBatchSize) + if err != nil { + log.WithError(err).WithField("deleted", total).Error("failed to list expired sessions") + + return err + } + + if len(sessions) == 0 { + break + } + + // Recordings first, rows second, because the row is the only thing that can still name + // the object. Sessions whose recording could not be purged are left out and keep their + // rows, so the next run finds them again. + deletable, err := s.pruneRecordings(ctx, sessions) + if err != nil { + log.WithError(err).WithField("deleted", total).Error("failed to prune recordings of expired sessions") + + return err + } + + // The batch is not empty but nothing in it can be deleted, so every session in it is + // blocked on its recording. Retrying inside this run would list the same rows again and + // spin until the cap; leave it for the next one, by which time the storage may answer. + if len(deletable) == 0 { + log.WithFields(log.Fields{"deleted": total, "blocked": len(sessions)}). + Warn("no expired session in the batch could be deleted; ending the run") + + break + } + + deleted, err := s.store.SessionDeleteMany(ctx, deletable) + if err != nil { + log.WithError(err).WithField("deleted", total).Error("failed to prune expired sessions") + + return err + } + + total += deleted + batches++ + + // A batch that came back short means the store ran out of sessions older than the + // cutoff, so there is nothing left for this run to do. + if len(sessions) < sessionCleanupBatchSize { + break + } + + select { + case <-ctx.Done(): + return ctx.Err() + case <-time.After(pause): + } + } + + if total > 0 { + log.WithFields(log.Fields{"deleted": total, "cutoff": cutoff, "capped": batches == sessionCleanupMaxBatches}). + Info("pruned sessions past the retention window") + } + + return nil +} + // EnrollmentCallbackCleanup prunes single-use callback redemption records once older than the maximum // token TTL, past which the token has expired and can no longer gate a replay. The table only gains a // row per resolved deferred webhook, so this keeps its growth bounded. diff --git a/server/api/services/task_test.go b/server/api/services/task_test.go index 3820f2c91c1..6c7eb3acbf8 100644 --- a/server/api/services/task_test.go +++ b/server/api/services/task_test.go @@ -3,6 +3,7 @@ package services import ( "context" "errors" + "fmt" "testing" "time" @@ -14,6 +15,7 @@ import ( "github.com/shellhub-io/shellhub/pkg/models" "github.com/shellhub-io/shellhub/server/api/store" storemock "github.com/shellhub-io/shellhub/server/api/store/mocks" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) @@ -433,3 +435,292 @@ func TestService_NamespaceDeviceCountSync(t *testing.T) { }) } } + +func TestService_SessionCleanup(t *testing.T) { + now := time.Date(2026, 8, 11, 12, 0, 0, 0, time.UTC) + retention := 180 * 24 * time.Hour + cutoff := now.Add(-retention) + + storeMock := storemock.NewMockStore(t) + clockMock := clockmock.NewMockClock(t) + + prevClock := clock.DefaultBackend + t.Cleanup(func() { clock.DefaultBackend = prevClock }) + clock.DefaultBackend = clockMock + clockMock.On("Now").Return(now).Maybe() + + // expired builds a batch of n sessions, none of them recorded. + expired := func(n int) []store.ExpiredSession { + batch := make([]store.ExpiredSession, n) + for i := range batch { + batch[i] = store.ExpiredSession{UID: fmt.Sprintf("session-%d", i)} + } + + return batch + } + + uids := func(sessions []store.ExpiredSession) []string { + out := make([]string, len(sessions)) + for i, session := range sessions { + out[i] = session.UID + } + + return out + } + + full := expired(sessionCleanupBatchSize) + + cases := []struct { + description string + retention time.Duration + requiredMocks func(context.Context, *mockSessionRecordingPruner) + withPruner bool + expected error + }{ + { + description: "does not prune when retention is not positive", + retention: 0, + requiredMocks: func(_ context.Context, _ *mockSessionRecordingPruner) { + // No store call is set up: a non-positive window must never be read as a cutoff + // of now, which would delete every session there is. + }, + expected: nil, + }, + { + description: "fails when listing fails", + retention: retention, + requiredMocks: func(ctx context.Context, _ *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(nil, errors.New("database error")). + Once() + }, + expected: errors.New("database error"), + }, + { + description: "stops when nothing is expired", + retention: retention, + requiredMocks: func(ctx context.Context, _ *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return([]store.ExpiredSession{}, nil). + Once() + }, + expected: nil, + }, + { + description: "fails when deleting fails", + retention: retention, + requiredMocks: func(ctx context.Context, _ *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(expired(3), nil). + Once() + storeMock. + On("SessionDeleteMany", ctx, uids(expired(3))). + Return(int64(0), errors.New("database error")). + Once() + }, + expected: errors.New("database error"), + }, + { + description: "stops after a partial batch", + retention: retention, + requiredMocks: func(ctx context.Context, _ *mockSessionRecordingPruner) { + batch := expired(sessionCleanupBatchSize - 1) + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(batch, nil). + Once() + storeMock. + On("SessionDeleteMany", ctx, uids(batch)). + Return(int64(len(batch)), nil). + Once() + }, + expected: nil, + }, + { + description: "keeps batching while each batch comes back full", + retention: retention, + requiredMocks: func(ctx context.Context, _ *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(full, nil). + Once() + storeMock. + On("SessionDeleteMany", ctx, uids(full)). + Return(int64(sessionCleanupBatchSize), nil). + Once() + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return([]store.ExpiredSession{}, nil). + Once() + }, + expected: nil, + }, + { + description: "stops at the per-run cap and leaves the rest for the next run", + retention: retention, + requiredMocks: func(ctx context.Context, _ *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(full, nil). + Times(sessionCleanupMaxBatches) + storeMock. + On("SessionDeleteMany", ctx, uids(full)). + Return(int64(sessionCleanupBatchSize), nil). + Times(sessionCleanupMaxBatches) + }, + expected: nil, + }, + { + description: "does not reach for the bucket when nothing in the batch was recorded", + retention: retention, + withPruner: true, + requiredMocks: func(ctx context.Context, _ *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(expired(3), nil). + Once() + storeMock. + On("SessionDeleteMany", ctx, uids(expired(3))). + Return(int64(3), nil). + Once() + // No DeleteRecordings: an instance that records nothing must not pay a storage + // lookup per session it deletes. + }, + expected: nil, + }, + { + description: "purges a recorded session's recording before deleting its row", + retention: retention, + withPruner: true, + requiredMocks: func(ctx context.Context, pruner *mockSessionRecordingPruner) { + batch := []store.ExpiredSession{ + {UID: "plain"}, + {UID: "recorded", Recorded: true}, + } + + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(batch, nil). + Once() + + purged := false + pruner. + On("DeleteRecordings", ctx, []string{"recorded"}). + Run(func(_ mock.Arguments) { purged = true }). + Return([]string{"recorded"}, nil). + Once() + storeMock. + On("SessionDeleteMany", ctx, []string{"plain", "recorded"}). + Run(func(_ mock.Arguments) { + assert.True(t, purged, "the row must not be deleted before its recording") + }). + Return(int64(2), nil). + Once() + }, + expected: nil, + }, + { + description: "keeps the row of a session whose recording could not be purged", + retention: retention, + withPruner: true, + requiredMocks: func(ctx context.Context, pruner *mockSessionRecordingPruner) { + batch := []store.ExpiredSession{ + {UID: "ok", Recorded: true}, + {UID: "stuck", Recorded: true}, + } + + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return(batch, nil). + Once() + pruner. + On("DeleteRecordings", ctx, []string{"ok", "stuck"}). + Return([]string{"ok"}, nil). + Once() + // "stuck" keeps its row, which is what leaves its object reachable next run. + storeMock. + On("SessionDeleteMany", ctx, []string{"ok"}). + Return(int64(1), nil). + Once() + }, + expected: nil, + }, + { + description: "ends the run when no session in the batch can be deleted", + retention: retention, + withPruner: true, + requiredMocks: func(ctx context.Context, pruner *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return([]store.ExpiredSession{{UID: "stuck", Recorded: true}}, nil). + Once() + pruner. + On("DeleteRecordings", ctx, []string{"stuck"}). + Return([]string{}, nil). + Once() + // No SessionDeleteMany, and only one listing: retrying inside the run would + // re-list the same blocked rows and spin until the cap. + }, + expected: nil, + }, + { + description: "fails when the pruner reports the batch is moot", + retention: retention, + withPruner: true, + requiredMocks: func(ctx context.Context, pruner *mockSessionRecordingPruner) { + storeMock. + On("SessionListExpired", ctx, cutoff, sessionCleanupBatchSize). + Return([]store.ExpiredSession{{UID: "a", Recorded: true}}, nil). + Once() + pruner. + On("DeleteRecordings", ctx, []string{"a"}). + Return(nil, context.Canceled). + Once() + }, + expected: context.Canceled, + }, + } + + for _, tc := range cases { + t.Run(tc.description, func(tt *testing.T) { + ctx := context.Background() + + pruner := new(mockSessionRecordingPruner) + tt.Cleanup(func() { pruner.AssertExpectations(tt) }) + tc.requiredMocks(ctx, pruner) + + opts := []Option{} + if tc.withPruner { + opts = append(opts, WithSessionRecordingPruner(pruner)) + } + + s := NewService(storeMock, privateKey, publicKey, cache.NewNullCache(), opts...) + require.Equal(tt, tc.expected, s.sessionCleanup(ctx, tc.retention, 0)) + }) + } + + // The cases above drive the loop directly so they do not pay the batch pause. This one goes + // through the cron handler itself, which is what the composition root registers. + t.Run("the cron handler prunes nothing when retention is not positive", func(tt *testing.T) { + s := NewService(storeMock, privateKey, publicKey, cache.NewNullCache()) + require.NoError(tt, s.SessionCleanup(0)(context.Background())) + }) +} + +// mockSessionRecordingPruner stands in for SessionRecordingPruner. It lives here rather than in +// the generated mocks package for the same reason as mockLicenseEvaluator: services/mocks +// imports services, so an internal test importing it would form a cycle. +type mockSessionRecordingPruner struct { + mock.Mock +} + +func (m *mockSessionRecordingPruner) DeleteRecordings(ctx context.Context, uids []string) ([]string, error) { + args := m.Called(ctx, uids) + + purged, _ := args.Get(0).([]string) + + return purged, args.Error(1) +} diff --git a/server/api/store/mocks/mock_store.go b/server/api/store/mocks/mock_store.go index 36e543d3cd0..d5a035f8a6d 100644 --- a/server/api/store/mocks/mock_store.go +++ b/server/api/store/mocks/mock_store.go @@ -6016,6 +6016,72 @@ func (_c *MockStore_SessionCreate_Call) RunAndReturn(run func(ctx context.Contex return _c } +// SessionDeleteMany provides a mock function for the type MockStore +func (_mock *MockStore) SessionDeleteMany(ctx context.Context, uids []string) (int64, error) { + ret := _mock.Called(ctx, uids) + + if len(ret) == 0 { + panic("no return value specified for SessionDeleteMany") + } + + var r0 int64 + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, []string) (int64, error)); ok { + return returnFunc(ctx, uids) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, []string) int64); ok { + r0 = returnFunc(ctx, uids) + } else { + r0 = ret.Get(0).(int64) + } + if returnFunc, ok := ret.Get(1).(func(context.Context, []string) error); ok { + r1 = returnFunc(ctx, uids) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// MockStore_SessionDeleteMany_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SessionDeleteMany' +type MockStore_SessionDeleteMany_Call struct { + *mock.Call +} + +// SessionDeleteMany is a helper method to define mock.On call +// - ctx context.Context +// - uids []string +func (_e *MockStore_Expecter) SessionDeleteMany(ctx any, uids any) *MockStore_SessionDeleteMany_Call { + return &MockStore_SessionDeleteMany_Call{Call: _e.mock.On("SessionDeleteMany", ctx, uids)} +} + +func (_c *MockStore_SessionDeleteMany_Call) Run(run func(ctx context.Context, uids []string)) *MockStore_SessionDeleteMany_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 []string + if args[1] != nil { + arg1 = args[1].([]string) + } + run( + arg0, + arg1, + ) + }) + return _c +} + +func (_c *MockStore_SessionDeleteMany_Call) Return(n int64, err error) *MockStore_SessionDeleteMany_Call { + _c.Call.Return(n, err) + return _c +} + +func (_c *MockStore_SessionDeleteMany_Call) RunAndReturn(run func(ctx context.Context, uids []string) (int64, error)) *MockStore_SessionDeleteMany_Call { + _c.Call.Return(run) + return _c +} + // SessionEventsCreate provides a mock function for the type MockStore func (_mock *MockStore) SessionEventsCreate(ctx context.Context, event *models.SessionEvent) error { ret := _mock.Called(ctx, event) @@ -6389,6 +6455,80 @@ func (_c *MockStore_SessionList_Call) RunAndReturn(run func(ctx context.Context, return _c } +// SessionListExpired provides a mock function for the type MockStore +func (_mock *MockStore) SessionListExpired(ctx context.Context, before time.Time, limit int) ([]store.ExpiredSession, error) { + ret := _mock.Called(ctx, before, limit) + + if len(ret) == 0 { + panic("no return value specified for SessionListExpired") + } + + var r0 []store.ExpiredSession + var r1 error + if returnFunc, ok := ret.Get(0).(func(context.Context, time.Time, int) ([]store.ExpiredSession, error)); ok { + return returnFunc(ctx, before, limit) + } + if returnFunc, ok := ret.Get(0).(func(context.Context, time.Time, int) []store.ExpiredSession); ok { + r0 = returnFunc(ctx, before, limit) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).([]store.ExpiredSession) + } + } + if returnFunc, ok := ret.Get(1).(func(context.Context, time.Time, int) error); ok { + r1 = returnFunc(ctx, before, limit) + } else { + r1 = ret.Error(1) + } + return r0, r1 +} + +// MockStore_SessionListExpired_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'SessionListExpired' +type MockStore_SessionListExpired_Call struct { + *mock.Call +} + +// SessionListExpired is a helper method to define mock.On call +// - ctx context.Context +// - before time.Time +// - limit int +func (_e *MockStore_Expecter) SessionListExpired(ctx any, before any, limit any) *MockStore_SessionListExpired_Call { + return &MockStore_SessionListExpired_Call{Call: _e.mock.On("SessionListExpired", ctx, before, limit)} +} + +func (_c *MockStore_SessionListExpired_Call) Run(run func(ctx context.Context, before time.Time, limit int)) *MockStore_SessionListExpired_Call { + _c.Call.Run(func(args mock.Arguments) { + var arg0 context.Context + if args[0] != nil { + arg0 = args[0].(context.Context) + } + var arg1 time.Time + if args[1] != nil { + arg1 = args[1].(time.Time) + } + var arg2 int + if args[2] != nil { + arg2 = args[2].(int) + } + run( + arg0, + arg1, + arg2, + ) + }) + return _c +} + +func (_c *MockStore_SessionListExpired_Call) Return(expiredSessions []store.ExpiredSession, err error) *MockStore_SessionListExpired_Call { + _c.Call.Return(expiredSessions, err) + return _c +} + +func (_c *MockStore_SessionListExpired_Call) RunAndReturn(run func(ctx context.Context, before time.Time, limit int) ([]store.ExpiredSession, error)) *MockStore_SessionListExpired_Call { + _c.Call.Return(run) + return _c +} + // SessionResolve provides a mock function for the type MockStore func (_mock *MockStore) SessionResolve(ctx context.Context, sc scope.Scope, resolver store.SessionResolver, value string, opts ...store.QueryOption) (*models.Session, error) { var tmpRet mock.Arguments diff --git a/server/api/store/pg/migration_autovacuum_test.go b/server/api/store/pg/migration_autovacuum_test.go new file mode 100644 index 00000000000..2d6df024e22 --- /dev/null +++ b/server/api/store/pg/migration_autovacuum_test.go @@ -0,0 +1,56 @@ +package pg_test + +import ( + "context" + "testing" + + "github.com/shellhub-io/shellhub/server/api/store/storetest/pgprovider" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestSessionAutovacuumThresholds covers migration 019. The knobs are per-table storage +// parameters, so the only honest assertion is what pg_class.reloptions holds on a migrated +// database — reading the migration file back would assert nothing but its own text. +func TestSessionAutovacuumThresholds(t *testing.T) { + ctx := context.Background() + + provider, err := pgprovider.NewProvider(ctx) + require.NoError(t, err) + t.Cleanup(func() { provider.Close(t) }) + + for _, table := range []string{"sessions", "session_events"} { + t.Run(table, func(t *testing.T) { + // unnest rather than reading the array whole: it yields one row per option, so + // the assertion matches a setting exactly instead of finding it as a substring + // of another. A table with no storage parameters yields no rows. + var options []string + require.NoError(t, + provider.DB().NewSelect(). + ColumnExpr("unnest(reloptions)"). + TableExpr("pg_class"). + Where("relname = ?", table). + Where("relkind = 'r'"). + Scan(ctx, &options), + ) + + assert.Contains(t, options, "autovacuum_vacuum_scale_factor=0.05") + assert.Contains(t, options, "autovacuum_analyze_scale_factor=0.02") + + // The new analyze factor only decides when the daemon next looks; it does nothing + // about statistics that are already stale, which on the instance this came from + // were 45 days old. The migration refreshes them once itself. + var analyzed int + require.NoError(t, + provider.DB().NewSelect(). + ColumnExpr("count(*)"). + TableExpr("pg_stat_user_tables"). + Where("relname = ?", table). + Where("last_analyze IS NOT NULL"). + Scan(ctx, &analyzed), + ) + + assert.Equal(t, 1, analyzed, "migration must leave %s with fresh statistics", table) + }) + } +} diff --git a/server/api/store/pg/migrations/019_set_session_autovacuum_thresholds.tx.down.sql b/server/api/store/pg/migrations/019_set_session_autovacuum_thresholds.tx.down.sql new file mode 100644 index 00000000000..5f295383735 --- /dev/null +++ b/server/api/store/pg/migrations/019_set_session_autovacuum_thresholds.tx.down.sql @@ -0,0 +1,14 @@ +-- RESET drops the per-table override, returning both tables to whatever the server-level +-- autovacuum_*_scale_factor is set to. It does not restore a previous per-table value, because +-- there was none to restore. +ALTER TABLE sessions RESET ( + autovacuum_vacuum_scale_factor, + autovacuum_analyze_scale_factor +); + +--bun:split + +ALTER TABLE session_events RESET ( + autovacuum_vacuum_scale_factor, + autovacuum_analyze_scale_factor +); diff --git a/server/api/store/pg/migrations/019_set_session_autovacuum_thresholds.tx.up.sql b/server/api/store/pg/migrations/019_set_session_autovacuum_thresholds.tx.up.sql new file mode 100644 index 00000000000..5df8d2c93c6 --- /dev/null +++ b/server/api/store/pg/migrations/019_set_session_autovacuum_thresholds.tx.up.sql @@ -0,0 +1,48 @@ +-- Autovacuum's default scale factors are proportions of the table, so the bigger a table +-- gets the longer it waits: at the default vacuum factor of 0.2, a 6.7M-row session_events +-- does not get vacuumed until ~1.3M rows are dead, and at the default analyze factor of 0.1 +-- its statistics are only refreshed after ~670k changes. Measured on a production instance, +-- that left session_events 45 days stale on a table the planner has to estimate against, and +-- sessions carrying 24k dead tuples ten days after its last vacuum. +-- +-- The proportion is the wrong shape for these two tables specifically: they are the largest +-- in the database and the fastest growing, which is exactly the combination the default +-- punishes. Lowering the factors converts "a fifth of the table" into a bound that stays +-- workable as the table grows — ~335k dead rows before a vacuum and ~134k changes before an +-- analyze at today's size, instead of 1.3M and 670k. +-- +-- These are storage parameters, not a rewrite: the ALTER takes a brief SHARE UPDATE EXCLUSIVE +-- lock, touches only the catalog and cannot block reads or writes. It also does not vacuum or +-- analyze anything by itself — it only changes when the daemon next decides to, which is why +-- the ANALYZE below is needed to clear the backlog the old factor already allowed. +-- +-- Retention (the cron this migration ships alongside) reduces how much dead tuple churn these +-- tables see at all, but does not replace this: a prune is itself a large source of dead +-- tuples, so the thresholds matter more once it is running, not less. +ALTER TABLE sessions SET ( + autovacuum_vacuum_scale_factor = 0.05, + autovacuum_analyze_scale_factor = 0.02 +); + +--bun:split + +ALTER TABLE session_events SET ( + autovacuum_vacuum_scale_factor = 0.05, + autovacuum_analyze_scale_factor = 0.02 +); + +--bun:split + +-- Statistics on the instance this was measured on were 45 days stale, and the lowered factor +-- above does nothing about a backlog that already exists — it only brings the *next* analyze +-- forward. The planner estimates against these tables on every session list, so leaving them +-- stale is the part that actually produces bad plans. +-- +-- Unlike a VACUUM FULL this is safe to run at boot: ANALYZE reads a bounded random +-- sample (default_statistics_target * 300, so ~30k rows) rather than the whole table, takes +-- only SHARE UPDATE EXCLUSIVE, and unlike VACUUM it is allowed inside a transaction block. +ANALYZE sessions; + +--bun:split + +ANALYZE session_events; diff --git a/server/api/store/pg/migrations/migrations_test.go b/server/api/store/pg/migrations/migrations_test.go index 341d7d96c49..e792db87be4 100644 --- a/server/api/store/pg/migrations/migrations_test.go +++ b/server/api/store/pg/migrations/migrations_test.go @@ -26,3 +26,165 @@ func TestNoDuplicateMigrationVersions(t *testing.T) { seen[version] = file } } + +// nonTransactionalStatements are statements PostgreSQL refuses to run inside a transaction +// block. +var nonTransactionalStatements = []string{ + "VACUUM", + "ALTER SYSTEM", + "CREATE INDEX CONCURRENTLY", + "DROP INDEX CONCURRENTLY", + "REINDEX INDEX CONCURRENTLY", + "REINDEX TABLE CONCURRENTLY", + "CREATE DATABASE", + "DROP DATABASE", +} + +// TestNonTransactionalMigrations guards the two conditions such a statement needs, neither of +// which is visible in the SQL itself: bun decides transactionality from the ".tx." filename +// suffix, and the pool runs in pgx simple-protocol mode where a multi-statement Exec is itself +// an implicit transaction block. Getting either wrong fails at boot, not in review. +func TestNonTransactionalMigrations(t *testing.T) { + files, err := fs.Glob(sqlMigrations, "*.sql") + if err != nil { + t.Fatalf("failed to list migrations: %v", err) + } + + for _, file := range files { + raw, err := fs.ReadFile(sqlMigrations, file) + if err != nil { + t.Fatalf("failed to read %q: %v", file, err) + } + + for _, chunk := range strings.Split(string(raw), "--bun:split") { + statement := stripSQLComments(chunk) + + keyword := findNonTransactionalStatement(statement) + if keyword == "" { + continue + } + + if strings.Contains(file, ".tx.") { + t.Errorf("%s: %q cannot run inside a transaction, so the file must not carry the .tx. suffix", file, keyword) + } + + if n := countStatements(statement); n > 1 { + t.Errorf("%s: %q shares its --bun:split chunk with %d other statement(s), which simple-protocol mode batches into an implicit transaction", file, keyword, n-1) + } + } + } +} + +// stripSQLComments removes "--" line comments so the scan below reads statements rather than +// the prose around them. It does not attempt to honour string literals, which no migration +// needs it to. +func stripSQLComments(chunk string) string { + lines := strings.Split(chunk, "\n") + kept := make([]string, 0, len(lines)) + + for _, line := range lines { + if i := strings.Index(line, "--"); i >= 0 { + line = line[:i] + } + + kept = append(kept, line) + } + + return strings.Join(kept, "\n") +} + +// findNonTransactionalStatement returns the offending keyword, or "" when the statement can run +// inside a transaction. +// +// Every keyword above opens a statement, so the match is anchored to the start of one rather +// than searched for anywhere in the chunk. Unanchored, "VACUUM" is also a substring of the +// autovacuum_* storage parameters, which would condemn an ALTER TABLE ... SET that is +// transactional in every respect. +func findNonTransactionalStatement(chunk string) string { + for _, statement := range strings.Split(chunk, ";") { + normalized := strings.Join(strings.Fields(strings.ToUpper(statement)), " ") + + for _, keyword := range nonTransactionalStatements { + if strings.HasPrefix(normalized, keyword) { + return keyword + } + } + } + + return "" +} + +// TestNonTransactionalDetection covers the two things the guard above has to get right: that +// prose merely naming one of these statements does not count, and that a statement sharing its +// chunk with another one does. +func TestNonTransactionalDetection(t *testing.T) { + tests := []struct { + name string + chunk string + keyword string + statements int + }{ + { + name: "vacuum cannot run in a transaction", + chunk: "VACUUM (FULL, ANALYZE) devices;", + keyword: "VACUUM", + statements: 1, + }, + { + name: "prose naming a vacuum is not a statement", + chunk: "-- recover by running VACUUM (FULL, ANALYZE) devices; by hand\nSELECT 1;", + keyword: "", + statements: 1, + }, + { + name: "keyword split across lines is still found", + chunk: "CREATE INDEX CONCURRENTLY\n devices_last_seen ON devices USING btree (last_seen);", + keyword: "CREATE INDEX CONCURRENTLY", + statements: 1, + }, + { + name: "a shared chunk is counted", + chunk: "SET lock_timeout = '60s';\nVACUUM (FULL, ANALYZE) devices;", + keyword: "VACUUM", + statements: 2, + }, + { + name: "ordinary ddl is transactional", + chunk: "DROP INDEX IF EXISTS devices_last_seen;", + keyword: "", + statements: 1, + }, + { + name: "a storage parameter merely spelling a keyword is not that statement", + chunk: "ALTER TABLE sessions SET (autovacuum_vacuum_scale_factor = 0.05);", + keyword: "", + statements: 1, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + statement := stripSQLComments(tc.chunk) + + if keyword := findNonTransactionalStatement(statement); keyword != tc.keyword { + t.Errorf("findNonTransactionalStatement() = %q, want %q", keyword, tc.keyword) + } + + if count := countStatements(statement); count != tc.statements { + t.Errorf("countStatements() = %d, want %d", count, tc.statements) + } + }) + } +} + +func countStatements(statement string) int { + count := 0 + + for _, s := range strings.Split(statement, ";") { + if strings.TrimSpace(s) != "" { + count++ + } + } + + return count +} diff --git a/server/api/store/pg/session.go b/server/api/store/pg/session.go index a08f93d3ca1..2e0b40a0321 100644 --- a/server/api/store/pg/session.go +++ b/server/api/store/pg/session.go @@ -2,6 +2,7 @@ package pg import ( "context" + "time" "github.com/shellhub-io/shellhub/pkg/api/scope" "github.com/shellhub-io/shellhub/pkg/clock" @@ -346,6 +347,60 @@ func (pg *Pg) SessionUpdateDeviceUID(ctx context.Context, oldUID models.UID, new return nil } +func (pg *Pg) SessionListExpired(ctx context.Context, before time.Time, limit int) ([]store.ExpiredSession, error) { + if limit <= 0 { + return []store.ExpiredSession{}, nil + } + + db := pg.GetConnection(ctx) + + // Ordering by started_at walks sessions_started_at_idx and takes the oldest rows first, so + // successive batches drain the backlog from its far end instead of re-reading it. + // + // The anti-join is the safety rail: closed is not reliable for this (a session whose server + // died never gets closed and would otherwise be immortal), whereas a row in active_sessions + // is what a live session actually holds. Deleting one would cascade that row away underneath + // the connection still using it. + uids := make([]string, 0, limit) + recorded := make([]bool, 0, limit) + + if err := db.NewSelect(). + Model((*entity.Session)(nil)). + Column("id", "recorded"). + Where("started_at < ?", before). + Where("NOT EXISTS (SELECT 1 FROM active_sessions WHERE active_sessions.session_id = session.id)"). + Order("started_at ASC"). + Limit(limit). + Scan(ctx, &uids, &recorded); err != nil { + return nil, fromSQLError(err) + } + + sessions := make([]store.ExpiredSession, len(uids)) + for i, uid := range uids { + sessions[i] = store.ExpiredSession{UID: uid, Recorded: recorded[i]} + } + + return sessions, nil +} + +func (pg *Pg) SessionDeleteMany(ctx context.Context, uids []string) (int64, error) { + if len(uids) == 0 { + return 0, nil + } + + db := pg.GetConnection(ctx) + + res, err := db.NewDelete(). + Model((*entity.Session)(nil)). + Where("id IN (?)", bun.List(uids)). + Exec(ctx) + if err != nil { + return 0, fromSQLError(err) + } + + return res.RowsAffected() +} + // SessionSelectQuery applies the standard session SELECT decorations: relations, // computed columns (active, event_types, event_seats), and the active_sessions JOIN. // The caller provides the base query with the desired model (core or cloud entity). diff --git a/server/api/store/pg/store_test.go b/server/api/store/pg/store_test.go index 7fc9dc9927f..ff8352aeb7c 100644 --- a/server/api/store/pg/store_test.go +++ b/server/api/store/pg/store_test.go @@ -71,6 +71,7 @@ func TestPgStore(t *testing.T) { suite.TestSessionEventsCreate(t) suite.TestSessionEventsList(t) suite.TestSessionEventsDelete(t) + suite.TestSessionCleanup(t) }) runSubSuite(t, "TagStore", func(suite *storetest.Suite, t *testing.T) { diff --git a/server/api/store/session.go b/server/api/store/session.go index 3e7b9732a2b..27aa4a25398 100644 --- a/server/api/store/session.go +++ b/server/api/store/session.go @@ -2,6 +2,7 @@ package store import ( "context" + "time" "github.com/shellhub-io/shellhub/pkg/api/scope" "github.com/shellhub-io/shellhub/pkg/models" @@ -13,6 +14,15 @@ const ( SessionUIDResolver SessionResolver = iota + 1 ) +// ExpiredSession is a session that has outlived the retention window, reduced to what deleting +// it needs to know. Recorded travels with the UID because it decides whether the session owns a +// recording at all: without it, an instance that records nothing still pays a storage lookup per +// session it deletes. +type ExpiredSession struct { + UID string + Recorded bool +} + type SessionStore interface { // SessionList retrieves a list of sessions based on the provided filters and pagination settings. // It returns the list of sessions, the total count of matching documents, and an error if any. @@ -48,4 +58,17 @@ type SessionStore interface { // SessionUpdateDeviceUID updates device UID references across sessions. It returns an error if any. SessionUpdateDeviceUID(ctx context.Context, oldUID models.UID, newUID models.UID) error + + // SessionListExpired returns up to limit sessions started before the given time, oldest + // first. A session that is still active is never returned, however old it is, and a limit + // that is not positive returns nothing. + // + // Listing is separate from deleting so a caller can act on what a session owns outside the + // database while the row that names it still exists. + SessionListExpired(ctx context.Context, before time.Time, limit int) ([]ExpiredSession, error) + + // SessionDeleteMany deletes the given sessions, cascading into their events. It returns the + // number deleted, which may be lower than the number asked for if a session went away in + // between. An empty slice is a no-op. + SessionDeleteMany(ctx context.Context, uids []string) (int64, error) } diff --git a/server/api/store/storetest/session_tests.go b/server/api/store/storetest/session_tests.go index 037e12be473..e9678fd8e0a 100644 --- a/server/api/store/storetest/session_tests.go +++ b/server/api/store/storetest/session_tests.go @@ -890,3 +890,193 @@ func (s *Suite) TestSessionEventsDelete(t *testing.T) { assert.Len(t, events2, 1) }) } + +// TestSessionCleanup tests the SessionListExpired/SessionDeleteMany pair across all +// implementations. They are exercised together because retention only ever uses them as a pair: +// list a batch, act on it, delete it. +// +// The cases below age sessions through pinClock rather than by setting StartedAt, because +// SessionCreate stamps started_at from the clock and ignores whatever the caller passed. Pinning +// the clock is the only way to build the age distribution retention is about. +func (s *Suite) TestSessionCleanup(t *testing.T) { + ctx := context.Background() + st := s.provider.Store() + + now := time.Date(2026, 8, 11, 12, 0, 0, 0, time.UTC) + cutoff := now.AddDate(0, 0, -180) + + // listUIDs returns every surviving session's UID. + listUIDs := func(t *testing.T) []string { + t.Helper() + + sessions, _, err := st.SessionList(ctx, scope.NewUnbounded(reasonTestQueryMechanics), + st.Options().Match(&query.Filters{}), + st.Options().Paginate(&query.Paginator{Page: -1, PerPage: -1})) + require.NoError(t, err) + + uids := make([]string, 0, len(sessions)) + for _, session := range sessions { + uids = append(uids, session.UID) + } + + return uids + } + + // prune runs the pair the way retention does. + prune := func(t *testing.T, limit int) int64 { + t.Helper() + + expired, err := st.SessionListExpired(ctx, cutoff, limit) + require.NoError(t, err) + + uids := make([]string, len(expired)) + for i, session := range expired { + uids[i] = session.UID + } + + deleted, err := st.SessionDeleteMany(ctx, uids) + require.NoError(t, err) + + return deleted + } + + t.Run("finds nothing when every session is newer than the cutoff", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, 1)) + uid := s.CreateSession(t, WithSessionActive(false)) + clk.now = now + + assert.Equal(t, int64(0), prune(t, 100)) + assert.Equal(t, []string{string(uid)}, listUIDs(t)) + }) + + t.Run("deletes only the sessions started before the cutoff", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, -10)) + device := s.CreateDevice(t) + s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + + clk.now = cutoff.AddDate(0, 0, 10) + kept := s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + + clk.now = now + + assert.Equal(t, int64(2), prune(t, 100)) + assert.Equal(t, []string{string(kept)}, listUIDs(t)) + }) + + t.Run("lists the oldest first and stops at the limit", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, -30)) + device := s.CreateDevice(t) + oldest := s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + + clk.now = cutoff.AddDate(0, 0, -20) + middle := s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + + clk.now = cutoff.AddDate(0, 0, -10) + newest := s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + + clk.now = now + + expired, err := st.SessionListExpired(ctx, cutoff, 2) + require.NoError(t, err) + assert.Equal(t, []store.ExpiredSession{ + {UID: string(oldest), Recorded: false}, + {UID: string(middle), Recorded: false}, + }, expired) + + deleted, err := st.SessionDeleteMany(ctx, []string{string(oldest), string(middle)}) + require.NoError(t, err) + assert.Equal(t, int64(2), deleted) + assert.Equal(t, []string{string(newest)}, listUIDs(t)) + }) + + t.Run("reports whether each expired session was recorded", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, -30)) + device := s.CreateDevice(t) + plain := s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + + clk.now = cutoff.AddDate(0, 0, -20) + recorded := s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + require.NoError(t, st.SessionUpdate(ctx, &models.Session{UID: string(recorded), Recorded: true})) + + clk.now = now + + expired, err := st.SessionListExpired(ctx, cutoff, 100) + require.NoError(t, err) + assert.Equal(t, []store.ExpiredSession{ + {UID: string(plain), Recorded: false}, + {UID: string(recorded), Recorded: true}, + }, expired) + }) + + t.Run("leaves an active session in place however old it is", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, -30)) + device := s.CreateDevice(t) + active := s.CreateSession(t, WithSessionDevice(device), WithSessionActive(true)) + s.CreateSession(t, WithSessionDevice(device), WithSessionActive(false)) + + clk.now = now + + assert.Equal(t, int64(1), prune(t, 100)) + assert.Equal(t, []string{string(active)}, listUIDs(t)) + }) + + t.Run("takes the session's events with it", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, -30)) + uid := s.CreateSession(t, WithSessionActive(false)) + + require.NoError(t, st.SessionEventsCreate(ctx, &models.SessionEvent{ + Session: string(uid), + Type: models.SessionEventTypePtyOutput, + Timestamp: clk.now, + Data: map[string]interface{}{"output": "test output"}, + Seat: 1, + })) + + clk.now = now + + assert.Equal(t, int64(1), prune(t, 100)) + + _, count, err := st.SessionEventsList(ctx, uid, 1, models.SessionEventTypePtyOutput) + require.NoError(t, err) + assert.Equal(t, 0, count) + }) + + t.Run("lists nothing when the limit is not positive", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, -30)) + uid := s.CreateSession(t, WithSessionActive(false)) + clk.now = now + + expired, err := st.SessionListExpired(ctx, cutoff, 0) + require.NoError(t, err) + assert.Empty(t, expired) + assert.Equal(t, []string{string(uid)}, listUIDs(t)) + }) + + t.Run("deleting none is a no-op", func(t *testing.T) { + require.NoError(t, s.provider.CleanDatabase(t)) + + clk := pinClock(t, cutoff.AddDate(0, 0, -30)) + uid := s.CreateSession(t, WithSessionActive(false)) + clk.now = now + + deleted, err := st.SessionDeleteMany(ctx, []string{}) + require.NoError(t, err) + assert.Equal(t, int64(0), deleted) + assert.Equal(t, []string{string(uid)}, listUIDs(t)) + }) +} diff --git a/server/api/store/storetest/suite.go b/server/api/store/storetest/suite.go index 44a1816448e..6912a9aabf7 100644 --- a/server/api/store/storetest/suite.go +++ b/server/api/store/storetest/suite.go @@ -63,6 +63,7 @@ func (s *Suite) Run(t *testing.T) { s.TestSessionEventsCreate(t) s.TestSessionEventsList(t) s.TestSessionEventsDelete(t) + s.TestSessionCleanup(t) }) t.Run("TagStore", func(t *testing.T) { diff --git a/server/app/server.go b/server/app/server.go index 9f45e9f6e21..fbf6f3e9c49 100644 --- a/server/app/server.go +++ b/server/app/server.go @@ -70,6 +70,15 @@ type Env struct { // AutoSSL reports whether the console is served over HTTPS; it selects the // scheme of that approval URL. AutoSSL bool `env:"SHELLHUB_AUTO_SSL,default=false"` + + // SessionRetentionDays is how long a session and its events are kept after the session + // started; 0, the default, keeps them indefinitely. + // + // The default is off because the deletion is permanent and unattended — session events are + // the recording. Choosing a window is a deployment decision, made where the compliance + // commitment and the volume are both known, so it is the deployment that sets this rather + // than the binary assuming one. docker-compose.enterprise.yml does exactly that. + SessionRetentionDays int `env:"SHELLHUB_SESSION_RETENTION_DAYS,default=0"` } // sshEnv is parsed with the SSH_ prefix, keeping the names the ssh service used. @@ -186,6 +195,13 @@ func (s *Server) Setup(ctx context.Context) error { servicesOptions = append(servicesOptions, feOpts...) + rpOpts, err := s.sessionRecordingPrunerOption(ctx, store, cache) + if err != nil { + return err + } + + servicesOptions = append(servicesOptions, rpOpts...) + routerOptions, err := s.routerOptions() if err != nil { return err @@ -211,6 +227,13 @@ func (s *Server) Setup(ctx context.Context) error { s.worker.HandleCron(services.CronEnrollmentCallbackCleanup, service.EnrollmentCallbackCleanup(), asynq.Unique()) s.worker.HandleCron(services.CronSSHApprovalCleanup, service.SSHApprovalCleanup(), asynq.Unique()) + if retention := time.Duration(s.env.SessionRetentionDays) * 24 * time.Hour; retention > 0 { + s.worker.HandleCron(services.CronSessionCleanup, service.SessionCleanup(retention), asynq.Unique()) + log.WithField("days", s.env.SessionRetentionDays).Info("session retention enabled") + } else { + log.Warn("session retention disabled; sessions and their events are kept indefinitely") + } + // Apply any worker extensions registered by cloud/enterprise packages. routes.ApplyWorkerExtensions(s.worker, store, cache) @@ -411,6 +434,28 @@ func (s *Server) licenseEvaluatorOption(ctx context.Context, st store.Store, c c return nil, nil } +// sessionRecordingPrunerOption initialises the recording pruner when an enterprise package +// registered a factory. Its factory also returns nil on an enterprise instance with no object +// storage configured, where there are no recordings to prune. Same nil guard, same reason as +// licenseEvaluatorOption. +func (s *Server) sessionRecordingPrunerOption(ctx context.Context, st store.Store, c cache.Cache) ([]services.Option, error) { + factory := services.SessionRecordingPrunerFactory() + if factory == nil { + return nil, nil + } + + rp, err := factory(ctx, st, c) + if err != nil { + return nil, errors.Join(errors.New("init session recording pruner"), err) + } + + if rp != nil { + return []services.Option{services.WithSessionRecordingPruner(rp)}, nil + } + + return nil, nil +} + // firewallEvaluatorOption initialises the firewall evaluator when an enterprise package // registered a factory. The nil guard carries the same weight as in // licenseEvaluatorOption: injecting a typed nil would panic on first use.