feat: serve SecurityReport and SecurityAcknowledgement

README.md

@@ -18,7 +18,7 @@ entries. It runs as an aggregated API server under Milo, like
 
 Early. Every resource in the design has a v1alpha1 type in
 `pkg/apis/feedback/v1alpha1` and a sample in `config/samples`, and the server
-serves everything except `SecurityReport` and `SecurityAcknowledgement`. Storage for the rest is tracked as
+serves every resource in the design. Storage for the rest is tracked as
 sub-issues of the epic.
 
 | Resource | Storage | State |
@@ -28,7 +28,7 @@ sub-issues of the epic.
 | `PostQuery` | Postgres (not stored) | served |
 | `Vote`, `Subscription` | Postgres | served |
 | `Submission`, `AttachmentUpload` | Postgres, bucket | served |
-| `SecurityReport`, `SecurityAcknowledgement` | Postgres | planned |
+| `SecurityReport`, `SecurityAcknowledgement` | Postgres, bucket | served |
 
 ## Layout
 
@@ -169,6 +169,35 @@ to the submitter and staff. Files live in `--attachments-bucket`
 lifecycle rule deleting `uploads/` after a day. Without a bucket,
 attachments are off and submissions without them still work.
 
+## Security reports
+
+Private vulnerability reports, with their own role: only the Security Team
+can list them or write their status. Feedback reviewers can't see them;
+Milo IAM enforces that (see #10).
+
+- **Report:** reporters create reports in the user context (verified email,
+  same rate limit as submissions) and read their own, without
+  `status.internal`. Names are generated (`sec-…`). Attachments are
+  `AttachmentUpload`s with `purpose: SecurityReport`, stored under their own
+  prefix and served by `securityreports/{name}/attachment?ref=` to the
+  reporter and the Security Team.
+- **Log:** the Security Team logs reports from email or GitHub in the
+  platform context with `spec.reporter` set. Those belong to the platform.
+- **Work:** the Security Team writes `status` through the status
+  subresource. A phase change gets a timeline entry if the team didn't add
+  one; timeline entries can be added but never changed. `NotApplicable` and
+  `Duplicate` need a reason, `Duplicate` needs `internal.duplicateOf`, and
+  `credits` rewards need an amount.
+- **Move:** staff send a misfiled submission to the security track with
+  `create` on `submissions/{name}/to-security-report`. In one transaction a
+  report owned by the submitter is created from it, its attachments are
+  copied, and the submission is deleted.
+- **Acknowledge:** `SecurityAcknowledgement` lists reports in `Fixed` or
+  `Disclosed` whose reporter chose public credit, with only the credited
+  name, link, month fixed and severity.
+
+Reports are never embedded, indexed, synced or sent to the AI gateway.
+
 ## Development
 
 ```bash

internal/apiserver/apiserver.go

@@ -26,6 +26,7 @@ import (
 	"go.miloapis.com/feedback/internal/registry/post"
 	"go.miloapis.com/feedback/internal/registry/postquery"
 	"go.miloapis.com/feedback/internal/registry/release"
+	"go.miloapis.com/feedback/internal/registry/securityreport"
 	"go.miloapis.com/feedback/internal/registry/submission"
 	"go.miloapis.com/feedback/internal/registry/vote"
 	"go.miloapis.com/feedback/internal/storage/pg"
@@ -259,8 +260,20 @@ func (c completedConfig) New(ctx context.Context) (*FeedbackServer, error) {
 	}
 	// Uploads: 20 per 10 minutes per user, per replica.
 	v1alpha1Storage["attachmentuploads"] = attachmentupload.NewREST(files, ratelimit.NewLimiter(2, 20))
+	reports, err := securityreport.NewREST(db, Scheme, securityreport.Options{
+		Identities: ids, Files: files, Limit: sc.Limit, Window: sc.Window,
+	})
+	if err != nil {
+		pool.Close()
+		return nil, fmt.Errorf("create SecurityReport storage: %w", err)
+	}
+	v1alpha1Storage["securityreports"] = reports.Store
+	v1alpha1Storage["securityreports/status"] = reports.Status
+	v1alpha1Storage["securityreports/attachment"] = reports.Attachment
+	v1alpha1Storage["securityacknowledgements"] = securityreport.NewAcknowledgementREST(db)
+
 	submissions, err := submission.NewREST(db, Scheme, submission.Options{
-		Identities: ids, Files: files, Votes: votes, Subscriptions: subs,
+		Identities: ids, Files: files, Votes: votes, Subscriptions: subs, Reports: reports,
 		Limit: sc.Limit, Window: sc.Window,
 	})
 	if err != nil {
@@ -271,9 +284,7 @@ func (c completedConfig) New(ctx context.Context) (*FeedbackServer, error) {
 	v1alpha1Storage["submissions/status"] = submissions.Status
 	v1alpha1Storage["submissions/review"] = submissions.Review
 	v1alpha1Storage["submissions/attachment"] = submissions.Attachment
-
-	// Other Postgres-backed resources (SecurityReport, ...) are added here as
-	// they are implemented. See datum-cloud/enhancements#943.
+	v1alpha1Storage["submissions/to-security-report"] = submissions.ToSecurity
 
 	apiGroupInfo.VersionedResourcesStorageMap[feedbackv1alpha1.SchemeGroupVersion.Version] = v1alpha1Storage
 	if err := genericServer.InstallAPIGroup(&apiGroupInfo); err != nil {

internal/attachments/attachments.go

@@ -157,9 +157,10 @@ func (s *Store) Attach(ctx context.Context, purpose v1.AttachmentPurpose, owner,
 	return out, nil
 }
 
-// Move copies a parent's attachments to another parent and deletes the
-// originals, e.g. when a submission becomes a security report.
-func (s *Store) Move(ctx context.Context, from, to v1.AttachmentPurpose, fromParent, toParent string) error {
+// CopyParent copies a parent's attachments to another parent, e.g. when a
+// submission becomes a security report. Deleting the old parent deletes the
+// originals.
+func (s *Store) CopyParent(ctx context.Context, from, to v1.AttachmentPurpose, fromParent, toParent string) error {
 	src := ParentPrefix(from, fromParent)
 	it := s.bucket.List(&blob.ListOptions{Prefix: src})
 	for {
@@ -173,9 +174,6 @@ func (s *Store) Move(ctx context.Context, from, to v1.AttachmentPurpose, fromPar
 		if err := s.bucket.Copy(ctx, ParentPrefix(to, toParent)+strings.TrimPrefix(obj.Key, src), obj.Key, nil); err != nil {
 			return err
 		}
-		if err := s.bucket.Delete(ctx, obj.Key); err != nil {
-			return err
-		}
 	}
 }
 

internal/registry/attachmentupload/download.go

@@ -0,0 +1,83 @@
+package attachmentupload
+
+import (
+	"context"
+	"errors"
+	"mime"
+	"net/http"
+	"slices"
+	"strconv"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	"k8s.io/apiserver/pkg/registry/rest"
+	"k8s.io/klog/v2"
+
+	"go.miloapis.com/feedback/internal/attachments"
+	v1 "go.miloapis.com/feedback/pkg/apis/feedback/v1alpha1"
+)
+
+// DownloadREST serves GET {resource}/{name}/attachment?ref=: one attached
+// file, to whoever can read the parent.
+type DownloadREST struct {
+	// Get reads the parent as the caller and returns its attachments. It
+	// must apply the parent's own access rules (scope, redaction).
+	Get     func(ctx context.Context, name string) ([]v1.AttachmentReference, error)
+	NewFunc func() runtime.Object
+	Files   *attachments.Store
+	Purpose v1.AttachmentPurpose
+	// Resource names the subresource in NotFound errors.
+	Resource schema.GroupResource
+}
+
+var _ rest.Connecter = &DownloadREST{}
+
+func (r *DownloadREST) New() runtime.Object { return r.NewFunc() }
+func (r *DownloadREST) Destroy()            {}
+
+func (r *DownloadREST) NewConnectOptions() (runtime.Object, bool, string) {
+	return &v1.AttachmentOptions{}, false, ""
+}
+
+func (r *DownloadREST) ConnectMethods() []string { return []string{http.MethodGet} }
+
+// Connect checks the caller can read the parent and that ref is attached to
+// it, then returns a handler that sends the file.
+func (r *DownloadREST) Connect(ctx context.Context, name string, opts runtime.Object, _ rest.Responder) (http.Handler, error) {
+	ref := opts.(*v1.AttachmentOptions).Ref
+	if ref == "" {
+		return nil, apierrors.NewBadRequest("set the ref query parameter to an attachment's uploadRef")
+	}
+	refs, err := r.Get(ctx, name)
+	if err != nil {
+		return nil, err
+	}
+	if !slices.ContainsFunc(refs, func(a v1.AttachmentReference) bool { return a.UploadRef == ref }) {
+		return nil, apierrors.NewNotFound(r.Resource, ref)
+	}
+	if r.Files == nil {
+		return nil, apierrors.NewServiceUnavailable("Attachments are not available.")
+	}
+	f, err := r.Files.Get(ctx, r.Purpose, name, ref)
+	if errors.Is(err, attachments.ErrNotFound) {
+		return nil, apierrors.NewNotFound(r.Resource, ref)
+	}
+	if err != nil {
+		klog.ErrorS(err, "Reading attachment failed", "resource", r.Resource, "name", name, "ref", ref)
+		return nil, apierrors.NewServiceUnavailable("Attachments are not available.")
+	}
+	return http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { serveFile(w, f) }), nil
+}
+
+// serveFile sends a file as a download that browsers won't render or run.
+func serveFile(w http.ResponseWriter, f *attachments.File) {
+	h := w.Header()
+	h.Set("Content-Type", f.ContentType)
+	h.Set("Content-Length", strconv.Itoa(len(f.Data)))
+	h.Set("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{"filename": f.Name}))
+	h.Set("X-Content-Type-Options", "nosniff")
+	h.Set("Content-Security-Policy", "sandbox; default-src 'none'")
+	h.Set("Cache-Control", "private, no-store")
+	_, _ = w.Write(f.Data)
+}

internal/registry/securityreport/acknowledgement.go

@@ -0,0 +1,175 @@
+package securityreport
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"slices"
+	"strings"
+	"time"
+
+	"github.com/jackc/pgx/v5"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	metainternalversion "k8s.io/apimachinery/pkg/apis/meta/internalversion"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apiserver/pkg/registry/rest"
+
+	"go.miloapis.com/feedback/internal/storage/pg"
+	v1 "go.miloapis.com/feedback/pkg/apis/feedback/v1alpha1"
+)
+
+// AcknowledgementREST serves SecurityAcknowledgement: a read-only view of
+// reports in Fixed or Disclosed whose reporter chose public credit. Each
+// holds the credited name, link, month and severity, and nothing else from
+// the report.
+type AcknowledgementREST struct {
+	db    *pg.DB
+	table rest.TableConvertor
+}
+
+var (
+	_ rest.Getter               = &AcknowledgementREST{}
+	_ rest.Lister               = &AcknowledgementREST{}
+	_ rest.Scoper               = &AcknowledgementREST{}
+	_ rest.SingularNameProvider = &AcknowledgementREST{}
+)
+
+// NewAcknowledgementREST returns the view.
+func NewAcknowledgementREST(db *pg.DB) *AcknowledgementREST {
+	return &AcknowledgementREST{db: db, table: ackTable{}}
+}
+
+func (r *AcknowledgementREST) New() runtime.Object     { return &v1.SecurityAcknowledgement{} }
+func (r *AcknowledgementREST) NewList() runtime.Object { return &v1.SecurityAcknowledgementList{} }
+func (r *AcknowledgementREST) Destroy()                {}
+func (r *AcknowledgementREST) NamespaceScoped() bool   { return false }
+func (r *AcknowledgementREST) GetSingularName() string { return "securityacknowledgement" }
+func (r *AcknowledgementREST) ConvertToTable(ctx context.Context, obj, opts runtime.Object) (*metav1.Table, error) {
+	return r.table.ConvertToTable(ctx, obj, opts)
+}
+
+const ackQuery = `SELECT name, created_at, object->'spec'->'credit', object->'status' FROM security_reports
+	WHERE phase IN ('Fixed', 'Disclosed')
+	  AND coalesce((object->'spec'->'credit'->>'public')::boolean, false)
+	  AND coalesce(object->'spec'->'credit'->>'name', '') <> ''`
+
+// Get returns one acknowledgement, named after its report.
+func (r *AcknowledgementREST) Get(ctx context.Context, name string, _ *metav1.GetOptions) (runtime.Object, error) {
+	acks, err := r.query(ctx, ackQuery+" AND name = $1", name)
+	if err != nil {
+		return nil, err
+	}
+	if len(acks) == 0 {
+		return nil, apierrors.NewNotFound(v1.Resource("securityacknowledgements"), name)
+	}
+	return &acks[0], nil
+}
+
+// List returns every acknowledgement, newest month first. The list is short,
+// so it isn't paged; selectors aren't supported.
+func (r *AcknowledgementREST) List(ctx context.Context, opts *metainternalversion.ListOptions) (runtime.Object, error) {
+	if opts != nil && ((opts.LabelSelector != nil && !opts.LabelSelector.Empty()) ||
+		(opts.FieldSelector != nil && !opts.FieldSelector.Empty() && !isNameSelector(opts))) {
+		return nil, apierrors.NewBadRequest("securityacknowledgements support no selectors except metadata.name")
+	}
+	q, args := ackQuery, []any{}
+	if opts != nil && isNameSelector(opts) {
+		v, _ := opts.FieldSelector.RequiresExactMatch("metadata.name")
+		q, args = q+" AND name = $1", []any{v}
+	}
+	acks, err := r.query(ctx, q, args...)
+	if err != nil {
+		return nil, err
+	}
+	slices.SortFunc(acks, func(a, b v1.SecurityAcknowledgement) int {
+		if c := strings.Compare(b.Status.Month, a.Status.Month); c != 0 {
+			return c
+		}
+		return strings.Compare(a.Name, b.Name)
+	})
+	return &v1.SecurityAcknowledgementList{Items: acks}, nil
+}
+
+func isNameSelector(opts *metainternalversion.ListOptions) bool {
+	if opts.FieldSelector == nil {
+		return false
+	}
+	_, ok := opts.FieldSelector.RequiresExactMatch("metadata.name")
+	return ok && len(opts.FieldSelector.Requirements()) == 1
+}
+
+func (r *AcknowledgementREST) query(ctx context.Context, q string, args ...any) ([]v1.SecurityAcknowledgement, error) {
+	rows, err := r.db.Q(ctx).Query(ctx, q, args...)
+	if err != nil {
+		return nil, apierrors.NewInternalError(err)
+	}
+	acks, err := pgx.CollectRows(rows, func(row pgx.CollectableRow) (v1.SecurityAcknowledgement, error) {
+		var (
+			name           string
+			created        time.Time
+			rawCredit, raw []byte
+			credit         v1.SecurityCredit
+			st             v1.SecurityReportStatus
+		)
+		if err := row.Scan(&name, &created, &rawCredit, &raw); err != nil {
+			return v1.SecurityAcknowledgement{}, err
+		}
+		if err := json.Unmarshal(rawCredit, &credit); err != nil {
+			return v1.SecurityAcknowledgement{}, fmt.Errorf("decode credit of %s: %w", name, err)
+		}
+		if err := json.Unmarshal(raw, &st); err != nil {
+			return v1.SecurityAcknowledgement{}, fmt.Errorf("decode status of %s: %w", name, err)
+		}
+		return v1.SecurityAcknowledgement{
+			ObjectMeta: metav1.ObjectMeta{Name: name},
+			Status: v1.SecurityAcknowledgementStatus{
+				Name: credit.Name, URL: credit.URL, Month: fixedMonth(st, created), Severity: st.Severity,
+			},
+		}, nil
+	})
+	if err != nil {
+		return nil, apierrors.NewInternalError(err)
+	}
+	return acks, nil
+}
+
+// fixedMonth is when the report was fixed: its first Fixed timeline entry,
+// else its first Disclosed one, else when it was received.
+func fixedMonth(st v1.SecurityReportStatus, received time.Time) string {
+	for _, phase := range []v1.SecurityReportPhase{v1.SecurityReportPhaseFixed, v1.SecurityReportPhaseDisclosed} {
+		for _, e := range st.Timeline {
+			if e.Phase == phase {
+				return e.At.UTC().Format("2006-01")
+			}
+		}
+	}
+	return received.UTC().Format("2006-01")
+}
+
+type ackTable struct{}
+
+func (ackTable) ConvertToTable(_ context.Context, obj runtime.Object, _ runtime.Object) (*metav1.Table, error) {
+	t := &metav1.Table{ColumnDefinitions: []metav1.TableColumnDefinition{
+		{Name: "Name", Type: "string", Format: "name"},
+		{Name: "Credit", Type: "string"},
+		{Name: "Month", Type: "string"},
+		{Name: "Severity", Type: "string"},
+		{Name: "URL", Type: "string", Priority: 1},
+	}}
+	add := func(a *v1.SecurityAcknowledgement) {
+		t.Rows = append(t.Rows, metav1.TableRow{
+			Object: runtime.RawExtension{Object: a},
+			Cells:  []any{a.Name, a.Status.Name, a.Status.Month, string(a.Status.Severity), a.Status.URL},
+		})
+	}
+	switch o := obj.(type) {
+	case *v1.SecurityAcknowledgement:
+		add(o)
+	case *v1.SecurityAcknowledgementList:
+		for i := range o.Items {
+			add(&o.Items[i])
+		}
+	}
+	return t, nil
+}