feat(submit): add user modpack build and approval pipeline

A user-directed extension over the build subsystem: an uploaded modpack stays in pending_review and is never built until an admin approves. Approval is a single-winner compare-and-swap that hands off to the image-build Job, keeping the mandatory vulnerability scan in front of any push.

internal/submit/pgstore.go

@@ -0,0 +1,159 @@
+package submit
+
+import (
+	"context"
+	"database/sql"
+	"time"
+)
+
+// PGStore is the Postgres-backed Store (image_submissions, migration 0002). It
+// is the only submit component that holds database credentials and exposes only
+// the narrow operations the Manager needs — no generic UPDATE escape hatch. It
+// compiles here but is exercised by integration tests against a live database;
+// the Manager's logic is unit-tested against the in-memory fake instead.
+type PGStore struct {
+	db *sql.DB
+}
+
+// NewPGStore wraps an open *sql.DB.
+func NewPGStore(db *sql.DB) *PGStore { return &PGStore{db: db} }
+
+// Compile-time proof PGStore satisfies the Store interface.
+var _ Store = (*PGStore)(nil)
+
+const submissionColumns = `id, submitted_by, display_name, context_ref, status,
+	image_ref, build_id, reviewed_by, reject_reason, created_at, reviewed_at`
+
+func (s *PGStore) CreateSubmission(ctx context.Context, sub *Submission) error {
+	const q = `INSERT INTO image_submissions
+		(id, submitted_by, display_name, context_ref, status, created_at)
+		VALUES ($1, $2, $3, $4, $5, $6)`
+	_, err := s.db.ExecContext(ctx, q,
+		sub.ID, sub.SubmittedBy, sub.DisplayName, sub.ContextRef, string(sub.Status), sub.CreatedAt)
+	return err
+}
+
+func (s *PGStore) GetSubmission(ctx context.Context, id string) (*Submission, error) {
+	const q = `SELECT ` + submissionColumns + ` FROM image_submissions WHERE id = $1`
+	return scanSubmission(s.db.QueryRowContext(ctx, q, id))
+}
+
+func (s *PGStore) ListSubmissions(ctx context.Context) ([]Submission, error) {
+	const q = `SELECT ` + submissionColumns + ` FROM image_submissions ORDER BY created_at DESC`
+	return s.querySubmissions(ctx, q)
+}
+
+func (s *PGStore) ListSubmissionsBy(ctx context.Context, submittedBy string) ([]Submission, error) {
+	const q = `SELECT ` + submissionColumns + `
+		FROM image_submissions WHERE submitted_by = $1 ORDER BY created_at DESC`
+	return s.querySubmissions(ctx, q, submittedBy)
+}
+
+// cas executes a compare-and-set UPDATE and reports whether THIS call moved the
+// row. The `status = 'pending_review'` guard is the actual CAS predicate and is
+// deliberately kept INLINE in each caller's query — it is security-visible, so a
+// reader auditing "can a non-pending row be flipped?" must see it next to the SET.
+// cas only folds the shared ExecContext + RowsAffected tail so the two reviewers
+// (approve, reject) cannot drift in how they report a lost race or a RowsAffected
+// error. n == 0 means a concurrent review already won the row — reported as
+// won=false (never an error), which the Manager maps to ErrAlreadyReviewed.
+func (s *PGStore) cas(ctx context.Context, q string, args ...any) (bool, error) {
+	res, err := s.db.ExecContext(ctx, q, args...)
+	if err != nil {
+		return false, err
+	}
+	n, err := res.RowsAffected()
+	return n > 0, err
+}
+
+// ApproveSubmission is the approve CAS: it flips the row only while it is still
+// pending_review, so a concurrent reviewer cannot also win.
+func (s *PGStore) ApproveSubmission(ctx context.Context, id, reviewedBy, imageRef string, at time.Time) (bool, error) {
+	const q = `UPDATE image_submissions
+		SET status = 'approved', image_ref = $2, reviewed_by = $3, reviewed_at = $4
+		WHERE id = $1 AND status = 'pending_review'`
+	return s.cas(ctx, q, id, imageRef, reviewedBy, at)
+}
+
+// RejectSubmission is the reject CAS, mirroring ApproveSubmission.
+func (s *PGStore) RejectSubmission(ctx context.Context, id, reviewedBy, reason string, at time.Time) (bool, error) {
+	const q = `UPDATE image_submissions
+		SET status = 'rejected', reviewed_by = $2, reject_reason = $3, reviewed_at = $4
+		WHERE id = $1 AND status = 'pending_review'`
+	return s.cas(ctx, q, id, reviewedBy, reason, at)
+}
+
+func (s *PGStore) LinkBuild(ctx context.Context, id, buildID string) error {
+	const q = `UPDATE image_submissions SET build_id = $2 WHERE id = $1`
+	res, err := s.db.ExecContext(ctx, q, id, buildID)
+	if err != nil {
+		return err
+	}
+	n, err := res.RowsAffected()
+	if err != nil {
+		return err
+	}
+	if n == 0 {
+		return ErrNotFound
+	}
+	return nil
+}
+
+func (s *PGStore) querySubmissions(ctx context.Context, q string, args ...any) ([]Submission, error) {
+	rows, err := s.db.QueryContext(ctx, q, args...)
+	if err != nil {
+		return nil, err
+	}
+	defer rows.Close()
+	var out []Submission
+	for rows.Next() {
+		sub, err := scanSubmissionRows(rows)
+		if err != nil {
+			return nil, err
+		}
+		out = append(out, *sub)
+	}
+	return out, rows.Err()
+}
+
+// rowScanner is the read surface shared by *sql.Row (single) and *sql.Rows (in a
+// list loop), so one scan body serves both without copy-paste.
+type rowScanner interface {
+	Scan(dest ...any) error
+}
+
+// scanSubmission scans a single row, translating no-rows into ErrNotFound.
+func scanSubmission(row *sql.Row) (*Submission, error) {
+	sub, err := scanSubmissionRows(row)
+	if err == sql.ErrNoRows {
+		return nil, ErrNotFound
+	}
+	return sub, err
+}
+
+// scanSubmissionRows decodes one row's columns, mapping nullable text/time
+// columns through sql.Null* (NULLIF/absent values become zero, omitted in JSON).
+func scanSubmissionRows(row rowScanner) (*Submission, error) {
+	var (
+		sub                                         Submission
+		status                                      string
+		imageRef, buildID, reviewedBy, rejectReason sql.NullString
+		reviewedAt                                  sql.NullTime
+	)
+	if err := row.Scan(
+		&sub.ID, &sub.SubmittedBy, &sub.DisplayName, &sub.ContextRef, &status,
+		&imageRef, &buildID, &reviewedBy, &rejectReason, &sub.CreatedAt, &reviewedAt,
+	); err != nil {
+		return nil, err
+	}
+	sub.Status = Status(status)
+	sub.ImageRef = imageRef.String
+	sub.BuildID = buildID.String
+	sub.ReviewedBy = reviewedBy.String
+	sub.RejectReason = rejectReason.String
+	if reviewedAt.Valid {
+		t := reviewedAt.Time
+		sub.ReviewedAt = &t
+	}
+	return &sub, nil
+}

