feat: audit feedback writes from the apiserver

README.md

@@ -82,6 +82,28 @@ only in their owner's context, submissions and security reports are created
 only by their author (or logged by the Security Team), and
 `FeedbackSource` is platform-only.
 
+## Audit
+
+Milo proxies requests to this API without their bodies, so the apiserver is
+the audit source for its own writes (`config/components/audit-logging`,
+included in the core-control-plane overlay):
+
+- Writes to posts, releases, changelog entries, votes, subscriptions and
+  feedback sources are logged at `RequestResponse`. The search indexer reads
+  posts, releases and changelog entries from `responseObject`.
+- Submissions, security reports, uploads and `PostQuery` are logged at
+  `Metadata`, so private context, vulnerability details, files and draft
+  feedback never reach the audit log, activity or search.
+- Reads aren't logged here; Milo's events cover them.
+- Side effects are annotations on the request that caused them:
+  `feedback.miloapis.com/subscribed`, `votes-moved-to`, `vote-granted` and
+  `security-report`.
+
+Events go to activity's Vector webhook in `activity-system`, like Milo's.
+The deployment supplies the webhook's CA as the `feedback-audit-webhook-ca`
+ConfigMap, and drops Milo's own events for `feedback.miloapis.com` writes so
+each write is recorded once.
+
 ## Postgres
 
 The apiserver is the only component that connects to Postgres. Other

config/components/audit-logging/audit-policy.yaml

