From a1d50815d496dc3d301b7c93704da2b86327623c Mon Sep 17 00:00:00 2001 From: veryCrunchy Date: Sat, 15 Aug 2026 02:55:35 +0200 Subject: [PATCH] fix(intake): tombstone cancelled submissions --- README.md | 2 + internal/intake/service.go | 25 +++++++ internal/intake/service_test.go | 46 ++++++++++++ internal/store/memory.go | 32 +++++++-- .../003_submission_cancellations.sql | 8 +++ internal/store/postgres.go | 70 ++++++++++++++++++- internal/store/store.go | 6 +- internal/web/server.go | 11 +++ internal/web/server_test.go | 22 ++++++ openapi.yaml | 12 ++++ 10 files changed, 225 insertions(+), 9 deletions(-) create mode 100644 internal/store/migrations/003_submission_cancellations.sql diff --git a/README.md b/README.md index b143684..579bac9 100644 --- a/README.md +++ b/README.md @@ -10,6 +10,7 @@ The public portal is a Vue 3 and TypeScript application. A small Go service owns - A versioned multipart API shared by the web portal and native applications. - Explicit 4 MiB diagnostic upload bounds and per-product ZIP entry allowlists. - Idempotent submission and receipt reconciliation after an uncertain response. +- Atomic cancellation tombstones that prevent a delayed upload from recreating a cancelled private report. - Human-readable support codes and unguessable private status/deletion links. - Application-level AES-256-GCM encryption for private report fields, capabilities, and diagnostic objects. - Automatic private-data expiration and immediate deletion through the private capability. @@ -85,6 +86,7 @@ npm run build - New reports are private. Nothing is published automatically. - Diagnostic attachments are optional and must match the selected product's registered schema. - The service does not store raw idempotency keys or raw status capabilities in database lookup columns. +- Cancellation retains only the one-way idempotency hash needed to reject a delayed submission; it does not retain report content. - Private report text, contact details, receipt capabilities, and diagnostic objects are encrypted before storage. - Support codes are identifiers, not authentication secrets. - Private status URLs are bearer capabilities. Applications must never put them in diagnostics, telemetry, or public issues. diff --git a/internal/intake/service.go b/internal/intake/service.go index 7a5b824..1c7ed98 100644 --- a/internal/intake/service.go +++ b/internal/intake/service.go @@ -29,6 +29,7 @@ var ( ErrInvalid = errors.New("invalid report") ErrNotFound = errors.New("report not found") ErrKeyReused = errors.New("idempotency key was already used for another report") + ErrCancelled = errors.New("report submission was cancelled") idempotencyKey = regexp.MustCompile(`^[A-Za-z0-9_-]{32,128}$`) productID = regexp.MustCompile(`^[a-z0-9][a-z0-9-]{1,62}[a-z0-9]$`) archiveFileName = regexp.MustCompile(`^[A-Za-z0-9][A-Za-z0-9._-]{0,95}$`) @@ -73,6 +74,8 @@ func (service *Service) Submit(ctx context.Context, submission Submission) (doma return domain.Receipt{}, ErrKeyReused } return service.receipt(existing) + } else if errors.Is(err, store.ErrCancelled) { + return domain.Receipt{}, ErrCancelled } else if !errors.Is(err, store.ErrNotFound) { return domain.Receipt{}, err } @@ -130,6 +133,9 @@ func (service *Service) Submit(ctx context.Context, submission Submission) (doma if report.DiagnosticObjectKey != nil { _ = service.objects.Delete(*report.DiagnosticObjectKey) } + if errors.Is(err, store.ErrCancelled) { + return domain.Receipt{}, ErrCancelled + } if errors.Is(err, store.ErrConflict) { if existing, lookupErr := service.reports.ByIdempotencyHash(ctx, idempotencyHash); lookupErr == nil { if !bytes.Equal(existing.RequestHash, requestHash) { @@ -151,6 +157,9 @@ func (service *Service) Reconcile(ctx context.Context, key string) (domain.Recei return domain.Receipt{}, ErrInvalid } report, err := service.reports.ByIdempotencyHash(ctx, hash([]byte(key))) + if errors.Is(err, store.ErrCancelled) { + return domain.Receipt{}, ErrCancelled + } if errors.Is(err, store.ErrNotFound) { return domain.Receipt{}, ErrNotFound } @@ -160,6 +169,22 @@ func (service *Service) Reconcile(ctx context.Context, key string) (domain.Recei return service.receipt(report) } +func (service *Service) Cancel(ctx context.Context, key string) error { + if !idempotencyKey.MatchString(key) { + return ErrInvalid + } + report, err := service.reports.CancelByIdempotencyHash(ctx, hash([]byte(key)), service.now()) + if err != nil { + return err + } + if report != nil && report.DiagnosticObjectKey != nil { + if err := service.objects.Delete(*report.DiagnosticObjectKey); err != nil { + return fmt.Errorf("delete cancelled private diagnostic object: %w", err) + } + } + return nil +} + func (service *Service) Status(ctx context.Context, capability string) (domain.PrivateStatus, error) { if !validCapability(capability) { return domain.PrivateStatus{}, ErrNotFound diff --git a/internal/intake/service_test.go b/internal/intake/service_test.go index d512198..6d730de 100644 --- a/internal/intake/service_test.go +++ b/internal/intake/service_test.go @@ -55,6 +55,52 @@ func TestSubmitRejectsIdempotencyKeyReuseWithDifferentContent(t *testing.T) { } } +func TestCancelBeforeSubmitPreventsLatePrivateReport(t *testing.T) { + service, reports, objects := testService(t) + submission := validSubmission(t) + if err := service.Cancel(context.Background(), submission.IdempotencyKey); err != nil { + t.Fatal(err) + } + if err := service.Cancel(context.Background(), submission.IdempotencyKey); err != nil { + t.Fatalf("repeated cancel: %v", err) + } + if _, err := service.Submit(context.Background(), submission); !errors.Is(err, ErrCancelled) { + t.Fatalf("late submit error = %v, want ErrCancelled", err) + } + if _, err := service.Reconcile(context.Background(), submission.IdempotencyKey); !errors.Is(err, ErrCancelled) { + t.Fatalf("reconcile error = %v, want ErrCancelled", err) + } + if _, err := reports.ByIdempotencyHash(context.Background(), hash([]byte(submission.IdempotencyKey))); !errors.Is(err, store.ErrCancelled) { + t.Fatalf("stored cancellation error = %v, want ErrCancelled", err) + } + if len(objects.Values) != 0 { + t.Fatalf("private object count = %d, want 0", len(objects.Values)) + } +} + +func TestCancelExistingReportDeletesPrivateDataAndPreventsRecreation(t *testing.T) { + service, reports, objects := testService(t) + submission := validSubmission(t) + if _, err := service.Submit(context.Background(), submission); err != nil { + t.Fatal(err) + } + if err := service.Cancel(context.Background(), submission.IdempotencyKey); err != nil { + t.Fatal(err) + } + if err := service.Cancel(context.Background(), submission.IdempotencyKey); err != nil { + t.Fatalf("repeated cancel: %v", err) + } + if len(objects.Values) != 0 { + t.Fatalf("private object count after cancellation = %d, want 0", len(objects.Values)) + } + if _, err := reports.ByIdempotencyHash(context.Background(), hash([]byte(submission.IdempotencyKey))); !errors.Is(err, store.ErrCancelled) { + t.Fatalf("stored cancellation error = %v, want ErrCancelled", err) + } + if _, err := service.Submit(context.Background(), submission); !errors.Is(err, ErrCancelled) { + t.Fatalf("recreated submit error = %v, want ErrCancelled", err) + } +} + func TestSubmitRejectsUnregisteredAndExpandingArchiveEntries(t *testing.T) { service, _, _ := testService(t) submission := validSubmission(t) diff --git a/internal/store/memory.go b/internal/store/memory.go index 47ccc1c..694ded2 100644 --- a/internal/store/memory.go +++ b/internal/store/memory.go @@ -10,16 +10,20 @@ import ( ) type Memory struct { - mu sync.Mutex - reports []domain.Report - sessions []domain.AdminSession + mu sync.Mutex + reports []domain.Report + cancellations map[string]time.Time + sessions []domain.AdminSession } -func NewMemory() *Memory { return &Memory{} } +func NewMemory() *Memory { return &Memory{cancellations: make(map[string]time.Time)} } func (memory *Memory) Create(_ context.Context, report domain.Report) error { memory.mu.Lock() defer memory.mu.Unlock() + if _, cancelled := memory.cancellations[string(report.IdempotencyHash)]; cancelled { + return ErrCancelled + } for _, existing := range memory.reports { if existing.SupportCode == report.SupportCode || bytes.Equal(existing.IdempotencyHash, report.IdempotencyHash) { return ErrConflict @@ -37,9 +41,29 @@ func (memory *Memory) ByIdempotencyHash(_ context.Context, hash []byte) (domain. return report, nil } } + if _, cancelled := memory.cancellations[string(hash)]; cancelled { + return domain.Report{}, ErrCancelled + } return domain.Report{}, ErrNotFound } +func (memory *Memory) CancelByIdempotencyHash(_ context.Context, hash []byte, now time.Time) (*domain.Report, error) { + memory.mu.Lock() + defer memory.mu.Unlock() + memory.cancellations[string(hash)] = now + for index, report := range memory.reports { + if bytes.Equal(report.IdempotencyHash, hash) { + if report.DeletedAt == nil { + memory.reports[index].DeletedAt = &now + memory.reports[index].UpdatedAt = now + } + cancelled := memory.reports[index] + return &cancelled, nil + } + } + return nil, nil +} + func (memory *Memory) ByCapabilityHash(_ context.Context, hash []byte) (domain.Report, error) { memory.mu.Lock() defer memory.mu.Unlock() diff --git a/internal/store/migrations/003_submission_cancellations.sql b/internal/store/migrations/003_submission_cancellations.sql new file mode 100644 index 0000000..6be1b35 --- /dev/null +++ b/internal/store/migrations/003_submission_cancellations.sql @@ -0,0 +1,8 @@ +CREATE TABLE IF NOT EXISTS support_submission_states ( + idempotency_hash BYTEA PRIMARY KEY CHECK (octet_length(idempotency_hash) = 32), + cancelled_at TIMESTAMPTZ +); + +INSERT INTO support_submission_states (idempotency_hash) +SELECT idempotency_hash FROM support_reports +ON CONFLICT (idempotency_hash) DO NOTHING; diff --git a/internal/store/postgres.go b/internal/store/postgres.go index 2976208..eef1688 100644 --- a/internal/store/postgres.go +++ b/internal/store/postgres.go @@ -28,7 +28,24 @@ func OpenPostgres(ctx context.Context, databaseURL string) (*Postgres, error) { } func (postgres *Postgres) Create(ctx context.Context, report domain.Report) error { - _, err := postgres.pool.Exec(ctx, ` + transaction, err := postgres.pool.Begin(ctx) + if err != nil { + return err + } + defer transaction.Rollback(ctx) + if _, err := transaction.Exec(ctx, `INSERT INTO support_submission_states (idempotency_hash) + VALUES ($1) ON CONFLICT (idempotency_hash) DO NOTHING`, report.IdempotencyHash); err != nil { + return err + } + var cancelledAt *time.Time + if err := transaction.QueryRow(ctx, `SELECT cancelled_at FROM support_submission_states + WHERE idempotency_hash = $1 FOR UPDATE`, report.IdempotencyHash).Scan(&cancelledAt); err != nil { + return err + } + if cancelledAt != nil { + return ErrCancelled + } + _, err = transaction.Exec(ctx, ` INSERT INTO support_reports ( id, support_code, product_id, request_type, status, private_payload, capability_ciphertext, diagnostic_object_key, idempotency_hash, request_hash, @@ -43,11 +60,58 @@ func (postgres *Postgres) Create(ctx context.Context, report domain.Report) erro if errors.As(err, &postgresError) && postgresError.Code == "23505" { return ErrConflict } - return err + if err != nil { + return err + } + return transaction.Commit(ctx) } func (postgres *Postgres) ByIdempotencyHash(ctx context.Context, hash []byte) (domain.Report, error) { - return scanReport(postgres.pool.QueryRow(ctx, reportSelect+` WHERE idempotency_hash = $1 AND deleted_at IS NULL`, hash)) + report, err := scanReport(postgres.pool.QueryRow(ctx, reportSelect+` WHERE idempotency_hash = $1 AND deleted_at IS NULL`, hash)) + if !errors.Is(err, ErrNotFound) { + return report, err + } + var cancelled bool + if lookupErr := postgres.pool.QueryRow(ctx, `SELECT EXISTS ( + SELECT 1 FROM support_submission_states WHERE idempotency_hash = $1 AND cancelled_at IS NOT NULL + )`, hash).Scan(&cancelled); lookupErr != nil { + return domain.Report{}, lookupErr + } + if cancelled { + return domain.Report{}, ErrCancelled + } + return domain.Report{}, ErrNotFound +} + +func (postgres *Postgres) CancelByIdempotencyHash(ctx context.Context, hash []byte, now time.Time) (*domain.Report, error) { + transaction, err := postgres.pool.Begin(ctx) + if err != nil { + return nil, err + } + defer transaction.Rollback(ctx) + if _, err := transaction.Exec(ctx, `INSERT INTO support_submission_states (idempotency_hash, cancelled_at) + VALUES ($1, $2) + ON CONFLICT (idempotency_hash) DO UPDATE + SET cancelled_at = COALESCE(support_submission_states.cancelled_at, EXCLUDED.cancelled_at)`, hash, now); err != nil { + return nil, err + } + report, updateErr := scanReport(transaction.QueryRow(ctx, `UPDATE support_reports + SET deleted_at = COALESCE(deleted_at, $1), + updated_at = CASE WHEN deleted_at IS NULL THEN $1 ELSE updated_at END + WHERE idempotency_hash = $2 + RETURNING id, support_code, product_id, request_type, status, private_payload, + capability_ciphertext, diagnostic_object_key, idempotency_hash, request_hash, + capability_hash, created_at, updated_at, retention_until, deleted_at`, now, hash)) + if updateErr != nil && !errors.Is(updateErr, ErrNotFound) { + return nil, updateErr + } + if err := transaction.Commit(ctx); err != nil { + return nil, err + } + if errors.Is(updateErr, ErrNotFound) { + return nil, nil + } + return &report, nil } func (postgres *Postgres) ByCapabilityHash(ctx context.Context, hash []byte) (domain.Report, error) { diff --git a/internal/store/store.go b/internal/store/store.go index 0694d73..16fbbab 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -9,13 +9,15 @@ import ( ) var ( - ErrNotFound = errors.New("report not found") - ErrConflict = errors.New("report already exists") + ErrNotFound = errors.New("report not found") + ErrConflict = errors.New("report already exists") + ErrCancelled = errors.New("report submission was cancelled") ) type Reports interface { Create(context.Context, domain.Report) error ByIdempotencyHash(context.Context, []byte) (domain.Report, error) + CancelByIdempotencyHash(context.Context, []byte, time.Time) (*domain.Report, error) ByCapabilityHash(context.Context, []byte) (domain.Report, error) DeleteByCapabilityHash(context.Context, []byte) (domain.Report, error) Expired(context.Context, time.Time, int) ([]domain.Report, error) diff --git a/internal/web/server.go b/internal/web/server.go index 4765107..93b2fd3 100644 --- a/internal/web/server.go +++ b/internal/web/server.go @@ -51,6 +51,7 @@ func (server *Server) Handler() http.Handler { mux.HandleFunc("GET /api/v1/products", server.listProducts) mux.HandleFunc("POST /api/v1/reports", server.createReport) mux.HandleFunc("GET /api/v1/receipts", server.reconcileReceipt) + mux.HandleFunc("DELETE /api/v1/receipts", server.cancelSubmission) mux.HandleFunc("GET /api/v1/reports/{capability}", server.reportStatus) mux.HandleFunc("DELETE /api/v1/reports/{capability}", server.deleteReport) mux.HandleFunc("POST /api/v1/admin/login", server.adminLogin) @@ -168,6 +169,14 @@ func (server *Server) reconcileReceipt(response http.ResponseWriter, request *ht writeJSON(response, http.StatusOK, receipt) } +func (server *Server) cancelSubmission(response http.ResponseWriter, request *http.Request) { + if err := server.intake.Cancel(request.Context(), request.Header.Get("Idempotency-Key")); err != nil { + server.writeIntakeError(response, err) + return + } + response.WriteHeader(http.StatusNoContent) +} + func (server *Server) reportStatus(response http.ResponseWriter, request *http.Request) { status, err := server.intake.Status(request.Context(), request.PathValue("capability")) if err != nil { @@ -192,6 +201,8 @@ func (server *Server) writeIntakeError(response http.ResponseWriter, err error) writeProblem(response, http.StatusBadRequest, "invalid_report", "Check the report details and try again.") case errors.Is(err, intake.ErrKeyReused): writeProblem(response, http.StatusConflict, "idempotency_conflict", "This retry identifier belongs to different report content.") + case errors.Is(err, intake.ErrCancelled): + writeProblem(response, http.StatusGone, "submission_cancelled", "This private report submission was cancelled.") case errors.Is(err, intake.ErrNotFound): writeProblem(response, http.StatusNotFound, "not_found", "This private report link is not available.") default: diff --git a/internal/web/server_test.go b/internal/web/server_test.go index 6f695b6..c7a4981 100644 --- a/internal/web/server_test.go +++ b/internal/web/server_test.go @@ -3,6 +3,7 @@ package web import ( "net/http" "net/http/httptest" + "strings" "testing" ) @@ -29,3 +30,24 @@ func TestSecurityHeadersKeepPrivateRoutesOutOfIndexes(t *testing.T) { t.Fatalf("public X-Robots-Tag = %q", got) } } + +func TestCancellationEndpointReturnsTerminalResult(t *testing.T) { + handler, _ := testAdminHandler(t) + idempotencyKey := strings.Repeat("A", 43) + + cancelRequest := httptest.NewRequest(http.MethodDelete, "/api/v1/receipts", nil) + cancelRequest.Header.Set("Idempotency-Key", idempotencyKey) + cancelled := httptest.NewRecorder() + handler.ServeHTTP(cancelled, cancelRequest) + if cancelled.Code != http.StatusNoContent || cancelled.Body.Len() != 0 { + t.Fatalf("cancellation = %d, body = %q", cancelled.Code, cancelled.Body.String()) + } + + reconcileRequest := httptest.NewRequest(http.MethodGet, "/api/v1/receipts", nil) + reconcileRequest.Header.Set("Idempotency-Key", idempotencyKey) + reconciled := httptest.NewRecorder() + handler.ServeHTTP(reconciled, reconcileRequest) + if reconciled.Code != http.StatusGone || !strings.Contains(reconciled.Body.String(), "submission_cancelled") { + t.Fatalf("reconciliation = %d, body = %q", reconciled.Code, reconciled.Body.String()) + } +} diff --git a/openapi.yaml b/openapi.yaml index 131e5ef..23b2084 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -50,6 +50,7 @@ paths: $ref: "#/components/schemas/Receipt" "400": { $ref: "#/components/responses/Problem" } "409": { $ref: "#/components/responses/Problem" } + "410": { $ref: "#/components/responses/Problem" } /api/v1/receipts: get: summary: Reconcile an uncertain report submission @@ -65,6 +66,17 @@ paths: schema: $ref: "#/components/schemas/Receipt" "404": { $ref: "#/components/responses/Problem" } + "410": { $ref: "#/components/responses/Problem" } + delete: + summary: Cancel a private report submission + operationId: cancelReportSubmission + description: Atomically prevent a pending submission from creating a report and delete any report already created with the same idempotency key. + parameters: + - $ref: "#/components/parameters/IdempotencyKey" + responses: + "204": + description: The submission is terminally cancelled and no private report remains available. + "400": { $ref: "#/components/responses/Problem" } /api/v1/reports/{capability}: parameters: - name: capability