internal/submit/submit.go

@@ -0,0 +1,418 @@
+// Package submit implements the user-uploaded modpack approval lane.
+//
+// PROVENANCE — read this before trusting the spec citations elsewhere. This lane
+// is a USER-DIRECTED extension, not a feature of Felis-Spec-V4.1 as written. The
+// spec's §16 build subsystem deliberately DELETED a manual-approve flow ("简化掉
+// (admin 信任,删):人工 approve 流程 → 删 ... 无人工 approve 端点") under a trust
+// model where "上传仅 SysAdmin" (§1 invariant 9: "build 鉴权简化(admin-only)").
+// The product owner subsequently required the opposite: ordinary, untrusted
+// users may upload their own modpacks, and an admin must approve each one before
+// it is usable. Re-introducing untrusted-origin uploads is exactly the condition
+// under which §16's deleted gate must come BACK — the spec removed the approve
+// step only because it had assumed every uploader was already a trusted SysAdmin.
+// So this lane departs from §16's *ingress* simplification on purpose, with the
+// owner's instruction as the authority; it does NOT relax anything §16 calls
+// non-negotiable. (Do not cite "spec §8" for this lane — §8 is Readiness/优雅关闭
+// /Console. Earlier drafts mis-cited it; those citations have been corrected.)
+//
+// What is preserved (§16, non-negotiable). The build subsystem treats the
+// Dockerfile as hostile at runtime (build/build.go): Kaniko reads the Dockerfile
+// from inside the uploaded context, so every build — admin or user — runs in the
+// isolated felis-build namespace under a weak service account behind a default-
+// deny egress policy, and a CRITICAL CVE fails the Trivy gate ("运行时隔离不能
+// 省"). This lane changes none of that: an approved submission is built through
+// the SAME gated Builder.Submit. The approval gate is layered in FRONT of the
+// still-mandatory Trivy scan, never instead of it — a human "yes" does not skip
+// the scan, so a CRITICAL CVE still blocks admission after approval.
+//
+// Two further guards close the gap an untrusted origin opens:
+//
+//   - The push target (image_ref) is DERIVED by the Manager as
+//     {registry}/user-uploads/{submissionID}:latest — a namespace platform refs
+//     never use, so a submitter can neither collide with nor target the platform
+//     image namespace.
+//   - The build context (context_ref) is DERIVED as {contextStore}/{id}/...
+//     too, NOT taken as free-form user input. Kaniko interpolates the context
+//     ref straight into its `--context=` argument (build/jobspec.go); a user-
+//     chosen ref would let an untrusted origin point the build at an arbitrary
+//     source. By deriving it from the submission id the user selects nothing
+//     that reaches the executor — only the modpack blob behind the pinned,
+//     id-namespaced location (uploaded by a separate, deferred transport).
+//
+// Source of truth. submissions is a NEW Postgres business-truth domain, added to
+// §1 invariant 2's enumeration (owner/claim/accounts/audit/quota/images/builds/
+// backups) by the same owner instruction that introduced this lane. The
+// image_submissions row is the business authority — the approval record — while
+// the build is the execution record in image_builds. Approve never copies the
+// build's authoritative fields; it records the derived push target it asked the
+// Builder to build plus the reviewer identity, and links the build id once
+// Submit succeeds.
+//
+// The Manager depends on the Store and Builds interfaces, so creation, the
+// approve CAS, rejection, and build hand-off are all unit-tested against
+// in-memory fakes. The Postgres implementation (PGStore) compiles here but is
+// exercised only by integration tests against a live database.
+package submit
+
+import (
+	"context"
+	"crypto/rand"
+	"encoding/hex"
+	"errors"
+	"fmt"
+	"regexp"
+	"strings"
+	"time"
+
+	"felis.lolicon.best/internal/build"
+)
+
+// Status mirrors the submission_status enum (spec §6, migration 0002).
+type Status string
+
+const (
+	StatusPendingReview Status = "pending_review"
+	StatusApproved      Status = "approved"
+	StatusRejected      Status = "rejected"
+)
+
+// Sentinels. The API layer maps ErrInvalid→400, ErrNotFound→404 and
+// ErrAlreadyReviewed→409; they are kept distinct from store/cluster failures so
+// those surface as 500.
+var (
+	ErrInvalid         = errors.New("submit: invalid request")
+	ErrNotFound        = errors.New("submit: submission not found")
+	ErrAlreadyReviewed = errors.New("submit: submission already reviewed")
+)
+
+// invalidf wraps ErrInvalid so every malformed-request case maps to one 400.
+func invalidf(format string, a ...any) error {
+	return fmt.Errorf("%w: "+format, append([]any{ErrInvalid}, a...)...)
+}
+
+const (
+	maxDisplayName  = 200
+	maxRejectReason = 1000
+)
+
+// displayNameRE constrains the user-supplied label to a calm, single-line set:
+// it is the only free-form string a submission carries, and although JSON
+// encoding already neutralises it in responses, rejecting control characters
+// and odd whitespace here keeps it clean in the admin review queue and any log
+// line. Length is bounded separately so this stays a charset check.
+var displayNameRE = regexp.MustCompile(`^[\p{L}\p{N} ._:()\[\]/+-]+$`)
+
+// Submission mirrors an image_submissions row (spec §6, migration 0002): a
+// user's request to have a modpack built into a runnable image, plus the admin
+// review verdict. The user supplies only DisplayName; ContextRef/ImageRef/
+// BuildID/ReviewedBy are filled by the platform, never by the submitter.
+type Submission struct {
+	ID           string     `json:"id"`
+	SubmittedBy  string     `json:"submitted_by"`
+	DisplayName  string     `json:"display_name"`
+	ContextRef   string     `json:"context_ref"`
+	Status       Status     `json:"status"`
+	ImageRef     string     `json:"image_ref,omitempty"`
+	BuildID      string     `json:"build_id,omitempty"`
+	ReviewedBy   string     `json:"reviewed_by,omitempty"`
+	RejectReason string     `json:"reject_reason,omitempty"`
+	CreatedAt    time.Time  `json:"created_at"`
+	ReviewedAt   *time.Time `json:"reviewed_at,omitempty"`
+}
+
+// Store is the business-layer persistence the Manager depends on. It is an
+// interface so the Manager is tested against an in-memory fake; the Postgres
+// implementation (PGStore) is integration-tested only.
+type Store interface {
+	// CreateSubmission inserts a pending_review row.
+	CreateSubmission(ctx context.Context, s *Submission) error
+	// GetSubmission loads one submission, or ErrNotFound.
+	GetSubmission(ctx context.Context, id string) (*Submission, error)
+	// ListSubmissions returns every submission, newest first (admin queue).
+	ListSubmissions(ctx context.Context) ([]Submission, error)
+	// ListSubmissionsBy returns one user's submissions, newest first.
+	ListSubmissionsBy(ctx context.Context, submittedBy string) ([]Submission, error)
+	// ApproveSubmission atomically flips pending_review -> approved, recording the
+	// derived image_ref, the reviewer and reviewed_at. It reports whether THIS
+	// call won the transition: false means a concurrent review already moved the
+	// row, so the caller must NOT start a build.
+	ApproveSubmission(ctx context.Context, id, reviewedBy, imageRef string, at time.Time) (won bool, err error)
+	// RejectSubmission atomically flips pending_review -> rejected, recording the
+	// reviewer, the reason and reviewed_at. Reports whether THIS call won.
+	RejectSubmission(ctx context.Context, id, reviewedBy, reason string, at time.Time) (won bool, err error)
+	// LinkBuild records the build id on an approved submission. It runs only after
+	// Builder.Submit succeeds, so the implication is ONE-WAY: build_id non-null ⇒
+	// a build started. The converse does NOT hold — if Submit succeeds but this
+	// link write then fails, build_id stays NULL while a build is genuinely running
+	// (Approve surfaces that distinctly so remediation does not double-build; see
+	// the Approve ordering note and the package KNOWN-LIMITATION).
+	LinkBuild(ctx context.Context, id, buildID string) error
+}
+
+// Builds is the slice of the build subsystem the approval lane drives. An
+// approved submission is built through the SAME gated Builder.Submit as an
+// admin's direct build, so the Trivy scan-gate applies identically (spec §16).
+type Builds interface {
+	Submit(ctx context.Context, req build.Request) (*build.Build, error)
+}
+
+// Manager orchestrates the approval lane. It holds no mutable state; the clock
+// and id generator are injectable for hermetic tests.
+type Manager struct {
+	Store  Store
+	Builds Builds
+
+	// Registry is the internal registry host[:port] the derived push target
+	// addresses. It MUST be the SAME value the Builder is configured with
+	// (build.Config.RegistryURL): the Manager pre-validates the derived ref
+	// against this host before the approve CAS, and the Builder re-validates
+	// against its own config at Submit — if the two disagree the pre-check passes
+	// but Submit rejects, stranding an approved row. cmd/felis wires both from one
+	// field. The derived ref is {Registry}/user-uploads/{id}:latest.
+	Registry string
+	// ContextStore is the pinned Kaniko build-context base for user uploads, e.g.
+	// "s3://felis-user-uploads" (mirrors a configured object store). The derived
+	// context ref is {ContextStore}/{id}/context.tar.gz; the modpack blob is
+	// placed there by a separate upload transport (deferred — see package doc).
+	ContextStore string
+
+	Now   func() time.Time
+	IDGen func() string
+}
+
+func (m *Manager) now() time.Time {
+	if m.Now != nil {
+		return m.Now()
+	}
+	return time.Now()
+}
+
+func (m *Manager) newID() string {
+	if m.IDGen != nil {
+		return m.IDGen()
+	}
+	// The id namespaces the derived image ref (deriveImageRef) and is the PK of
+	// image_submissions, so it must be lowercase (imageNameRE in build/validate.go),
+	// path/argv-safe, and collision-resistant. 8 bytes of crypto/rand hex give all
+	// three: lowercase hex never contains a slash or shell metachar, and the id is
+	// unguessable rather than a sequential timestamp. crypto/rand.Read only fails on
+	// a broken entropy source; fall back to a nanosecond stamp so creation degrades
+	// instead of panicking (a PK clash would surface as a normal insert error).
+	var b [8]byte
+	if _, err := rand.Read(b[:]); err != nil {
+		return fmt.Sprintf("sub-%d", time.Now().UnixNano())
+	}
+	return "sub-" + hex.EncodeToString(b[:])
+}
+
+// deriveImageRef is the platform-controlled push target for a submission: a pure
+// function of the registry and the submission id, in a user-uploads/ path no
+// platform ref uses, so an untrusted submitter can never collide with or target
+// the platform image namespace.
+func (m *Manager) deriveImageRef(id string) string {
+	return fmt.Sprintf("%s/user-uploads/%s:latest", strings.TrimRight(m.Registry, "/"), id)
+}
+
+// deriveContextRef is the platform-controlled build context for a submission.
+// Like the image ref it is a pure function of the submission id, so the user
+// selects nothing that reaches Kaniko's --context argument; only the blob behind
+// this pinned, id-namespaced location (placed by the upload transport) varies.
+func (m *Manager) deriveContextRef(id string) string {
+	return fmt.Sprintf("%s/%s/context.tar.gz", strings.TrimRight(m.ContextStore, "/"), id)
+}
+
+// auditDockerfile is the audit-archive Dockerfile recorded on the build row. It
+// is NOT what Kaniko executes — Kaniko reads the real Dockerfile from inside the
+// uploaded context (build/jobspec.go) — so this honestly documents the
+// provenance instead of implying a recipe the platform controls. It satisfies
+// build.Validate's non-empty requirement.
+func auditDockerfile(id, contextRef string) string {
+	return fmt.Sprintf(
+		"# felis user-modpack submission %s\n"+
+			"# The executed Dockerfile is provided by the uploaded build context:\n"+
+			"#   %s\n"+
+			"# Built in the isolated felis-build sandbox and Trivy-gated (spec §16).\n",
+		id, contextRef)
+}
+
+// CreateRequest is the validated user upload. The user supplies ONLY a
+// human-friendly name; identity comes from the authenticated principal and the
+// build inputs (image/context refs) are platform-derived, never from the body.
+type CreateRequest struct {
+	DisplayName string
+	SubmittedBy string
+}
+
+// Create records a new pending_review submission. It does NOT start a build:
+// nothing is built until an admin approves (the whole point of the lane).
+func (m *Manager) Create(ctx context.Context, req CreateRequest) (*Submission, error) {
+	name := strings.TrimSpace(req.DisplayName)
+	switch {
+	case name == "":
+		return nil, invalidf("display name is required")
+	case len(name) > maxDisplayName:
+		return nil, invalidf("display name exceeds %d characters", maxDisplayName)
+	case !displayNameRE.MatchString(name):
+		return nil, invalidf("display name contains unsupported characters")
+	}
+	if strings.TrimSpace(req.SubmittedBy) == "" {
+		return nil, invalidf("submitter identity is required")
+	}
+
+	id := m.newID()
+	s := &Submission{
+		ID:          id,
+		SubmittedBy: req.SubmittedBy,
+		DisplayName: name,
+		ContextRef:  m.deriveContextRef(id),
+		Status:      StatusPendingReview,
+		CreatedAt:   m.now(),
+	}
+	if err := m.Store.CreateSubmission(ctx, s); err != nil {
+		return nil, err
+	}
+	return s, nil
+}
+
+// Approve is the admin gate. It atomically claims the pending_review -> approved
+// transition (CAS) and ONLY the winner starts the build, so concurrent approvals
+// can never double-build. The build runs through the SAME gated Builder.Submit as
+// an admin's direct build: approval is layered in FRONT of the Trivy scan, never
+// instead of it (spec §16), so a CRITICAL CVE still fails the build and nothing
+// is admitted even after a human approved.
+//
+// Ordering and its two post-CAS partial states. Deterministic validation (a
+// misconfigured registry/context, an id that does not form a valid ref) runs
+// BEFORE the CAS via build.Validate, so such failures never leave a stuck
+// approved row. The CAS is then committed before Submit. Two failures can occur
+// after it, and they are NOT equally benign:
+//
+//   - Submit fails (transient cluster error): the row is approved with build_id
+//     NULL and NO build is running. Benign and recoverable — an admin re-drives
+//     it via the direct build path.
+//   - Submit SUCCEEDS but the follow-up LinkBuild fails: the row is approved with
+//     build_id NULL while a build IS running and will push to the derived
+//     image_ref. On the row this is indistinguishable from the benign case, so a
+//     blind "re-drive" would double-build and double-push to :latest (no unique
+//     constraint stops it). Approve therefore returns a DISTINCT error naming the
+//     running build id and warning not to re-submit.
+//
+// Both are labeled KNOWN-LIMITATIONs (see package doc); the second is the worse
+// one and the reason the error is differentiated. The alternative ordering
+// (Submit-then-CAS) would either double-build under a concurrent approve or
+// orphan a build on a lost race, both worse still.
+func (m *Manager) Approve(ctx context.Context, id, reviewedBy string) (*Submission, error) {
+	if strings.TrimSpace(reviewedBy) == "" {
+		return nil, invalidf("reviewer identity is required")
+	}
+
+	sub, err := m.Store.GetSubmission(ctx, id)
+	if err != nil {
+		return nil, err
+	}
+	if sub.Status != StatusPendingReview {
+		return nil, ErrAlreadyReviewed
+	}
+
+	imageRef := m.deriveImageRef(id)
+	req := build.Request{
+		ImageRef:    imageRef,
+		Dockerfile:  auditDockerfile(id, sub.ContextRef),
+		ContextRef:  sub.ContextRef,
+		BaseImage:   "", // declared inside the uploaded context, unknown here
+		RequestedBy: reviewedBy,
+	}
+	// Pre-validate against the SAME registry the Builder enforces, BEFORE the CAS,
+	// so a deterministic config error cannot strand the row in approved.
+	if err := build.Validate(req, build.Config{RegistryURL: m.Registry}); err != nil {
+		return nil, err
+	}
+
+	now := m.now()
+	won, err := m.Store.ApproveSubmission(ctx, id, reviewedBy, imageRef, now)
+	if err != nil {
+		return nil, err
+	}
+	if !won {
+		// Lost the race to a concurrent approve/reject.
+		return nil, ErrAlreadyReviewed
+	}
+
+	// Only the CAS winner starts the build.
+	bld, err := m.Builds.Submit(ctx, req)
+	if err != nil {
+		// Approved but unbuilt and NOTHING is running — admin remediation via the
+		// direct build path is safe (the benign partial state).
+		return nil, fmt.Errorf("submit: approved but build hand-off failed: %w", err)
+	}
+	if err := m.Store.LinkBuild(ctx, id, bld.ID); err != nil {
+		// The worse partial state: the build IS running and will push to image_ref,
+		// but the row still shows build_id NULL. A blind re-drive would double-build.
+		// Name the running build id and warn explicitly so an operator reconciles it
+		// (link it by hand / let it finish) instead of re-submitting.
+		return nil, fmt.Errorf("submit: approved and build %s started but linking it to the submission failed (build is running — do NOT re-submit, reconcile build_id by hand): %w", bld.ID, err)
+	}
+
+	reviewedAt := now
+	sub.Status = StatusApproved
+	sub.ImageRef = imageRef
+	sub.BuildID = bld.ID
+	sub.ReviewedBy = reviewedBy
+	sub.ReviewedAt = &reviewedAt
+	return sub, nil
+}
+
+// Reject is the admin's other verdict: it atomically claims
+// pending_review -> rejected with a required reason and starts NO build.
+func (m *Manager) Reject(ctx context.Context, id, reviewedBy, reason string) (*Submission, error) {
+	if strings.TrimSpace(reviewedBy) == "" {
+		return nil, invalidf("reviewer identity is required")
+	}
+	reason = strings.TrimSpace(reason)
+	switch {
+	case reason == "":
+		return nil, invalidf("a reject reason is required")
+	case len(reason) > maxRejectReason:
+		return nil, invalidf("reject reason exceeds %d characters", maxRejectReason)
+	}
+
+	sub, err := m.Store.GetSubmission(ctx, id)
+	if err != nil {
+		return nil, err
+	}
+	if sub.Status != StatusPendingReview {
+		return nil, ErrAlreadyReviewed
+	}
+
+	now := m.now()
+	won, err := m.Store.RejectSubmission(ctx, id, reviewedBy, reason, now)
+	if err != nil {
+		return nil, err
+	}
+	if !won {
+		return nil, ErrAlreadyReviewed
+	}
+
+	reviewedAt := now
+	sub.Status = StatusRejected
+	sub.ReviewedBy = reviewedBy
+	sub.RejectReason = reason
+	sub.ReviewedAt = &reviewedAt
+	return sub, nil
+}
+
+// List returns every submission, newest first (the admin review queue).
+func (m *Manager) List(ctx context.Context) ([]Submission, error) {
+	return m.Store.ListSubmissions(ctx)
+}
+
+// ListBy returns one user's submissions, newest first (the "my uploads" view).
+func (m *Manager) ListBy(ctx context.Context, submittedBy string) ([]Submission, error) {
+	if strings.TrimSpace(submittedBy) == "" {
+		return nil, invalidf("submitter identity is required")
+	}
+	return m.Store.ListSubmissionsBy(ctx, submittedBy)
+}
+
+// Compile-time proof that the production build subsystem satisfies Builds.
+var _ Builds = (*build.Builder)(nil)

