diff --git a/client/api.go b/client/api.go index a7317f5..0cd7653 100644 --- a/client/api.go +++ b/client/api.go @@ -205,8 +205,10 @@ func (c *Client) ListAcks(ctx context.Context) ([]AckRecord, error) { return out, nil } -// AckFinding records a finding acknowledgement. It requires a bearer token -// unless the server runs with authentication disabled. +// AckFinding records a finding acknowledgement. The fingerprint must belong to +// a finding in the latest report, or the server answers 404, and the caller's +// key must reach that finding's cluster. It requires a bearer token unless the +// server runs with authentication disabled. func (c *Client) AckFinding(ctx context.Context, fingerprint string, req AckRequest) (*AckRecord, error) { var out AckRecord path := "/api/findings/" + url.PathEscape(fingerprint) + "/ack" diff --git a/client/client_test.go b/client/client_test.go index b096571..4820d6c 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -56,8 +56,8 @@ func newContractServer(t *testing.T, insecure bool, token string) (*client.Clien func seedScan(t *testing.T, st store.Store) string { t.Helper() results := map[string]map[string]scanner.Result{ - "prod-a": {"version": {Scanner: "version", Data: map[string]any{"gitVersion": "v1.29.0"}}}, - "prod-b": {"version": {Scanner: "version", Data: map[string]any{"gitVersion": "v1.28.3"}}}, + "prod-a": {"version": {Scanner: "version", Data: map[string]any{"git_version": "v1.29.0"}}}, + "prod-b": {"version": {Scanner: "version", Data: map[string]any{"git_version": "v1.28.3"}}}, } id, err := st.SaveScan(context.Background(), []string{"prod-a", "prod-b"}, results) if err != nil { @@ -203,13 +203,26 @@ func TestContract(t *testing.T) { }) t.Run("findings ack", func(t *testing.T) { - const fp = "fp-contract-1" + report, err := c.GetScanReport(ctx, scanID) + if err != nil { + t.Fatalf("get scan report: %v", err) + } + if len(report.Findings) == 0 { + t.Fatal("seeded report has no findings to ack") + } + f := report.Findings[0] + fp := f.Fingerprint ack, err := c.AckFinding(ctx, fp, client.AckRequest{AckBy: "tester", Reason: "known noise"}) if err != nil { t.Fatalf("ack finding: %v", err) } - if ack.Fingerprint != fp { - t.Errorf("ack fingerprint = %q, want %q", ack.Fingerprint, fp) + want := client.AckRecord{ + Fingerprint: fp, Cluster: f.Cluster, Scanner: f.Scanner, Title: f.Title, + AckBy: "tester", Reason: "known noise", + } + ignore := cmpopts.IgnoreFields(client.AckRecord{}, "CreatedAt") + if diff := cmp.Diff(want, *ack, ignore); diff != "" { + t.Errorf("ack mismatch (-want +got):\n%s", diff) } acks, err := c.ListAcks(ctx) if err != nil { @@ -273,6 +286,17 @@ func TestContract(t *testing.T) { } }) + t.Run("ack finding unknown", func(t *testing.T) { + _, err := c.AckFinding(ctx, "fp-unknown", client.AckRequest{AckBy: "tester"}) + var apiErr *client.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("ack finding: want *APIError, got %v", err) + } + if apiErr.StatusCode != http.StatusNotFound { + t.Errorf("status = %d, want %d", apiErr.StatusCode, http.StatusNotFound) + } + }) + t.Run("ack alert unknown", func(t *testing.T) { _, err := c.AckAlert(ctx, "fp-unknown", client.AckAlertRequest{AckBy: "tester"}) var apiErr *client.APIError diff --git a/client/types.go b/client/types.go index f8d2188..4d8df7a 100644 --- a/client/types.go +++ b/client/types.go @@ -358,14 +358,9 @@ type IntegrationStatus struct { Hint string `json:"hint,omitempty"` } -// AckRequest is the body for acknowledging a finding. +// AckRequest is the body for acknowledging a finding. The server takes the +// cluster, scanner, and title from the finding it holds. type AckRequest struct { - // Cluster narrows the ack to one cluster. - Cluster string `json:"cluster,omitempty"` - // Scanner narrows the ack to one scanner. - Scanner string `json:"scanner,omitempty"` - // Title narrows the ack to one finding title. - Title string `json:"title,omitempty"` // AckBy identifies who acknowledged the finding. AckBy string `json:"ack_by,omitempty"` // Reason records why the finding was acknowledged. diff --git a/docs/operator/rbac.md b/docs/operator/rbac.md index 497bad6..47e7d9c 100644 --- a/docs/operator/rbac.md +++ b/docs/operator/rbac.md @@ -14,7 +14,7 @@ Fleetsweeper has two authentication paths, designed to coexist: | Role | Permissions | | --- | --- | | `admin` | Everything, including key management and audit log. | -| `operator` | Trigger scans, acknowledge findings, manage groups and locations within scope. Cannot manage keys. | +| `operator` | Trigger scans, acknowledge findings, and manage tags and locations within scope. Cannot manage groups or keys. | | `viewer` | Read-only. Cannot reach `/admin/*`. | Read-only endpoints (GET) are open to viewers and above (or to anyone when @@ -33,14 +33,38 @@ Each key carries `cluster_scope`, a JSON array. The semantics are: Cluster scope is enforced on: -- `POST /api/scans` (contexts list is filtered; a request becomes 403 only - when no contexts remain). +- `POST /api/scans`. The contexts list is filtered to the key's scope. A + request becomes 403 only when no contexts remain. - `PUT /api/locations/{cluster}` and `DELETE /api/locations/{cluster}`. -- `POST /api/findings/{fingerprint}/ack` (when the request body carries a - `cluster` field). - -Group management (`POST /api/groups`, `DELETE /api/groups/{name}`) is -admin-only because groups cross cluster boundaries. +- `PUT /api/clusters/{name}/tags/{key}` and + `DELETE /api/clusters/{name}/tags/{key}`. +- `POST /api/whatif`, against the cluster the request names. +- `POST /api/findings/{fingerprint}/ack` and + `DELETE /api/findings/{fingerprint}/ack`. +- `POST /api/alerts/{fingerprint}/ack`, against the cluster stored on the + alert. An alert with no cluster needs a key with `*` scope. + +Acks resolve the cluster on the server. The ack body carries only `ack_by`, +`reason`, and `snooze_until`, and a body that names `cluster`, `scanner`, or +`title` is rejected with 400. To create an ack, the server finds the finding +with that fingerprint in the latest report and checks the key's scope against +the finding's cluster. A fingerprint that no current finding carries returns +404. To remove an ack, the server checks the cluster stored on the ack. A +fleet-scoped finding or ack (cluster `fleet`) needs a key with `*` scope, so a +key scoped to `prod-east` cannot silence a fleet-wide finding. + +Group management (`POST /api/groups`, `DELETE /api/groups/{name}`) needs +the `admin` role. An operator is refused even with `*` scope. A group widens +every key scoped to it, so defining one is an admin act. + +Cluster scope limits writes. Most read endpoints are not scoped, so a key +scoped to `prod-west` can still read the findings and full scanner results of +`prod-east`. Treat any caller that can read the API as able to see the whole +fleet. + +`POST /api/webhooks/scan-trigger` takes no API key. Its HMAC secret is the +authority, and a request signed with that secret may scan any context. Guard +the secret as you would an admin token. ## Creating a key diff --git a/docs/reference/api.md b/docs/reference/api.md index 727d81e..1b37ad6 100644 --- a/docs/reference/api.md +++ b/docs/reference/api.md @@ -58,7 +58,7 @@ Paths below are under `/api` unless they start with a root path. | GET | `/integrations` | Which optional integrations are on. | | GET | `/scanners` | Every scanner with its label, description, and category. | | GET | `/acks` | Active finding acknowledgements. | -| POST, DELETE | `/findings/{fingerprint}/ack` | Acknowledge a finding or remove the acknowledgement. | +| POST, DELETE | `/findings/{fingerprint}/ack` | Acknowledge a finding in the latest report or remove the acknowledgement. The server takes the cluster, scanner, and title from the finding. | | GET | `/findings/timeline`, `/findings/persistence` | Findings over recent scans and how often each recurs. | | GET | `/alerts` | Inbound AlertManager and Falco alerts, each with its ack state. | | POST | `/alerts/{fingerprint}/ack` | Acknowledge an inbound alert. | diff --git a/internal/server/acks_handler.go b/internal/server/acks_handler.go index 84ee0cf..8cbbaea 100644 --- a/internal/server/acks_handler.go +++ b/internal/server/acks_handler.go @@ -1,23 +1,22 @@ package server import ( + "context" + "errors" "net/http" "time" "go.uber.org/zap" + "github.com/dcadolph/fleetsweeper/internal/report" "github.com/dcadolph/fleetsweeper/internal/store" ) -// ackRequest is the JSON body for POST /api/findings/{fingerprint}/ack. +// ackRequest is the optional JSON body for POST /api/findings/{fingerprint}/ack +// and POST /api/alerts/{fingerprint}/ack. The server takes cluster, scanner, and +// title from the finding or alert it holds, so the client supplies only the +// human context. type ackRequest struct { - // Cluster echoes the finding's cluster scope. Optional; helps when - // listing acks back. - Cluster string `json:"cluster"` - // Scanner echoes the finding's source scanner. Optional. - Scanner string `json:"scanner"` - // Title echoes the finding's title at ack time. Optional but recommended. - Title string `json:"title"` // AckBy identifies the operator. Optional, free-form. AckBy string `json:"ack_by"` // Reason is the operator's stated reason. Optional, free-form. @@ -41,66 +40,38 @@ func (s *Server) handleListAcks(w http.ResponseWriter, r *http.Request) { // handleCreateAck records (or refreshes) an acknowledgement for the finding // identified by the fingerprint in the URL path. The fingerprint is the // finding's served fingerprint field, the SHA-256 of cluster|scanner|rule, so -// the ack keeps matching when a count in the finding's title changes. +// the ack keeps matching when a count in the finding's title changes. The +// finding must be in the latest report: its cluster decides the scope check +// and its cluster, scanner, and title are what the ack stores. func (s *Server) handleCreateAck(w http.ResponseWriter, r *http.Request) { fp := r.PathValue("fingerprint") if fp == "" { writeError(w, http.StatusBadRequest, "fingerprint required") return } - var req ackRequest - if err := decodeJSON(w, r, &req, maxJSONBody); err != nil { + f, err := s.currentFinding(r.Context(), fp) + if err != nil { + s.writeStoreError(w, err, "acks: find finding", "no current finding has this fingerprint") return } - if req.Cluster != "" && req.Cluster != "fleet" { - if !actorFromContext(r.Context()).AllowsCluster(req.Cluster, s.groupLookup(r.Context())) { - writeError(w, http.StatusForbidden, "cluster not in actor scope") - return - } - } - rec := store.AckRecord{ - Fingerprint: fp, - Cluster: req.Cluster, - Scanner: req.Scanner, - Title: req.Title, - AckBy: req.AckBy, - Reason: req.Reason, - CreatedAt: time.Now().UTC(), - } - if req.SnoozeUntil != "" { - t, err := time.Parse(time.RFC3339, req.SnoozeUntil) - if err != nil { - writeError(w, http.StatusBadRequest, "snooze_until must be RFC3339") - return - } - rec.SnoozeUntil = t - } - if err := s.store.SaveAck(r.Context(), rec); err != nil { - s.log.Warn("acks: save", zap.Error(err)) - writeError(w, http.StatusInternalServerError, "save ack failed") + if !s.allowsAckCluster(r.Context(), f.Cluster) { + writeError(w, http.StatusForbidden, "finding cluster not in actor scope") return } - writeJSON(w, http.StatusCreated, rec) -} - -// alertAckRequest is the smaller body the alert-ack endpoint takes — -// the cluster/title/scanner fields are derived server-side from the -// alert row so the client only supplies the human-supplied context. -type alertAckRequest struct { - // AckBy identifies the operator. Optional, free-form. - AckBy string `json:"ack_by"` - // Reason is the operator's stated reason. Optional, free-form. - Reason string `json:"reason"` - // SnoozeUntil, when set, is an RFC3339 timestamp at which the ack - // expires. Omit for a permanent ack. - SnoozeUntil string `json:"snooze_until,omitempty"` + s.saveAck(w, r, store.AckRecord{ + Fingerprint: fp, + Cluster: f.Cluster, + Scanner: f.Scanner, + Title: f.Title, + }) } // handleAckAlert records an ack against an inbound alert. The alert // row is fetched by fingerprint so the cluster, title, and source tag -// don't have to be re-supplied by the client. The same `finding_acks` -// table is reused — alerts share the snooze + tombstone machinery with -// scan findings so the dashboard surfaces them uniformly. +// don't have to be re-supplied by the client, and the scope check reads +// the alert's cluster the same way the ack delete reads it later. The same +// `finding_acks` table is reused: alerts share the snooze and tombstone +// machinery with scan findings so the dashboard surfaces them uniformly. func (s *Server) handleAckAlert(w http.ResponseWriter, r *http.Request) { fp := r.PathValue("fingerprint") if fp == "" { @@ -112,51 +83,41 @@ func (s *Server) handleAckAlert(w http.ResponseWriter, r *http.Request) { s.writeStoreError(w, err, "acks: get alert", "alert not found") return } - if alert.Cluster != "" { - if !actorFromContext(r.Context()).AllowsCluster(alert.Cluster, s.groupLookup(r.Context())) { - writeError(w, http.StatusForbidden, "alert cluster not in actor scope") - return - } - } - var req alertAckRequest - if r.ContentLength != 0 { - if err := decodeJSON(w, r, &req, maxJSONBody); err != nil { - return - } + if !s.allowsAckCluster(r.Context(), alert.Cluster) { + writeError(w, http.StatusForbidden, "alert cluster not in actor scope") + return } - rec := store.AckRecord{ + s.saveAck(w, r, store.AckRecord{ Fingerprint: fp, Cluster: alert.Cluster, Scanner: "alert:" + firstNonEmpty(alert.Labels["source"], "alertmanager"), Title: alert.AlertName, - AckBy: req.AckBy, - Reason: req.Reason, - CreatedAt: time.Now().UTC(), - } - if req.SnoozeUntil != "" { - t, err := time.Parse(time.RFC3339, req.SnoozeUntil) - if err != nil { - writeError(w, http.StatusBadRequest, "snooze_until must be RFC3339") - return - } - rec.SnoozeUntil = t - } - if err := s.store.SaveAck(r.Context(), rec); err != nil { - s.log.Warn("acks: alert save", zap.Error(err)) - writeError(w, http.StatusInternalServerError, "save ack failed") - return - } - writeJSON(w, http.StatusCreated, rec) + }) } -// handleDeleteAck removes the acknowledgement for the given fingerprint. -// Idempotent: a missing row returns 204 just like a successful delete. +// handleDeleteAck removes the acknowledgement for the given fingerprint. The +// scope check reads the cluster stored on the ack row. Idempotent: a missing +// row returns 204 just like a successful delete. func (s *Server) handleDeleteAck(w http.ResponseWriter, r *http.Request) { fp := r.PathValue("fingerprint") if fp == "" { writeError(w, http.StatusBadRequest, "fingerprint required") return } + rec, err := s.store.GetAck(r.Context(), fp) + switch { + case errors.Is(err, store.ErrNotFound): + w.WriteHeader(http.StatusNoContent) + return + case err != nil: + s.log.Warn("acks: get", zap.Error(err)) + writeError(w, http.StatusInternalServerError, "get ack failed") + return + } + if !s.allowsAckCluster(r.Context(), rec.Cluster) { + writeError(w, http.StatusForbidden, "ack cluster not in actor scope") + return + } if err := s.store.DeleteAck(r.Context(), fp); err != nil { s.log.Warn("acks: delete", zap.Error(err)) writeError(w, http.StatusInternalServerError, "delete ack failed") @@ -164,3 +125,60 @@ func (s *Server) handleDeleteAck(w http.ResponseWriter, r *http.Request) { } w.WriteHeader(http.StatusNoContent) } + +// currentFinding returns the finding in the latest report whose fingerprint is +// fp. Returns store.ErrNotFound when no scan exists or no finding carries it. +func (s *Server) currentFinding(ctx context.Context, fp string) (report.Finding, error) { + rpt, err := s.latestReport(ctx) + if err != nil { + return report.Finding{}, err + } + if rpt != nil { + for _, f := range rpt.Findings { + if f.Fingerprint == fp { + return f, nil + } + } + } + return report.Finding{}, store.ErrNotFound +} + +// allowsAckCluster reports whether the request's actor may change an ack on +// cluster. A fleet-scoped or cluster-less ack spans every cluster, so it needs +// wildcard scope. +func (s *Server) allowsAckCluster(ctx context.Context, cluster string) bool { + actor := actorFromContext(ctx) + if cluster == "" || cluster == "fleet" { + return actor.HasWildcardScope() + } + return actor.AllowsCluster(cluster, s.groupLookup(ctx)) +} + +// saveAck decodes the optional ack body into base, persists the record, and +// writes it back with 201. The caller fills base from a server-owned object and +// checks scope first, so the body cannot change what the ack covers. +func (s *Server) saveAck(w http.ResponseWriter, r *http.Request, base store.AckRecord) { + var req ackRequest + if r.ContentLength != 0 { + if err := decodeJSON(w, r, &req, maxJSONBody); err != nil { + return + } + } + base.AckBy = req.AckBy + base.Reason = req.Reason + base.CreatedAt = time.Now().UTC() + if req.SnoozeUntil != "" { + t, err := time.Parse(time.RFC3339, req.SnoozeUntil) + if err != nil { + writeError(w, http.StatusBadRequest, "snooze_until must be RFC3339") + return + } + base.SnoozeUntil = t + } + if err := s.store.SaveAck(r.Context(), base); err != nil { + s.log.Warn("acks: save", zap.Error(err)) + writeError(w, http.StatusInternalServerError, "save ack failed") + return + } + writeJSON(w, http.StatusCreated, base) +} diff --git a/internal/server/acks_handler_test.go b/internal/server/acks_handler_test.go index 0509f0a..1897817 100644 --- a/internal/server/acks_handler_test.go +++ b/internal/server/acks_handler_test.go @@ -3,12 +3,16 @@ package server import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "strings" "testing" "time" + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + "github.com/dcadolph/fleetsweeper/internal/report" "github.com/dcadolph/fleetsweeper/internal/scanner" "github.com/dcadolph/fleetsweeper/internal/store" @@ -36,13 +40,13 @@ func TestHandleListAcks_ReturnsStored(t *testing.T) { } // TestHandleCreateAck_Happy verifies a valid ack is persisted with the -// fingerprint taken from the path. +// fingerprint taken from the path and the scope fields taken from the finding. func TestHandleCreateAck_Happy(t *testing.T) { t.Parallel() srv, ss := testServer(t) - body := `{"cluster":"east","scanner":"version","title":"old","ack_by":"op","reason":"known"}` - req := httptest.NewRequest(http.MethodPost, "/api/findings/fp-xyz/ack", - strings.NewReader(body)) + east, _ := seedAckFindings(t, srv, ss) + req := httptest.NewRequest(http.MethodPost, "/api/findings/"+east.Fingerprint+"/ack", + strings.NewReader(`{"ack_by":"op","reason":"known"}`)) req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() srv.mux.ServeHTTP(w, req) @@ -53,8 +57,12 @@ func TestHandleCreateAck_Happy(t *testing.T) { if err != nil { t.Fatalf("list: %v", err) } - if len(acks) != 1 || acks[0].Fingerprint != "fp-xyz" || acks[0].AckBy != "op" { - t.Errorf("unexpected acks: %+v", acks) + want := []store.AckRecord{{ + Fingerprint: east.Fingerprint, Cluster: "east", Scanner: "workload-security", + Title: east.Title, AckBy: "op", Reason: "known", + }} + if diff := cmp.Diff(want, acks, cmpopts.IgnoreFields(store.AckRecord{}, "CreatedAt")); diff != "" { + t.Errorf("acks mismatch (-want +got):\n%s", diff) } } @@ -63,9 +71,10 @@ func TestHandleCreateAck_Happy(t *testing.T) { func TestHandleCreateAck_SnoozeRoundTrip(t *testing.T) { t.Parallel() srv, ss := testServer(t) + east, _ := seedAckFindings(t, srv, ss) snooze := time.Now().Add(time.Hour).UTC().Format(time.RFC3339) - body := `{"cluster":"east","title":"t","snooze_until":"` + snooze + `"}` - req := httptest.NewRequest(http.MethodPost, "/api/findings/fp-snooze/ack", + body := `{"snooze_until":"` + snooze + `"}` + req := httptest.NewRequest(http.MethodPost, "/api/findings/"+east.Fingerprint+"/ack", strings.NewReader(body)) req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() @@ -79,39 +88,35 @@ func TestHandleCreateAck_SnoozeRoundTrip(t *testing.T) { } } -// TestHandleCreateAck_Validation covers the ack-create 400/403 branches. +// TestHandleCreateAck_Validation covers the ack-create 400 branches. func TestHandleCreateAck_Validation(t *testing.T) { t.Parallel() tests := []struct { - Name string - Fingerprint string - Body string - Actor *Actor - WantStatus int + Name string + Path bool + Body string + WantStatus int }{{ // Test 0: Empty fingerprint. - Name: "empty fp", Fingerprint: "", Body: `{}`, Actor: adminActor(), - WantStatus: http.StatusBadRequest, + Name: "empty fp", Body: `{}`, WantStatus: http.StatusBadRequest, }, { // Test 1: Malformed JSON. - Name: "bad json", Fingerprint: "fp", Body: "{", Actor: adminActor(), - WantStatus: http.StatusBadRequest, - }, { // Test 2: Cluster outside actor scope. - Name: "scope reject", Fingerprint: "fp", Body: `{"cluster":"east"}`, - Actor: &Actor{ID: "s", Role: store.RoleOperator, ClusterScope: []string{"west"}}, - WantStatus: http.StatusForbidden, - }, { // Test 3: Malformed snooze timestamp. - Name: "bad snooze", Fingerprint: "fp", Body: `{"cluster":"fleet","snooze_until":"nope"}`, - Actor: adminActor(), + Name: "bad json", Path: true, Body: "{", WantStatus: http.StatusBadRequest, + }, { // Test 2: Malformed snooze timestamp. + Name: "bad snooze", Path: true, Body: `{"snooze_until":"nope"}`, WantStatus: http.StatusBadRequest, }} for _, test := range tests { t.Run(test.Name, func(t *testing.T) { t.Parallel() - srv, _ := testServer(t) - req := httptest.NewRequest(http.MethodPost, - "/api/findings/"+test.Fingerprint+"/ack", + srv, ss := testServer(t) + east, _ := seedAckFindings(t, srv, ss) + fp := "" + if test.Path { + fp = east.Fingerprint + } + req := httptest.NewRequest(http.MethodPost, "/api/findings/"+fp+"/ack", strings.NewReader(test.Body)) - req.SetPathValue("fingerprint", test.Fingerprint) - req = req.WithContext(withActor(req.Context(), test.Actor)) + req.SetPathValue("fingerprint", fp) + req = req.WithContext(withActor(req.Context(), adminActor())) w := httptest.NewRecorder() srv.handleCreateAck(w, req) if w.Code != test.WantStatus { @@ -191,8 +196,7 @@ func TestAckSurvivesCountChange(t *testing.T) { if first.Acked { t.Fatal("finding is acked before any ack") } - body := `{"cluster":"a","scanner":"workload-security","title":"` + first.Title + `"}` - req := httptest.NewRequest(http.MethodPost, "/api/findings/"+first.Fingerprint+"/ack", strings.NewReader(body)) + req := httptest.NewRequest(http.MethodPost, "/api/findings/"+first.Fingerprint+"/ack", strings.NewReader(`{}`)) w := httptest.NewRecorder() srv.mux.ServeHTTP(w, req) if w.Code != http.StatusCreated { @@ -207,3 +211,177 @@ func TestAckSurvivesCountChange(t *testing.T) { t.Errorf("finding %q is not acked at the new count", second.Title) } } + +// seedAckFindings saves one scan over east and west whose report carries a +// privileged-container finding on east and a version skew finding on the +// fleet, and returns both findings. +func seedAckFindings(t *testing.T, srv *Server, ss *store.SQLite) (report.Finding, report.Finding) { + t.Helper() + ctx := context.Background() + version := func(v string) scanner.Result { + return scanner.Result{Scanner: "version", State: scanner.StateOK, Data: map[string]any{"git_version": v}} + } + results := map[string]map[string]scanner.Result{ + "east": { + "version": version("v1.29.0"), + "workload-security": {Scanner: "workload-security", State: scanner.StateOK, Data: map[string]any{ + "privileged_containers": 3, "total_pods": 10, + }}, + }, + "west": {"version": version("v1.28.3")}, + } + if _, err := ss.SaveScan(ctx, []string{"east", "west"}, results); err != nil { + t.Fatalf("seed scan: %v", err) + } + rpt := srv.latestReportForMetrics(ctx) + if rpt == nil { + t.Fatal("no latest report") + } + var east, fleet report.Finding + for _, f := range rpt.Findings { + switch { + case f.Cluster == "east" && f.Rule == "workload-security/privileged": + east = f + case f.Cluster == "fleet" && f.Rule == "version/skew": + fleet = f + } + } + if east.Fingerprint == "" || fleet.Fingerprint == "" { + t.Fatalf("seeded report lacks the east or fleet finding: %+v", rpt.Findings) + } + return east, fleet +} + +// scopedOperator returns an operator Actor limited to the given cluster scope. +func scopedOperator(scope ...string) *Actor { + return &Actor{ID: "op", Name: "op", Role: store.RoleOperator, ClusterScope: scope} +} + +// TestHandleCreateAck_ServerOwnedScope verifies the ack route takes cluster, +// scanner, and title from the finding the server holds, checks the actor's +// scope against that cluster, and refuses fingerprints no finding carries. +func TestHandleCreateAck_ServerOwnedScope(t *testing.T) { + t.Parallel() + tests := []struct { + Actor *Actor + Target string + Body string + WantStatus int + WantStored bool + }{{ // Test 0: Out-of-scope operator omits the cluster from the body. + Actor: scopedOperator("west"), Target: "east", Body: `{}`, WantStatus: http.StatusForbidden, + }, { // Test 1: Out-of-scope operator names the fleet in the body. + Actor: scopedOperator("west"), Target: "east", Body: `{"cluster":"fleet"}`, + WantStatus: http.StatusForbidden, + }, { // Test 2: Out-of-scope operator names its own cluster in the body. + Actor: scopedOperator("west"), Target: "east", Body: `{"cluster":"west"}`, + WantStatus: http.StatusForbidden, + }, { // Test 3: Cluster-scoped operator acks a fleet-scoped finding. + Actor: scopedOperator("east"), Target: "fleet", Body: `{}`, WantStatus: http.StatusForbidden, + }, { // Test 4: In-scope operator acks its cluster's finding. + Actor: scopedOperator("east"), Target: "east", Body: `{"ack_by":"op","reason":"known"}`, + WantStatus: http.StatusCreated, WantStored: true, + }, { // Test 5: Wildcard operator acks a fleet-scoped finding. + Actor: scopedOperator(store.ScopeWildcard), Target: "fleet", Body: `{"reason":"skew"}`, + WantStatus: http.StatusCreated, WantStored: true, + }, { // Test 6: Fingerprint no current finding carries. + Actor: adminActor(), Target: "unknown", Body: `{}`, WantStatus: http.StatusNotFound, + }, { // Test 7: In-scope operator still may not name a cluster in the body. + Actor: scopedOperator("east"), Target: "east", Body: `{"cluster":"east"}`, + WantStatus: http.StatusBadRequest, + }, { // Test 8: Empty body is accepted. + Actor: scopedOperator("east"), Target: "east", Body: "", + WantStatus: http.StatusCreated, WantStored: true, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + srv, ss := testServer(t) + east, fleet := seedAckFindings(t, srv, ss) + target := map[string]report.Finding{ + "east": east, "fleet": fleet, + "unknown": {Fingerprint: "fp-unknown", Cluster: "east", Scanner: "spoof", Title: "spoof"}, + }[test.Target] + req := httptest.NewRequest(http.MethodPost, "/api/findings/"+target.Fingerprint+"/ack", + strings.NewReader(test.Body)) + req.SetPathValue("fingerprint", target.Fingerprint) + req = req.WithContext(withActor(req.Context(), test.Actor)) + w := httptest.NewRecorder() + srv.handleCreateAck(w, req) + if w.Code != test.WantStatus { + t.Fatalf("want %d, got %d body=%s", test.WantStatus, w.Code, w.Body.String()) + } + acks, err := ss.ListAcks(context.Background()) + if err != nil { + t.Fatalf("list acks: %v", err) + } + var want []store.AckRecord + if test.WantStored { + want = []store.AckRecord{{ + Fingerprint: target.Fingerprint, Cluster: target.Cluster, + Scanner: target.Scanner, Title: target.Title, + }} + } + ignore := cmpopts.IgnoreFields(store.AckRecord{}, "AckBy", "Reason", "SnoozeUntil", "CreatedAt") + if diff := cmp.Diff(want, acks, ignore, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("stored acks mismatch (-want +got):\n%s", diff) + } + }) + } +} + +// TestHandleDeleteAck_ServerOwnedScope verifies removing an ack checks the +// actor's scope against the cluster stored on the ack row. +func TestHandleDeleteAck_ServerOwnedScope(t *testing.T) { + t.Parallel() + tests := []struct { + Actor *Actor + Fingerprint string + WantStatus int + WantAcked bool + }{{ // Test 0: Out-of-scope operator removes another cluster's ack. + Actor: scopedOperator("west"), Fingerprint: "fp-east", + WantStatus: http.StatusForbidden, WantAcked: true, + }, { // Test 1: In-scope operator removes its cluster's ack. + Actor: scopedOperator("east"), Fingerprint: "fp-east", WantStatus: http.StatusNoContent, + }, { // Test 2: Cluster-scoped operator removes a fleet-scoped ack. + Actor: scopedOperator("east"), Fingerprint: "fp-fleet", + WantStatus: http.StatusForbidden, WantAcked: true, + }, { // Test 3: Wildcard operator removes a fleet-scoped ack. + Actor: scopedOperator(store.ScopeWildcard), Fingerprint: "fp-fleet", + WantStatus: http.StatusNoContent, + }, { // Test 4: Missing ack stays idempotent. + Actor: scopedOperator("west"), Fingerprint: "fp-none", WantStatus: http.StatusNoContent, + }, { // Test 5: Ack with no cluster needs wildcard scope. + Actor: scopedOperator("east"), Fingerprint: "fp-blank", + WantStatus: http.StatusForbidden, WantAcked: true, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + srv, ss := testServer(t) + ctx := context.Background() + seeds := map[string]string{"fp-east": "east", "fp-fleet": "fleet", "fp-blank": ""} + for fp, cluster := range seeds { + if err := ss.SaveAck(ctx, store.AckRecord{Fingerprint: fp, Cluster: cluster}); err != nil { + t.Fatalf("seed ack: %v", err) + } + } + req := httptest.NewRequest(http.MethodDelete, "/api/findings/"+test.Fingerprint+"/ack", nil) + req.SetPathValue("fingerprint", test.Fingerprint) + req = req.WithContext(withActor(req.Context(), test.Actor)) + w := httptest.NewRecorder() + srv.handleDeleteAck(w, req) + if w.Code != test.WantStatus { + t.Fatalf("want %d, got %d body=%s", test.WantStatus, w.Code, w.Body.String()) + } + acked, err := ss.IsAcked(ctx, test.Fingerprint) + if err != nil { + t.Fatalf("is acked: %v", err) + } + if acked != test.WantAcked { + t.Errorf("acked = %v, want %v", acked, test.WantAcked) + } + }) + } +} diff --git a/internal/server/auth.go b/internal/server/auth.go index 7d5512d..de0a8ec 100644 --- a/internal/server/auth.go +++ b/internal/server/auth.go @@ -35,9 +35,9 @@ func (a *Actor) HasWildcardScope() bool { } // CanManageGroups reports whether the actor may create or delete groups. A group widens every -// key scoped to it, so only an admin or an actor that already reaches every cluster may define one. +// key scoped to it, so defining one is an admin act whatever the actor's cluster scope. func (a *Actor) CanManageGroups() bool { - return a.Role == store.RoleAdmin || a.HasWildcardScope() + return a.Role == store.RoleAdmin } // AllowsCluster reports whether the actor may act on the given cluster. diff --git a/internal/server/auth_test.go b/internal/server/auth_test.go index dffd29f..09a02a5 100644 --- a/internal/server/auth_test.go +++ b/internal/server/auth_test.go @@ -91,12 +91,12 @@ func TestAuthMiddlewareAPIKey(t *testing.T) { t.Fatalf("save api key: %v", err) } - req := httptest.NewRequest(http.MethodPost, "/api/groups", strings.NewReader(`{"name":"x","clusters":[]}`)) + req := httptest.NewRequest(http.MethodPut, "/api/locations/east", strings.NewReader(`{"lat":1,"lng":2}`)) req.Header.Set("Authorization", "Bearer "+raw) w := httptest.NewRecorder() srv.mux.ServeHTTP(w, req) - if w.Code != http.StatusCreated { - t.Fatalf("create group: want 201, got %d body %s", w.Code, w.Body.String()) + if w.Code != http.StatusOK { + t.Fatalf("set location: want 200, got %d body %s", w.Code, w.Body.String()) } } @@ -279,9 +279,9 @@ func saveTestKey(t *testing.T, st *store.SQLite, id, role string, scope ...strin return raw } -// TestGroupWritesRequireFleetScope verifies only an admin or a wildcard-scoped actor may create -// or delete a group, and that a refused write leaves the group untouched. -func TestGroupWritesRequireFleetScope(t *testing.T) { +// TestGroupWritesRequireAdmin verifies only an admin may create or delete a group, whatever the +// caller's cluster scope, and that a refused write leaves the group untouched. +func TestGroupWritesRequireAdmin(t *testing.T) { t.Parallel() tests := []struct { Role string @@ -294,9 +294,9 @@ func TestGroupWritesRequireFleetScope(t *testing.T) { }, { // Test 1: A cluster-scoped operator cannot define groups. Role: store.RoleOperator, Scope: []string{"edge-a"}, WantCreateStatus: http.StatusForbidden, WantDeleteStatus: http.StatusForbidden, - }, { // Test 2: A wildcard operator may manage groups. + }, { // Test 2: A wildcard operator cannot define groups either. Role: store.RoleOperator, Scope: []string{store.ScopeWildcard}, - WantCreateStatus: http.StatusCreated, WantDeleteStatus: http.StatusNoContent, + WantCreateStatus: http.StatusForbidden, WantDeleteStatus: http.StatusForbidden, }, { // Test 3: An admin may manage groups even with a narrow scope. Role: store.RoleAdmin, Scope: []string{"group:edge"}, WantCreateStatus: http.StatusCreated, WantDeleteStatus: http.StatusNoContent, @@ -351,7 +351,7 @@ func TestCanManageGroups(t *testing.T) { }, { // Test 1: Admin with a narrow scope. Actor: Actor{Role: store.RoleAdmin, ClusterScope: []string{"east"}}, WantResult: true, }, { // Test 2: Operator with wildcard scope. - Actor: Actor{Role: store.RoleOperator, ClusterScope: []string{"east", store.ScopeWildcard}}, WantResult: true, + Actor: Actor{Role: store.RoleOperator, ClusterScope: []string{"east", store.ScopeWildcard}}, WantResult: false, }, { // Test 3: Operator with a group scope. Actor: Actor{Role: store.RoleOperator, ClusterScope: []string{"group:prod"}}, WantResult: false, }, { // Test 4: Anonymous viewer with no scope. diff --git a/internal/server/e2e_demo_test.go b/internal/server/e2e_demo_test.go index a47e8cf..7f89e68 100644 --- a/internal/server/e2e_demo_test.go +++ b/internal/server/e2e_demo_test.go @@ -13,6 +13,7 @@ import ( "github.com/google/go-cmp/cmp" "go.uber.org/zap" + "github.com/dcadolph/fleetsweeper/internal/report" "github.com/dcadolph/fleetsweeper/internal/scanner" "github.com/dcadolph/fleetsweeper/internal/store" ) @@ -181,8 +182,10 @@ type mutationCase struct { var demoMutations = []mutationCase{ //nolint:gochecknoglobals // Not modified, simplifies testing. {Name: "trigger", Method: http.MethodPost, Path: "/api/scans", Body: `{"all_contexts":true}`, WantDemo: http.StatusAccepted}, - {Name: "ack", Method: http.MethodPost, Path: "/api/findings/abc123/ack", - Body: `{"cluster":"fleet","reason":"demo"}`, WantDemo: http.StatusCreated}, + {Name: "ack", Method: http.MethodPost, Path: "/api/findings/" + + report.FindingFingerprint(report.Finding{ + Cluster: "prod-eu-central-1", Scanner: "deprecated-apis", Rule: "deprecated-apis/in-use", + }) + "/ack", Body: `{"reason":"demo"}`, WantDemo: http.StatusCreated}, {Name: "tag", Method: http.MethodPut, Path: "/api/clusters/prod-us-east-1/tags/cohort", Body: `{"value":"edge"}`, WantDemo: http.StatusOK}, } diff --git a/internal/server/handlers.go b/internal/server/handlers.go index 4472819..3b0d5b1 100644 --- a/internal/server/handlers.go +++ b/internal/server/handlers.go @@ -429,7 +429,7 @@ type createGroupRequest struct { // it, because a group-scoped key gains whatever clusters its group lists. func (s *Server) handleCreateGroup(w http.ResponseWriter, r *http.Request) { if !actorFromContext(r.Context()).CanManageGroups() { - writeError(w, http.StatusForbidden, "admin role or wildcard scope required") + writeError(w, http.StatusForbidden, "admin role required") return } var req createGroupRequest @@ -453,7 +453,7 @@ func (s *Server) handleCreateGroup(w http.ResponseWriter, r *http.Request) { // handleCreateGroup, since deleting a group revokes every key scoped to it. func (s *Server) handleDeleteGroup(w http.ResponseWriter, r *http.Request) { if !actorFromContext(r.Context()).CanManageGroups() { - writeError(w, http.StatusForbidden, "admin role or wildcard scope required") + writeError(w, http.StatusForbidden, "admin role required") return } name := r.PathValue("name") diff --git a/internal/server/handlers_clusters_tags_test.go b/internal/server/handlers_clusters_tags_test.go index 06db20b..69d3f94 100644 --- a/internal/server/handlers_clusters_tags_test.go +++ b/internal/server/handlers_clusters_tags_test.go @@ -9,6 +9,7 @@ import ( "strings" "testing" + "github.com/dcadolph/fleetsweeper/internal/report" "github.com/dcadolph/fleetsweeper/internal/scanner" "github.com/dcadolph/fleetsweeper/internal/store" ) @@ -55,6 +56,9 @@ func TestListClusters_IncludesTags(t *testing.T) { func TestJSONBodyLimits(t *testing.T) { t.Parallel() huge := `{"name":"` + strings.Repeat("a", 2<<20) + `"}` + eastFP := report.FindingFingerprint(report.Finding{ + Cluster: "east", Scanner: "workload-security", Rule: "workload-security/privileged", + }) tests := []struct { Method string Path string @@ -67,7 +71,7 @@ func TestJSONBodyLimits(t *testing.T) { }, { // Test 3: Set location. Method: http.MethodPut, Path: "/api/locations/east", }, { // Test 4: Ack a finding. - Method: http.MethodPost, Path: "/api/findings/abc123/ack", + Method: http.MethodPost, Path: "/api/findings/" + eastFP + "/ack", }, { // Test 5: Ack an alert. Method: http.MethodPost, Path: "/api/alerts/fp-1/ack", }, { // Test 6: Create an API key. @@ -82,6 +86,7 @@ func TestJSONBodyLimits(t *testing.T) { }); err != nil { t.Fatalf("seed alert: %v", err) } + seedAckFindings(t, srv, ss) req := httptest.NewRequest(test.Method, test.Path, strings.NewReader(huge)) w := httptest.NewRecorder() diff --git a/internal/server/handlers_read_test.go b/internal/server/handlers_read_test.go index 70739b4..d0ad728 100644 --- a/internal/server/handlers_read_test.go +++ b/internal/server/handlers_read_test.go @@ -523,9 +523,8 @@ func TestHandleGetOutliers_AckDoesNotTouchCache(t *testing.T) { t.Fatal("want findings above zero, got none") } f := first.Findings[0] - body := `{"cluster":"` + f.Cluster + `","scanner":"` + f.Scanner + `","title":"` + f.Title + `"}` req := httptest.NewRequest(http.MethodPost, "/api/findings/"+f.Fingerprint+"/ack", - strings.NewReader(body)) + strings.NewReader(`{"reason":"known"}`)) w := httptest.NewRecorder() srv.mux.ServeHTTP(w, req) if w.Code != http.StatusCreated { diff --git a/internal/server/metrics.go b/internal/server/metrics.go index f7f7669..2c2f31f 100644 --- a/internal/server/metrics.go +++ b/internal/server/metrics.go @@ -199,26 +199,36 @@ func emitPolicyReportMetrics(w io.Writer, r *report.Report, log *zap.Logger) { } // latestReportForMetrics returns the rebuilt report for the most recent scan, -// or nil when no scan exists. +// or nil when no scan exists or the store fails. func (s *Server) latestReportForMetrics(ctx context.Context) *report.Report { - scans, err := s.store.ListScans(ctx, 1) + rpt, err := s.latestReport(ctx) if err != nil { - s.log.Warn("metrics: list scans", zap.Error(err)) + s.log.Warn("metrics: latest report", zap.Error(err)) return nil } + return rpt +} + +// latestReport returns the cached report for the most recent scan, or nil with +// no error when no scan exists. The report is shared, so callers must not +// modify it. +func (s *Server) latestReport(ctx context.Context) (*report.Report, error) { + scans, err := s.store.ListScans(ctx, 1) + if err != nil { + return nil, fmt.Errorf("list scans: %w", err) + } if len(scans) == 0 { - return nil + return nil, nil } latest := scans[0] results, err := s.store.GetScanResults(ctx, latest.ID) if err != nil { - s.log.Warn("metrics: get scan results", zap.Error(err)) - return nil + return nil, fmt.Errorf("get scan results: %w", err) } opts := s.reportOptions(ctx, s.log) opts.Timestamp = latest.Timestamp - return s.reports.get(latest.ID, latest.Clusters, results, opts) + return s.reports.get(latest.ID, latest.Clusters, results, opts), nil } // emit writes one line of Prometheus exposition followed by a newline. diff --git a/internal/server/openapi.yaml b/internal/server/openapi.yaml index ec81d9a..7ae7690 100644 --- a/internal/server/openapi.yaml +++ b/internal/server/openapi.yaml @@ -528,10 +528,10 @@ components: AckRequest: type: object + description: | + Human context for an ack. The server takes cluster, scanner, and title + from the finding it holds, and rejects a body that names them. properties: - cluster: {type: string} - scanner: {type: string} - title: {type: string} ack_by: {type: string} reason: {type: string} snooze_until: {type: string, format: date-time} @@ -989,13 +989,18 @@ paths: /api/findings/{fingerprint}/ack: post: summary: Record a finding acknowledgement + description: | + The fingerprint must belong to a finding in the latest report. The + server checks the caller's cluster scope against that finding's + cluster and stores the finding's cluster, scanner, and title. A + fleet-scoped finding needs wildcard scope. parameters: - in: path name: fingerprint required: true schema: {type: string} requestBody: - required: true + required: false content: application/json: schema: {$ref: "#/components/schemas/AckRequest"} @@ -1005,15 +1010,23 @@ paths: content: application/json: schema: {$ref: "#/components/schemas/AckRecord"} + "400": {description: Unreadable body, unknown field, or snooze_until not RFC 3339} + "403": {description: Finding cluster outside the caller's scope} + "404": {description: No finding in the latest report carries this fingerprint} delete: summary: Remove a finding acknowledgement + description: | + The server checks the caller's cluster scope against the cluster stored + on the ack. A fleet-scoped ack needs wildcard scope. A missing ack + returns 204. parameters: - in: path name: fingerprint required: true schema: {type: string} responses: - "204": {description: Deleted} + "204": {description: Deleted, or no ack existed} + "403": {description: Ack cluster outside the caller's scope} /api/alerts: get: diff --git a/internal/server/scope_test.go b/internal/server/scope_test.go new file mode 100644 index 0000000..8db6b6b --- /dev/null +++ b/internal/server/scope_test.go @@ -0,0 +1,309 @@ +package server + +import ( + "context" + "fmt" + "net/http" + "net/http/httptest" + "slices" + "strings" + "testing" + "time" + + "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" + + "github.com/dcadolph/fleetsweeper/internal/scanner" + "github.com/dcadolph/fleetsweeper/internal/store" +) + +// scopeNeed names the scope an actor needs to change a route's server-owned object. +type scopeNeed int + +const ( + // needEast means the object lives in cluster east. + needEast scopeNeed = iota + // needWildcard means the object spans the fleet, so only wildcard scope reaches it. + needWildcard + // needAdmin means the route is for admins whatever their cluster scope. + needAdmin +) + +// scopeCase builds one request against a mutating route. +type scopeCase struct { + // Name labels the case in failure output. + Name string + // Need is the scope the request's target requires. + Need scopeNeed + // WantAllowed is the status an actor that meets Need receives. + WantAllowed int + // Build seeds the server-owned object and returns the request path under /api and its body. + Build func(t *testing.T, srv *Server, ss *store.SQLite) (path, body string) +} + +// scopeCases returns the scope cases for every mutating API route, keyed by "METHOD path" as +// apiRoutes declares it. Each object the server owns lives in east unless the case names the +// fleet. +func scopeCases() map[string][]scopeCase { + fixed := func(path, body string) func(*testing.T, *Server, *store.SQLite) (string, string) { + return func(*testing.T, *Server, *store.SQLite) (string, string) { return path, body } + } + seedAck := func(fp, cluster string) func(*testing.T, *Server, *store.SQLite) (string, string) { + return func(t *testing.T, _ *Server, ss *store.SQLite) (string, string) { + t.Helper() + rec := store.AckRecord{Fingerprint: fp, Cluster: cluster} + if err := ss.SaveAck(context.Background(), rec); err != nil { + t.Fatalf("seed ack: %v", err) + } + return "/findings/" + fp + "/ack", "" + } + } + seedAlert := func(fp, cluster string) func(*testing.T, *Server, *store.SQLite) (string, string) { + return func(t *testing.T, _ *Server, ss *store.SQLite) (string, string) { + t.Helper() + if err := ss.UpsertAlert(context.Background(), store.AlertRecord{ + Fingerprint: fp, Cluster: cluster, Status: "firing", AlertName: "Drift", + ReceivedAt: time.Now(), Labels: map[string]string{}, Annotations: map[string]string{}, + }); err != nil { + t.Fatalf("seed alert: %v", err) + } + return "/alerts/" + fp + "/ack", `{}` + } + } + return map[string][]scopeCase{ + "POST /scans": {{ + Name: "contexts in east", Need: needEast, WantAllowed: http.StatusTooManyRequests, + Build: func(_ *testing.T, srv *Server, _ *store.SQLite) (string, string) { + // A held scan slot turns an allowed trigger into 429 instead of a real scan. + srv.scanBusy.Store(true) + return "/scans", `{"contexts":["east"]}` + }, + }}, + "POST /findings/{fingerprint}/ack": {{ + Name: "east finding", Need: needEast, WantAllowed: http.StatusCreated, + Build: func(t *testing.T, srv *Server, ss *store.SQLite) (string, string) { + east, _ := seedAckFindings(t, srv, ss) + return "/findings/" + east.Fingerprint + "/ack", `{}` + }, + }, { + Name: "fleet finding", Need: needWildcard, WantAllowed: http.StatusCreated, + Build: func(t *testing.T, srv *Server, ss *store.SQLite) (string, string) { + _, fleet := seedAckFindings(t, srv, ss) + return "/findings/" + fleet.Fingerprint + "/ack", `{}` + }, + }}, + "DELETE /findings/{fingerprint}/ack": {{ + Name: "east ack", Need: needEast, WantAllowed: http.StatusNoContent, + Build: seedAck("fp-east", "east"), + }, { + Name: "cluster-less ack", Need: needWildcard, WantAllowed: http.StatusNoContent, + Build: seedAck("fp-blank", ""), + }}, + "POST /alerts/{fingerprint}/ack": {{ + Name: "east alert", Need: needEast, WantAllowed: http.StatusCreated, + Build: seedAlert("alert-east", "east"), + }, { + Name: "cluster-less alert", Need: needWildcard, WantAllowed: http.StatusCreated, + Build: seedAlert("alert-blank", ""), + }}, + "PUT /clusters/{name}/tags/{key}": {{ + Name: "east tag", Need: needEast, WantAllowed: http.StatusOK, + Build: fixed("/clusters/east/tags/team", `{"value":"core"}`), + }}, + "DELETE /clusters/{name}/tags/{key}": {{ + Name: "east tag", Need: needEast, WantAllowed: http.StatusNoContent, + Build: func(t *testing.T, _ *Server, ss *store.SQLite) (string, string) { + t.Helper() + if err := ss.SetClusterTag(context.Background(), "east", "team", "core"); err != nil { + t.Fatalf("seed tag: %v", err) + } + return "/clusters/east/tags/team", "" + }, + }}, + "PUT /locations/{cluster}": {{ + Name: "east location", Need: needEast, WantAllowed: http.StatusOK, + Build: fixed("/locations/east", `{"lat":40.7,"lng":-74}`), + }}, + "DELETE /locations/{cluster}": {{ + Name: "east location", Need: needEast, WantAllowed: http.StatusNoContent, + Build: func(t *testing.T, _ *Server, ss *store.SQLite) (string, string) { + t.Helper() + rec := store.LocationRecord{Cluster: "east", Lat: 40.7, Lng: -74} + if err := ss.SetLocation(context.Background(), rec); err != nil { + t.Fatalf("seed location: %v", err) + } + return "/locations/east", "" + }, + }}, + "POST /whatif": {{ + Name: "east field", Need: needEast, WantAllowed: http.StatusOK, + Build: func(t *testing.T, _ *Server, ss *store.SQLite) (string, string) { + t.Helper() + privileged := func(n int) map[string]scanner.Result { + return map[string]scanner.Result{"workload-security": { + Scanner: "workload-security", State: scanner.StateOK, + Data: map[string]any{"privileged_containers": n, "total_pods": 10}, + }} + } + clusters := []string{"east", "west", "north", "south"} + results := map[string]map[string]scanner.Result{ + "east": privileged(3), "west": privileged(0), + "north": privileged(0), "south": privileged(0), + } + if _, err := ss.SaveScan(context.Background(), clusters, results); err != nil { + t.Fatalf("seed scan: %v", err) + } + body := `{"cluster":"east","scanner":"workload-security","field":"privileged_containers"}` + return "/whatif", body + }, + }}, + "POST /groups": {{ + Name: "group of east", Need: needAdmin, WantAllowed: http.StatusCreated, + Build: fixed("/groups", `{"name":"edge","clusters":["east"]}`), + }}, + "DELETE /groups/{name}": {{ + Name: "group of east", Need: needAdmin, WantAllowed: http.StatusNoContent, + Build: func(t *testing.T, _ *Server, ss *store.SQLite) (string, string) { + t.Helper() + if err := ss.SaveGroup(context.Background(), "edge", []string{"east"}); err != nil { + t.Fatalf("seed group: %v", err) + } + return "/groups/edge", "" + }, + }}, + } +} + +// mutatingKeys returns the "METHOD path" key of every route that changes state outside the +// admin and webhook namespaces, which carry their own authorization. +func mutatingKeys(routes []route) []string { + var keys []string + for _, r := range routes { + switch r.Method { + case http.MethodPost, http.MethodPut, http.MethodPatch, http.MethodDelete: + default: + continue + } + if strings.HasPrefix(r.Path, "/admin/") || strings.HasPrefix(r.Path, "/webhooks/") { + continue + } + keys = append(keys, r.Method+" "+r.Path) + } + return keys +} + +// unscopedRoutes returns the mutating route keys that have no scope case. +func unscopedRoutes(routes []route, cases map[string][]scopeCase) []string { + var missing []string + for _, key := range mutatingKeys(routes) { + if len(cases[key]) == 0 { + missing = append(missing, key) + } + } + return missing +} + +// serveRoute registers only the route under key on a fresh mux, so path values resolve as in +// production, and serves the request as actor. It fails the test when apiRoutes has no such key. +func serveRoute(t *testing.T, srv *Server, key string, actor *Actor, path, body string) int { + t.Helper() + i := slices.IndexFunc(srv.apiRoutes(), func(r route) bool { return r.Method+" "+r.Path == key }) + if i < 0 { + t.Fatalf("apiRoutes has no route %q", key) + } + r := srv.apiRoutes()[i] + mux := http.NewServeMux() + mux.HandleFunc(key, r.Handler) + req := httptest.NewRequest(r.Method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + req = req.WithContext(withActor(req.Context(), actor)) + w := httptest.NewRecorder() + mux.ServeHTTP(w, req) + return w.Code +} + +// TestMutatingRoutesAreScoped walks every mutating API route and checks that the scope test +// reads the server-owned object: an operator scoped to west is refused an object in east, an +// operator scoped to east is not, fleet-spanning objects need wildcard scope, and group +// management needs the admin role. A mutating route with no case fails the test, so a new +// endpoint cannot ship unscoped. +func TestMutatingRoutesAreScoped(t *testing.T) { + t.Parallel() + cases := scopeCases() + probe, _ := testServer(t) + if missing := unscopedRoutes(probe.apiRoutes(), cases); len(missing) > 0 { + t.Fatalf("mutating routes with no scope case: %v", missing) + } + for key := range cases { + if !slices.Contains(mutatingKeys(probe.apiRoutes()), key) { + t.Errorf("scope case %q matches no mutating route", key) + } + } + tests := []struct { + Actor *Actor + Meets []scopeNeed + }{{ // Test 0: Operator scoped to west reaches nothing in east. + Actor: scopedOperator("west"), + }, { // Test 1: Operator scoped to east reaches east objects only. + Actor: scopedOperator("east"), Meets: []scopeNeed{needEast}, + }, { // Test 2: Operator reaching east through a group. + Actor: scopedOperator("group:east-only"), Meets: []scopeNeed{needEast}, + }, { // Test 3: Wildcard operator reaches the fleet but cannot manage groups. + Actor: scopedOperator(store.ScopeWildcard), Meets: []scopeNeed{needEast, needWildcard}, + }, { // Test 4: Admin reaches everything. + Actor: adminActor(), Meets: []scopeNeed{needEast, needWildcard, needAdmin}, + }} + for testNum, test := range tests { + for key, scs := range cases { + for _, sc := range scs { + t.Run(fmt.Sprintf("test %d/%s/%s", testNum, key, sc.Name), func(t *testing.T) { + t.Parallel() + srv, ss := testServer(t) + if err := ss.SaveGroup(context.Background(), "east-only", []string{"east"}); err != nil { + t.Fatalf("seed group: %v", err) + } + path, body := sc.Build(t, srv, ss) + want := http.StatusForbidden + if slices.Contains(test.Meets, sc.Need) { + want = sc.WantAllowed + } + if got := serveRoute(t, srv, key, test.Actor, path, body); got != want { + t.Errorf("%s %s: want %d, got %d", key, path, want, got) + } + }) + } + } + } +} + +// TestUnscopedRoutesNamesMissing verifies the route walk names a mutating route with no scope +// case and ignores reads, admin routes, and webhooks. +func TestUnscopedRoutesNamesMissing(t *testing.T) { + t.Parallel() + probe, _ := testServer(t) + tests := []struct { + Extra []route + WantMissing []string + }{{ // Test 0: The real route table is fully covered. + Extra: nil, WantMissing: nil, + }, { // Test 1: A new POST route with no case is named. + Extra: []route{{Method: http.MethodPost, Path: "/fake/{id}"}}, + WantMissing: []string{"POST /fake/{id}"}, + }, { // Test 2: Reads, admin routes, and webhooks need no case. + Extra: []route{ + {Method: http.MethodGet, Path: "/fake"}, + {Method: http.MethodPost, Path: "/admin/fake"}, + {Method: http.MethodPost, Path: "/webhooks/fake"}, + }, + }} + for testNum, test := range tests { + t.Run(fmt.Sprintf("test %d", testNum), func(t *testing.T) { + t.Parallel() + routes := append(probe.apiRoutes(), test.Extra...) + got := unscopedRoutes(routes, scopeCases()) + if diff := cmp.Diff(test.WantMissing, got, cmpopts.EquateEmpty()); diff != "" { + t.Errorf("missing mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/internal/server/static/js/components/finding.js b/internal/server/static/js/components/finding.js index 65f2b34..b0507a2 100644 --- a/internal/server/static/js/components/finding.js +++ b/internal/server/static/js/components/finding.js @@ -161,7 +161,6 @@ function openAckDrawer(el) { snooze_until: snoozeUntil(Number(panel.querySelector('#ackSnooze').value)), }; const path = isAlert ? '/alerts/' + encodeURIComponent(fp) + '/ack' : '/findings/' + encodeURIComponent(fp) + '/ack'; - if (!isAlert) Object.assign(body, { cluster: el.dataset.cluster || '', scanner: el.dataset.scanner || '', title: el.dataset.title || '' }); if (!await send('POST', path, body, 'Acknowledge')) return false; setAcked(fp, true); toast('Acknowledged.', 'ok'); diff --git a/internal/store/acks.go b/internal/store/acks.go index 3025fe6..34cd8f2 100644 --- a/internal/store/acks.go +++ b/internal/store/acks.go @@ -75,6 +75,29 @@ func (s *sqlStore) DeleteAck(ctx context.Context, fingerprint string) error { return nil } +// GetAck returns the active ack for fingerprint. Expired snoozes are pruned +// first, so an elapsed ack reads as missing. Returns the wrapped ErrNotFound +// when no row matches. +func (s *sqlStore) GetAck(ctx context.Context, fingerprint string) (AckRecord, error) { + if err := s.pruneExpiredAcks(ctx); err != nil { + return AckRecord{}, err + } + rows, err := s.query(ctx, ` +SELECT fingerprint, cluster, scanner, title, ack_by, reason, snooze_until, created_at +FROM finding_acks WHERE fingerprint = ?`, fingerprint) + if err != nil { + return AckRecord{}, fmt.Errorf("%w: get ack: %w", ErrQuery, err) + } + defer rows.Close() + if !rows.Next() { + if err := rows.Err(); err != nil { + return AckRecord{}, fmt.Errorf("%w: get ack: %w", ErrQuery, err) + } + return AckRecord{}, fmt.Errorf("%w: get ack", ErrNotFound) + } + return scanAckRow(rows) +} + // ListAcks returns every active ack. Expired snoozes are filtered out and // removed from the database as a side effect so the table stays small. func (s *sqlStore) ListAcks(ctx context.Context) ([]AckRecord, error) { diff --git a/internal/store/contract_test.go b/internal/store/contract_test.go index beed55a..ff2ab68 100644 --- a/internal/store/contract_test.go +++ b/internal/store/contract_test.go @@ -420,7 +420,7 @@ func contractLocations(t *testing.T, st Store) { } } -// contractAcks checks ack upsert, list, IsAcked, snooze expiry, delete, and +// contractAcks checks ack upsert, list, IsAcked, GetAck, snooze expiry, delete, and // the empty-fingerprint refusal. func contractAcks(t *testing.T, st Store) { ctx := context.Background() @@ -462,6 +462,27 @@ func contractAcks(t *testing.T, st Store) { t.Errorf("IsAcked(%s) = %v, want %v", fp, acked, wantAcked) } } + for _, test := range []struct { + Fingerprint string + WantAck AckRecord + Want error + }{{ // Test 0: Refreshed ack returns its latest values. + Fingerprint: "fp-1", WantAck: acks[1], + }, { // Test 1: Snoozed ack is still active. + Fingerprint: "fp-2", WantAck: acks[2], + }, { // Test 2: Expired snooze reads as missing. + Fingerprint: "fp-3", Want: ErrNotFound, + }, { // Test 3: Unknown fingerprint. + Fingerprint: "fp-none", Want: ErrNotFound, + }} { + got, err := st.GetAck(ctx, test.Fingerprint) + if !errors.Is(err, test.Want) { + t.Errorf("GetAck(%s) error = %v, want %v", test.Fingerprint, err, test.Want) + } + if diff := cmp.Diff(test.WantAck, got); diff != "" { + t.Errorf("GetAck(%s) mismatch (-want +got):\n%s", test.Fingerprint, diff) + } + } if err := st.DeleteAck(ctx, "fp-1"); err != nil { t.Fatalf("delete ack: %v", err) diff --git a/internal/store/store.go b/internal/store/store.go index 943e1ce..84b3309 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -152,6 +152,10 @@ type Store interface { // DeleteAck removes an acknowledgement. Idempotent. DeleteAck(ctx context.Context, fingerprint string) error + // GetAck returns the active acknowledgement for a fingerprint. Returns the + // wrapped ErrNotFound when no row matches or its snooze has expired. + GetAck(ctx context.Context, fingerprint string) (AckRecord, error) + // ListAcks returns every active acknowledgement (expired snoozes pruned). ListAcks(ctx context.Context) ([]AckRecord, error)