Files
Felis/internal/api/submissions_test.go
T
Lemon-miaow 72c4aa3895 fix(submissions): surface each linked build's outcome to the submitter
/me/submissions (and the admin queue) now attach build_status/build_error by
a read-only Builder.Get — until now a failed build was visible only on the
admin-tier /images/build routes, so the person who submitted the modpack
never learned the build died. A missing build row renders as "no outcome";
any other lookup failure surfaces instead of being swallowed. The panel's
My Submissions page renders the outcome in the expanded row, localised.
2026-09-23 06:31:13 +08:00

526 lines
20 KiB
Go

package api
import (
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"strings"
"testing"
"felis.lolicon.best/internal/build"
"felis.lolicon.best/internal/submit"
)
// fakeSubmissions is an in-memory SubmissionService for the handler tests. Each
// *Err field injects a canned outcome; the recorders let a test assert exactly
// what the handler forwarded (the point of the owner-scoping checks: the
// submitter and reviewer must come from the principal, never the body).
type fakeSubmissions struct {
created *submit.CreateRequest
createErr error
uploadedID string
uploadedBy string
uploadedN int64
uploadErr error
listedBy string
byResult []submit.Submission
byErr error
listed []submit.Submission
listErr error
approvedID string
approvedBy string
approveErr error
rejectedID string
rejectedBy string
rejectReas string
rejectErr error
openedID string
openBody string
openErr error
}
func (f *fakeSubmissions) Create(_ context.Context, req submit.CreateRequest) (*submit.Submission, error) {
if f.createErr != nil {
return nil, f.createErr
}
cp := req
f.created = &cp
return &submit.Submission{ID: "sub-1", SubmittedBy: req.SubmittedBy,
DisplayName: req.DisplayName, Status: submit.StatusPendingReview}, nil
}
func (f *fakeSubmissions) UploadContext(_ context.Context, id, submittedBy string, r io.Reader) (*submit.Submission, error) {
f.uploadedID, f.uploadedBy = id, submittedBy
if f.uploadErr != nil {
return nil, f.uploadErr
}
n, _ := io.Copy(io.Discard, r)
f.uploadedN = n
return &submit.Submission{ID: id, SubmittedBy: submittedBy, Status: submit.StatusPendingReview}, nil
}
func (f *fakeSubmissions) ListBy(_ context.Context, submittedBy string) ([]submit.Submission, error) {
f.listedBy = submittedBy
return f.byResult, f.byErr
}
func (f *fakeSubmissions) List(_ context.Context) ([]submit.Submission, error) {
return f.listed, f.listErr
}
func (f *fakeSubmissions) Approve(_ context.Context, id, reviewedBy string) (*submit.Submission, error) {
f.approvedID, f.approvedBy = id, reviewedBy
if f.approveErr != nil {
return nil, f.approveErr
}
return &submit.Submission{ID: id, Status: submit.StatusApproved, ReviewedBy: reviewedBy, BuildID: "bld-1"}, nil
}
func (f *fakeSubmissions) Reject(_ context.Context, id, reviewedBy, reason string) (*submit.Submission, error) {
f.rejectedID, f.rejectedBy, f.rejectReas = id, reviewedBy, reason
if f.rejectErr != nil {
return nil, f.rejectErr
}
return &submit.Submission{ID: id, Status: submit.StatusRejected, ReviewedBy: reviewedBy, RejectReason: reason}, nil
}
// openErr injects the OpenContext outcome; the body recorder lets the internal
// route test assert byte-exact streaming and the 404 mapping.
func (f *fakeSubmissions) OpenContext(_ context.Context, id string) (io.ReadCloser, error) {
f.openedID = id
if f.openErr != nil {
return nil, f.openErr
}
return io.NopCloser(strings.NewReader(f.openBody)), nil
}
// appSubAPI wires a submissions service behind an ordinary user principal (the
// app tier — /me/submissions). [email protected] / .test are deliberately not the
// deployment domain.
func appSubAPI(s SubmissionService) *API {
api := newTestAPI(newFakeRepo(), newFakeCluster())
api.Submissions = s
api.External = staticExternal{p: &Principal{UserID: "user-7", Email: "[email protected]", Role: "user"}}
return api
}
// adminSubAPI wires a submissions service behind an admin principal (the admin
// tier — /submissions approve/reject/list).
func adminSubAPI(s SubmissionService) *API {
api := newTestAPI(newFakeRepo(), newFakeCluster())
api.Submissions = s
api.External = staticExternal{p: &Principal{UserID: "admin-1", Email: "[email protected]",
Role: "admin", ViaAdminAccess: true}}
return api
}
// The submitter is the principal, never the body: a valid create stamps the
// authenticated user's id onto the submission.
func TestCreateSubmissionStampsPrincipal(t *testing.T) {
fs := &fakeSubmissions{}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions", `{"display_name":"My Pack"}`, nil)
if w.Code != http.StatusCreated {
t.Fatalf("code = %d, want 201 (%s)", w.Code, w.Body.String())
}
if fs.created == nil {
t.Fatal("Create was not called")
}
if fs.created.SubmittedBy != "user-7" {
t.Errorf("submitted_by = %q, want the principal id user-7", fs.created.SubmittedBy)
}
if fs.created.DisplayName != "My Pack" {
t.Errorf("display_name = %q", fs.created.DisplayName)
}
}
// A client cannot smuggle a submitted_by through the body: decodeJSON rejects the
// unknown field with 400 and the handler never reaches Create.
func TestCreateSubmissionRejectsBodySubmittedBy(t *testing.T) {
fs := &fakeSubmissions{}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions",
`{"display_name":"X","submitted_by":"someone-else"}`, nil)
if w.Code != http.StatusBadRequest {
t.Fatalf("code = %d, want 400 (%s)", w.Code, w.Body.String())
}
if fs.created != nil {
t.Errorf("Create must not be called for an unknown-field body; got %+v", fs.created)
}
}
// A validation failure from the submit layer surfaces as 400 bad_request.
func TestCreateSubmissionValidationIs400(t *testing.T) {
fs := &fakeSubmissions{createErr: fmt.Errorf("%w: display name is required", submit.ErrInvalid)}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions", `{"display_name":""}`, nil)
if w.Code != http.StatusBadRequest {
t.Fatalf("code = %d, want 400 (%s)", w.Code, w.Body.String())
}
if got := decodeErr(t, w); got != "bad_request" {
t.Errorf("error code = %q, want bad_request", got)
}
}
// The upload endpoint forwards the raw body to the transport and stamps the
// submitter from the principal, never the body — a user can only upload to a
// submission under their own identity.
func TestUploadSubmissionContextStreamsBody(t *testing.T) {
fs := &fakeSubmissions{}
api := appSubAPI(fs)
// A tiny gzip-magic-prefixed body stands in for a real context.tar.gz.
body := "\x1f\x8b\x08\x00 the modpack bytes"
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", body,
map[string]string{"Content-Type": "application/gzip"})
if w.Code != http.StatusOK {
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
}
if fs.uploadedID != "sub-9" {
t.Errorf("uploaded id = %q, want sub-9", fs.uploadedID)
}
if fs.uploadedBy != "user-7" {
t.Errorf("submitter = %q, want the principal id user-7", fs.uploadedBy)
}
if fs.uploadedN != int64(len(body)) {
t.Errorf("streamed %d bytes, want %d", fs.uploadedN, len(body))
}
}
// A submission the caller does not own reads back as 404 (the transport reports
// ErrNotFound), so the endpoint cannot probe another user's submission.
func TestUploadSubmissionContextNotOwnedIs404(t *testing.T) {
fs := &fakeSubmissions{uploadErr: submit.ErrNotFound}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-x/context", "\x1f\x8bdata", nil)
if w.Code != http.StatusNotFound {
t.Fatalf("code = %d, want 404 (%s)", w.Code, w.Body.String())
}
}
// Uploading to an already-reviewed submission is a 409.
func TestUploadSubmissionContextAlreadyReviewedIs409(t *testing.T) {
fs := &fakeSubmissions{uploadErr: submit.ErrAlreadyReviewed}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", nil)
if w.Code != http.StatusConflict {
t.Fatalf("code = %d, want 409 (%s)", w.Code, w.Body.String())
}
}
// A wrong-format / oversize body surfaces as 400 (the transport wraps ErrInvalid).
func TestUploadSubmissionContextBadFormatIs400(t *testing.T) {
fs := &fakeSubmissions{uploadErr: fmt.Errorf("%w: build context must be a gzip-compressed tarball (.tar.gz)", submit.ErrInvalid)}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "not gzip", nil)
if w.Code != http.StatusBadRequest {
t.Fatalf("code = %d, want 400 (%s)", w.Code, w.Body.String())
}
if got := decodeErr(t, w); got != "bad_request" {
t.Errorf("error code = %q, want bad_request", got)
}
}
// When the deployment's store has no upload transport, the endpoint reports 503
// (ErrUploadsUnavailable) — an honest "not available here", not a 500.
func TestUploadSubmissionContextNoTransportIs503(t *testing.T) {
fs := &fakeSubmissions{uploadErr: submit.ErrUploadsUnavailable}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", nil)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("code = %d, want 503 (%s)", w.Code, w.Body.String())
}
if got := decodeErr(t, w); got != "uploads_unavailable" {
t.Errorf("error code = %q, want uploads_unavailable", got)
}
}
// The upload route is app-tier: with no service configured it is 503, exactly
// like the other /me/submissions routes.
func TestUploadSubmissionContextWithoutServiceIs503(t *testing.T) {
app := appSubAPI(nil)
app.Submissions = nil
w := do(app.ExternalHandler(), "POST", "/api/v1/me/submissions/sub-9/context", "\x1f\x8bdata", nil)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("code = %d, want 503 (%s)", w.Code, w.Body.String())
}
}
// The "my uploads" list scopes strictly to the principal's id — there is no
// parameter that could widen it to another user's submissions.
func TestMySubmissionsScopesToPrincipal(t *testing.T) {
fs := &fakeSubmissions{byResult: []submit.Submission{
{ID: "sub-1", SubmittedBy: "user-7", DisplayName: "p", Status: submit.StatusPendingReview},
}}
api := appSubAPI(fs)
w := do(api.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil)
if w.Code != http.StatusOK {
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
}
if fs.listedBy != "user-7" {
t.Errorf("ListBy scoped to %q, want the principal id user-7", fs.listedBy)
}
var got map[string][]submit.Submission
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
t.Fatalf("body not JSON: %v", err)
}
if len(got["submissions"]) != 1 {
t.Fatalf("submissions = %d, want 1", len(got["submissions"]))
}
}
// The "my uploads" list carries the linked build's outcome — for a submitter it
// is the only visible outlet for a failed build (the /images/build routes are
// admin-tier). A row with no linked build gains no build fields.
func TestMySubmissionsCarriesBuildOutcome(t *testing.T) {
fs := &fakeSubmissions{byResult: []submit.Submission{
{ID: "sub-1", SubmittedBy: "user-7", Status: submit.StatusApproved, BuildID: "bld-9"},
{ID: "sub-2", SubmittedBy: "user-7", Status: submit.StatusPendingReview},
}}
api := appSubAPI(fs)
api.Builder = &fakeBuilder{getBuilds: map[string]*build.Build{
"bld-9": {ID: "bld-9", Status: build.StatusFailed, Error: "build job failed or scan found a CRITICAL CVE"},
}}
w := do(api.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil)
if w.Code != http.StatusOK {
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
}
var got struct {
Submissions []struct {
ID string `json:"id"`
BuildStatus string `json:"build_status"`
BuildError string `json:"build_error"`
} `json:"submissions"`
}
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
t.Fatalf("body not JSON: %v", err)
}
if len(got.Submissions) != 2 {
t.Fatalf("submissions = %d, want 2", len(got.Submissions))
}
if got.Submissions[0].BuildStatus != "failed" || got.Submissions[0].BuildError == "" {
t.Errorf("sub-1 outcome = %+v, want failed with the error text", got.Submissions[0])
}
if got.Submissions[1].BuildStatus != "" || got.Submissions[1].BuildError != "" {
t.Errorf("sub-2 outcome = %+v, want no build fields without a linked build", got.Submissions[1])
}
}
// A linked build whose row is gone renders as "no outcome" rather than failing
// the whole list; any other lookup failure must surface, never be swallowed.
func TestMySubmissionsBuildLookupSemantics(t *testing.T) {
// Missing row (ErrNotFound): 200 with no build fields.
fs := &fakeSubmissions{byResult: []submit.Submission{{ID: "sub-1", BuildID: "bld-gone"}}}
api := appSubAPI(fs)
api.Builder = &fakeBuilder{}
w := do(api.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil)
if w.Code != http.StatusOK {
t.Fatalf("missing build row: code = %d, want 200 (%s)", w.Code, w.Body.String())
}
if strings.Contains(w.Body.String(), "build_status") {
t.Errorf("missing build row: body carries build fields: %s", w.Body.String())
}
// Store fault: the failure is reported, not hidden behind a 200.
fs = &fakeSubmissions{byResult: []submit.Submission{{ID: "sub-1", BuildID: "bld-1"}}}
api = appSubAPI(fs)
api.Builder = &fakeBuilder{getErr: errors.New("db down")}
w = do(api.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil)
if w.Code != http.StatusInternalServerError {
t.Fatalf("store fault: code = %d, want 500 (%s)", w.Code, w.Body.String())
}
}
// Every /submissions route is admin-tier: a plain user is rejected before the
// handler runs.
func TestSubmissionAdminRoutesAreAdminOnly(t *testing.T) {
cases := []struct {
method, target, body string
}{
{"GET", "/api/v1/submissions", ""},
{"POST", "/api/v1/submissions/sub-1/approve", ""},
{"POST", "/api/v1/submissions/sub-1/reject", `{"reason":"no"}`},
}
for _, c := range cases {
api := adminSubAPI(&fakeSubmissions{})
// downgrade to a plain user
api.External = staticExternal{p: &Principal{UserID: "u", Role: "user"}}
w := do(api.ExternalHandler(), c.method, c.target, c.body, nil)
if w.Code != http.StatusForbidden {
t.Errorf("%s %s: code = %d, want 403", c.method, c.target, w.Code)
}
}
}
func TestListSubmissionsAdmin(t *testing.T) {
fs := &fakeSubmissions{listed: []submit.Submission{
{ID: "sub-1", SubmittedBy: "user-7", Status: submit.StatusPendingReview},
{ID: "sub-2", SubmittedBy: "user-9", Status: submit.StatusApproved, BuildID: "bld-2"},
}}
api := adminSubAPI(fs)
api.Builder = &fakeBuilder{getBuilds: map[string]*build.Build{
"bld-2": {ID: "bld-2", Status: build.StatusSucceeded},
}}
w := do(api.ExternalHandler(), "GET", "/api/v1/submissions", "", nil)
if w.Code != http.StatusOK {
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
}
var got map[string][]submit.Submission
if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil {
t.Fatalf("body not JSON: %v", err)
}
if len(got["submissions"]) != 2 {
t.Fatalf("submissions = %d, want 2", len(got["submissions"]))
}
// The admin queue carries the same build outcome enrichment.
if !strings.Contains(w.Body.String(), `"build_status":"succeeded"`) {
t.Errorf("admin queue lacks the linked build outcome: %s", w.Body.String())
}
}
// The reviewer is the admin principal's email, never client input.
func TestApproveSubmission(t *testing.T) {
fs := &fakeSubmissions{}
api := adminSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/submissions/sub-9/approve", "", nil)
if w.Code != http.StatusOK {
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
}
if fs.approvedID != "sub-9" {
t.Errorf("approved id = %q, want sub-9", fs.approvedID)
}
if fs.approvedBy != "[email protected]" {
t.Errorf("reviewer = %q, want the admin email", fs.approvedBy)
}
}
func TestApproveSubmissionAlreadyReviewedIs409(t *testing.T) {
fs := &fakeSubmissions{approveErr: submit.ErrAlreadyReviewed}
api := adminSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/submissions/sub-9/approve", "", nil)
if w.Code != http.StatusConflict {
t.Fatalf("code = %d, want 409 (%s)", w.Code, w.Body.String())
}
if got := decodeErr(t, w); got != "already_reviewed" {
t.Errorf("error code = %q, want already_reviewed", got)
}
}
func TestApproveSubmissionNotFoundIs404(t *testing.T) {
fs := &fakeSubmissions{approveErr: submit.ErrNotFound}
api := adminSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/submissions/missing/approve", "", nil)
if w.Code != http.StatusNotFound {
t.Fatalf("code = %d, want 404 (%s)", w.Code, w.Body.String())
}
}
// A build hand-off failure post-approval is a server-side fault (the client did
// nothing wrong), so it collapses to 500 — never a 4xx.
func TestApproveBuildHandoffFailureIs500(t *testing.T) {
fs := &fakeSubmissions{approveErr: fmt.Errorf("submit: approved but build hand-off failed: %w",
errors.New("cluster unreachable"))}
api := adminSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/submissions/sub-9/approve", "", nil)
if w.Code != http.StatusInternalServerError {
t.Fatalf("code = %d, want 500 (%s)", w.Code, w.Body.String())
}
if got := decodeErr(t, w); got != "internal" {
t.Errorf("error code = %q, want internal", got)
}
}
func TestRejectSubmission(t *testing.T) {
fs := &fakeSubmissions{}
api := adminSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/submissions/sub-3/reject",
`{"reason":"contains malware"}`, nil)
if w.Code != http.StatusOK {
t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String())
}
if fs.rejectedID != "sub-3" {
t.Errorf("rejected id = %q, want sub-3", fs.rejectedID)
}
if fs.rejectedBy != "[email protected]" {
t.Errorf("reviewer = %q, want the admin email", fs.rejectedBy)
}
if fs.rejectReas != "contains malware" {
t.Errorf("reason = %q", fs.rejectReas)
}
}
func TestRejectSubmissionValidationIs400(t *testing.T) {
fs := &fakeSubmissions{rejectErr: fmt.Errorf("%w: a reject reason is required", submit.ErrInvalid)}
api := adminSubAPI(fs)
w := do(api.ExternalHandler(), "POST", "/api/v1/submissions/sub-3/reject", `{"reason":""}`, nil)
if w.Code != http.StatusBadRequest {
t.Fatalf("code = %d, want 400 (%s)", w.Code, w.Body.String())
}
if got := decodeErr(t, w); got != "bad_request" {
t.Errorf("error code = %q, want bad_request", got)
}
}
// When no Submissions service is configured, the routes report 503 — the app
// route directly, and the admin route only AFTER the admin gate, so the boundary
// is still enforced first.
func TestSubmissionRoutesWithoutServiceAre503(t *testing.T) {
// app tier
app := appSubAPI(nil)
app.Submissions = nil
if w := do(app.ExternalHandler(), "GET", "/api/v1/me/submissions", "", nil); w.Code != http.StatusServiceUnavailable {
t.Fatalf("app route: code = %d, want 503", w.Code)
}
// admin tier
adm := adminSubAPI(nil)
adm.Submissions = nil
if w := do(adm.ExternalHandler(), "GET", "/api/v1/submissions", "", nil); w.Code != http.StatusServiceUnavailable {
t.Fatalf("admin route: code = %d, want 503", w.Code)
}
}
// The internal context route is the build Pod's only read path to a submission's
// blob: it streams the bytes verbatim, and its error mapping distinguishes a
// missing blob (404) from an unwired transport (503).
func TestInternalSubmissionContextRoute(t *testing.T) {
newAPI := func(s SubmissionService) *API {
api := newTestAPI(newFakeRepo(), newFakeCluster())
api.Submissions = s
return api
}
t.Run("streams the blob", func(t *testing.T) {
fs := &fakeSubmissions{openBody: "\x1f\x8b\x08\x00blob"}
w := do(newAPI(fs).InternalHandler(), "GET", "/api/v1/internal/submissions/sub-7/context", "", nil)
if w.Code != http.StatusOK {
t.Fatalf("code = %d body %s", w.Code, w.Body.String())
}
if w.Body.String() != fs.openBody {
t.Fatalf("body = %q, want the stored blob %q", w.Body.String(), fs.openBody)
}
if fs.openedID != "sub-7" {
t.Fatalf("opened id = %q, want the path id", fs.openedID)
}
if ct := w.Header().Get("Content-Type"); ct != "application/gzip" {
t.Fatalf("content-type = %q, want application/gzip", ct)
}
})
t.Run("missing blob is 404", func(t *testing.T) {
fs := &fakeSubmissions{openErr: fmt.Errorf("%w: gone", submit.ErrBlobNotFound)}
w := do(newAPI(fs).InternalHandler(), "GET", "/api/v1/internal/submissions/sub-7/context", "", nil)
if w.Code != http.StatusNotFound {
t.Fatalf("code = %d, want 404", w.Code)
}
})
t.Run("unwired transport is 503", func(t *testing.T) {
w := do(newAPI(nil).InternalHandler(), "GET", "/api/v1/internal/submissions/sub-7/context", "", nil)
if w.Code != http.StatusServiceUnavailable {
t.Fatalf("code = %d, want 503", w.Code)
}
})
}