internal/submit/submit_test.go

@@ -0,0 +1,454 @@
+package submit
+
+import (
+	"context"
+	"errors"
+	"strings"
+	"testing"
+	"time"
+
+	"felis.lolicon.best/internal/build"
+)
+
+// testNow is the frozen clock for hermetic assertions.
+var testNow = time.Date(2026, 1, 1, 12, 0, 0, 0, time.UTC)
+
+const (
+	testRegistry = "registry.felis.svc:5000"
+	testContext  = "s3://felis-user-uploads"
+)
+
+// fakeStore is an in-memory Store with CAS semantics and error injection.
+type fakeStore struct {
+	subs map[string]*Submission
+
+	createErr  error
+	approveErr error
+	rejectErr  error
+	linkErr    error
+
+	linked []string // "id=buildID" recorder
+}
+
+func newFakeStore() *fakeStore { return &fakeStore{subs: map[string]*Submission{}} }
+
+func (f *fakeStore) CreateSubmission(_ context.Context, s *Submission) error {
+	if f.createErr != nil {
+		return f.createErr
+	}
+	cp := *s
+	f.subs[s.ID] = &cp
+	return nil
+}
+
+func (f *fakeStore) GetSubmission(_ context.Context, id string) (*Submission, error) {
+	s, ok := f.subs[id]
+	if !ok {
+		return nil, ErrNotFound
+	}
+	cp := *s
+	return &cp, nil
+}
+
+func (f *fakeStore) ListSubmissions(_ context.Context) ([]Submission, error) {
+	out := make([]Submission, 0, len(f.subs))
+	for _, s := range f.subs {
+		out = append(out, *s)
+	}
+	return out, nil
+}
+
+func (f *fakeStore) ListSubmissionsBy(_ context.Context, by string) ([]Submission, error) {
+	var out []Submission
+	for _, s := range f.subs {
+		if s.SubmittedBy == by {
+			out = append(out, *s)
+		}
+	}
+	return out, nil
+}
+
+func (f *fakeStore) ApproveSubmission(_ context.Context, id, reviewedBy, imageRef string, at time.Time) (bool, error) {
+	if f.approveErr != nil {
+		return false, f.approveErr
+	}
+	s, ok := f.subs[id]
+	if !ok || s.Status != StatusPendingReview {
+		return false, nil // CAS lost / nonexistent
+	}
+	s.Status = StatusApproved
+	s.ImageRef = imageRef
+	s.ReviewedBy = reviewedBy
+	t := at
+	s.ReviewedAt = &t
+	return true, nil
+}
+
+func (f *fakeStore) RejectSubmission(_ context.Context, id, reviewedBy, reason string, at time.Time) (bool, error) {
+	if f.rejectErr != nil {
+		return false, f.rejectErr
+	}
+	s, ok := f.subs[id]
+	if !ok || s.Status != StatusPendingReview {
+		return false, nil
+	}
+	s.Status = StatusRejected
+	s.ReviewedBy = reviewedBy
+	s.RejectReason = reason
+	t := at
+	s.ReviewedAt = &t
+	return true, nil
+}
+
+func (f *fakeStore) LinkBuild(_ context.Context, id, buildID string) error {
+	if f.linkErr != nil {
+		return f.linkErr
+	}
+	s, ok := f.subs[id]
+	if !ok {
+		return ErrNotFound
+	}
+	s.BuildID = buildID
+	f.linked = append(f.linked, id+"="+buildID)
+	return nil
+}
+
+// fakeBuilds records Submit calls and can inject a failure.
+type fakeBuilds struct {
+	submitErr error
+	calls     int
+	got       []build.Request
+}
+
+func (f *fakeBuilds) Submit(_ context.Context, req build.Request) (*build.Build, error) {
+	f.calls++
+	f.got = append(f.got, req)
+	if f.submitErr != nil {
+		return nil, f.submitErr
+	}
+	return &build.Build{ID: "bld-x", ImageRef: req.ImageRef, Status: build.StatusBuilding}, nil
+}
+
+// newManager wires the fakes with a frozen clock and a deterministic lowercase
+// id generator (sub-1, sub-2, ...).
+func newManager() (*Manager, *fakeStore, *fakeBuilds) {
+	st := newFakeStore()
+	bl := &fakeBuilds{}
+	n := 0
+	m := &Manager{
+		Store:        st,
+		Builds:       bl,
+		Registry:     testRegistry,
+		ContextStore: testContext,
+		Now:          func() time.Time { return testNow },
+		IDGen:        func() string { n++; return "sub-" + string(rune('0'+n)) },
+	}
+	return m, st, bl
+}
+
+func TestCreatePendingDoesNotBuild(t *testing.T) {
+	m, st, bl := newManager()
+
+	sub, err := m.Create(context.Background(), CreateRequest{
+		DisplayName: "My Modpack",
+		SubmittedBy: "user-1",
+	})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	if sub.Status != StatusPendingReview {
+		t.Fatalf("status = %q, want pending_review", sub.Status)
+	}
+	if sub.ID != "sub-1" {
+		t.Fatalf("id = %q, want sub-1", sub.ID)
+	}
+	// Context ref is derived, never user-supplied.
+	if want := testContext + "/sub-1/context.tar.gz"; sub.ContextRef != want {
+		t.Fatalf("context_ref = %q, want %q", sub.ContextRef, want)
+	}
+	if sub.ImageRef != "" || sub.BuildID != "" {
+		t.Fatalf("image/build set before approval: %+v", sub)
+	}
+	if bl.calls != 0 {
+		t.Fatalf("Create started %d builds, want 0", bl.calls)
+	}
+	if _, ok := st.subs["sub-1"]; !ok {
+		t.Fatal("submission not persisted")
+	}
+}
+
+func TestCreateValidation(t *testing.T) {
+	cases := []struct {
+		name string
+		req  CreateRequest
+	}{
+		{"empty name", CreateRequest{DisplayName: "  ", SubmittedBy: "user-1"}},
+		{"oversize name", CreateRequest{DisplayName: strings.Repeat("a", maxDisplayName+1), SubmittedBy: "user-1"}},
+		{"control chars", CreateRequest{DisplayName: "bad\nname", SubmittedBy: "user-1"}},
+		{"no submitter", CreateRequest{DisplayName: "ok", SubmittedBy: ""}},
+	}
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			m, st, bl := newManager()
+			_, err := m.Create(context.Background(), tc.req)
+			if !errors.Is(err, ErrInvalid) {
+				t.Fatalf("err = %v, want ErrInvalid", err)
+			}
+			if len(st.subs) != 0 || bl.calls != 0 {
+				t.Fatal("invalid Create persisted or built")
+			}
+		})
+	}
+}
+
+func TestApproveStartsExactlyOneBuildAndLinks(t *testing.T) {
+	m, st, bl := newManager()
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+
+	sub, err := m.Approve(context.Background(), seed.ID, "[email protected]")
+	if err != nil {
+		t.Fatalf("Approve: %v", err)
+	}
+	if sub.Status != StatusApproved {
+		t.Fatalf("status = %q, want approved", sub.Status)
+	}
+	wantImage := testRegistry + "/user-uploads/sub-1:latest"
+	if sub.ImageRef != wantImage {
+		t.Fatalf("image_ref = %q, want %q", sub.ImageRef, wantImage)
+	}
+	if sub.BuildID != "bld-x" {
+		t.Fatalf("build_id = %q, want bld-x", sub.BuildID)
+	}
+	if sub.ReviewedBy != "[email protected]" || sub.ReviewedAt == nil {
+		t.Fatalf("review metadata missing: %+v", sub)
+	}
+	// Exactly one build, carrying the derived refs + reviewer + audit Dockerfile.
+	if bl.calls != 1 {
+		t.Fatalf("builds started = %d, want 1", bl.calls)
+	}
+	req := bl.got[0]
+	if req.ImageRef != wantImage {
+		t.Fatalf("build ImageRef = %q, want %q", req.ImageRef, wantImage)
+	}
+	if req.ContextRef != seed.ContextRef {
+		t.Fatalf("build ContextRef = %q, want %q", req.ContextRef, seed.ContextRef)
+	}
+	if req.RequestedBy != "[email protected]" {
+		t.Fatalf("build RequestedBy = %q, want admin", req.RequestedBy)
+	}
+	if strings.TrimSpace(req.Dockerfile) == "" {
+		t.Fatal("audit Dockerfile must be non-empty (build.Validate requires it)")
+	}
+	// The persisted row links the build.
+	if got := st.subs[seed.ID]; got.BuildID != "bld-x" || got.Status != StatusApproved {
+		t.Fatalf("persisted row not linked/approved: %+v", got)
+	}
+	if len(st.linked) != 1 {
+		t.Fatalf("LinkBuild called %d times, want 1", len(st.linked))
+	}
+}
+
+func TestApproveDerivedRefValidates(t *testing.T) {
+	// The derived push target must satisfy the build subsystem's own admission
+	// rules, else Approve would CAS then fail at Submit. Prove it against the SAME
+	// validator the Builder uses.
+	m, _, _ := newManager()
+	ref := m.deriveImageRef("sub-1")
+	req := build.Request{
+		ImageRef:   ref,
+		Dockerfile: auditDockerfile("sub-1", m.deriveContextRef("sub-1")),
+		ContextRef: m.deriveContextRef("sub-1"),
+	}
+	if err := build.Validate(req, build.Config{RegistryURL: testRegistry}); err != nil {
+		t.Fatalf("derived build request does not validate: %v", err)
+	}
+	if !strings.HasPrefix(ref, testRegistry+"/user-uploads/") {
+		t.Fatalf("derived ref %q is not in the user-uploads namespace", ref)
+	}
+}
+
+func TestApproveIsCASNoDoubleBuild(t *testing.T) {
+	m, _, bl := newManager()
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+
+	if _, err := m.Approve(context.Background(), seed.ID, "admin@x"); err != nil {
+		t.Fatalf("first Approve: %v", err)
+	}
+	// A second approve loses the CAS and must NOT start another build.
+	_, err := m.Approve(context.Background(), seed.ID, "admin@x")
+	if !errors.Is(err, ErrAlreadyReviewed) {
+		t.Fatalf("second Approve err = %v, want ErrAlreadyReviewed", err)
+	}
+	if bl.calls != 1 {
+		t.Fatalf("builds started = %d, want 1 (no double-build)", bl.calls)
+	}
+}
+
+func TestApproveRejectedSubmission(t *testing.T) {
+	m, _, bl := newManager()
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+	if _, err := m.Reject(context.Background(), seed.ID, "admin@x", "nope"); err != nil {
+		t.Fatalf("Reject: %v", err)
+	}
+	_, err := m.Approve(context.Background(), seed.ID, "admin@x")
+	if !errors.Is(err, ErrAlreadyReviewed) {
+		t.Fatalf("Approve after reject err = %v, want ErrAlreadyReviewed", err)
+	}
+	if bl.calls != 0 {
+		t.Fatalf("builds started = %d, want 0", bl.calls)
+	}
+}
+
+func TestApproveUnknown(t *testing.T) {
+	m, _, _ := newManager()
+	_, err := m.Approve(context.Background(), "sub-nope", "admin@x")
+	if !errors.Is(err, ErrNotFound) {
+		t.Fatalf("err = %v, want ErrNotFound", err)
+	}
+}
+
+func TestApproveBuildFailureLeavesApprovedUnlinked(t *testing.T) {
+	// The post-CAS Submit failure is the documented KNOWN-LIMITATION: the row is
+	// left approved with build_id NULL, recoverable by an admin via the direct
+	// build path. It must NOT roll back to pending or double-count.
+	m, st, bl := newManager()
+	bl.submitErr = errors.New("apiserver unreachable")
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+
+	_, err := m.Approve(context.Background(), seed.ID, "admin@x")
+	if err == nil {
+		t.Fatal("Approve should surface the build hand-off failure")
+	}
+	if bl.calls != 1 {
+		t.Fatalf("builds attempted = %d, want 1", bl.calls)
+	}
+	got := st.subs[seed.ID]
+	if got.Status != StatusApproved {
+		t.Fatalf("status = %q, want approved (CAS already committed)", got.Status)
+	}
+	if got.BuildID != "" {
+		t.Fatalf("build_id = %q, want empty (Submit failed before LinkBuild)", got.BuildID)
+	}
+	if len(st.linked) != 0 {
+		t.Fatal("LinkBuild must not run when Submit failed")
+	}
+}
+
+func TestApproveLinkFailureNamesRunningBuild(t *testing.T) {
+	// The WORSE post-CAS partial state: Submit SUCCEEDS (a build is genuinely
+	// running and will push to image_ref) but the follow-up LinkBuild write fails,
+	// so the row is approved with build_id still NULL. On the row this is
+	// indistinguishable from the benign Submit-fail case above, so a blind re-drive
+	// would double-build/double-push to :latest. Approve must therefore return a
+	// DISTINCT error that names the running build id and forbids re-submission —
+	// this exercises that branch (the benign test above does not).
+	m, st, bl := newManager()
+	st.linkErr = errors.New("db write timeout")
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+
+	_, err := m.Approve(context.Background(), seed.ID, "admin@x")
+	if err == nil {
+		t.Fatal("Approve must surface the link-write failure (a build is running)")
+	}
+	// A build WAS started — that is precisely why re-driving is unsafe.
+	if bl.calls != 1 {
+		t.Fatalf("builds started = %d, want 1 (Submit succeeded, build is running)", bl.calls)
+	}
+	// The error must name the running build id and the do-not-resubmit warning, and
+	// wrap the underlying store error so callers can still inspect the cause.
+	if !strings.Contains(err.Error(), "bld-x") {
+		t.Fatalf("error must name the running build id bld-x: %v", err)
+	}
+	if !strings.Contains(err.Error(), "do NOT re-submit") {
+		t.Fatalf("error must warn against re-submission: %v", err)
+	}
+	if !errors.Is(err, st.linkErr) {
+		t.Fatalf("error must wrap the underlying link failure: %v", err)
+	}
+	// The row is approved (CAS committed) but build_id stays empty — the very
+	// ambiguity the distinct error compensates for.
+	got := st.subs[seed.ID]
+	if got.Status != StatusApproved {
+		t.Fatalf("status = %q, want approved (CAS already committed)", got.Status)
+	}
+	if got.BuildID != "" {
+		t.Fatalf("build_id = %q, want empty (LinkBuild failed to record it)", got.BuildID)
+	}
+	if len(st.linked) != 0 {
+		t.Fatal("LinkBuild errored before recording — must not appear linked")
+	}
+}
+
+func TestApproveValidatesBeforeCAS(t *testing.T) {
+	// A misconfigured registry makes the derived ref fail build.Validate. That
+	// deterministic failure must happen BEFORE the CAS, so the row stays pending
+	// and no build is attempted — never a stranded approved row.
+	m, st, bl := newManager()
+	m.Registry = "" // derived ref becomes "/user-uploads/...", not host-qualified
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+
+	_, err := m.Approve(context.Background(), seed.ID, "admin@x")
+	if !errors.Is(err, build.ErrInvalid) {
+		t.Fatalf("err = %v, want build.ErrInvalid (pre-CAS validation)", err)
+	}
+	if got := st.subs[seed.ID]; got.Status != StatusPendingReview {
+		t.Fatalf("status = %q, want still pending_review (CAS not reached)", got.Status)
+	}
+	if bl.calls != 0 {
+		t.Fatalf("builds started = %d, want 0", bl.calls)
+	}
+}
+
+func TestRejectDoesNotBuild(t *testing.T) {
+	m, st, bl := newManager()
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+
+	sub, err := m.Reject(context.Background(), seed.ID, "admin@x", "ships a coin miner")
+	if err != nil {
+		t.Fatalf("Reject: %v", err)
+	}
+	if sub.Status != StatusRejected || sub.RejectReason != "ships a coin miner" {
+		t.Fatalf("reject verdict not recorded: %+v", sub)
+	}
+	if sub.ReviewedBy != "admin@x" || sub.ReviewedAt == nil {
+		t.Fatalf("review metadata missing: %+v", sub)
+	}
+	if bl.calls != 0 {
+		t.Fatalf("builds started = %d, want 0", bl.calls)
+	}
+	if st.subs[seed.ID].Status != StatusRejected {
+		t.Fatal("rejection not persisted")
+	}
+}
+
+func TestRejectRequiresReason(t *testing.T) {
+	m, st, _ := newManager()
+	seed, _ := m.Create(context.Background(), CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"})
+	_, err := m.Reject(context.Background(), seed.ID, "admin@x", "   ")
+	if !errors.Is(err, ErrInvalid) {
+		t.Fatalf("err = %v, want ErrInvalid", err)
+	}
+	if got := st.subs[seed.ID]; got.Status != StatusPendingReview {
+		t.Fatalf("status = %q, want pending_review", got.Status)
+	}
+}
+
+func TestListBy(t *testing.T) {
+	m, _, _ := newManager()
+	if _, err := m.Create(context.Background(), CreateRequest{DisplayName: "A", SubmittedBy: "user-1"}); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := m.Create(context.Background(), CreateRequest{DisplayName: "B", SubmittedBy: "user-2"}); err != nil {
+		t.Fatal(err)
+	}
+	mine, err := m.ListBy(context.Background(), "user-1")
+	if err != nil {
+		t.Fatalf("ListBy: %v", err)
+	}
+	if len(mine) != 1 || mine[0].SubmittedBy != "user-1" {
+		t.Fatalf("ListBy scoped wrong: %+v", mine)
+	}
+	if _, err := m.ListBy(context.Background(), ""); !errors.Is(err, ErrInvalid) {
+		t.Fatalf("empty submitter err = %v, want ErrInvalid", err)
+	}
+}