diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 0692b34..aed73f1 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -4355,6 +4355,41 @@ paths: '503': $ref: '#/components/responses/ServiceUnavailable' + /api/v1/me/submissions/{id}: + delete: + tags: [submissions] + operationId: withdrawSubmission + summary: Withdraw your own pending submission (user side; user-directed lane over §16). + description: >- + Retracts the caller's own submission while it is still pending review: + the row and its uploaded build context are deleted, freeing the pending + slot and the per-user storage budget for a fresh submission. A reviewed + submission is frozen (409 — its build may already be consuming the + context), and a submission the caller does not own reads back as 404, so + this endpoint cannot probe or clear another user's uploads. + x-felis-face: [external] + x-felis-tier: app + security: [{ accessJWT: [] }] + parameters: + - { name: id, in: path, required: true, schema: { type: string } } + responses: + '200': + description: The withdrawn submission, as it was before the deletion. + content: + application/json: + schema: { $ref: '#/components/schemas/Submission' } + '401': + $ref: '#/components/responses/Unauthorized' + '404': + $ref: '#/components/responses/NotFound' + '409': + description: Submission has already been reviewed and cannot be withdrawn. + content: + application/json: + schema: { $ref: '#/components/schemas/Error' } + '503': + $ref: '#/components/responses/ServiceUnavailable' + # ------------------------------------------------------ external: admin ---- /api/v1/servers/{name}: patch: @@ -4750,3 +4785,35 @@ paths: schema: { $ref: '#/components/schemas/Error' } '503': $ref: '#/components/responses/ServiceUnavailable' + + /api/v1/submissions/{id}: + delete: + tags: [submissions] + operationId: deleteSubmission + summary: Retire a submission outright — row and uploaded context (admin; user-directed lane over §16). + description: >- + Removes the submission and its uploaded build context, any status — the + lane's only lifecycle valve, and the path that reclaims a rejected or + consumed upload from the uploads PVC. The reviewer identity is recorded + in the audit event, not on the (now deleted) row. Deleting an approved + submission whose build is still running fails that build's context + fetch; the admin has explicitly chosen to retire the artifact. + x-felis-face: [external] + x-felis-tier: admin + security: [{ accessJWT: [] }] + parameters: + - { name: id, in: path, required: true, schema: { type: string } } + responses: + '200': + description: The deleted submission, as it was before the deletion. + content: + application/json: + schema: { $ref: '#/components/schemas/Submission' } + '401': + $ref: '#/components/responses/Unauthorized' + '403': + $ref: '#/components/responses/Forbidden' + '404': + $ref: '#/components/responses/NotFound' + '503': + $ref: '#/components/responses/ServiceUnavailable' diff --git a/internal/api/api.go b/internal/api/api.go index a9ec66f..a8eda55 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -518,6 +518,11 @@ func (a *API) externalAPIRoutes() []apiRoute { // App-tier and owner-scoped (the id must belong to the principal), exactly // like the create/list routes above. {Method: "POST", Pattern: "/api/v1/me/submissions/{id}/context", h: a.handleUploadSubmissionContext}, + // Withdraw the caller's OWN pending submission: the row and its uploaded + // context are deleted, freeing the pending slot and storage budget. Same + // owner-scoping as the upload route — a reviewed submission is frozen (409) + // and another user's id is invisible (404). + {Method: "DELETE", Pattern: "/api/v1/me/submissions/{id}", h: a.handleWithdrawSubmission}, // Admin (Zero-Trust) tier: create / mutate spec / image admission. These gate // on Principal.IsAdmin() inside the handler via the adminOnly wrapper, so the // boundary is exercised even where the body is a later-phase stub. @@ -547,6 +552,10 @@ func (a *API) externalAPIRoutes() []apiRoute { {Method: "GET", Pattern: "/api/v1/submissions", Admin: true, h: a.handleListSubmissions}, {Method: "POST", Pattern: "/api/v1/submissions/{id}/approve", Admin: true, h: a.handleApproveSubmission}, {Method: "POST", Pattern: "/api/v1/submissions/{id}/reject", Admin: true, h: a.handleRejectSubmission}, + // Retire a submission outright (row + uploaded context), any status. The + // lane's lifecycle valve: without it, rejected/consumed uploads accumulated + // on the uploads PVC forever — there is no other delete path. + {Method: "DELETE", Pattern: "/api/v1/submissions/{id}", Admin: true, h: a.handleDeleteSubmission}, // The reviewer's read path to the uploaded blob: the executed Dockerfile // lives inside it, so approval would otherwise be blind. {Method: "GET", Pattern: "/api/v1/submissions/{id}/context", Admin: true, h: a.handleAdminSubmissionContext}, diff --git a/internal/api/submissions.go b/internal/api/submissions.go index 4d39100..c551f58 100644 --- a/internal/api/submissions.go +++ b/internal/api/submissions.go @@ -43,6 +43,13 @@ type SubmissionService interface { // Reject is the admin's other verdict: pending_review -> rejected with a // required reason; it starts no build. Reject(ctx context.Context, id, reviewedBy, reason string) (*submit.Submission, error) + // Withdraw retracts the caller's OWN pending submission: the row and its + // uploaded context are deleted. A reviewed submission is frozen (409) and a + // submission the caller does not own reads back as 404, like the upload route. + Withdraw(ctx context.Context, id, submittedBy string) (*submit.Submission, error) + // Delete retires any submission outright (the admin lifecycle valve): the row + // and its uploaded context are removed, any status. + Delete(ctx context.Context, id string) (*submit.Submission, error) // OpenContext returns the stored build-context blob for the internal // context-fetch route: the build Pod's initContainer cannot mount the uploads // PVC across namespaces and holds no object-store credentials, so it streams @@ -294,6 +301,48 @@ func (a *API) handleRejectSubmission(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, sub) } +// handleWithdrawSubmission retracts the caller's own pending submission +// (app-tier): the row and its uploaded context are deleted, freeing the pending +// slot and the storage budget for a fresh submission. The submitter is the +// principal, never the body; a submission the caller does not own is reported as +// 404, so this endpoint cannot probe or clear another user's uploads, and a +// reviewed submission is 409 (its build may already be consuming the context). +func (a *API) handleWithdrawSubmission(w http.ResponseWriter, r *http.Request) { + if a.Submissions == nil { + writeError(w, r, errSubmissionsUnavailable) + return + } + p := principalFromContext(r.Context()) + sub, err := a.Submissions.Withdraw(r.Context(), r.PathValue("id"), p.UserID) + if err != nil { + writeSubmitError(w, r, err) + return + } + a.audit(r, p.Email, "submission.withdraw", sub.ID) + writeJSON(w, http.StatusOK, sub) +} + +// handleDeleteSubmission retires any submission outright (admin-tier): the row +// and its uploaded context are removed, any status. This is the lane's lifecycle +// valve — the only path that reclaims a rejected or consumed upload from the +// uploads PVC. The reviewer identity goes to the audit event, not the (now +// nonexistent) row. Deleting an approved submission whose build is still running +// fails that build's context fetch; the admin has explicitly chosen to retire it. +func (a *API) handleDeleteSubmission(w http.ResponseWriter, r *http.Request) { + if a.Submissions == nil { + writeError(w, r, errSubmissionsUnavailable) + return + } + p := principalFromContext(r.Context()) + sub, err := a.Submissions.Delete(r.Context(), r.PathValue("id")) + if err != nil { + writeSubmitError(w, r, err) + return + } + a.audit(r, p.Email, "submission.delete", sub.ID) + writeJSON(w, http.StatusOK, sub) +} + // errSubmissionsUnavailable is returned when the approval lane is not configured // on this api instance (a nil Submissions service), so the admin/app boundary is // still exercised before the subsystem is wired in. diff --git a/internal/api/submissions_test.go b/internal/api/submissions_test.go index 89aafc3..192681c 100644 --- a/internal/api/submissions_test.go +++ b/internal/api/submissions_test.go @@ -20,27 +20,32 @@ import ( // 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 + 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 + withdrawnID string + withdrawBy string + withdrawErr error + deletedID string + deleteErr error + openedID string + openBody string + openErr error } func (f *fakeSubmissions) Create(_ context.Context, req submit.CreateRequest) (*submit.Submission, error) { @@ -88,6 +93,22 @@ func (f *fakeSubmissions) Reject(_ context.Context, id, reviewedBy, reason strin return &submit.Submission{ID: id, Status: submit.StatusRejected, ReviewedBy: reviewedBy, RejectReason: reason}, nil } +func (f *fakeSubmissions) Withdraw(_ context.Context, id, submittedBy string) (*submit.Submission, error) { + f.withdrawnID, f.withdrawBy = id, submittedBy + if f.withdrawErr != nil { + return nil, f.withdrawErr + } + return &submit.Submission{ID: id, SubmittedBy: submittedBy, Status: submit.StatusPendingReview}, nil +} + +func (f *fakeSubmissions) Delete(_ context.Context, id string) (*submit.Submission, error) { + f.deletedID = id + if f.deleteErr != nil { + return nil, f.deleteErr + } + return &submit.Submission{ID: id, Status: submit.StatusRejected}, 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) { @@ -249,6 +270,69 @@ func TestUploadSubmissionContextWithoutServiceIs503(t *testing.T) { } } +// Withdraw retracts the caller's OWN pending submission: the submitter is the +// principal (never the body), a reviewed submission is 409, and a foreign id is +// 404 — the same posture as the upload route. +func TestWithdrawSubmission(t *testing.T) { + t.Run("withdraws as the principal", func(t *testing.T) { + fs := &fakeSubmissions{} + w := do(appSubAPI(fs).ExternalHandler(), "DELETE", "/api/v1/me/submissions/sub-3", "", nil) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + if fs.withdrawnID != "sub-3" || fs.withdrawBy != "user-7" { + t.Fatalf("withdraw forwarded (%q, %q), want (sub-3, user-7)", fs.withdrawnID, fs.withdrawBy) + } + }) + t.Run("reviewed submission is 409", func(t *testing.T) { + fs := &fakeSubmissions{withdrawErr: submit.ErrAlreadyReviewed} + w := do(appSubAPI(fs).ExternalHandler(), "DELETE", "/api/v1/me/submissions/sub-3", "", 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) + } + }) + t.Run("foreign or unknown id is 404", func(t *testing.T) { + fs := &fakeSubmissions{withdrawErr: submit.ErrNotFound} + w := do(appSubAPI(fs).ExternalHandler(), "DELETE", "/api/v1/me/submissions/sub-x", "", nil) + if w.Code != http.StatusNotFound { + t.Fatalf("code = %d, want 404 (%s)", w.Code, w.Body.String()) + } + }) + t.Run("no service is 503", func(t *testing.T) { + app := appSubAPI(nil) + app.Submissions = nil + w := do(app.ExternalHandler(), "DELETE", "/api/v1/me/submissions/sub-3", "", nil) + if w.Code != http.StatusServiceUnavailable { + t.Fatalf("code = %d, want 503 (%s)", w.Code, w.Body.String()) + } + }) +} + +// The admin delete retires any submission and maps the lane's 404; the route's +// admin gate itself is pinned by TestSubmissionAdminRoutesAreAdminOnly. +func TestDeleteSubmissionAdmin(t *testing.T) { + t.Run("deletes the named row", func(t *testing.T) { + fs := &fakeSubmissions{} + w := do(adminSubAPI(fs).ExternalHandler(), "DELETE", "/api/v1/submissions/sub-8", "", nil) + if w.Code != http.StatusOK { + t.Fatalf("code = %d, want 200 (%s)", w.Code, w.Body.String()) + } + if fs.deletedID != "sub-8" { + t.Fatalf("delete forwarded id %q, want sub-8", fs.deletedID) + } + }) + t.Run("unknown is 404", func(t *testing.T) { + fs := &fakeSubmissions{deleteErr: submit.ErrNotFound} + w := do(adminSubAPI(fs).ExternalHandler(), "DELETE", "/api/v1/submissions/sub-x", "", nil) + if w.Code != http.StatusNotFound { + t.Fatalf("code = %d, want 404 (%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) { @@ -343,6 +427,7 @@ func TestSubmissionAdminRoutesAreAdminOnly(t *testing.T) { {"GET", "/api/v1/submissions", ""}, {"POST", "/api/v1/submissions/sub-1/approve", ""}, {"POST", "/api/v1/submissions/sub-1/reject", `{"reason":"no"}`}, + {"DELETE", "/api/v1/submissions/sub-1", ""}, {"GET", "/api/v1/submissions/sub-1/context", ""}, } for _, c := range cases { diff --git a/internal/pgint/pgint_test.go b/internal/pgint/pgint_test.go index 6f59753..1186ab5 100644 --- a/internal/pgint/pgint_test.go +++ b/internal/pgint/pgint_test.go @@ -1034,6 +1034,36 @@ func TestSubmitStoreContract(t *testing.T) { if got, _ = s.GetSubmission(ctx, id); got.BuildID == "" { t.Fatal("LinkBuild must persist build_id") } + + // Lifecycle: the withdraw CAS deletes ONLY the owner's still-pending row — a + // wrong owner or a reviewed row can never delete through it — and the admin + // path deletes any status, exactly once. + id2 := "sub-w-" + suffix(t) + if err := s.CreateSubmission(ctx, &submit.Submission{ + ID: id2, SubmittedBy: u.ID, DisplayName: "withdraw me", + ContextRef: "s3://bucket/" + id2 + "/context.tar.gz", + Status: submit.StatusPendingReview, CreatedAt: now, + }); err != nil { + t.Fatalf("CreateSubmission(2): %v", err) + } + if ok, err := s.DeletePendingSubmission(ctx, id2, "someone-else"); err != nil || ok { + t.Fatalf("withdraw by a non-owner = (%v, %v), want (false, nil)", ok, err) + } + if ok, err := s.DeletePendingSubmission(ctx, id, u.ID); err != nil || ok { + t.Fatalf("withdraw of a reviewed row = (%v, %v), want (false, nil)", ok, err) + } + if ok, err := s.DeletePendingSubmission(ctx, id2, u.ID); err != nil || !ok { + t.Fatalf("withdraw by the owner = (%v, %v), want (true, nil)", ok, err) + } + if _, err := s.GetSubmission(ctx, id2); !errors.Is(err, submit.ErrNotFound) { + t.Fatalf("withdrawn row still readable: %v", err) + } + if ok, err := s.DeleteSubmission(ctx, id); err != nil || !ok { + t.Fatalf("admin delete = (%v, %v), want (true, nil)", ok, err) + } + if ok, err := s.DeleteSubmission(ctx, id); err != nil || ok { + t.Fatalf("second admin delete = (%v, %v), want (false, nil)", ok, err) + } } // ---- builds -------------------------------------------------------------------- diff --git a/internal/submit/blobstore.go b/internal/submit/blobstore.go index c82403f..3841aa2 100644 --- a/internal/submit/blobstore.go +++ b/internal/submit/blobstore.go @@ -149,5 +149,19 @@ func (s *LocalContextStore) Size(_ context.Context, id string) (int64, bool, err } } +// Delete removes everything stored for id — the blob plus its id-namespaced +// directory (a stray temp file from an interrupted upload goes with it) — after +// the submission row is gone. Idempotent: nothing stored is success. +func (s *LocalContextStore) Delete(_ context.Context, id string) error { + dir, err := s.dir(id) + if err != nil { + return err + } + if err := os.RemoveAll(dir); err != nil { + return fmt.Errorf("submit: remove context blob: %w", err) + } + return nil +} + // Compile-time proof that the filesystem store satisfies the Blobs transport. var _ Blobs = (*LocalContextStore)(nil) diff --git a/internal/submit/blobstore_test.go b/internal/submit/blobstore_test.go index fe36ed6..ab3b2ff 100644 --- a/internal/submit/blobstore_test.go +++ b/internal/submit/blobstore_test.go @@ -109,6 +109,35 @@ func TestLocalContextStorePutOverwrites(t *testing.T) { } } +// Delete removes the blob and its id-namespaced directory, and is idempotent — +// the retry-safety the withdraw/delete cleanup depends on. +func TestLocalContextStoreDelete(t *testing.T) { + base := t.TempDir() + s := &LocalContextStore{Base: base} + ctx := context.Background() + + if _, err := s.Put(ctx, "sub-abc", strings.NewReader("\x1f\x8bbytes")); err != nil { + t.Fatalf("Put: %v", err) + } + if err := s.Delete(ctx, "sub-abc"); err != nil { + t.Fatalf("Delete: %v", err) + } + if ok, err := s.Exists(ctx, "sub-abc"); err != nil || ok { + t.Fatalf("Exists after Delete = (%v, %v), want (false, nil)", ok, err) + } + if _, err := os.Stat(filepath.Join(base, "sub-abc")); !os.IsNotExist(err) { + t.Fatalf("per-submission dir still present after Delete (err=%v)", err) + } + // Idempotent: deleting nothing is success, so a retried cleanup cannot fail. + if err := s.Delete(ctx, "sub-abc"); err != nil { + t.Fatalf("second Delete = %v, want nil (idempotent)", err) + } + // The same path guard as Put/Open. + if err := s.Delete(ctx, "../etc"); err == nil { + t.Fatal("Delete must reject an unsafe id") + } +} + func TestLocalContextStoreRejectsUnsafeID(t *testing.T) { base := t.TempDir() s := &LocalContextStore{Base: base} diff --git a/internal/submit/pgstore.go b/internal/submit/pgstore.go index 58a0252..d0dea34 100644 --- a/internal/submit/pgstore.go +++ b/internal/submit/pgstore.go @@ -57,14 +57,16 @@ func (s *PGStore) ListSubmissionsBy(ctx context.Context, submittedBy string) ([] 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. +// cas executes a single-statement compare-and-set — an UPDATE or DELETE whose +// WHERE clause is the predicate — and reports whether THIS call moved a row. The +// `status = 'pending_review'` guard is the actual CAS predicate in each caller's +// query and is deliberately kept INLINE — it is security-visible, so a reader +// auditing "can a non-pending row be flipped or deleted?" must see it next to the +// SET or DELETE. cas only folds the shared ExecContext + RowsAffected tail so +// the reviewers and deleters cannot drift in how they report a lost race or a +// RowsAffected error. n == 0 means a concurrent actor already won the row — +// reported as won=false (never an error), which the Manager maps to +// ErrAlreadyReviewed / ErrNotFound. func (s *PGStore) cas(ctx context.Context, q string, args ...any) (bool, error) { res, err := s.db.ExecContext(ctx, q, args...) if err != nil { @@ -107,6 +109,21 @@ func (s *PGStore) LinkBuild(ctx context.Context, id, buildID string) error { return nil } +// DeleteSubmission is the admin delete: any status, one row, no predicate beyond +// the id. False means the id was already gone (a concurrent delete won). +func (s *PGStore) DeleteSubmission(ctx context.Context, id string) (bool, error) { + const q = `DELETE FROM image_submissions WHERE id = $1` + return s.cas(ctx, q, id) +} + +// DeletePendingSubmission is the withdraw CAS: owner + pending_review must both +// still hold, so a reviewed submission can never be deleted through this path. +func (s *PGStore) DeletePendingSubmission(ctx context.Context, id, submittedBy string) (bool, error) { + const q = `DELETE FROM image_submissions + WHERE id = $1 AND submitted_by = $2 AND status = 'pending_review'` + return s.cas(ctx, q, id, submittedBy) +} + func (s *PGStore) querySubmissions(ctx context.Context, q string, args ...any) ([]Submission, error) { rows, err := s.db.QueryContext(ctx, q, args...) if err != nil { diff --git a/internal/submit/s3store.go b/internal/submit/s3store.go index 80c4f98..aa75619 100644 --- a/internal/submit/s3store.go +++ b/internal/submit/s3store.go @@ -21,6 +21,7 @@ type s3Client interface { PutObject(ctx context.Context, bucket, object string, reader io.Reader, size int64, opts minio.PutObjectOptions) (minio.UploadInfo, error) StatObject(ctx context.Context, bucket, object string, opts minio.StatObjectOptions) (minio.ObjectInfo, error) GetObject(ctx context.Context, bucket, object string, opts minio.GetObjectOptions) (s3Object, error) + RemoveObject(ctx context.Context, bucket, object string, opts minio.RemoveObjectOptions) error } // s3Object is the handle GetObject yields: a stream whose Stat performs the HEAD @@ -205,6 +206,20 @@ func (s *S3ContextStore) Size(ctx context.Context, id string) (int64, bool, erro return info.Size, true, nil } +// Delete removes the stored object for the withdrawn/deleted submission. S3's +// DELETE is idempotent — removing an absent key succeeds — which is exactly the +// contract the cleanup path needs on a retry. +func (s *S3ContextStore) Delete(ctx context.Context, id string) error { + key, err := s.keyFor(id) + if err != nil { + return err + } + if err := s.client.RemoveObject(ctx, s.bucket, key, minio.RemoveObjectOptions{}); err != nil { + return fmt.Errorf("submit: remove context blob: %w", err) + } + return nil +} + // Open returns the stored context blob for id — the read side of the transport the // build Pod's fetch initContainer uses. minio's GetObject returns only once the // server answered with an object (it surfaces NoSuchKey up front), so a missing diff --git a/internal/submit/s3store_test.go b/internal/submit/s3store_test.go index 9c99a39..6e6e810 100644 --- a/internal/submit/s3store_test.go +++ b/internal/submit/s3store_test.go @@ -15,9 +15,10 @@ import ( // error for a missing stat, so S3ContextStore's key derivation and not-found // handling are exercised without a live bucket. type fakeS3 struct { - objects map[string][]byte - putErr error - statErr error // when set, StatObject returns it (e.g. auth rejected / bucket missing) + objects map[string][]byte + putErr error + statErr error // when set, StatObject returns it (e.g. auth rejected / bucket missing) + removeErr error } func (f *fakeS3) PutObject(_ context.Context, bucket, object string, r io.Reader, _ int64, _ minio.PutObjectOptions) (minio.UploadInfo, error) { @@ -45,6 +46,16 @@ func (f *fakeS3) StatObject(_ context.Context, bucket, object string, _ minio.St return minio.ObjectInfo{}, minio.ErrorResponse{Code: "NoSuchKey", StatusCode: http.StatusNotFound} } +// RemoveObject mirrors S3's idempotent DELETE: removing a key (absent or not) +// succeeds unless removeErr injects a failure. +func (f *fakeS3) RemoveObject(_ context.Context, bucket, object string, _ minio.RemoveObjectOptions) error { + if f.removeErr != nil { + return f.removeErr + } + delete(f.objects, bucket+"/"+object) + return nil +} + // fakeS3Object is the object handle fakeS3.GetObject yields: Stat mirrors // StatObject's not-found behaviour, Read serves the stored bytes. type fakeS3Object struct { @@ -142,6 +153,32 @@ func TestS3ContextStorePutAndExists(t *testing.T) { } } +// Delete removes the derived object and is idempotent (S3 DELETE of an absent +// key succeeds), so a retried cleanup after a partial failure cannot stick. +func TestS3ContextStoreDelete(t *testing.T) { + fake := &fakeS3{} + s := &S3ContextStore{client: fake, bucket: "felis-uploads", prefix: "builds"} + ctx := context.Background() + + if _, err := s.Put(ctx, "sub-abc", strings.NewReader("\x1f\x8bbytes")); err != nil { + t.Fatalf("Put: %v", err) + } + if err := s.Delete(ctx, "sub-abc"); err != nil { + t.Fatalf("Delete: %v", err) + } + if ok, err := s.Exists(ctx, "sub-abc"); err != nil || ok { + t.Fatalf("Exists after Delete = (%v, %v), want (false, nil)", ok, err) + } + if err := s.Delete(ctx, "sub-abc"); err != nil { + t.Fatalf("second Delete = %v, want nil (idempotent)", err) + } + // A transport failure is surfaced, not swallowed. + fake.removeErr = errors.New("s3 unavailable") + if err := s.Delete(ctx, "sub-abc"); err == nil { + t.Fatal("Delete with a failing transport = nil, want error") + } +} + // Open serves the stored object's bytes and maps a missing key to ErrBlobNotFound // (the internal fetch route's 404), eagerly — before the caller reads a byte. func TestS3ContextStoreOpen(t *testing.T) { diff --git a/internal/submit/submit.go b/internal/submit/submit.go index a06a0c7..337e7dc 100644 --- a/internal/submit/submit.go +++ b/internal/submit/submit.go @@ -203,6 +203,17 @@ type Store interface { // (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 + // DeleteSubmission removes a submission row outright — the admin delete path. + // Any status is deletable: the row is the business record and the caller has + // chosen to retire it. Reports whether a row was actually deleted; false + // means a concurrent delete won, which the Manager surfaces as ErrNotFound. + DeleteSubmission(ctx context.Context, id string) (bool, error) + // DeletePendingSubmission is the submitter's withdraw CAS: it deletes the row + // only while it is still owned by submittedBy AND still pending_review, so a + // concurrent approve/reject wins or loses cleanly and a reviewed submission + // can never be withdrawn out from under its build. Reports whether THIS call + // deleted the row. + DeletePendingSubmission(ctx context.Context, id, submittedBy string) (bool, error) } // Builds is the slice of the build subsystem the approval lane drives. An @@ -235,6 +246,11 @@ type Blobs interface { // store — never a recorded number that could drift from it (a re-upload // supersedes the previous blob in place). Size(ctx context.Context, id string) (int64, bool, error) + // Delete removes everything stored for id — the withdrawal/review-cleanup + // path. It is idempotent: deleting nothing is success, so a retried cleanup + // never fails on absence. Callers reap the blob only AFTER the row is gone + // (see Manager.deleteBlob), so a live row can never point at a reaped blob. + Delete(ctx context.Context, id string) error // Open returns the stored blob's bytes for the internal context-fetch route // the build Pod's initContainer dials (cmd/felis fetch-context). It returns an // error wrapping ErrBlobNotFound when no blob exists, so the route can answer @@ -711,6 +727,86 @@ func (m *Manager) Reject(ctx context.Context, id, reviewedBy, reason string) (*S return sub, nil } +// Withdraw retracts the submitter's own pending submission: the row is deleted +// (CAS-guarded on owner + pending_review, so a concurrent review can never be +// undercut) and its uploaded context is reaped, freeing both the user's pending +// slot and their storage budget for a fresh submission. Once reviewed the +// context is frozen — an approved build may already be consuming it — so a +// non-pending row reports ErrAlreadyReviewed, exactly like UploadContext, and +// another user's id stays invisible (ErrNotFound), like every other owner-scoped +// operation on the lane. +func (m *Manager) Withdraw(ctx context.Context, id, submittedBy string) (*Submission, error) { + if strings.TrimSpace(submittedBy) == "" { + return nil, invalidf("submitter identity is required") + } + sub, err := m.Store.GetSubmission(ctx, id) + if err != nil { + return nil, err + } + if sub.SubmittedBy != submittedBy { + return nil, ErrNotFound + } + if sub.Status != StatusPendingReview { + return nil, ErrAlreadyReviewed + } + won, err := m.Store.DeletePendingSubmission(ctx, id, submittedBy) + if err != nil { + return nil, err + } + if !won { + // A concurrent approve/reject (or delete) moved the row between the load + // and the CAS. The context must NOT be reaped: it belongs to the review + // outcome now. + return nil, ErrAlreadyReviewed + } + if err := m.deleteBlob(ctx, id); err != nil { + return nil, err + } + return sub, nil +} + +// Delete retires any submission outright (the admin path): the row and its +// stored context are both removed, any status. The reviewer identity is recorded +// by the API's audit event, not on the row — the row no longer exists. Deleting +// an approved submission whose build is still running can fail that build (its +// context fetch answers 404); the admin has explicitly chosen to retire the +// artifact. A concurrent delete reports ErrNotFound, the same outcome the +// initial load would have given had it lost the race. +func (m *Manager) Delete(ctx context.Context, id string) (*Submission, error) { + sub, err := m.Store.GetSubmission(ctx, id) + if err != nil { + return nil, err + } + deleted, err := m.Store.DeleteSubmission(ctx, id) + if err != nil { + return nil, err + } + if !deleted { + return nil, ErrNotFound + } + if err := m.deleteBlob(ctx, id); err != nil { + return nil, err + } + return sub, nil +} + +// deleteBlob reaps a submission's stored context after its row is gone. The row +// is deleted FIRST (for Withdraw, under the status CAS), so a cleanup failure +// here can never leave a live row pointing at a reaped blob — the failure mode +// is the other direction: the row is retired and the blob is orphaned on the +// uploads store, which the returned error names explicitly so the operator knows +// exactly what is left behind. A deployment with no upload transport (Blobs nil) +// has no blobs to reap. +func (m *Manager) deleteBlob(ctx context.Context, id string) error { + if m.Blobs == nil { + return nil + } + if err := m.Blobs.Delete(ctx, id); err != nil { + return fmt.Errorf("submit: submission removed, but its uploaded context could not be deleted (it may remain on the uploads store): %w", err) + } + return 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) diff --git a/internal/submit/submit_test.go b/internal/submit/submit_test.go index 3ea0c5f..ea12bc8 100644 --- a/internal/submit/submit_test.go +++ b/internal/submit/submit_test.go @@ -25,6 +25,7 @@ type fakeBlobs struct { putErr error existsErr error sizeErr error + deleteErr error forceExists *bool // overrides the stored-map lookup for the approve-gate tests } @@ -65,6 +66,15 @@ func (f *fakeBlobs) Size(_ context.Context, id string) (int64, bool, error) { return int64(len(b)), true, nil } +// Delete mirrors the real stores: idempotent, nothing stored is success. +func (f *fakeBlobs) Delete(_ context.Context, id string) error { + if f.deleteErr != nil { + return f.deleteErr + } + delete(f.stored, id) + return nil +} + func (f *fakeBlobs) Open(_ context.Context, id string) (io.ReadCloser, error) { b, ok := f.stored[id] if !ok { @@ -90,6 +100,7 @@ type fakeStore struct { approveErr error rejectErr error linkErr error + deleteErr error linked []string // "id=buildID" recorder } @@ -190,6 +201,29 @@ func (f *fakeStore) LinkBuild(_ context.Context, id, buildID string) error { return nil } +func (f *fakeStore) DeleteSubmission(_ context.Context, id string) (bool, error) { + if f.deleteErr != nil { + return false, f.deleteErr + } + if _, ok := f.subs[id]; !ok { + return false, nil // concurrent delete won / nonexistent + } + delete(f.subs, id) + return true, nil +} + +func (f *fakeStore) DeletePendingSubmission(_ context.Context, id, by string) (bool, error) { + if f.deleteErr != nil { + return false, f.deleteErr + } + s, ok := f.subs[id] + if !ok || s.SubmittedBy != by || s.Status != StatusPendingReview { + return false, nil + } + delete(f.subs, id) + return true, nil +} + // fakeBuilds records Submit calls and can inject a failure. type fakeBuilds struct { submitErr error @@ -819,6 +853,155 @@ func TestCreatePendingCountFailureSurfaces(t *testing.T) { } } +// Withdraw retracts the submitter's own pending submission: the row AND its +// uploaded context are gone — which is what frees the pending slot and the +// storage budget for a fresh submission. +func TestWithdrawDeletesRowAndBlob(t *testing.T) { + m, st, _ := newManager() + fb := newFakeBlobs() + m.Blobs = fb + ctx := context.Background() + seed, _ := m.Create(ctx, CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"}) + if _, err := m.UploadContext(ctx, seed.ID, "user-1", strings.NewReader(gzBody("bytes"))); err != nil { + t.Fatalf("UploadContext: %v", err) + } + + sub, err := m.Withdraw(ctx, seed.ID, "user-1") + if err != nil { + t.Fatalf("Withdraw: %v", err) + } + if sub.ID != seed.ID || sub.Status != StatusPendingReview { + t.Fatalf("withdrawn row = %+v, want the pending row back", sub) + } + if _, ok := st.subs[seed.ID]; ok { + t.Fatal("withdraw must delete the row") + } + if _, ok := fb.stored[seed.ID]; ok { + t.Fatal("withdraw must reap the uploaded context") + } +} + +// Another user's id is invisible on the withdraw path (404, not 403), and +// nothing is touched. +func TestWithdrawNotOwnerIsNotFound(t *testing.T) { + m, st, _ := newManager() + fb := newFakeBlobs() + m.Blobs = fb + ctx := context.Background() + seed, _ := m.Create(ctx, CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"}) + if _, err := m.UploadContext(ctx, seed.ID, "user-1", strings.NewReader(gzBody("x"))); err != nil { + t.Fatalf("UploadContext: %v", err) + } + + if _, err := m.Withdraw(ctx, seed.ID, "user-2"); !errors.Is(err, ErrNotFound) { + t.Fatalf("err = %v, want ErrNotFound", err) + } + if _, ok := st.subs[seed.ID]; !ok { + t.Fatal("a non-owner withdraw must not delete the row") + } + if _, ok := fb.stored[seed.ID]; !ok { + t.Fatal("a non-owner withdraw must not reap the context") + } +} + +// A reviewed submission is frozen: the withdraw CAS refuses it (409), keeping +// the context a running/queued build may still consume. +func TestWithdrawReviewedIsAlreadyReviewed(t *testing.T) { + m, st, _ := newManager() + fb := newFakeBlobs() + m.Blobs = fb + ctx := context.Background() + seed, _ := m.Create(ctx, CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"}) + if _, err := m.UploadContext(ctx, seed.ID, "user-1", strings.NewReader(gzBody("x"))); err != nil { + t.Fatalf("UploadContext: %v", err) + } + if _, err := m.Reject(ctx, seed.ID, "admin@example.net", "no"); err != nil { + t.Fatalf("Reject: %v", err) + } + + if _, err := m.Withdraw(ctx, seed.ID, "user-1"); !errors.Is(err, ErrAlreadyReviewed) { + t.Fatalf("err = %v, want ErrAlreadyReviewed", err) + } + if _, ok := st.subs[seed.ID]; !ok { + t.Fatal("a reviewed row must survive a withdraw attempt") + } + if _, ok := fb.stored[seed.ID]; !ok { + t.Fatal("a reviewed row's context must survive a withdraw attempt") + } +} + +// The admin delete retires ANY status, reaping the context with it. +func TestDeleteAnyStatusReapsContext(t *testing.T) { + m, st, _ := newManager() + fb := newFakeBlobs() + m.Blobs = fb + ctx := context.Background() + seed, _ := m.Create(ctx, CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"}) + if _, err := m.UploadContext(ctx, seed.ID, "user-1", strings.NewReader(gzBody("x"))); err != nil { + t.Fatalf("UploadContext: %v", err) + } + if _, err := m.Reject(ctx, seed.ID, "admin@example.net", "no"); err != nil { + t.Fatalf("Reject: %v", err) + } + + sub, err := m.Delete(ctx, seed.ID) + if err != nil { + t.Fatalf("Delete: %v", err) + } + if sub.Status != StatusRejected { + t.Fatalf("returned status = %q, want the row as it was before the delete", sub.Status) + } + if _, ok := st.subs[seed.ID]; ok { + t.Fatal("delete must remove the row") + } + if _, ok := fb.stored[seed.ID]; ok { + t.Fatal("delete must reap the context") + } +} + +// Deleting an unknown id — including the second delete of one already gone — +// is ErrNotFound (404), not a crash and not a silent success. +func TestDeleteUnknownIsNotFound(t *testing.T) { + m, _, _ := newManager() + m.Blobs = newFakeBlobs() + if _, err := m.Delete(context.Background(), "sub-nope"); !errors.Is(err, ErrNotFound) { + t.Fatalf("err = %v, want ErrNotFound", err) + } +} + +// A deployment without an upload transport has no blobs to reap: delete still +// retires the row. +func TestDeleteWithoutTransport(t *testing.T) { + m, st, _ := newManager() // Blobs nil + ctx := context.Background() + seed, _ := m.Create(ctx, CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"}) + if _, err := m.Delete(ctx, seed.ID); err != nil { + t.Fatalf("Delete: %v", err) + } + if _, ok := st.subs[seed.ID]; ok { + t.Fatal("delete must remove the row even with no transport") + } +} + +// A blob-cleanup failure after the row is gone must surface loudly (naming what +// was left behind), never masquerade as success. +func TestDeleteBlobCleanupFailureSurfaces(t *testing.T) { + m, st, _ := newManager() + fb := newFakeBlobs() + fb.deleteErr = errors.New("pvc read-only") + m.Blobs = fb + ctx := context.Background() + seed, _ := m.Create(ctx, CreateRequest{DisplayName: "Pack", SubmittedBy: "user-1"}) + + _, err := m.Delete(ctx, seed.ID) + if err == nil || !strings.Contains(err.Error(), "could not be deleted") { + t.Fatalf("err = %v, want the cleanup failure named", err) + } + if _, ok := st.subs[seed.ID]; ok { + t.Fatal("the row is deleted before the blob reap; it must stay deleted") + } +} + func TestApproveRefusesMissingContext(t *testing.T) { // With a transport wired, approving a submission whose context was never // uploaded fails BEFORE the CAS: the row stays pending and no build starts. diff --git a/panel/src/i18n/resources/en-US/admin.json b/panel/src/i18n/resources/en-US/admin.json index b116a9d..91b598e 100644 --- a/panel/src/i18n/resources/en-US/admin.json +++ b/panel/src/i18n/resources/en-US/admin.json @@ -53,6 +53,9 @@ "table_display_name": "Name", "approve_btn": "Approve", "reject_btn": "Reject", + "delete_submission_btn": "Delete submission", + "delete_submission_confirm": "Delete", + "delete_submission_cancel": "Cancel", "reject_reason_label": "Rejection Reason", "reject_reason_placeholder": "Please provide a reason (max 1000 characters)", "reject_dialog_title": "Reject Submission", diff --git a/panel/src/i18n/resources/en-US/submissions.json b/panel/src/i18n/resources/en-US/submissions.json index 7df83ad..459c46c 100644 --- a/panel/src/i18n/resources/en-US/submissions.json +++ b/panel/src/i18n/resources/en-US/submissions.json @@ -36,5 +36,9 @@ "field_context_ref": "Context Reference", "field_image_ref": "Image Reference", "clear_btn": "Clear", - "file_hint": "Supports .tar.gz (max 1GB)" + "file_hint": "Supports .tar.gz (max 1GB)", + "withdraw_btn": "Withdraw", + "withdraw_hint": "Withdrawing deletes this submission and its uploaded context, freeing your pending slot and storage budget.", + "withdraw_confirm": "Withdraw", + "withdraw_cancel": "Cancel" } diff --git a/panel/src/i18n/resources/zh-CN/admin.json b/panel/src/i18n/resources/zh-CN/admin.json index 7792619..d3a634c 100644 --- a/panel/src/i18n/resources/zh-CN/admin.json +++ b/panel/src/i18n/resources/zh-CN/admin.json @@ -53,6 +53,9 @@ "table_display_name": "名称", "approve_btn": "通过", "reject_btn": "驳回", + "delete_submission_btn": "删除提交", + "delete_submission_confirm": "确认删除", + "delete_submission_cancel": "取消", "reject_reason_label": "驳回原因", "reject_reason_placeholder": "请填写驳回原因（限 1000 字符）", "reject_dialog_title": "驳回申请", diff --git a/panel/src/i18n/resources/zh-CN/submissions.json b/panel/src/i18n/resources/zh-CN/submissions.json index 711a6a0..9255a44 100644 --- a/panel/src/i18n/resources/zh-CN/submissions.json +++ b/panel/src/i18n/resources/zh-CN/submissions.json @@ -36,5 +36,9 @@ "field_context_ref": "构建上下文引用", "field_image_ref": "目标镜像引用", "clear_btn": "清除", - "file_hint": "支持 .tar.gz 格式 (最大 1GB)" + "file_hint": "支持 .tar.gz 格式 (最大 1GB)", + "withdraw_btn": "撤回提交", + "withdraw_hint": "撤回会删除该提交及其已上传的构建上下文，并释放你的待审核名额与存储配额。", + "withdraw_confirm": "确认撤回", + "withdraw_cancel": "取消" } diff --git a/panel/src/lib/api.test.ts b/panel/src/lib/api.test.ts index aba7135..6cf5ad4 100644 --- a/panel/src/lib/api.test.ts +++ b/panel/src/lib/api.test.ts @@ -530,6 +530,28 @@ describe("image whitelist and builds wire shapes", () => { expect(humanizeError({ code: "submission_quota_exceeded" })).toMatch(/quota/i); expect(humanizeError({ code: "submission_cooldown" })).toMatch(/try again/i); }); + + it("withdrawSubmission DELETEs /me/submissions/{id}", async () => { + const sub = { id: "sub-4", status: "pending_review" }; + const fetchSpy = fakeFetch(sub); + vi.stubGlobal("fetch", fetchSpy); + const res = await api.withdrawSubmission("sub-4"); + expect(res).toEqual(sub); + const [url, opts] = (fetchSpy as unknown as ReturnType).mock.calls[0]; + expect(String(url)).toBe("/me/submissions/sub-4"); + expect((opts as RequestInit).method).toBe("DELETE"); + }); + + it("deleteSubmission DELETEs /submissions/{id}", async () => { + const sub = { id: "sub-5", status: "rejected" }; + const fetchSpy = fakeFetch(sub); + vi.stubGlobal("fetch", fetchSpy); + const res = await api.deleteSubmission("sub-5"); + expect(res).toEqual(sub); + const [url, opts] = (fetchSpy as unknown as ReturnType).mock.calls[0]; + expect(String(url)).toBe("/submissions/sub-5"); + expect((opts as RequestInit).method).toBe("DELETE"); + }); }); describe("updates maintenance window", () => { diff --git a/panel/src/lib/api.ts b/panel/src/lib/api.ts index 6c10113..296594e 100644 --- a/panel/src/lib/api.ts +++ b/panel/src/lib/api.ts @@ -474,6 +474,10 @@ export const api = { rejectSubmission: (id: string, reason: string) => request("POST", `/submissions/${id}/reject`, { reason }), + // Retire a submission outright (row + uploaded context) — the review queue's + // lifecycle valve, the only way an upload is reclaimed from the PVC. + deleteSubmission: (id: string) => request("DELETE", `/submissions/${id}`), + // The reviewer's read path to the uploaded build context: the executed // Dockerfile lives inside the tarball, so approving without this would be // blind. The body is the attacker-supplied archive — download it, never @@ -519,6 +523,10 @@ export const api = { "Content-Type": "application/x-gzip", }), + // Retract the caller's own pending submission (and its uploaded context), which + // frees their pending slot and storage budget. Reviewed submissions are frozen. + withdrawSubmission: (id: string) => request("DELETE", `/me/submissions/${id}`), + getUpdateWindow: () => request("GET", "/updates/window"), setUpdateWindow: (window: UpdateWindow) => request("PUT", "/updates/window", window), diff --git a/panel/src/pages/MySubmissionsPage.tsx b/panel/src/pages/MySubmissionsPage.tsx index 6c681ad..8a4399f 100644 --- a/panel/src/pages/MySubmissionsPage.tsx +++ b/panel/src/pages/MySubmissionsPage.tsx @@ -11,6 +11,7 @@ import { CheckCircle, CircleSlash, ClipboardCheck, + Trash2, } from "lucide-react"; import { SearchInput } from "@/components/SearchInput"; import { @@ -30,6 +31,7 @@ import { } from "@/components/ui/dialog"; import { cn } from "@/lib/utils"; import { MessageLine } from "@/components/MessageLine"; +import { InlineConfirm } from "@/components/InlineConfirm"; import { Loading, ErrorState, EmptyState } from "@/components/States"; import { Pagination } from "@/components/Pagination"; import { StatCard } from "@/components/StatCard"; @@ -90,6 +92,12 @@ export function MySubmissionsPage() { const [submitStep, setSubmitStep] = useState<"create" | "upload" | null>(null); const [error, setError] = useState(null); + // Row action state: withdraw arms a row (trash → confirm/cancel) before it + // fires, and a failure lands in actionError above the list. + const [confirmingWithdraw, setConfirmingWithdraw] = useState(null); + const [withdrawing, setWithdrawing] = useState(null); + const [actionError, setActionError] = useState(null); + const fileInputRef = useRef(null); // Search & Filtering State @@ -225,6 +233,23 @@ export function MySubmissionsPage() { } }; + // Withdraw retracts a still-pending submission and its uploaded context, + // freeing the pending slot and the storage budget for a fresh submission. + async function handleWithdraw(id: string) { + if (withdrawing) return; + setWithdrawing(id); + setActionError(null); + try { + await api.withdrawSubmission(id); + setConfirmingWithdraw(null); + reload(); + } catch (err) { + setActionError(humanizeError(err)); + } finally { + setWithdrawing(null); + } + } + return (
{/* Header */} @@ -250,6 +275,9 @@ export function MySubmissionsPage() { className="mb-6" /> + {/* Row-action error (withdraw) */} + {actionError && } + {/* Stats Cards Row */}
@@ -410,6 +438,35 @@ export function MySubmissionsPage() { )}
+ {sub.status === "pending_review" && ( +
+ {t("withdraw_hint")} + {confirmingWithdraw === sub.id ? ( + handleWithdraw(sub.id)} + onCancel={() => setConfirmingWithdraw(null)} + confirmLabel={t("withdraw_confirm")} + cancelLabel={t("withdraw_cancel")} + className="flex shrink-0 items-center gap-1" + /> + ) : ( + + )} +
+ )} + {sub.status !== "pending_review" && (
{sub.reviewed_by && ( diff --git a/panel/src/pages/admin/SubmissionsPage.tsx b/panel/src/pages/admin/SubmissionsPage.tsx index b13d08e..2787603 100644 --- a/panel/src/pages/admin/SubmissionsPage.tsx +++ b/panel/src/pages/admin/SubmissionsPage.tsx @@ -1,5 +1,5 @@ import { useState, useMemo } from "react"; -import { ClipboardCheck, CheckCircle2, CircleSlash, ChevronDown, ChevronUp, Check, X, Loader2, Download } from "lucide-react"; +import { ClipboardCheck, CheckCircle2, CircleSlash, ChevronDown, ChevronUp, Check, X, Loader2, Download, Trash2 } from "lucide-react"; import { useTranslation } from "react-i18next"; import { Card, CardContent } from "@/components/ui/card"; import { StatCard } from "@/components/StatCard"; @@ -18,6 +18,7 @@ import { } from "@/components/ui/dialog"; import { cn } from "@/lib/utils"; import { MessageLine } from "@/components/MessageLine"; +import { InlineConfirm } from "@/components/InlineConfirm"; import { Loading, ErrorState, EmptyState } from "@/components/States"; import { Pagination } from "@/components/Pagination"; import { api, humanizeError } from "@/lib/api"; @@ -43,8 +44,11 @@ export function SubmissionsPage() { // Pending actions (for button spinners) const [busyId, setBusyId] = useState(null); - const [busyType, setBusyType] = useState<"approve" | "reject" | null>(null); + const [busyType, setBusyType] = useState<"approve" | "reject" | "delete" | null>(null); const [downloadingId, setDownloadingId] = useState(null); + // Delete arms the row (trash → confirm/cancel) before it fires; a row gone on + // one stray click would take its uploaded context with it. + const [confirmingDelete, setConfirmingDelete] = useState(null); // Search & Filtering State const [search, setSearch] = useState(""); @@ -120,6 +124,26 @@ export function SubmissionsPage() { setRejectDialogOpen(true); } + // Delete retires the submission outright — any status — along with its + // uploaded context: the lane's only lifecycle valve, the path that reclaims + // a rejected or consumed upload from the uploads PVC. + async function handleDelete(id: string) { + if (busyId) return; + setBusyId(id); + setBusyType("delete"); + setActionError(null); + try { + await api.deleteSubmission(id); + setConfirmingDelete(null); + reload(); + } catch (err) { + setActionError(humanizeError(err)); + } finally { + setBusyId(null); + setBusyType(null); + } + } + async function handleRejectSubmit(e?: React.FormEvent) { e?.preventDefault(); if (!rejectingId || !rejectReason.trim()) return; @@ -266,6 +290,7 @@ export function SubmissionsPage() { const isExpanded = expandedId === sub.id; const isBusyApprove = busyId === sub.id && busyType === "approve"; const isBusyReject = busyId === sub.id && busyType === "reject"; + const isBusyDelete = busyId === sub.id && busyType === "delete"; return (
- {/* Action buttons (only in table row if NOT pending, else show triggers) */} + {/* Action buttons: approve/reject on pending rows, plus the + two-step delete on every row — it is the only path that + reclaims an upload from the uploads PVC. */}
e.stopPropagation()}> - {sub.status === "pending_review" ? ( -
+
+ {sub.status === "pending_review" && ( + <> -
- ) : ( - - — - - )} + + )} + {confirmingDelete === sub.id ? ( + handleDelete(sub.id)} + onCancel={() => setConfirmingDelete(null)} + confirmLabel={t("delete_submission_confirm")} + cancelLabel={t("delete_submission_cancel")} + className="flex shrink-0 items-center gap-1" + /> + ) : ( + + )} +