fix(diagnostics): address round-5 review findings on PR #445

- service: reject supplied child-profile attribution with a distinct
  ErrChildProfileForbidden (403 child_profile_forbidden) instead of
  silently dropping it as if the profile were not found; a profile that
  is simply not the user's still drops attribution unchanged
- repo: add a manifest-free list projection (reportListSelectSQL /
  scanReportSummary) for admin list and retention/stale cleanup queries
  so they no longer drag the full manifest JSONB per row; keep the full
  projection for GetByID/DeleteByID and mark Manifest omitempty
- cleanup: delete/mark the DB row before the blob in retention and stale
  loops so a mid-run DB failure can't leave a ready report pointing at a
  missing bundle; blob-delete failures are logged with bucket/keys for
  orphan cleanup to reap rather than aborting the run (shared helper with
  the admin DeleteReport path)
- admin: reject diagnostics settings where max_bytes_per_user would fall
  below max_bundle_bytes (and the reciprocal), which would make every
  max-size upload fail quota
- router/demo: route POST /diagnostics/reports through DemoGuard and block
  the reports prefix in demo mode while keeping GET /diagnostics/status
  available

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012e3QjbPo96ed9Mn2qRiUkh

internal/api/handlers/admin.go

@@ -2472,6 +2472,11 @@ func (h *AdminHandler) normalizeDiagnosticsNumericSetting(ctx context.Context, k
 		if value > settings.MaxUncompressedBytes {
 			return "", fmt.Errorf("%s must not exceed %s (%d bytes)", key, diagnostics.KeyMaxUncompressedBytes, settings.MaxUncompressedBytes)
 		}
+		// A single bundle can never exceed the per-user byte cap, or every upload
+		// at this size would fail quota; keep the two bounds consistent.
+		if value > settings.MaxBytesPerUser {
+			return "", fmt.Errorf("%s must not exceed %s (%d bytes)", key, diagnostics.KeyMaxBytesPerUser, settings.MaxBytesPerUser)
+		}
 	case diagnostics.KeyMaxUncompressedBytes:
 		if value < settings.MaxBundleBytes || value > gib {
 			return "", fmt.Errorf("%s must be between %s (%d bytes) and 1 GiB (%d bytes)", key, diagnostics.KeyMaxBundleBytes, settings.MaxBundleBytes, gib)
@@ -2488,6 +2493,12 @@ func (h *AdminHandler) normalizeDiagnosticsNumericSetting(ctx context.Context, k
 		if value < 10*mib || value > 10*gib {
 			return "", fmt.Errorf("%s must be between 10 MiB (%d bytes) and 10 GiB (%d bytes)", key, 10*mib, 10*gib)
 		}
+		// The per-user cap must leave room for at least one max-size bundle, or
+		// /diagnostics/status would advertise a bundle size InsertReceiving always
+		// rejects as quota_exceeded.
+		if value < settings.MaxBundleBytes {
+			return "", fmt.Errorf("%s must be at least %s (%d bytes)", key, diagnostics.KeyMaxBundleBytes, settings.MaxBundleBytes)
+		}
 	default:
 		return "", fmt.Errorf("unsupported diagnostics numeric setting %s", key)
 	}

internal/api/handlers/diagnostics.go

@@ -323,6 +323,8 @@ func writeDiagnosticsServiceError(w http.ResponseWriter, err error) {
 		writeError(w, http.StatusBadRequest, "archive_mismatch", "Diagnostics archive metadata does not match")
 	case errors.Is(err, diagnostics.ErrProfileMismatch):
 		writeError(w, http.StatusBadRequest, "profile_mismatch", "Diagnostics profile does not match the captured report")
+	case errors.Is(err, diagnostics.ErrChildProfileForbidden):
+		writeError(w, http.StatusForbidden, "child_profile_forbidden", "Diagnostics cannot be attributed to a child profile")
 	case errors.Is(err, diagnostics.ErrInvalidBundle):
 		writeError(w, http.StatusBadRequest, "invalid_bundle", "Invalid diagnostics bundle")
 	default:

internal/api/handlers/diagnostics_test.go

@@ -447,7 +447,7 @@ func TestDiagnosticsNumericSettingsAcceptBoundaries(t *testing.T) {
 		initial map[string]string
 	}{
 		{name: "bundle minimum", key: diagnostics.KeyMaxBundleBytes, value: "1048576", initial: map[string]string{diagnostics.KeyMaxUncompressedBytes: "1073741824"}},
-		{name: "bundle maximum", key: diagnostics.KeyMaxBundleBytes, value: "268435456", initial: map[string]string{diagnostics.KeyMaxUncompressedBytes: "1073741824"}},
+		{name: "bundle maximum", key: diagnostics.KeyMaxBundleBytes, value: "268435456", initial: map[string]string{diagnostics.KeyMaxUncompressedBytes: "1073741824", diagnostics.KeyMaxBytesPerUser: "10737418240"}},
 		{name: "uncompressed bundle floor", key: diagnostics.KeyMaxUncompressedBytes, value: "1048576", initial: map[string]string{diagnostics.KeyMaxBundleBytes: "1048576"}},
 		{name: "uncompressed maximum", key: diagnostics.KeyMaxUncompressedBytes, value: "1073741824", initial: map[string]string{diagnostics.KeyMaxBundleBytes: "1048576"}},
 		{name: "reports minimum", key: diagnostics.KeyMaxReportsPerUserDay, value: "1"},
@@ -482,6 +482,8 @@ func TestDiagnosticsNumericSettingsRejectOutOfRangeValues(t *testing.T) {
 		{name: "bundle above maximum", key: diagnostics.KeyMaxBundleBytes, value: "268435457", initial: map[string]string{diagnostics.KeyMaxUncompressedBytes: "1073741824"}},
 		{name: "bundle above uncompressed", key: diagnostics.KeyMaxBundleBytes, value: "67108865", initial: map[string]string{diagnostics.KeyMaxUncompressedBytes: "67108864"}},
 		{name: "uncompressed below bundle", key: diagnostics.KeyMaxUncompressedBytes, value: "10485759", initial: map[string]string{diagnostics.KeyMaxBundleBytes: "10485760"}},
+		{name: "bundle above per-user cap", key: diagnostics.KeyMaxBundleBytes, value: "104857600", initial: map[string]string{diagnostics.KeyMaxUncompressedBytes: "1073741824", diagnostics.KeyMaxBytesPerUser: "10485760"}},
+		{name: "per-user below bundle", key: diagnostics.KeyMaxBytesPerUser, value: "10485760", initial: map[string]string{diagnostics.KeyMaxBundleBytes: "104857600", diagnostics.KeyMaxUncompressedBytes: "1073741824"}},
 		{name: "uncompressed above maximum", key: diagnostics.KeyMaxUncompressedBytes, value: "1073741825"},
 		{name: "reports below minimum", key: diagnostics.KeyMaxReportsPerUserDay, value: "0"},
 		{name: "reports above maximum", key: diagnostics.KeyMaxReportsPerUserDay, value: "1001"},

internal/api/middleware/demo_guard.go

@@ -17,7 +17,8 @@ type DemoSettingsReader interface {
 // Allowed: browsing, playback, favorites, watchlist, ratings, collections,
 // profiles, playback progress, watched state.
 //
-// Blocked: API key management, downloads, history imports, subtitle downloads.
+// Blocked: API key management, downloads, history imports, subtitle downloads,
+// diagnostics report uploads/deletes.
 type DemoGuard struct {
 	settings DemoSettingsReader
 }
@@ -41,6 +42,9 @@ var demoBlockedRoutes = []blockedRoute{
 	{methods: []string{"POST"}, prefix: "/api/v1/subtitles/download"},
 	{methods: []string{"POST"}, prefix: "/api/v1/subtitles/upload"},
 	{methods: []string{"DELETE"}, prefix: "/api/v1/subtitles/"},
+	// Diagnostics report uploads/deletes write DB rows and private-bucket blobs;
+	// GET /api/v1/diagnostics/status is unaffected (GETs always pass).
+	{methods: []string{"POST", "DELETE"}, prefix: "/api/v1/diagnostics/reports"},
 }
 
 // Guard is an HTTP middleware that enforces demo mode restrictions.

internal/api/router.go

@@ -1816,6 +1816,12 @@ func NewRouter(deps Dependencies) chi.Router {
 		if diagnosticsHandler != nil && authMiddleware != nil {
 			r.Group(func(r chi.Router) {
 				r.Use(authMiddleware.RequireAuth)
+				// Demo mode blocks non-admin report uploads (a write to the
+				// private bucket and DB); the read-only status endpoint stays
+				// available because DemoGuard always lets GETs through.
+				if demoGuard != nil {
+					r.Use(demoGuard.Guard)
+				}
 				if deps.RateLimitMW != nil {
 					r.Use(deps.RateLimitMW.Handler)
 				}

internal/diagnostics/admin.go

@@ -78,13 +78,7 @@ func (s *Service) DeleteReport(ctx context.Context, id string) (*Report, error)
 		return nil, err
 	}
 	if err := deleteReportObjects(ctx, s.store, deleted, s.logger); err != nil {
-		s.logger.ErrorContext(ctx, "diagnostic report blob deletion failed after row deletion",
-			"component", "diagnostics",
-			"report_id", deleted.ID,
-			"bucket", reportBlobBucket(deleted, s.store),
-			"keys", reportObjectKeys(*deleted),
-			"error", err,
-		)
+		logDeferredBlobDeletion(ctx, s.logger, s.store, deleted, err)
 	}
 	return deleted, nil
 }

internal/diagnostics/cleanup.go

@@ -94,14 +94,17 @@ func CleanupReports(
 		return result, errors.Join(errs...)
 	}
 	for _, report := range candidates {
-		if err := deleteReportObjects(ctx, store, &report, logger); err != nil {
-			errs = append(errs, fmt.Errorf("delete diagnostic retention blob %s: %w", report.ID, err))
-			continue
-		}
+		// Row first: a DB failure after the object is gone would leave a visible
+		// ready report whose bundle 404s on download. If the blob delete then
+		// fails the row is already gone, so log it for the orphan pass below to
+		// reap rather than aborting the run (see the admin DeleteReport path).
 		if _, err := repo.DeleteByID(ctx, report.ID); err != nil && !IsReportNotFound(err) {
 			errs = append(errs, fmt.Errorf("delete diagnostic retention row %s: %w", report.ID, err))
 			continue
 		}
+		if err := deleteReportObjects(ctx, store, &report, logger); err != nil {
+			logDeferredBlobDeletion(ctx, logger, store, &report, err)
+		}
 		result.RetentionReportsDeleted++
 	}
 
@@ -111,10 +114,9 @@ func CleanupReports(
 		return result, errors.Join(errs...)
 	}
 	for _, report := range stale {
-		if err := deleteReportObjects(ctx, store, &report, logger); err != nil {
-			errs = append(errs, fmt.Errorf("delete stale diagnostic blob %s: %w", report.ID, err))
-			continue
-		}
+		// Same rows-first ordering as retention: mark/delete the row before the
+		// blob so a mid-cleanup DB failure never leaves a row pointing at a
+		// missing object, and let orphan cleanup reap a blob that fails to delete.
 		if report.State == StateReceiving {
 			if err := repo.MarkFailed(ctx, report.ID); err != nil && !IsReportNotFound(err) {
 				errs = append(errs, fmt.Errorf("mark stale diagnostic report failed %s: %w", report.ID, err))
@@ -125,6 +127,9 @@ func CleanupReports(
 			errs = append(errs, fmt.Errorf("delete stale diagnostic row %s: %w", report.ID, err))
 			continue
 		}
+		if err := deleteReportObjects(ctx, store, &report, logger); err != nil {
+			logDeferredBlobDeletion(ctx, logger, store, &report, err)
+		}
 		result.StaleReportsDeleted++
 	}
 
@@ -196,6 +201,25 @@ func logSkippedObjectDeletion(ctx context.Context, logger *slog.Logger, reportID
 	)
 }
 
+// logDeferredBlobDeletion records a report blob that could not be deleted after
+// its DB row was already removed. The row is the source of truth, so the object
+// is now an orphan for the orphan-cleanup pass (this run or a later one) to
+// reap; we log the bucket and keys rather than fail so one unreachable object
+// can't block row deletion. Shared by the retention/stale loops and the admin
+// DeleteReport path.
+func logDeferredBlobDeletion(ctx context.Context, logger *slog.Logger, store ObjectStore, report *Report, err error) {
+	if logger == nil {
+		logger = slog.Default()
+	}
+	logger.ErrorContext(ctx, "diagnostic report blob deletion failed after row deletion",
+		"component", "diagnostics",
+		"report_id", report.ID,
+		"bucket", reportBlobBucket(report, store),
+		"keys", reportObjectKeys(*report),
+		"error", err,
+	)
+}
+
 func deleteObjectIfPresent(ctx context.Context, store ObjectStore, bucket, key string) error {
 	key = strings.TrimSpace(key)
 	if key == "" {

internal/diagnostics/cleanup_test.go

@@ -9,7 +9,7 @@ import (
 	"time"
 )
 
-func TestCleanupReportsDeletesBlobBeforeRowAndToleratesMissingObject(t *testing.T) {
+func TestCleanupReportsDeletesRowBeforeBlobAndToleratesMissingObject(t *testing.T) {
 	ops := []string{}
 	repo := &fakeCleanupRepo{
 		retention: []Report{
@@ -38,15 +38,15 @@ func TestCleanupReportsDeletesBlobBeforeRowAndToleratesMissingObject(t *testing.
 		t.Fatalf("RetentionReportsDeleted = %d, want 2", result.RetentionReportsDeleted)
 	}
 	wantOps := []string{
-		"delete-object:diagnostics/7/r1.tar.gz",
 		"delete-row:r1",
-		"delete-object:diagnostics/7/r2.tar.gz",
+		"delete-object:diagnostics/7/r1.tar.gz",
 		"delete-row:r2",
+		"delete-object:diagnostics/7/r2.tar.gz",
 	}
 	assertStrings(t, ops, wantOps)
 }
 
-func TestCleanupReportsContinuesPastPerReportFailure(t *testing.T) {
+func TestCleanupReportsTreatsBlobFailureAsNonFatal(t *testing.T) {
 	ops := []string{}
 	repo := &fakeCleanupRepo{
 		retention: []Report{
@@ -68,17 +68,20 @@ func TestCleanupReportsContinuesPastPerReportFailure(t *testing.T) {
 		Now:    func() time.Time { return time.Date(2026, 7, 20, 12, 0, 0, 0, time.UTC) },
 		Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
 	})
-	if err == nil {
-		t.Fatal("CleanupReports error = nil, want aggregated per-report failure")
+	// r1's row is deleted first, so its blob delete failing does not abort the
+	// run or roll back the row: it is logged for orphan cleanup to reap, both
+	// rows count as deleted, and no error is surfaced.
+	if err != nil {
+		t.Fatalf("CleanupReports: %v", err)
 	}
-	// r1's blob delete failed, so its row is left intact; r2 is still processed.
-	if result.RetentionReportsDeleted != 1 {
-		t.Fatalf("RetentionReportsDeleted = %d, want 1", result.RetentionReportsDeleted)
+	if result.RetentionReportsDeleted != 2 {
+		t.Fatalf("RetentionReportsDeleted = %d, want 2", result.RetentionReportsDeleted)
 	}
 	assertStrings(t, ops, []string{
+		"delete-row:r1",
 		"delete-object:diagnostics/7/r1.tar.gz",
-		"delete-object:diagnostics/7/r2.tar.gz",
 		"delete-row:r2",
+		"delete-object:diagnostics/7/r2.tar.gz",
 	})
 }
 
@@ -103,9 +106,9 @@ func TestCleanupReportsCleansStaleReceiving(t *testing.T) {
 		t.Fatalf("StaleReportsDeleted = %d, want 1", result.StaleReportsDeleted)
 	}
 	assertStrings(t, ops, []string{
-		"delete-object:diagnostics/7/r1.tar.gz",
 		"mark-failed:r1",
 		"delete-row:r1",
+		"delete-object:diagnostics/7/r1.tar.gz",
 	})
 }
 

internal/diagnostics/repo.go

@@ -260,7 +260,7 @@ func (r *PostgresRepository) ListForAdmin(ctx context.Context, filters ListFilte
 	}
 
 	query := fmt.Sprintf("%s\n\t\tWHERE %s\n\t\tORDER BY received_at DESC, id DESC\n\t\tLIMIT $%d",
-		reportSelectSQL(), strings.Join(conditions, " AND "), argIdx)
+		reportListSelectSQL(), strings.Join(conditions, " AND "), argIdx)
 	args = append(args, limit+1)
 
 	rows, err := r.pool.Query(ctx, query, args...)
@@ -271,7 +271,7 @@ func (r *PostgresRepository) ListForAdmin(ctx context.Context, filters ListFilte
 
 	reports := make([]Report, 0, limit+1)
 	for rows.Next() {
-		report, err := scanReport(rows)
+		report, err := scanReportSummary(rows)
 		if err != nil {
 			return ListResult{}, err
 		}
@@ -314,13 +314,13 @@ func (r *PostgresRepository) RetentionCandidates(ctx context.Context, olderThan
 		return nil, nil
 	}
 
-	query := reportSelectSQL() + `
+	query := reportListSelectSQL() + `
 		WHERE received_at < $1
 		ORDER BY received_at ASC, id ASC
 	`
 	args := []any{olderThan}
 	if olderThan.IsZero() {
-		query = reportSelectSQL() + `
+		query = reportListSelectSQL() + `
 			WHERE false
 		`
 		args = nil
@@ -341,12 +341,12 @@ func (r *PostgresRepository) RetentionCandidates(ctx context.Context, olderThan
 				) ranked
 				WHERE retained_bytes > $1
 			)
-		` + reportSelectSQL() + `
+		` + reportListSelectSQL() + `
 			WHERE id IN (SELECT id FROM quota_candidates)
 			ORDER BY received_at ASC, id ASC
 		`
 		if olderThan.IsZero() {
-			return r.queryReports(ctx, byteCapQuery, perUserByteCap)
+			return r.queryReportSummaries(ctx, byteCapQuery, perUserByteCap)
 		}
 
 		query = `
@@ -363,14 +363,14 @@ func (r *PostgresRepository) RetentionCandidates(ctx context.Context, olderThan
 				) ranked
 				WHERE retained_bytes > $2
 			)
-		` + reportSelectSQL() + `
+		` + reportListSelectSQL() + `
 			WHERE received_at < $1 OR id IN (SELECT id FROM quota_candidates)
 			ORDER BY received_at ASC, id ASC
 		`
 		args = []any{olderThan, perUserByteCap}
 	}
 
-	return r.queryReports(ctx, query, args...)
+	return r.queryReportSummaries(ctx, query, args...)
 }
 
 func (r *PostgresRepository) StaleReceiving(ctx context.Context, grace time.Duration) ([]Report, error) {
@@ -378,7 +378,7 @@ func (r *PostgresRepository) StaleReceiving(ctx context.Context, grace time.Dura
 		grace = time.Hour
 	}
 	cutoff := time.Now().UTC().Add(-grace)
-	return r.queryReports(ctx, reportSelectSQL()+`
+	return r.queryReportSummaries(ctx, reportListSelectSQL()+`
 		WHERE state IN ('receiving', 'failed')
 		  AND received_at < $1
 		ORDER BY received_at ASC, id ASC
@@ -417,7 +417,9 @@ func (r *PostgresRepository) LiveBlobKeys(ctx context.Context, keys []string) (m
 	return live, nil
 }
 
-func (r *PostgresRepository) queryReports(ctx context.Context, query string, args ...any) ([]Report, error) {
+// queryReportSummaries runs a reportListSelectSQL-shaped query; the returned
+// reports carry no manifest. Only list/cleanup paths use it.
+func (r *PostgresRepository) queryReportSummaries(ctx context.Context, query string, args ...any) ([]Report, error) {
 	rows, err := r.pool.Query(ctx, query, args...)
 	if err != nil {
 		return nil, fmt.Errorf("query diagnostic reports: %w", err)
@@ -426,7 +428,7 @@ func (r *PostgresRepository) queryReports(ctx context.Context, query string, arg
 
 	reports := []Report{}
 	for rows.Next() {
-		report, err := scanReport(rows)
+		report, err := scanReportSummary(rows)
 		if err != nil {
 			return nil, err
 		}
@@ -463,6 +465,10 @@ func validateInsertReceivingInput(input InsertReceivingInput) error {
 	return nil
 }
 
+// reportSelectSQL is the full projection including the manifest JSONB (up to
+// MaxManifestBytes per row). Reserve it for single-report detail/download paths;
+// list and cleanup queries use reportListSelectSQL so browsing a page or running
+// a retention/stale batch doesn't drag every report's manifest through the DB.
 func reportSelectSQL() string {
 	return `SELECT id::text, short_id, user_id, profile_id, state, captured_at, received_at,
 		       report_type, platform, app_version, crash_summary, manifest, playback_session_ids,
@@ -470,64 +476,120 @@ func reportSelectSQL() string {
 		FROM client_diagnostic_reports`
 }
 
+// reportListSelectSQL mirrors reportSelectSQL but omits the manifest column.
+// Reports scanned with scanReportSummary therefore have a nil Manifest; the
+// list and cleanup callers only need summary and blob fields.
+func reportListSelectSQL() string {
+	return `SELECT id::text, short_id, user_id, profile_id, state, captured_at, received_at,
+		       report_type, platform, app_version, crash_summary, playback_session_ids,
+		       blob_bucket, blob_key, blob_bytes, uncompressed_bytes, blob_sha256
+		FROM client_diagnostic_reports`
+}
+
 type reportScanner interface {
 	Scan(dest ...any) error
 }
 
+// reportNulls holds the nullable columns shared by the full and summary
+// projections so both scanners apply them the same way.
+type reportNulls struct {
+	profileID         sql.NullString
+	crashSummary      sql.NullString
+	blobBucket        sql.NullString
+	blobKey           sql.NullString
+	blobSHA256        sql.NullString
+	blobBytes         sql.NullInt64
+	uncompressedBytes sql.NullInt64
+}
+
+func (n *reportNulls) apply(report *Report) {
+	if n.profileID.Valid {
+		report.ProfileID = &n.profileID.String
+	}
+	if n.crashSummary.Valid {
+		report.CrashSummary = &n.crashSummary.String
+	}
+	if report.PlaybackSessionIDs == nil {
+		report.PlaybackSessionIDs = []string{}
+	}
+	if n.blobBucket.Valid {
+		report.BlobBucket = &n.blobBucket.String
+	}
+	if n.blobKey.Valid {
+		report.BlobKey = &n.blobKey.String
+	}
+	if n.blobBytes.Valid {
+		report.BlobBytes = &n.blobBytes.Int64
+	}
+	if n.uncompressedBytes.Valid {
+		report.UncompressedBytes = &n.uncompressedBytes.Int64
+	}
+	if n.blobSHA256.Valid {
+		report.BlobSHA256 = &n.blobSHA256.String
+	}
+}
+
+// scanReport scans the full reportSelectSQL projection, including the manifest.
 func scanReport(row reportScanner) (*Report, error) {
 	var report Report
-	var profileID, crashSummary, blobBucket, blobKey, blobSHA256 sql.NullString
-	var blobBytes, uncompressedBytes sql.NullInt64
+	var nulls reportNulls
 	var manifest []byte
 	if err := row.Scan(
 		&report.ID,
 		&report.ShortID,
 		&report.UserID,
-		&profileID,
+		&nulls.profileID,
 		&report.State,
 		&report.CapturedAt,
 		&report.ReceivedAt,
 		&report.ReportType,
 		&report.Platform,
 		&report.AppVersion,
-		&crashSummary,
+		&nulls.crashSummary,
 		&manifest,
 		&report.PlaybackSessionIDs,
-		&blobBucket,
-		&blobKey,
-		&blobBytes,
-		&uncompressedBytes,
-		&blobSHA256,
+		&nulls.blobBucket,
+		&nulls.blobKey,
+		&nulls.blobBytes,
+		&nulls.uncompressedBytes,
+		&nulls.blobSHA256,
 	); err != nil {
 		return nil, fmt.Errorf("scan diagnostic report: %w", err)
 	}
-	if profileID.Valid {
-		report.ProfileID = &profileID.String
-	}
-	if crashSummary.Valid {
-		report.CrashSummary = &crashSummary.String
-	}
 	if len(manifest) > 0 {
 		report.Manifest = append(json.RawMessage(nil), manifest...)
 	}
-	if report.PlaybackSessionIDs == nil {
-		report.PlaybackSessionIDs = []string{}
-	}
-	if blobBucket.Valid {
-		report.BlobBucket = &blobBucket.String
-	}
-	if blobKey.Valid {
-		report.BlobKey = &blobKey.String
-	}
-	if blobBytes.Valid {
-		report.BlobBytes = &blobBytes.Int64
-	}
-	if uncompressedBytes.Valid {
-		report.UncompressedBytes = &uncompressedBytes.Int64
-	}
-	if blobSHA256.Valid {
-		report.BlobSHA256 = &blobSHA256.String
+	nulls.apply(&report)
+	return &report, nil
+}
+
+// scanReportSummary scans the reportListSelectSQL projection, which omits the
+// manifest; the returned report's Manifest is left nil.
+func scanReportSummary(row reportScanner) (*Report, error) {
+	var report Report
+	var nulls reportNulls
+	if err := row.Scan(
+		&report.ID,
+		&report.ShortID,
+		&report.UserID,
+		&nulls.profileID,
+		&report.State,
+		&report.CapturedAt,
+		&report.ReceivedAt,
+		&report.ReportType,
+		&report.Platform,
+		&report.AppVersion,
+		&nulls.crashSummary,
+		&report.PlaybackSessionIDs,
+		&nulls.blobBucket,
+		&nulls.blobKey,
+		&nulls.blobBytes,
+		&nulls.uncompressedBytes,
+		&nulls.blobSHA256,
+	); err != nil {
+		return nil, fmt.Errorf("scan diagnostic report: %w", err)
 	}
+	nulls.apply(&report)
 	return &report, nil
 }
 

internal/diagnostics/service.go

@@ -34,6 +34,7 @@ var (
 	ErrArchiveMismatch        = errors.New("diagnostics archive metadata mismatch")
 	ErrReportStoreUnavailable = errors.New("diagnostics report store unavailable")
 	ErrProfileMismatch        = errors.New("diagnostics profile id mismatch")
+	ErrChildProfileForbidden  = errors.New("diagnostics child profile attribution forbidden")
 )
 
 type AvailabilityStatus string
@@ -99,15 +100,20 @@ type ProfileLookup func(ctx context.Context, userID int, profileID string) (foun
 // attributes a report to a profile only when it belongs to the user and is not
 // a child profile. The client diagnostics design forbids child profiles from
 // performing diagnostics actions, so a nonconforming client that sends a child
-// profile's ID (via X-Profile-Id or a manifest profile_id) has that attribution
-// rejected rather than recorded against the child.
+// profile's ID (via X-Profile-Id or a manifest profile_id) has the upload
+// rejected with ErrChildProfileForbidden rather than silently losing the
+// attribution. A profile that is simply not the user's (found == false) is not
+// an error: attribution is dropped and the upload proceeds unattributed.
 func NewProfileAttributionValidator(lookup ProfileLookup) ProfileAttributionValidator {
 	return ProfileAttributionValidatorFunc(func(ctx context.Context, userID int, profileID string) (bool, error) {
 		found, isChild, err := lookup(ctx, userID, profileID)
 		if err != nil {
 			return false, err
 		}
-		return found && !isChild, nil
+		if found && isChild {
+			return false, ErrChildProfileForbidden
+		}
+		return found, nil
 	})
 }
 
@@ -479,6 +485,12 @@ func (s *Service) validatedProfileID(ctx context.Context, userID int, profileID
 	}
 	found, err := s.profileValidator.ProfileBelongsToUser(ctx, userID, *candidate)
 	if err != nil {
+		// A child-profile attribution is a client-facing rejection, not an
+		// internal failure: surface it verbatim so the handler can return a
+		// clear error instead of a generic 500.
+		if errors.Is(err, ErrChildProfileForbidden) {
+			return nil, err
+		}
 		return nil, fmt.Errorf("validate diagnostic profile attribution: %w", err)
 	}
 	if !found {

internal/diagnostics/service_test.go

@@ -301,16 +301,17 @@ func TestServiceIngestAcceptsMatchingProfileID(t *testing.T) {
 
 func TestNewProfileAttributionValidatorRejectsChildProfiles(t *testing.T) {
 	tests := []struct {
-		name    string
-		found   bool
-		isChild bool
-		lookErr error
-		want    bool
-		wantErr bool
+		name      string
+		found     bool
+		isChild   bool
+		lookErr   error
+		want      bool
+		wantErr   bool
+		wantChild bool
 	}{
 		{name: "non-child profile attributed", found: true, isChild: false, want: true},
-		{name: "child profile rejected", found: true, isChild: true, want: false},
-		{name: "missing profile rejected", found: false, isChild: false, want: false},
+		{name: "child profile forbidden", found: true, isChild: true, wantErr: true, wantChild: true},
+		{name: "missing profile dropped", found: false, isChild: false, want: false},
 		{name: "lookup error propagated", lookErr: errors.New("boom"), wantErr: true},
 	}
 	for _, tc := range tests {
@@ -326,6 +327,9 @@ func TestNewProfileAttributionValidatorRejectsChildProfiles(t *testing.T) {
 				if err == nil {
 					t.Fatal("ProfileBelongsToUser error = nil, want error")
 				}
+				if tc.wantChild && !errors.Is(err, ErrChildProfileForbidden) {
+					t.Fatalf("ProfileBelongsToUser error = %v, want ErrChildProfileForbidden", err)
+				}
 				return
 			}
 			if err != nil {
@@ -338,6 +342,30 @@ func TestNewProfileAttributionValidatorRejectsChildProfiles(t *testing.T) {
 	}
 }
 
+func TestServiceIngestRejectsChildProfileAttribution(t *testing.T) {
+	// A nonconforming client attributes the report to a child profile. The
+	// validator rejects it with ErrChildProfileForbidden, so the upload is
+	// refused outright rather than silently stored without attribution.
+	bundle, manifest, _ := testDiagnosticsUpload(t, "server-1", DefaultConsentNoticeVer, "prof_child")
+	repo := &fakeDiagnosticReportStore{}
+	store := &fakeDiagnosticObjectStore{bucket: "private"}
+	svc := newTestDiagnosticsService(repo, store)
+	svc.SetProfileAttributionValidator(ProfileAttributionValidatorFunc(func(context.Context, int, string) (bool, error) {
+		return false, ErrChildProfileForbidden
+	}))
+
+	_, err := svc.Ingest(context.Background(), 42, nil, manifest, bytes.NewReader(bundle))
+	if !errors.Is(err, ErrChildProfileForbidden) {
+		t.Fatalf("Ingest error = %v, want ErrChildProfileForbidden", err)
+	}
+	if repo.insertInput.ProfileID != nil {
+		t.Fatalf("ProfileID = %v, want no insert on child profile", *repo.insertInput.ProfileID)
+	}
+	if len(repo.ready) != 0 || len(store.puts) != 0 {
+		t.Fatalf("no report should be stored on child rejection: ready=%v puts=%v", repo.ready, store.puts)
+	}
+}
+
 func TestServiceIngestAcceptsManifestWithoutProfileID(t *testing.T) {
 	bundle, manifest, _ := testDiagnosticsUpload(t, "server-1", DefaultConsentNoticeVer, "")
 	repo := &fakeDiagnosticReportStore{}

internal/diagnostics/types.go

@@ -82,7 +82,7 @@ type Report struct {
 	Platform           string          `json:"platform"`
 	AppVersion         string          `json:"app_version"`
 	CrashSummary       *string         `json:"crash_summary,omitempty"`
-	Manifest           json.RawMessage `json:"manifest"`
+	Manifest           json.RawMessage `json:"manifest,omitempty"`
 	PlaybackSessionIDs []string        `json:"playback_session_ids"`
 	BlobBucket         *string         `json:"blob_bucket,omitempty"`
 	BlobKey            *string         `json:"blob_key,omitempty"`