diff --git a/api/logretention/agent_floor.go b/api/logretention/agent_floor.go index 0f78efc..374c01d 100644 --- a/api/logretention/agent_floor.go +++ b/api/logretention/agent_floor.go @@ -6,7 +6,7 @@ import ( "github.com/jackc/pgx/v5/pgxpool" ) -// AgentRetentionStore reads the protective retention floor set on +// AgentRetentionStore reads the protective retention floors set on // agents.ConfigOverride.LogRetentionDays -- a separate, Postgres-backed // concern from Store's ClickHouse access above, so it lives in its own // file. Deliberately its own narrow query against the same `agents` @@ -23,24 +23,31 @@ func NewAgentRetentionStore(pool *pgxpool.Pool) *AgentRetentionStore { return &AgentRetentionStore{pool: pool} } -// MaxRetentionDays reports the largest log_retention_days configured -// across any agent's desired_override, if any are set at all -- this is -// the floor a non-owner's deletion request must not reach into (see -// Handler.checkRetentionFloor). The second return value is false when -// no agent has this field configured, distinct from a configured floor -// of 0 (which validateOverride never allows to be stored in the first -// place). -func (s *AgentRetentionStore) MaxRetentionDays(ctx context.Context) (int, bool, error) { - var days *int - err := s.pool.QueryRow(ctx, ` - SELECT max((desired_override->>'log_retention_days')::int) +// RetentionDaysByHost reports the configured log_retention_days for +// every agent that has one set, keyed by host -- a host absent from +// this map has no configured floor at all. Per-host rather than a +// single global maximum: now that deletion is host-scoped +// (Handler.partitionHosts), a floor on one host must never block +// deleting another host's logs that happen to be requested in the same +// call. +func (s *AgentRetentionStore) RetentionDaysByHost(ctx context.Context) (map[string]int, error) { + rows, err := s.pool.Query(ctx, ` + SELECT host, (desired_override->>'log_retention_days')::int FROM agents - WHERE desired_override->>'log_retention_days' IS NOT NULL`).Scan(&days) + WHERE desired_override->>'log_retention_days' IS NOT NULL`) if err != nil { - return 0, false, err + return nil, err } - if days == nil { - return 0, false, nil + defer rows.Close() + + out := map[string]int{} + for rows.Next() { + var host string + var days int + if err := rows.Scan(&host, &days); err != nil { + return nil, err + } + out[host] = days } - return *days, true, nil + return out, rows.Err() } diff --git a/api/logretention/handler.go b/api/logretention/handler.go index 57987cf..f1bb129 100644 --- a/api/logretention/handler.go +++ b/api/logretention/handler.go @@ -3,7 +3,6 @@ package logretention import ( "context" "encoding/json" - "fmt" "log/slog" "net/http" "strconv" @@ -16,15 +15,16 @@ import ( // is the production implementation; tests use a fake, same pattern as // agents.store/dashboards.store. type store interface { - CountOlderThan(ctx context.Context, cutoff time.Time) (uint64, error) - DeleteOlderThan(ctx context.Context, cutoff time.Time) error + HostsOlderThan(ctx context.Context, cutoff time.Time) ([]HostCount, error) + CountOlderThan(ctx context.Context, cutoff time.Time, hosts []string) (uint64, error) + DeleteOlderThan(ctx context.Context, cutoff time.Time, hosts []string) error } // retentionFloor is the narrow interface backing the owner-only // override check -- *AgentRetentionStore (agent_floor.go) is the // production implementation. type retentionFloor interface { - MaxRetentionDays(ctx context.Context) (int, bool, error) + RetentionDaysByHost(ctx context.Context) (map[string]int, error) } // maxOlderThanHours bounds the age a caller can specify -- 10 years is @@ -33,6 +33,12 @@ type retentionFloor interface { // digit) with a clear 400 rather than silently accepting it. const maxOlderThanHours = 10 * 365 * 24 +// maxHosts caps how many host filters one request can carry -- 1000 is +// far beyond any real fleet this deployment's homelab/small-scale +// target implies, and exists only to reject a pathological/malformed +// request rather than to meaningfully restrict real usage. +const maxHosts = 1000 + type Handler struct { logger *slog.Logger store store @@ -44,21 +50,23 @@ func NewHandler(logger *slog.Logger, store store, floor retentionFloor, authoriz return &Handler{logger: logger, store: store, floor: floor, authorizer: authorizer} } -// RegisterRoutes: both routes are RoleAdmin -- RoleOwner satisfies it -// too (Role.Satisfies is a floor, not an exact match), matching the +// RegisterRoutes: all three routes are RoleAdmin -- RoleOwner satisfies +// it too (Role.Satisfies is a floor, not an exact match), matching the // "owner and admin" requirement this feature shipped for. Permanently // deleting log data is at least as consequential as the RBAC matrix's // other RoleAdmin-floor actions (e.g. issuing an agent restart // command, api/agents/handler.go), so it gets the same floor rather // than a stricter RoleOwner-only one. func (h *Handler) RegisterRoutes(mux *http.ServeMux) { + mux.HandleFunc("GET /logs/retention/hosts", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handleHosts)) mux.HandleFunc("GET /logs/retention/preview", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handlePreview)) mux.HandleFunc("DELETE /logs/retention", authz.RequireRole(h.authorizer, authz.RoleAdmin, h.handleDelete)) } // parseOlderThanHours reads and validates the older_than_hours query -// param shared by both routes -- a caller must ask for at least 1 hour -// (an accidental empty/zero value must never mean "delete everything"). +// param shared by all three routes -- a caller must ask for at least 1 +// hour (an accidental empty/zero value must never mean "delete +// everything"). func parseOlderThanHours(r *http.Request) (int, bool) { hours, err := strconv.Atoi(r.URL.Query().Get("older_than_hours")) if err != nil || hours < 1 || hours > maxOlderThanHours { @@ -67,40 +75,145 @@ func parseOlderThanHours(r *http.Request) (int, bool) { return hours, true } -// checkRetentionFloor enforces api/agents.ConfigOverride.LogRetentionDays -// as a hard floor against anyone but an owner: if any agent has a -// configured retention, the largest one across all agents is the -// earliest boundary a non-owner may delete up to. An owner always -// bypasses this (identity.Role == RoleOwner short-circuits before ever -// querying the floor) -- "owner and admin" gates the routes themselves -// (RegisterRoutes), but this narrows what admin specifically can do -// once inside them, the same shape handleSetConfig's own -// log_retention_days gate uses on the agents side. A nil identity (no -// authorizer configured at all, Phase 0-3 default-open) skips this -// too, consistent with every other RBAC check in this codebase being a -// no-op when there's no RBAC to begin with. -func (h *Handler) checkRetentionFloor(ctx context.Context, cutoff time.Time) (string, error) { - identity, ok := authz.IdentityFromContext(ctx) - if !ok || identity.Role == authz.RoleOwner { - return "", nil +// parseHosts reads the repeated host query param shared by preview and +// delete -- deliberately required (at least one), never "omitted means +// every host": the whole point of this parameter existing is letting a +// caller target specific agents' logs instead of wholesale deleting +// everything, so there is no implicit "all hosts" shortcut here. GET +// /logs/retention/hosts is how a caller discovers what to pass. +// Duplicates are silently deduped; an empty host value is rejected +// outright rather than silently dropped, since a caller sending "" almost +// certainly has a client-side bug worth surfacing. +func parseHosts(r *http.Request) ([]string, bool) { + raw := r.URL.Query()["host"] + seen := make(map[string]struct{}, len(raw)) + var hosts []string + for _, host := range raw { + if host == "" { + return nil, false + } + if _, dup := seen[host]; dup { + continue + } + seen[host] = struct{}{} + hosts = append(hosts, host) } - maxDays, hasFloor, err := h.floor.MaxRetentionDays(ctx) + if len(hosts) == 0 || len(hosts) > maxHosts { + return nil, false + } + return hosts, true +} + +// blockedHost is one entry in a preview/delete response's blocked_hosts +// list -- a host the caller asked about that a configured retention +// floor (api/agents.ConfigOverride.LogRetentionDays) protects from +// anyone but an owner. +type blockedHost struct { + Host string `json:"host"` + ProtectedDays int `json:"protected_days"` +} + +// partitionHosts splits hosts into allowed (this caller may act on +// them) and blocked (a configured floor protects them from anyone but +// an owner, and this caller isn't one) -- role-scoped rather than a +// single all-or-nothing check, so a floor on one host never blocks +// acting on other hosts requested in the same call. An owner always +// gets everything back as allowed, no query needed. +func (h *Handler) partitionHosts(ctx context.Context, role authz.Role, hosts []string, cutoff time.Time) ([]string, []blockedHost, error) { + if role == authz.RoleOwner { + return hosts, nil, nil + } + floors, err := h.floor.RetentionDaysByHost(ctx) if err != nil { - return "", err + return nil, nil, err } - if !hasFloor { - return "", nil + now := time.Now().UTC() + var allowed []string + var blocked []blockedHost + for _, host := range hosts { + days, hasFloor := floors[host] + if !hasFloor { + allowed = append(allowed, host) + continue + } + protectedBoundary := now.Add(-time.Duration(days) * 24 * time.Hour) + if cutoff.After(protectedBoundary) { + blocked = append(blocked, blockedHost{Host: host, ProtectedDays: days}) + } else { + allowed = append(allowed, host) + } } - protectedBoundary := time.Now().UTC().Add(-time.Duration(maxDays) * 24 * time.Hour) - if cutoff.After(protectedBoundary) { - return fmt.Sprintf("a configured log retention policy protects logs newer than %d days; only an owner can override this", maxDays), nil + return allowed, blocked, nil +} + +// roleForFloorCheck returns the identity's role, or RoleOwner (i.e. +// "bypass the floor entirely") when no identity resolved at all -- a +// nil authorizer means no RBAC is configured (Phase 0-3 default-open), +// and this feature's owner-only override must stay a no-op in that +// case too, consistent with every other RBAC check in this codebase. +func roleForFloorCheck(ctx context.Context) authz.Role { + identity, ok := authz.IdentityFromContext(ctx) + if !ok { + return authz.RoleOwner } - return "", nil + return identity.Role +} + +type hostEntry struct { + Host string `json:"host"` + Count uint64 `json:"count"` + ProtectedDays *int `json:"protected_days,omitempty"` +} + +type hostsResponse struct { + Hosts []hostEntry `json:"hosts"` + Cutoff time.Time `json:"cutoff"` +} + +// handleHosts lists every host with at least one log record older than +// the requested age, annotated with any configured retention floor -- +// informational for every caller regardless of role (RegisterRoutes' +// RoleAdmin floor already gates who can reach this at all); it's what +// populates the host picker a caller then selects from for preview/ +// delete, not itself an action that needs partitionHosts. +func (h *Handler) handleHosts(w http.ResponseWriter, r *http.Request) { + hours, ok := parseOlderThanHours(r) + if !ok { + writeError(w, http.StatusBadRequest, "older_than_hours must be a positive integer") + return + } + cutoff := time.Now().UTC().Add(-time.Duration(hours) * time.Hour) + + counts, err := h.store.HostsOlderThan(r.Context(), cutoff) + if err != nil { + h.logger.Error("listing hosts for retention preview", "error", err) + writeError(w, http.StatusInternalServerError, "listing hosts failed") + return + } + floors, err := h.floor.RetentionDaysByHost(r.Context()) + if err != nil { + h.logger.Error("reading log retention floors", "error", err) + writeError(w, http.StatusInternalServerError, "listing hosts failed") + return + } + + entries := make([]hostEntry, len(counts)) + for i, c := range counts { + e := hostEntry{Host: c.Host, Count: c.Count} + if days, hasFloor := floors[c.Host]; hasFloor { + d := days + e.ProtectedDays = &d + } + entries[i] = e + } + writeJSON(w, http.StatusOK, hostsResponse{Hosts: entries, Cutoff: cutoff}) } type previewResponse struct { - Count uint64 `json:"count"` - Cutoff time.Time `json:"cutoff"` + Count uint64 `json:"count"` + Cutoff time.Time `json:"cutoff"` + Hosts []string `json:"hosts"` + BlockedHosts []blockedHost `json:"blocked_hosts,omitempty"` } func (h *Handler) handlePreview(w http.ResponseWriter, r *http.Request) { @@ -109,29 +222,37 @@ func (h *Handler) handlePreview(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "older_than_hours must be a positive integer") return } + hosts, ok := parseHosts(r) + if !ok { + writeError(w, http.StatusBadRequest, "at least one host must be specified") + return + } cutoff := time.Now().UTC().Add(-time.Duration(hours) * time.Hour) - if msg, err := h.checkRetentionFloor(r.Context(), cutoff); err != nil { + allowed, blocked, err := h.partitionHosts(r.Context(), roleForFloorCheck(r.Context()), hosts, cutoff) + if err != nil { h.logger.Error("checking log retention floor", "error", err) writeError(w, http.StatusInternalServerError, "checking retention policy failed") return - } else if msg != "" { - writeError(w, http.StatusForbidden, msg) - return } - count, err := h.store.CountOlderThan(r.Context(), cutoff) - if err != nil { - h.logger.Error("counting logs for retention preview", "error", err) - writeError(w, http.StatusInternalServerError, "counting logs failed") - return + var count uint64 + if len(allowed) > 0 { + count, err = h.store.CountOlderThan(r.Context(), cutoff, allowed) + if err != nil { + h.logger.Error("counting logs for retention preview", "error", err) + writeError(w, http.StatusInternalServerError, "counting logs failed") + return + } } - writeJSON(w, http.StatusOK, previewResponse{Count: count, Cutoff: cutoff}) + writeJSON(w, http.StatusOK, previewResponse{Count: count, Cutoff: cutoff, Hosts: allowed, BlockedHosts: blocked}) } type deleteResponse struct { - DeletedCount uint64 `json:"deleted_count"` - Cutoff time.Time `json:"cutoff"` + DeletedCount uint64 `json:"deleted_count"` + Cutoff time.Time `json:"cutoff"` + DeletedHosts []string `json:"deleted_hosts"` + BlockedHosts []blockedHost `json:"blocked_hosts,omitempty"` } // handleDelete counts immediately before deleting so the response can @@ -148,35 +269,41 @@ func (h *Handler) handleDelete(w http.ResponseWriter, r *http.Request) { writeError(w, http.StatusBadRequest, "older_than_hours must be a positive integer") return } + hosts, ok := parseHosts(r) + if !ok { + writeError(w, http.StatusBadRequest, "at least one host must be specified") + return + } cutoff := time.Now().UTC().Add(-time.Duration(hours) * time.Hour) - if msg, err := h.checkRetentionFloor(r.Context(), cutoff); err != nil { + allowed, blocked, err := h.partitionHosts(r.Context(), roleForFloorCheck(r.Context()), hosts, cutoff) + if err != nil { h.logger.Error("checking log retention floor", "error", err) writeError(w, http.StatusInternalServerError, "checking retention policy failed") return - } else if msg != "" { - writeError(w, http.StatusForbidden, msg) - return } - count, err := h.store.CountOlderThan(r.Context(), cutoff) - if err != nil { - h.logger.Error("counting logs before retention delete", "error", err) - writeError(w, http.StatusInternalServerError, "counting logs failed") - return + var count uint64 + if len(allowed) > 0 { + count, err = h.store.CountOlderThan(r.Context(), cutoff, allowed) + if err != nil { + h.logger.Error("counting logs before retention delete", "error", err) + writeError(w, http.StatusInternalServerError, "counting logs failed") + return + } + + if err := h.store.DeleteOlderThan(r.Context(), cutoff, allowed); err != nil { + h.logger.Error("deleting logs by retention age", "error", err) + writeError(w, http.StatusInternalServerError, "deleting logs failed") + return + } + + identity, _ := authz.IdentityFromContext(r.Context()) + h.logger.Info("logs deleted by retention age", + "deleted_count", count, "cutoff", cutoff, "hosts", allowed, "user_id", identity.UserID, "role", identity.Role) } - if err := h.store.DeleteOlderThan(r.Context(), cutoff); err != nil { - h.logger.Error("deleting logs by retention age", "error", err) - writeError(w, http.StatusInternalServerError, "deleting logs failed") - return - } - - identity, _ := authz.IdentityFromContext(r.Context()) - h.logger.Info("logs deleted by retention age", - "deleted_count", count, "cutoff", cutoff, "user_id", identity.UserID, "role", identity.Role) - - writeJSON(w, http.StatusOK, deleteResponse{DeletedCount: count, Cutoff: cutoff}) + writeJSON(w, http.StatusOK, deleteResponse{DeletedCount: count, Cutoff: cutoff, DeletedHosts: allowed, BlockedHosts: blocked}) } func writeJSON(w http.ResponseWriter, status int, v any) { diff --git a/api/logretention/handler_test.go b/api/logretention/handler_test.go index f767463..6222077 100644 --- a/api/logretention/handler_test.go +++ b/api/logretention/handler_test.go @@ -8,6 +8,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "reflect" "strconv" "testing" "time" @@ -19,27 +20,43 @@ func discardLogger() *slog.Logger { return slog.New(slog.NewTextHandler(io.Discard, nil)) } -// fakeStore records the cutoff it was called with so tests can assert -// the handler computed it correctly from older_than_hours, and lets a -// test inject a store error to exercise the failure paths. +// hostCall records one CountOlderThan/DeleteOlderThan invocation, so +// tests can assert both the cutoff and the exact host set a call used. +type hostCall struct { + cutoff time.Time + hosts []string +} + +// fakeStore lets a test inject store errors and a fixed host listing, +// and records every count/delete call it received so tests can assert +// the handler scoped them to the right hosts. type fakeStore struct { + hostList []HostCount + hostsErr error count uint64 countErr error deleteErr error - countedWith []time.Time - deletedWith []time.Time + countedWith []hostCall + deletedWith []hostCall } -func (f *fakeStore) CountOlderThan(_ context.Context, cutoff time.Time) (uint64, error) { - f.countedWith = append(f.countedWith, cutoff) +func (f *fakeStore) HostsOlderThan(_ context.Context, _ time.Time) ([]HostCount, error) { + if f.hostsErr != nil { + return nil, f.hostsErr + } + return f.hostList, nil +} + +func (f *fakeStore) CountOlderThan(_ context.Context, cutoff time.Time, hosts []string) (uint64, error) { + f.countedWith = append(f.countedWith, hostCall{cutoff, hosts}) if f.countErr != nil { return 0, f.countErr } return f.count, nil } -func (f *fakeStore) DeleteOlderThan(_ context.Context, cutoff time.Time) error { - f.deletedWith = append(f.deletedWith, cutoff) +func (f *fakeStore) DeleteOlderThan(_ context.Context, cutoff time.Time, hosts []string) error { + f.deletedWith = append(f.deletedWith, hostCall{cutoff, hosts}) return f.deleteErr } @@ -51,17 +68,16 @@ func (f fakeAuthorizer) Authorize(*http.Request) (authz.Identity, error) { return authz.Identity{TenantID: "default", UserID: "u1", Role: f.role}, nil } -// fakeFloor stands in for AgentRetentionStore -- hasFloor false (the -// zero value) means no agent has log_retention_days configured, same -// as every existing test in this file assumed before the floor existed. +// fakeFloor stands in for AgentRetentionStore -- a nil/empty byHost map +// means no agent has log_retention_days configured, same as every test +// that doesn't care about the floor assumed before it existed. type fakeFloor struct { - days int - hasFloor bool - err error + byHost map[string]int + err error } -func (f fakeFloor) MaxRetentionDays(context.Context) (int, bool, error) { - return f.days, f.hasFloor, f.err +func (f fakeFloor) RetentionDaysByHost(context.Context) (map[string]int, error) { + return f.byHost, f.err } func newTestHandler(s *fakeStore, role authz.Role) *Handler { @@ -78,12 +94,39 @@ func doRequest(t *testing.T, h *Handler, method, path string) *httptest.Response return rec } -func TestPreviewReturnsCountAndCutoff(t *testing.T) { +func hoursForDays(days int) string { + return strconv.Itoa(days * 24) +} + +func TestHostsListsHostsWithCountsAndFloors(t *testing.T) { + s := &fakeStore{hostList: []HostCount{{Host: "web-01", Count: 100}, {Host: "web-02", Count: 5}}} + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"web-02": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) + + rec := doRequest(t, h, "GET", "/logs/retention/hosts?older_than_hours=24") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + var resp hostsResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decoding response: %v", err) + } + if len(resp.Hosts) != 2 { + t.Fatalf("len(hosts) = %d, want 2", len(resp.Hosts)) + } + if resp.Hosts[0].Host != "web-01" || resp.Hosts[0].Count != 100 || resp.Hosts[0].ProtectedDays != nil { + t.Errorf("hosts[0] = %+v, want web-01/100/no floor", resp.Hosts[0]) + } + if resp.Hosts[1].Host != "web-02" || resp.Hosts[1].Count != 5 || resp.Hosts[1].ProtectedDays == nil || *resp.Hosts[1].ProtectedDays != 90 { + t.Errorf("hosts[1] = %+v, want web-02/5/floor=90", resp.Hosts[1]) + } +} + +func TestPreviewReturnsCountCutoffAndHosts(t *testing.T) { s := &fakeStore{count: 42} h := newTestHandler(s, authz.RoleAdmin) before := time.Now().UTC() - rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24") + rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01&host=web-02") after := time.Now().UTC() if rec.Code != http.StatusOK { @@ -96,6 +139,9 @@ func TestPreviewReturnsCountAndCutoff(t *testing.T) { if resp.Count != 42 { t.Errorf("count = %d, want 42", resp.Count) } + if !reflect.DeepEqual(resp.Hosts, []string{"web-01", "web-02"}) { + t.Errorf("hosts = %v, want [web-01 web-02]", resp.Hosts) + } wantEarliest := before.Add(-24 * time.Hour) wantLatest := after.Add(-24 * time.Hour) if resp.Cutoff.Before(wantEarliest) || resp.Cutoff.After(wantLatest) { @@ -104,13 +150,47 @@ func TestPreviewReturnsCountAndCutoff(t *testing.T) { if len(s.deletedWith) != 0 { t.Errorf("preview must never delete anything, but DeleteOlderThan was called %d time(s)", len(s.deletedWith)) } + if len(s.countedWith) != 1 || !reflect.DeepEqual(s.countedWith[0].hosts, []string{"web-01", "web-02"}) { + t.Errorf("CountOlderThan was not scoped to the requested hosts: %+v", s.countedWith) + } } -func TestDeleteReturnsDeletedCountAndCutoff(t *testing.T) { +func TestPreviewDedupesHosts(t *testing.T) { + s := &fakeStore{count: 1} + h := newTestHandler(s, authz.RoleAdmin) + + rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01&host=web-01") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + if len(s.countedWith) != 1 || !reflect.DeepEqual(s.countedWith[0].hosts, []string{"web-01"}) { + t.Fatalf("expected a deduped single-host call, got %+v", s.countedWith) + } +} + +func TestPreviewRequiresAtLeastOneHost(t *testing.T) { + h := newTestHandler(&fakeStore{}, authz.RoleAdmin) + + rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24") + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 with no host specified", rec.Code) + } +} + +func TestPreviewRejectsEmptyHostValue(t *testing.T) { + h := newTestHandler(&fakeStore{}, authz.RoleAdmin) + + rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=") + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 with an empty host value", rec.Code) + } +} + +func TestDeleteReturnsDeletedCountHostsAndCutoff(t *testing.T) { s := &fakeStore{count: 7} h := newTestHandler(s, authz.RoleAdmin) - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=720") + rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=720&host=web-01") if rec.Code != http.StatusOK { t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) } @@ -121,11 +201,14 @@ func TestDeleteReturnsDeletedCountAndCutoff(t *testing.T) { if resp.DeletedCount != 7 { t.Errorf("deleted_count = %d, want 7", resp.DeletedCount) } - if len(s.deletedWith) != 1 { - t.Fatalf("expected exactly one DeleteOlderThan call, got %d", len(s.deletedWith)) + if !reflect.DeepEqual(resp.DeletedHosts, []string{"web-01"}) { + t.Errorf("deleted_hosts = %v, want [web-01]", resp.DeletedHosts) } - if len(s.countedWith) != 1 || !s.countedWith[0].Equal(s.deletedWith[0]) { - t.Errorf("count and delete must use the same cutoff: counted=%v deleted=%v", s.countedWith, s.deletedWith) + if len(s.deletedWith) != 1 || !reflect.DeepEqual(s.deletedWith[0].hosts, []string{"web-01"}) { + t.Fatalf("expected exactly one scoped DeleteOlderThan call, got %+v", s.deletedWith) + } + if len(s.countedWith) != 1 || !s.countedWith[0].cutoff.Equal(s.deletedWith[0].cutoff) { + t.Errorf("count and delete must use the same cutoff: counted=%+v deleted=%+v", s.countedWith, s.deletedWith) } } @@ -133,11 +216,11 @@ func TestRejectsMissingOrInvalidOlderThanHours(t *testing.T) { h := newTestHandler(&fakeStore{}, authz.RoleAdmin) cases := []string{ - "/logs/retention/preview", - "/logs/retention/preview?older_than_hours=0", - "/logs/retention/preview?older_than_hours=-5", - "/logs/retention/preview?older_than_hours=notanumber", - "/logs/retention/preview?older_than_hours=999999999", + "/logs/retention/preview?host=web-01", + "/logs/retention/preview?older_than_hours=0&host=web-01", + "/logs/retention/preview?older_than_hours=-5&host=web-01", + "/logs/retention/preview?older_than_hours=notanumber&host=web-01", + "/logs/retention/preview?older_than_hours=999999999&host=web-01", } for _, path := range cases { rec := doRequest(t, h, "GET", path) @@ -151,7 +234,7 @@ func TestDeleteRejectsInvalidOlderThanHours(t *testing.T) { s := &fakeStore{} h := newTestHandler(s, authz.RoleAdmin) - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=0") + rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=0&host=web-01") if rec.Code != http.StatusBadRequest { t.Fatalf("status = %d, want 400", rec.Code) } @@ -160,11 +243,24 @@ func TestDeleteRejectsInvalidOlderThanHours(t *testing.T) { } } +func TestDeleteRejectsMissingHosts(t *testing.T) { + s := &fakeStore{} + h := newTestHandler(s, authz.RoleAdmin) + + rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24") + if rec.Code != http.StatusBadRequest { + t.Fatalf("status = %d, want 400 with no host specified", rec.Code) + } + if len(s.deletedWith) != 0 { + t.Error("a request with no host specified must never reach the store's delete path") + } +} + func TestDeletePropagatesStoreErrors(t *testing.T) { s := &fakeStore{deleteErr: errors.New("clickhouse mutation failed")} h := newTestHandler(s, authz.RoleAdmin) - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24") + rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") if rec.Code != http.StatusInternalServerError { t.Fatalf("status = %d, want 500", rec.Code) } @@ -175,11 +271,15 @@ func TestOwnerAndAdminCanUseRetentionRoutes(t *testing.T) { s := &fakeStore{count: 3} h := newTestHandler(s, role) - preview := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24") + hosts := doRequest(t, h, "GET", "/logs/retention/hosts?older_than_hours=24") + if hosts.Code != http.StatusOK { + t.Errorf("role %s: hosts status = %d, want 200", role, hosts.Code) + } + preview := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01") if preview.Code != http.StatusOK { t.Errorf("role %s: preview status = %d, want 200", role, preview.Code) } - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24") + del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") if del.Code != http.StatusOK { t.Errorf("role %s: delete status = %d, want 200", role, del.Code) } @@ -191,11 +291,15 @@ func TestViewerAndEditorAreForbiddenFromRetentionRoutes(t *testing.T) { s := &fakeStore{count: 3} h := newTestHandler(s, role) - preview := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24") + hosts := doRequest(t, h, "GET", "/logs/retention/hosts?older_than_hours=24") + if hosts.Code != http.StatusForbidden { + t.Errorf("role %s: hosts status = %d, want 403", role, hosts.Code) + } + preview := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01") if preview.Code != http.StatusForbidden { t.Errorf("role %s: preview status = %d, want 403", role, preview.Code) } - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24") + del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") if del.Code != http.StatusForbidden { t.Errorf("role %s: delete status = %d, want 403", role, del.Code) } @@ -214,31 +318,68 @@ func TestRetentionRoutesRequireAuth(t *testing.T) { // here too, same as every other RequireRole-wrapped route, rather // than this package accidentally being open or closed by default in // a way inconsistent with the rest of the API. - rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24") + rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=24&host=web-01") if rec.Code != http.StatusOK { t.Fatalf("status with nil authorizer = %d, want 200 (default-open, matches RequireRole elsewhere)", rec.Code) } } -// TestAdminBlockedByRetentionFloor is the core regression test for the -// owner-only override: an agent configured with a 90-day retention -// floor must block an admin's attempt to delete anything newer than -// that, on both preview and delete. -func TestAdminBlockedByRetentionFloor(t *testing.T) { - s := &fakeStore{count: 100} - h := NewHandler(discardLogger(), s, fakeFloor{days: 90, hasFloor: true}, fakeAuthorizer{role: authz.RoleAdmin}) +// TestAdminPartiallyBlockedByPerHostRetentionFloor is the core +// regression test for host-scoped floor enforcement: requesting two +// hosts where only one has a protective floor must delete the +// unprotected host and report the other as blocked, not reject the +// whole request. +func TestAdminPartiallyBlockedByPerHostRetentionFloor(t *testing.T) { + s := &fakeStore{count: 5} + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"protected-host": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) - // 30 days is newer than the 90-day floor -- must be blocked. - preview := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours="+hoursForDays(30)) - if preview.Code != http.StatusForbidden { - t.Fatalf("preview at 30d against a 90d floor: status = %d, want 403, body=%s", preview.Code, preview.Body.String()) + // 30 days is newer than protected-host's 90-day floor. + rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(30)+"&host=protected-host&host=open-host") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200 (partial success, not an error), body=%s", rec.Code, rec.Body.String()) } - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(30)) - if del.Code != http.StatusForbidden { - t.Fatalf("delete at 30d against a 90d floor: status = %d, want 403, body=%s", del.Code, del.Body.String()) + var resp deleteResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decoding response: %v", err) } - if len(s.countedWith) != 0 || len(s.deletedWith) != 0 { - t.Error("a blocked request must never reach the store at all") + if !reflect.DeepEqual(resp.DeletedHosts, []string{"open-host"}) { + t.Errorf("deleted_hosts = %v, want [open-host]", resp.DeletedHosts) + } + if len(resp.BlockedHosts) != 1 || resp.BlockedHosts[0].Host != "protected-host" || resp.BlockedHosts[0].ProtectedDays != 90 { + t.Errorf("blocked_hosts = %+v, want [{protected-host 90}]", resp.BlockedHosts) + } + if len(s.deletedWith) != 1 || !reflect.DeepEqual(s.deletedWith[0].hosts, []string{"open-host"}) { + t.Fatalf("DeleteOlderThan must only ever be scoped to the allowed host, got %+v", s.deletedWith) + } +} + +// TestAllHostsBlockedReturnsZeroCountNotError confirms a request where +// every requested host is protected still succeeds (200), just with +// nothing deleted -- informative, not an error condition, since the +// request itself was perfectly valid. +func TestAllHostsBlockedReturnsZeroCountNotError(t *testing.T) { + s := &fakeStore{count: 100} + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"protected-host": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) + + rec := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(30)+"&host=protected-host") + if rec.Code != http.StatusOK { + t.Fatalf("status = %d, want 200, body=%s", rec.Code, rec.Body.String()) + } + var resp deleteResponse + if err := json.Unmarshal(rec.Body.Bytes(), &resp); err != nil { + t.Fatalf("decoding response: %v", err) + } + if resp.DeletedCount != 0 { + t.Errorf("deleted_count = %d, want 0", resp.DeletedCount) + } + if len(resp.DeletedHosts) != 0 { + t.Errorf("deleted_hosts = %v, want empty", resp.DeletedHosts) + } + if len(resp.BlockedHosts) != 1 || resp.BlockedHosts[0].Host != "protected-host" { + t.Errorf("blocked_hosts = %+v, want [{protected-host 90}]", resp.BlockedHosts) + } + if len(s.deletedWith) != 0 || len(s.countedWith) != 0 { + t.Error("the store must never be called when every requested host is blocked") } } @@ -247,13 +388,20 @@ func TestAdminBlockedByRetentionFloor(t *testing.T) { // request older than the floor itself is unaffected by it. func TestAdminAllowedBeyondRetentionFloor(t *testing.T) { s := &fakeStore{count: 5} - h := NewHandler(discardLogger(), s, fakeFloor{days: 90, hasFloor: true}, fakeAuthorizer{role: authz.RoleAdmin}) + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"web-01": 90}}, fakeAuthorizer{role: authz.RoleAdmin}) // 120 days is older than the 90-day floor -- must be allowed. - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(120)) + del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(120)+"&host=web-01") if del.Code != http.StatusOK { t.Fatalf("delete at 120d against a 90d floor: status = %d, want 200, body=%s", del.Code, del.Body.String()) } + var resp deleteResponse + if err := json.Unmarshal(del.Body.Bytes(), &resp); err != nil { + t.Fatalf("decoding response: %v", err) + } + if len(resp.BlockedHosts) != 0 { + t.Errorf("blocked_hosts = %+v, want none", resp.BlockedHosts) + } } // TestOwnerBypassesRetentionFloor confirms the whole point of the @@ -261,24 +409,29 @@ func TestAdminAllowedBeyondRetentionFloor(t *testing.T) { // window that blocks everyone else. func TestOwnerBypassesRetentionFloor(t *testing.T) { s := &fakeStore{count: 100} - h := NewHandler(discardLogger(), s, fakeFloor{days: 90, hasFloor: true}, fakeAuthorizer{role: authz.RoleOwner}) + h := NewHandler(discardLogger(), s, fakeFloor{byHost: map[string]int{"web-01": 90}}, fakeAuthorizer{role: authz.RoleOwner}) - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(1)) + del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours="+hoursForDays(1)+"&host=web-01") if del.Code != http.StatusOK { t.Fatalf("owner deleting within the floor: status = %d, want 200, body=%s", del.Code, del.Body.String()) } + var resp deleteResponse + if err := json.Unmarshal(del.Body.Bytes(), &resp); err != nil { + t.Fatalf("decoding response: %v", err) + } + if !reflect.DeepEqual(resp.DeletedHosts, []string{"web-01"}) { + t.Errorf("deleted_hosts = %v, want [web-01] (owner bypasses the floor entirely)", resp.DeletedHosts) + } } // TestNoConfiguredFloorNeverBlocksAdmin confirms the default, common // case (no agent has log_retention_days set) behaves exactly as before -// this feature existed -- fakeFloor{} (hasFloor: false) is what every -// other test in this file already relies on, this just makes the -// no-floor-configured case explicit. +// this feature existed. func TestNoConfiguredFloorNeverBlocksAdmin(t *testing.T) { s := &fakeStore{count: 9} - h := NewHandler(discardLogger(), s, fakeFloor{hasFloor: false}, fakeAuthorizer{role: authz.RoleAdmin}) + h := NewHandler(discardLogger(), s, fakeFloor{}, fakeAuthorizer{role: authz.RoleAdmin}) - del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=1") + del := doRequest(t, h, "DELETE", "/logs/retention?older_than_hours=1&host=web-01") if del.Code != http.StatusOK { t.Fatalf("status = %d, want 200 with no configured floor", del.Code) } @@ -288,12 +441,8 @@ func TestRetentionFloorCheckPropagatesStoreErrors(t *testing.T) { s := &fakeStore{} h := NewHandler(discardLogger(), s, fakeFloor{err: errors.New("postgres unreachable")}, fakeAuthorizer{role: authz.RoleAdmin}) - rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24") + rec := doRequest(t, h, "GET", "/logs/retention/preview?older_than_hours=24&host=web-01") if rec.Code != http.StatusInternalServerError { t.Fatalf("status = %d, want 500", rec.Code) } } - -func hoursForDays(days int) string { - return strconv.Itoa(days * 24) -} diff --git a/api/logretention/store.go b/api/logretention/store.go index bbf7ee1..a95c318 100644 --- a/api/logretention/store.go +++ b/api/logretention/store.go @@ -1,8 +1,12 @@ // Package logretention lets an owner or admin permanently delete log -// records older than a chosen age -- storage/README.md has flagged "no -// TTL/retention clause yet" since Phase 0; this is the on-demand, -// operator-triggered half of that gap (not an automatic TTL, which is -// a different, engine-driven design nobody asked for here). +// records older than a chosen age, scoped to specific hosts -- deleting +// by age alone (with no way to target which agents' logs) turned out +// to be a real footgun for an operator who only wants to clean up one +// noisy host, not everything; storage/README.md has flagged "no +// TTL/retention clause yet" since Phase 0, this is the on-demand, +// operator-triggered, host-scoped half of that gap (not an automatic +// TTL, which is a different, engine-driven design nobody asked for +// here). // // Deliberately scoped to core's single ClickHouse `logs` table, not // enterprise/'s per-tenant ClickHouse routing @@ -25,6 +29,8 @@ package logretention import ( "context" + "fmt" + "strings" "time" "github.com/ClickHouse/clickhouse-go/v2/lib/driver" @@ -33,10 +39,10 @@ import ( // Store issues purpose-built, parameterized statements against the // `logs` table -- deliberately not querylang/executor.ChRunner, whose // one method (RunSQL) is scoped to arbitrary SELECT statements for the -// query language compiler. This package only ever needs two fixed -// statements (a count and a delete), so keeping them separate avoids -// stretching ChRunner's SELECT-shaped contract to also cover a DML -// mutation. +// query language compiler. This package only ever needs a handful of +// fixed statement shapes (list hosts, count, delete), so keeping them +// separate avoids stretching ChRunner's SELECT-shaped contract to also +// cover a DML mutation. type Store struct { conn driver.Conn } @@ -45,11 +51,59 @@ func NewStore(conn driver.Conn) *Store { return &Store{conn: conn} } -// CountOlderThan reports how many log records are older than cutoff -- -// backs the "this will delete N records" preview a caller shows before -// asking for confirmation. -func (s *Store) CountOlderThan(ctx context.Context, cutoff time.Time) (uint64, error) { - row := s.conn.QueryRow(ctx, "SELECT count() FROM logs WHERE timestamp < ?", cutoff) +type HostCount struct { + Host string `json:"host"` + Count uint64 `json:"count"` +} + +// HostsOlderThan lists every host with at least one log record older +// than cutoff, along with how many -- backs the host picker a caller +// selects from before previewing/deleting, so the list only ever shows +// hosts that actually have something to act on for the chosen age. +func (s *Store) HostsOlderThan(ctx context.Context, cutoff time.Time) ([]HostCount, error) { + rows, err := s.conn.Query(ctx, ` + SELECT host, count() AS n FROM logs WHERE timestamp < ? GROUP BY host ORDER BY n DESC`, cutoff) + if err != nil { + return nil, err + } + defer rows.Close() + + var out []HostCount + for rows.Next() { + var hc HostCount + if err := rows.Scan(&hc.Host, &hc.Count); err != nil { + return nil, err + } + out = append(out, hc) + } + return out, rows.Err() +} + +// hostPlaceholders builds "?, ?, ..." for n hosts and the matching +// []any argument slice (cutoff first, then each host) -- shared by +// CountOlderThan and DeleteOlderThan since both statements have the +// same "timestamp < ? AND host IN (...)" shape. Callers must never +// pass an empty hosts slice (an empty IN () is invalid SQL, and more +// importantly "no hosts specified" must never silently mean "every +// host" -- see Handler.parseHosts, which rejects that before this is +// ever called). +func hostPlaceholders(cutoff time.Time, hosts []string) (string, []any) { + placeholders := make([]string, len(hosts)) + args := make([]any, 0, len(hosts)+1) + args = append(args, cutoff) + for i, h := range hosts { + placeholders[i] = "?" + args = append(args, h) + } + return strings.Join(placeholders, ", "), args +} + +// CountOlderThan reports how many log records from any of hosts are +// older than cutoff -- backs the "this will delete N records" preview +// a caller shows before asking for confirmation. +func (s *Store) CountOlderThan(ctx context.Context, cutoff time.Time, hosts []string) (uint64, error) { + ph, args := hostPlaceholders(cutoff, hosts) + row := s.conn.QueryRow(ctx, fmt.Sprintf("SELECT count() FROM logs WHERE timestamp < ? AND host IN (%s)", ph), args...) var n uint64 if err := row.Scan(&n); err != nil { return 0, err @@ -58,13 +112,14 @@ func (s *Store) CountOlderThan(ctx context.Context, cutoff time.Time) (uint64, e } // DeleteOlderThan issues a synchronous ClickHouse mutation -// (SETTINGS mutations_sync = 1) deleting every log record older than -// cutoff. Synchronous rather than fire-and-forget so a 200 response -// means the data is actually gone, not just queued -- an owner/admin -// confirming a permanent delete should be able to trust the response. -// This does block for as long as the mutation takes, which could be a -// while against a very large table; a disclosed tradeoff for this -// deployment's homelab/small-scale target, not a hidden one. -func (s *Store) DeleteOlderThan(ctx context.Context, cutoff time.Time) error { - return s.conn.Exec(ctx, "ALTER TABLE logs DELETE WHERE timestamp < ? SETTINGS mutations_sync = 1", cutoff) +// (SETTINGS mutations_sync = 1) deleting every log record from any of +// hosts older than cutoff. Synchronous rather than fire-and-forget so +// a 200 response means the data is actually gone, not just queued -- an +// owner/admin confirming a permanent delete should be able to trust the +// response. This does block for as long as the mutation takes, which +// could be a while against a very large table; a disclosed tradeoff for +// this deployment's homelab/small-scale target, not a hidden one. +func (s *Store) DeleteOlderThan(ctx context.Context, cutoff time.Time, hosts []string) error { + ph, args := hostPlaceholders(cutoff, hosts) + return s.conn.Exec(ctx, fmt.Sprintf("ALTER TABLE logs DELETE WHERE timestamp < ? AND host IN (%s) SETTINGS mutations_sync = 1", ph), args...) } diff --git a/web/src/lib/api.ts b/web/src/lib/api.ts index 72ed0e4..ee33302 100644 --- a/web/src/lib/api.ts +++ b/web/src/lib/api.ts @@ -483,16 +483,38 @@ export function setUserRole(id: string, role: string): Promise { } // --- log retention (owner/admin only, see api/logretention) ----------- +// Deletion is host-scoped, not wholesale: a caller must name which +// hosts' logs to target (listRetentionHosts is how the UI discovers +// what to offer), and api/logretention never treats an omitted host +// list as "every host." -export type LogRetentionPreview = { count: number; cutoff: string }; -export type LogRetentionDeleteResult = { deleted_count: number; cutoff: string }; +export type BlockedHost = { host: string; protected_days: number }; +export type RetentionHost = { host: string; count: number; protected_days?: number }; +export type RetentionHostsResult = { hosts: RetentionHost[]; cutoff: string }; +export type LogRetentionPreview = { count: number; cutoff: string; hosts: string[]; blocked_hosts?: BlockedHost[] }; +export type LogRetentionDeleteResult = { + deleted_count: number; + cutoff: string; + deleted_hosts: string[]; + blocked_hosts?: BlockedHost[]; +}; -export function previewLogDeletion(olderThanHours: number): Promise { - return request(`/logs/retention/preview?older_than_hours=${olderThanHours}`, { credentials: 'include' }); +function hostsQuery(hosts: string[]): string { + return hosts.map((h) => `host=${encodeURIComponent(h)}`).join('&'); } -export function deleteLogsOlderThan(olderThanHours: number): Promise { - return request(`/logs/retention?older_than_hours=${olderThanHours}`, { +export function listRetentionHosts(olderThanHours: number): Promise { + return request(`/logs/retention/hosts?older_than_hours=${olderThanHours}`, { credentials: 'include' }); +} + +export function previewLogDeletion(olderThanHours: number, hosts: string[]): Promise { + return request(`/logs/retention/preview?older_than_hours=${olderThanHours}&${hostsQuery(hosts)}`, { + credentials: 'include' + }); +} + +export function deleteLogsOlderThan(olderThanHours: number, hosts: string[]): Promise { + return request(`/logs/retention?older_than_hours=${olderThanHours}&${hostsQuery(hosts)}`, { method: 'DELETE', credentials: 'include' }); diff --git a/web/src/routes/settings/+page.svelte b/web/src/routes/settings/+page.svelte index 9eb61af..18feee9 100644 --- a/web/src/routes/settings/+page.svelte +++ b/web/src/routes/settings/+page.svelte @@ -5,11 +5,13 @@ localAuthEnabled, getLocalSession, getCurrentSession, + listRetentionHosts, previewLogDeletion, deleteLogsOlderThan, type AuthFeatures, type LocalSession, type CurrentSession, + type RetentionHost, type LogRetentionPreview, type LogRetentionDeleteResult } from '$lib/api'; @@ -68,7 +70,10 @@ return !localAuthEnabled; }); - // --- log retention deletion --- + // --- log retention deletion -- host-scoped, not wholesale: a caller + // picks which hosts to target from what listRetentionHosts reports for + // the chosen age, same "select specific agents, not delete everything" + // requirement api/logretention's own parseHosts enforces server-side. --- const retentionOptions: { label: string; hours: number }[] = [ { label: '7 days', hours: 24 * 7 }, { label: '30 days', hours: 24 * 30 }, @@ -77,18 +82,67 @@ { label: '365 days', hours: 24 * 365 } ]; let retentionHours = $state(retentionOptions[1].hours); + + let hostsLoading = $state(false); + let hosts = $state([]); + let hostsError = $state(''); + let selectedHosts = $state>(new Set()); + let previewing = $state(false); let preview = $state(null); let deleting = $state(false); let deleteResult = $state(null); let retentionError = $state(''); + // Reloads whenever retentionHours changes (including on mount, since + // $effect runs once immediately too) -- selection/preview/result all + // reset because they were scoped to the previous age's host list. + async function loadHosts(hours: number) { + hostsLoading = true; + hostsError = ''; + preview = null; + deleteResult = null; + selectedHosts = new Set(); + try { + const result = await listRetentionHosts(hours); + hosts = result.hosts; + } catch (e) { + hostsError = e instanceof Error ? e.message : String(e); + hosts = []; + } finally { + hostsLoading = false; + } + } + $effect(() => { + loadHosts(retentionHours); + }); + + function toggleHost(host: string) { + const next = new Set(selectedHosts); + if (next.has(host)) next.delete(host); + else next.add(host); + selectedHosts = next; + preview = null; + deleteResult = null; + } + + function selectAllHosts() { + selectedHosts = new Set(hosts.map((h) => h.host)); + preview = null; + } + + function selectNoHosts() { + selectedHosts = new Set(); + preview = null; + } + async function handlePreview() { + if (selectedHosts.size === 0) return; previewing = true; retentionError = ''; deleteResult = null; try { - preview = await previewLogDeletion(retentionHours); + preview = await previewLogDeletion(retentionHours, [...selectedHosts]); } catch (e) { retentionError = e instanceof Error ? e.message : String(e); } finally { @@ -105,8 +159,16 @@ deleting = true; retentionError = ''; try { - deleteResult = await deleteLogsOlderThan(retentionHours); + // preview.hosts, not [...selectedHosts] -- already excludes any + // host the preview found protected, so this never re-asks for a + // host the response just said would be skipped. + const result = await deleteLogsOlderThan(retentionHours, preview.hosts); preview = null; + // loadHosts resets deleteResult as part of its "fresh state" load + // (stale counts/now-empty hosts shouldn't linger), so it runs + // before deleteResult is set here, not after. + await loadHosts(retentionHours); + deleteResult = result; } catch (e) { retentionError = e instanceof Error ? e.message : String(e); } finally { @@ -182,34 +244,87 @@