@@ -0,0 +1,54 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: feedback-audit-policy
+  namespace: feedback-system
+data:
+  audit-policy.yaml: |
+    # The feedback apiserver is the audit source for writes to its API. Milo
+    # proxies requests here without their bodies, so it records them at
+    # Metadata only; the deployment drops Milo's events for
+    # feedback.miloapis.com writes so each write is recorded once, here.
+    # Reads stay with Milo's events.
+    apiVersion: audit.k8s.io/v1
+    kind: Policy
+    omitStages:
+      - RequestReceived
+      - ResponseStarted
+      - Panic
+    rules:
+      - level: None
+        nonResourceURLs:
+          - /healthz*
+          - /livez*
+          - /readyz*
+          - /metrics
+          - /version
+          - /api
+          - /apis
+          - /apis/*
+          - /openapi
+          - /openapi/*
+      - level: None
+        verbs: [get, list, watch]
+      # Private content stays out of the audit log, activity and search:
+      # submissions (private context, triage), security reports
+      # (vulnerability details), uploads (file contents) and PostQuery
+      # (draft feedback in similarTo). Side effects show as annotations.
+      - level: Metadata
+        resources:
+          - group: feedback.miloapis.com
+            resources:
+              - submissions
+              - submissions/*
+              - securityreports
+              - securityreports/*
+              - attachmentuploads
+              - postqueries
+      # Everything else is written with its bodies. The search indexer reads
+      # posts, releases and changelog entries from responseObject.
+      - level: RequestResponse
+        verbs: [create, update, patch, delete, deletecollection]
+        resources:
+          - group: feedback.miloapis.com
+      - level: None

config/components/audit-logging/audit-webhook.yaml

@@ -0,0 +1,26 @@
+apiVersion: v1
+kind: ConfigMap
+metadata:
+  name: feedback-audit-webhook
+  namespace: feedback-system
+data:
+  audit-webhook.yaml: |
+    # The Vector webhook in activity-system that Milo sends to, so feedback
+    # events reach the audit log, activity and search the same way.
+    apiVersion: v1
+    kind: Config
+    clusters:
+      - name: activity-audit
+        cluster:
+          server: https://vector-sidecar.activity-system.svc.cluster.local:8080/events
+          certificate-authority: /etc/kubernetes/pki/audit-webhook/ca.crt
+    users:
+      - name: feedback-apiserver
+        user:
+          token: noop
+    contexts:
+      - name: activity-audit
+        context:
+          cluster: activity-audit
+          user: feedback-apiserver
+    current-context: activity-audit

config/components/audit-logging/kustomization.yaml

@@ -0,0 +1,48 @@
+# Audit logging for the feedback apiserver: a policy and a webhook to
+# activity's Vector. The deployer supplies the webhook's CA as the ConfigMap
+# feedback-audit-webhook-ca (key ca.crt), e.g. from the cluster's trust
+# bundle, or replaces feedback-audit-webhook.
+apiVersion: kustomize.config.k8s.io/v1alpha1
+kind: Component
+
+resources:
+  - audit-policy.yaml
+  - audit-webhook.yaml
+
+patches:
+  - target:
+      group: apps
+      version: v1
+      kind: Deployment
+      name: feedback-apiserver
+    patch: |-
+      - op: add
+        path: /spec/template/spec/containers/0/args/-
+        value: --audit-policy-file=/etc/feedback/audit/policy/audit-policy.yaml
+      - op: add
+        path: /spec/template/spec/containers/0/args/-
+        value: --audit-webhook-config-file=/etc/feedback/audit/webhook/audit-webhook.yaml
+      - op: add
+        path: /spec/template/spec/containers/0/args/-
+        value: --audit-webhook-mode=batch
+      - op: add
+        path: /spec/template/spec/containers/0/args/-
+        value: --audit-webhook-initial-backoff=10s
+      - op: add
+        path: /spec/template/spec/containers/0/volumeMounts/-
+        value: {name: audit-policy, mountPath: /etc/feedback/audit/policy, readOnly: true}
+      - op: add
+        path: /spec/template/spec/containers/0/volumeMounts/-
+        value: {name: audit-webhook, mountPath: /etc/feedback/audit/webhook, readOnly: true}
+      - op: add
+        path: /spec/template/spec/containers/0/volumeMounts/-
+        value: {name: audit-webhook-ca, mountPath: /etc/kubernetes/pki/audit-webhook, readOnly: true}
+      - op: add
+        path: /spec/template/spec/volumes/-
+        value: {name: audit-policy, configMap: {name: feedback-audit-policy}}
+      - op: add
+        path: /spec/template/spec/volumes/-
+        value: {name: audit-webhook, configMap: {name: feedback-audit-webhook}}
+      - op: add
+        path: /spec/template/spec/volumes/-
+        value: {name: audit-webhook-ca, configMap: {name: feedback-audit-webhook-ca, optional: true}}

config/overlays/core-control-plane/kustomization.yaml

@@ -4,6 +4,9 @@ kind: Kustomization
 resources:
   - ../../base
 
+components:
+  - ../../components/audit-logging
+
 images:
   - name: ghcr.io/milo-os/feedback
     newTag: latest

internal/apiserver/auditpolicy_test.go

@@ -0,0 +1,70 @@
+package apiserver
+
+import (
+	"os"
+	"testing"
+
+	auditinternal "k8s.io/apiserver/pkg/apis/audit"
+	"k8s.io/apiserver/pkg/audit/policy"
+	"k8s.io/apiserver/pkg/authentication/user"
+	"k8s.io/apiserver/pkg/authorization/authorizer"
+	"sigs.k8s.io/yaml"
+)
+
+// TestAuditPolicy evaluates the shipped policy: writes are recorded with
+// their bodies, except private kinds, which are recorded at Metadata; reads
+// are left to Milo.
+func TestAuditPolicy(t *testing.T) {
+	raw, err := os.ReadFile("../../config/components/audit-logging/audit-policy.yaml")
+	if err != nil {
+		t.Fatal(err)
+	}
+	var cm struct {
+		Data map[string]string `json:"data"`
+	}
+	if err := yaml.Unmarshal(raw, &cm); err != nil {
+		t.Fatal(err)
+	}
+	p, err := policy.LoadPolicyFromBytes([]byte(cm.Data["audit-policy.yaml"]))
+	if err != nil {
+		t.Fatal(err)
+	}
+	eval := policy.NewPolicyRuleEvaluator(p)
+
+	u := &user.DefaultInfo{Name: "someone"}
+	res := func(verb, resource, sub string) authorizer.Attributes {
+		return authorizer.AttributesRecord{User: u, Verb: verb, APIGroup: group, APIVersion: "v1alpha1",
+			Resource: resource, Subresource: sub, Name: "x", ResourceRequest: true}
+	}
+	for _, c := range []struct {
+		attrs authorizer.Attributes
+		want  auditinternal.Level
+	}{
+		{res("create", "posts", ""), auditinternal.LevelRequestResponse},
+		{res("patch", "posts", "status"), auditinternal.LevelRequestResponse},
+		{res("delete", "posts", ""), auditinternal.LevelRequestResponse},
+		{res("update", "releases", "status"), auditinternal.LevelRequestResponse},
+		{res("create", "changelogentries", ""), auditinternal.LevelRequestResponse},
+		{res("create", "votes", ""), auditinternal.LevelRequestResponse},
+		{res("delete", "subscriptions", ""), auditinternal.LevelRequestResponse},
+		{res("update", "feedbacksources", ""), auditinternal.LevelRequestResponse},
+		{res("create", "submissions", ""), auditinternal.LevelMetadata},
+		{res("update", "submissions", "status"), auditinternal.LevelMetadata},
+		{res("create", "submissions", "review"), auditinternal.LevelMetadata},
+		{res("create", "submissions", "to-security-report"), auditinternal.LevelMetadata},
+		{res("create", "securityreports", ""), auditinternal.LevelMetadata},
+		{res("patch", "securityreports", "status"), auditinternal.LevelMetadata},
+		{res("create", "attachmentuploads", ""), auditinternal.LevelMetadata},
+		{res("create", "postqueries", ""), auditinternal.LevelMetadata},
+		{res("get", "posts", ""), auditinternal.LevelNone},
+		{res("list", "submissions", ""), auditinternal.LevelNone},
+		{res("get", "securityreports", ""), auditinternal.LevelNone},
+		{res("get", "submissions", "attachment"), auditinternal.LevelNone},
+		{authorizer.AttributesRecord{User: u, Verb: "get", Path: "/readyz"}, auditinternal.LevelNone},
+	} {
+		got := eval.EvaluatePolicyRule(c.attrs).Level
+		if got != c.want {
+			t.Errorf("%s %s/%s: level %s, want %s", c.attrs.GetVerb(), c.attrs.GetResource(), c.attrs.GetSubresource(), got, c.want)
+		}
+	}
+}

internal/auditx/auditx.go

@@ -0,0 +1,28 @@
+// Package auditx records a request's side effects as audit annotations, so
+// the request's own audit event shows everything it changed without
+// carrying private content.
+package auditx
+
+import (
+	"context"
+
+	"k8s.io/apiserver/pkg/audit"
+)
+
+// Annotation keys.
+const (
+	// Subscribed is the post a vote subscribed its voter to.
+	Subscribed = "feedback.miloapis.com/subscribed"
+	// VotesMovedTo is the post a merged post's votes moved to.
+	VotesMovedTo = "feedback.miloapis.com/votes-moved-to"
+	// VoteGranted is "<user>/<post>" when a review or a linked post gave the
+	// submitter a vote.
+	VoteGranted = "feedback.miloapis.com/vote-granted"
+	// SecurityReport is the report a submission became.
+	SecurityReport = "feedback.miloapis.com/security-report"
+)
+
+// Annotate adds key=value to the request's audit event, if it has one.
+func Annotate(ctx context.Context, key, value string) {
+	audit.AddAuditAnnotation(ctx, key, value)
+}

internal/registry/post/storage.go

@@ -15,6 +15,7 @@ import (
 	"k8s.io/apiserver/pkg/registry/rest"
 	"k8s.io/klog/v2"
 
+	"go.miloapis.com/feedback/internal/auditx"
 	"go.miloapis.com/feedback/internal/identity"
 	"go.miloapis.com/feedback/internal/storage/pg"
 	v1 "go.miloapis.com/feedback/pkg/apis/feedback/v1alpha1"
@@ -176,6 +177,7 @@ func (h hooks) beforeWrite(ctx context.Context, op pg.Op, old, obj runtime.Objec
 		if err := moveVotes(ctx, q, p.Name, to); err != nil {
 			return apierrors.NewInternalError(fmt.Errorf("move votes to %s: %w", to, err))
 		}
+		auditx.Annotate(ctx, auditx.VotesMovedTo, to)
 		if _, err := Retally(ctx, h.db, to, now); err != nil {
 			return apierrors.NewInternalError(fmt.Errorf("retally %s: %w", to, err))
 		}

internal/registry/submission/storage.go

@@ -18,6 +18,7 @@ import (
 	"k8s.io/klog/v2"
 
 	"go.miloapis.com/feedback/internal/attachments"
+	"go.miloapis.com/feedback/internal/auditx"
 	"go.miloapis.com/feedback/internal/identity"
 	"go.miloapis.com/feedback/internal/registry/attachmentupload"
 	"go.miloapis.com/feedback/internal/registry/securityreport"
@@ -216,7 +217,10 @@ func (h hooks) grant(ctx context.Context, owner string, sub *v1.Submission, post
 	}
 	_, err := h.o.Votes.Create(as, v, nil, &metav1.CreateOptions{})
 	switch {
-	case err == nil, apierrors.IsAlreadyExists(err):
+	case err == nil:
+		auditx.Annotate(ctx, auditx.VoteGranted, owner+"/"+post)
+		return nil
+	case apierrors.IsAlreadyExists(err):
 		return nil
 	case apierrors.IsInvalid(err):
 		klog.InfoS("Not giving the submitter a vote", "submission", sub.Name, "post", post, "err", err)

internal/registry/submission/tosecurity.go

@@ -12,6 +12,7 @@ import (
 	"k8s.io/apiserver/pkg/registry/rest"
 
 	"go.miloapis.com/feedback/internal/attachments"
+	"go.miloapis.com/feedback/internal/auditx"
 	"go.miloapis.com/feedback/internal/identity"
 	"go.miloapis.com/feedback/internal/registry/securityreport"
 	"go.miloapis.com/feedback/internal/storage/pg"
@@ -88,6 +89,7 @@ func (r *ToSecurityReportREST) Create(ctx context.Context, name string, obj runt
 			return err
 		}
 		rep := created.(*v1.SecurityReport)
+		auditx.Annotate(ctx, auditx.SecurityReport, rep.Name)
 		if len(sub.Spec.Attachments) > 0 {
 			if r.files == nil {
 				return apierrors.NewServiceUnavailable("Attachments are not available, so this submission can't move yet.")

internal/registry/vote/subscription.go

@@ -11,6 +11,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/apiserver/pkg/storage/names"
 
+	"go.miloapis.com/feedback/internal/auditx"
 	"go.miloapis.com/feedback/internal/storage/pg"
 	v1 "go.miloapis.com/feedback/pkg/apis/feedback/v1alpha1"
 )
@@ -118,6 +119,9 @@ func (r *SubscriptionREST) Follow(ctx context.Context, postName string, reason v
 	if apierrors.IsAlreadyExists(err) {
 		return nil
 	}
+	if err == nil {
+		auditx.Annotate(ctx, auditx.Subscribed, postName)
+	}
 	return err
 }