Log retention

- Permanently delete log records older than a chosen age. Visible to owners and admins only. + Permanently delete log records from specific hosts, older than a chosen age. Visible to owners and + admins only.

- {#each retentionOptions as opt (opt.hours)} {/each} -
+ {#if hostsError}

{hostsError}

{/if} + + {#if hostsLoading} +

Loading hosts…

+ {:else if hosts.length === 0} +

No hosts have logs older than this.

+ {:else} +
+
+ + +
+
    + {#each hosts as h (h.host)} +
  • + +
    • + {/each} +
+ +
+ {/if} + {#if retentionError}

{retentionError}

{/if} {#if preview}
-

- This will permanently delete {preview.count.toLocaleString()} - log record{preview.count === 1 ? '' : 's'} older than {formatCutoff(preview.cutoff)}. - This cannot be undone. -

+ {#if preview.hosts.length > 0} +

+ This will permanently delete {preview.count.toLocaleString()} + log record{preview.count === 1 ? '' : 's'} older than {formatCutoff(preview.cutoff)} from + {preview.hosts.length} host{preview.hosts.length === 1 ? '' : 's'} ({preview.hosts.join(', ')}). + This cannot be undone. +

+ {:else} +

Every selected host is protected by a retention policy -- nothing to delete.

+ {/if} + {#if preview.blocked_hosts?.length} +

+ Protected by a retention policy, skipped: {preview.blocked_hosts + .map((b) => `${b.host} (${b.protected_days}d)`) + .join(', ')}. +

+ {/if}
- - + {#if preview.hosts.length > 0} + + {/if}
{/if} @@ -218,7 +333,13 @@

Deleted {deleteResult.deleted_count.toLocaleString()} log record{deleteResult.deleted_count === 1 ? '' - : 's'} older than {formatCutoff(deleteResult.cutoff)}. + : 's'} older than {formatCutoff(deleteResult.cutoff)} + {#if deleteResult.deleted_hosts.length}from {deleteResult.deleted_hosts.join(', ')}{/if}. + {#if deleteResult.blocked_hosts?.length} + Skipped (protected): {deleteResult.blocked_hosts + .map((b) => `${b.host} (${b.protected_days}d)`) + .join(', ')}. + {/if}

{/if}
@@ -338,21 +459,83 @@ border-radius: var(--radius-sm); padding: var(--space-2) var(--space-3); } - .retention-controls button { + .host-picker { + margin-top: var(--space-3); + padding: var(--space-3); + background: var(--color-surface); + border: 1px solid var(--color-border); + border-radius: var(--radius-md); + } + .host-picker-actions { + display: flex; + gap: var(--space-3); + margin-bottom: var(--space-2); + } + .link { + background: none; + border: none; + padding: 0; + color: var(--color-accent); + font-family: var(--font-ui); + font-size: var(--text-xs); + cursor: pointer; + } + .link:disabled { + cursor: default; + opacity: 0.6; + } + .host-list { + list-style: none; + margin: 0 0 var(--space-3); + padding: 0; + max-height: 14rem; + overflow-y: auto; + } + .host-list li { + border-bottom: 1px solid var(--color-border); + } + .host-list li:last-child { + border-bottom: none; + } + .host-list label { + display: flex; + align-items: center; + gap: var(--space-2); + padding: var(--space-2) var(--space-1); + font-size: var(--text-sm); + cursor: pointer; + } + .host-name { + font-family: var(--font-mono); + color: var(--color-text); + } + .host-count { + color: var(--color-text-muted); + font-size: var(--text-xs); + } + .protected-badge { + margin-left: auto; + font-size: var(--text-xs); + color: var(--color-text-muted); + border: 1px solid var(--color-border); + border-radius: var(--radius-sm); + padding: 0.05rem 0.4rem; + } + .host-picker > button { padding: var(--space-2) var(--space-4); font-family: var(--font-ui); font-size: var(--text-sm); font-weight: var(--font-weight-medium); color: var(--color-text); - background: var(--color-surface); + background: var(--color-bg); border: 1px solid var(--color-border); border-radius: var(--radius-sm); cursor: pointer; } - .retention-controls button:hover { + .host-picker > button:hover:not(:disabled) { border-color: var(--color-border-strong); } - .retention-controls button:disabled { + .host-picker > button:disabled { cursor: default; opacity: 0.6; }