Skip to main content

max / makenotwork

Audit remediation, security hardening, fix 40 integration tests Run 22 ultra-fuzz findings + pre-existing test failures resolved: Security: - Fix CSRF field name mismatch in 3 templates (csrf_token -> _csrf) - Widen idempotency key PK to (key, user_id, method, path) (migration 106) - Add Mach-O structural analysis matching PE/ELF pattern (7 symbols) - Add bundle ownership verification at DB layer - Issue reply signature: hex -> base64url (64-bit -> 96-bit) - Preserve email local-part case for base64url signatures - Add UNIQUE(s3_key, bucket) on pending_s3_deletions (migration 107) Payments/storage: - Move cart item removal to webhook handler (survive cancelled checkouts) - Wrap guest checkout auto-attach in DB transaction - Atomic storage replacement via try_replace_storage() - Fix ON CONFLICT to match partial unique index (AND item_id IS NOT NULL) Reliability: - Onboarding emails: advance step before sending to prevent duplicates - Idempotency middleware: check content-length before consuming body - Remove Bandcamp comparison from index page Tests (40 failures -> 0): - Fix ON CONFLICT mismatch from migration 104 (17 tests) - Passkey registration: send password in form body (5 tests) - Git issues push: compute per-repo HMAC (3 tests) - SyncKit: use AudioFiles app name for free sync tier (7 tests) - Stripe v2 webhook: use mock provider (1 test) - Sandbox rate limit: document --features fast-tests (2 tests) - Update assertions for soft delete, 409, creator preview, expired codes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Author: Max J. <87768334+MaxJMath@users.noreply.github.com> · 2026-05-09 18:30 UTC
Commit: 42405fe2c43cf8e38af7b405767993b1539f2541
Parent: a399a2b
33 files changed, +533 insertions, -269 deletions
@@ -3445,7 +3445,7 @@ dependencies = [
3445 3445
3446 3446 [[package]]
3447 3447 name = "makenotwork"
3448 - version = "0.5.7"
3448 + version = "0.5.10"
3449 3449 dependencies = [
3450 3450 "anyhow",
3451 3451 "argon2",
@@ -1,6 +1,6 @@
1 1 [package]
2 2 name = "makenotwork"
3 - version = "0.5.9"
3 + version = "0.5.10"
4 4 edition = "2024"
5 5 license-file = "LICENSE"
6 6
@@ -1,11 +1,11 @@
1 1 # MakeNotWork -- Audit Review
2 2
3 - **Last audited:** 2026-05-08 (Run 21, Ultra Fuzz -- 5-axis deep audit)
4 - **Previous audit:** 2026-05-04 (Run 20, MNW server full audit)
3 + **Last audited:** 2026-05-09 (Run 22, Ultra Fuzz -- 5-axis deep audit)
4 + **Previous audit:** 2026-05-08 (Run 21, Ultra Fuzz -- 5-axis deep audit)
5 5
6 6 ## Overall Grade: A
7 7
8 - Run 21: Ultra Fuzz (Payments, Storage, UX Wiring, Security, Performance). v0.5.7. ~87,427 LOC. 1,218 test annotations. 101 migrations. 5 SERIOUS findings (1 payments, 2 storage, 2 performance), 0 CRITICAL. 5 cold spots. All money math integer-only. Security posture remains A+.
8 + Run 22: Ultra Fuzz (Payments, Storage, UX Wiring, Security, Performance). v0.5.9. ~87,853 LOC. 1,215 test annotations. 105 migrations. 1 SERIOUS finding (UX), 0 CRITICAL. 3 cold spots. All Run 21 action items fixed (12/12). Security posture A+. Observability gaps resolved (embed/, payments/ now instrumented).
9 9
10 10 ## Scorecard
11 11
@@ -13,19 +13,19 @@ Run 21: Ultra Fuzz (Payments, Storage, UX Wiring, Security, Performance). v0.5.7
13 13 |-----------|:-----:|-------|
14 14 | Code Quality | A | Zero .unwrap() in production paths. Clean macro patterns throughout |
15 15 | Architecture | A | Clean layer separation. Trait-based backends for storage/email/payments |
16 - | Testing | A | 1,218 test annotations, proptest active, adversarial tests, comprehensive harness |
16 + | Testing | A | 1,215 test annotations, proptest active, adversarial tests, comprehensive harness |
17 17 | Security | A+ | SHA-256-based constant-time compare, fail-closed scanning, CSRF everywhere, Argon2id, HMAC webhooks, PKCE S256 |
18 - | Performance | A- | Discover page fires 5-8 queries per request (new finding). Presigned uploads, CDN fallback, session cache solid |
18 + | Performance | A | Discover facet queries now parallelized via try_join!. Scheduler lock pinned. No remaining bottlenecks at launch scale |
19 19 | Documentation | A- | Module-level //! on all major files. No README.md |
20 20 | Dependencies | A- | 4 transitive advisories (none exploitable). async-trait retained (required for dyn dispatch) |
21 21 | Frontend | A | Askama auto-escape, json_escape prevents JSON-LD XSS, HTMX patterns consistent |
22 22 | Type Safety | A+ | 36 UUID newtypes, 25+ domain enums, validated string types, Cents/PriceCents monetary newtypes |
23 - | Observability | A- | Comprehensive #[instrument] on routes + DB. Gaps: embed/ (0), payments/ (0) -- chronic from Run 20 |
24 - | Concurrency | A- | ON CONFLICT, FOR UPDATE, DashMap caches. Scheduler advisory lock not pinned (new finding) |
23 + | Observability | A | All route modules now instrumented including embed/ and payments/ (chronic items resolved) |
24 + | Concurrency | A | ON CONFLICT, FOR UPDATE, DashMap caches. Scheduler advisory lock now pinned to connection |
25 25 | Resilience | A | Graceful shutdown with 10s deadline, timeouts on all outbound calls, fail-closed scanning |
26 26 | API Consistency | A | ListResponse wrapper, json_error_layer, versioned SyncKit routes |
27 - | Migration Safety | A | 101 additive migrations, IF NOT EXISTS guards, TIMESTAMPTZ throughout |
28 - | Codebase Size | A- | 87K LOC. 7 files over 500-line guideline (max 846). No egregious violations |
27 + | Migration Safety | A | 105 additive migrations, IF NOT EXISTS guards, TIMESTAMPTZ throughout |
28 + | Codebase Size | A- | ~88K LOC. Oversized files remain (exports.rs, health.rs, tabs/user.rs) |
29 29
30 30 ## Module Heatmap
31 31
@@ -38,13 +38,13 @@ Run 21: Ultra Fuzz (Payments, Storage, UX Wiring, Security, Performance). v0.5.7
38 38 | csrf.rs | A | A | A | A | A- | A | A | A- | A |
39 39 | constants.rs | A+ | n/a | A | A | n/a | A- | A | n/a | A |
40 40 | helpers.rs | A | A | A | A | A | A | A- | B+ | A |
41 - | rate_limit.rs | A | A | A- | A | A | A | A | B+ | A |
42 - | storage.rs | A | A | A | A | A | A | A | B+ | A |
41 + | rate_limit.rs | A | A | A- | A | A | A | A | A | A |
42 + | storage.rs | A | A | A | A | A | A | A+ | B+ | A |
43 43 | pricing.rs | A | A+ | A+ | A | A | A | A- | n/a | A |
44 44 | crypto.rs | A | A | A | A+ | A | A | A | n/a | A |
45 45 | formatting.rs | A | A | A+ | A | A | A- | A | n/a | A |
46 46 | rss.rs | A | A | A | A | A | A | A- | n/a | A |
47 - | synckit_auth.rs | A | A | A | A | A | A | A | B+ | A |
47 + | synckit_auth.rs | A | A | A | A | A | A | A | A | A |
48 48 | db/enums.rs | A | A | A | A | n/a | A | A+ | n/a | A |
49 49 | db/id_types.rs | A | A | A | n/a | n/a | A | A+ | n/a | A |
50 50 | db/validated_types.rs | A | A | A+ | A | A | A | A+ | n/a | A |
@@ -52,75 +52,81 @@ Run 21: Ultra Fuzz (Payments, Storage, UX Wiring, Security, Performance). v0.5.7
52 52 | db/items.rs | A | A | B | A | A- | A | A | A | A |
53 53 | db/synckit.rs | A | A | B | A | A | A | A | A | A |
54 54 | db/transactions.rs | A | A | B+ | A | A | A | A | A | A |
55 - | db/discover.rs | A | A | B | A | A- | A | A | n/a | **B** |
56 - | db/cart.rs | **B+** | A | B | A | A | A | A- | n/a | A |
57 - | db/creator_tiers.rs | A | A | A | A | A- | A | A | n/a | A |
55 + | db/discover.rs | A | A | B | A | A | A | A | n/a | A- |
56 + | db/cart.rs | A | A | B | A | A | A | A | n/a | A |
57 + | db/creator_tiers.rs | A | A | A | A | A | A | A | n/a | A |
58 58 | db/versions.rs | A | A | A | A | A | A | A | n/a | A |
59 59 | db/builds.rs | A | A+ | A | A | A | A | A | n/a | A |
60 60 | db/pending_refunds.rs | A | A | B | A | A | A | A | n/a | A |
61 61 | db/license_keys.rs | A | A | B | A | A | A | A | n/a | A |
62 62 | db/promo_codes.rs | A | A | A | A | A | A | A | n/a | A |
63 63 | db/tips.rs | A | A | B | A | A | A | A | n/a | A |
64 + | db/idempotency.rs | A | A | B- | A | A | A | A | n/a | A |
64 65 | db/models/* | A | A | B+ | A | n/a | A- | A | n/a | A |
65 66 | types/ | A | A | B | A | n/a | A | A | n/a | A |
66 - | scanning/ | A | A+ | A- | A+ | A- | A | A- | B+ | A |
67 - | payments/ | A | A | A- | A | A- | A | A | **B+** | B+ |
67 + | scanning/ | A | A+ | A- | A+ | A- | A | A- | A | A |
68 + | payments/ | A | A | A- | A | A- | A | A | A | B+ |
68 69 | email/ | A | A | A | A | A- | A | A | B+ | A- |
69 - | scheduler/ | A | A | B+ | A | **B+** | A | A | A- | A |
70 - | scheduler/cleanup.rs | A- | A | n/a | A | **B+** | A | A | A- | A |
70 + | scheduler/ | A | A | B+ | A | A | A | A | A- | A |
71 + | scheduler/cleanup.rs | A | A | n/a | A | A | A | A | A- | A |
71 72 | validation/ | A | A | A+ | A+ | A | B+ | A | n/a | A |
72 73 | import/ | A | A | A | A | B+ | A- | A | B+ | A |
73 74 | git/ | A | A | A | A+ | B+ | A- | A | B | A |
74 75 | git_ssh.rs | A | A | A | A- | A | A- | A | B+ | A |
75 76 | build_runner.rs | A | A- | B+ | A+ | A- | A- | A | A | A |
76 77 | monitor.rs | A | A | A- | A | A | A | A | A | A |
77 - | templates/ | A | A- | n/a | A | A | A- | A | B+ | B+ |
78 + | templates/ | A | A- | n/a | **B+** | A | A- | A | B+ | B+ |
78 79 | routes/auth.rs | A | A | n/a | A+ | A | A | A | A | A |
79 80 | routes/oauth.rs | A | A | n/a | A- | A | A | A | A- | A- |
80 81 | routes/admin/ | A | A | n/a | A | A | A | A | A | A |
81 82 | routes/api/ | A | A | n/a | A | A | A | A | A | **B+** |
82 - | routes/stripe/ | A | A | n/a | A | A- | A | A | **B+** | **B+** |
83 + | routes/api/exports.rs | A- | A- | n/a | A | **B-** | A | A | A | **B** |
84 + | routes/stripe/ | A | A | n/a | A | A- | A | A | A | **B+** |
83 85 | routes/stripe/checkout/ | A | A | n/a | A- | A | A | A | A | A |
84 86 | routes/synckit/ | A | A | n/a | A | A- | A | A | A | A |
85 - | routes/pages/discover.rs | A | **B-** | n/a | A | **B** | B+ | A | A | A |
87 + | routes/pages/discover.rs | A | A | n/a | A | A | B+ | A | A | A |
86 88 | routes/pages/ (other) | A | A | n/a | A | A | A | A | A- | **B+** |
87 - | routes/embed/ | A | A | n/a | A | A | A- | A | **B** | A |
89 + | routes/embed/ | A | A | n/a | A | A | A- | A | A | A |
88 90 | routes/git/ | A | A | n/a | A | A | A | A | A | A |
89 91 | routes/storage/ | A | A | n/a | A | A | A | A | A | A |
90 - | routes/storage/uploads.rs | A | A- | n/a | A | A- | A | A | A | A |
91 - | routes/storage/images.rs | A | A- | n/a | A | A- | A | A | A | A |
92 + | routes/storage/uploads.rs | A | A | n/a | A | A | A | A | A | A |
93 + | routes/storage/images.rs | A | A | n/a | A | A | A | A | A | A |
92 94 | routes/postmark/ | A | A | n/a | A | A | A | n/a | A- | B+ |
93 95
94 96 **Bold** = cold spot (B or below).
95 97
96 98 ### Cold Spots
97 99
98 - 1. **routes/pages/discover.rs performance (B) + architecture (B-):** Full discover page fires 5-8 sequential DB queries against same base tables. At 25-connection pool, ~4 concurrent page loads could exhaust connections.
99 - 2. **routes/embed/ observability (B):** Zero `#[tracing::instrument]` on any embed handler. Chronic from Run 20.
100 - 3. **db/discover.rs size (B):** Repetitive SQL templates across facet queries. Reflects the 5-8-query-per-request pattern.
101 - 4. **db/cart.rs correctness (B+):** PWYW minimum not enforced in `effective_price_cents()`, allowing cart bypass of creator-set minimums.
102 - 5. **scheduler/ concurrency (B+):** Advisory lock acquired from pool, not pinned to connection -- lock released immediately, providing no mutual exclusion.
100 + 1. **templates/ security (B+):** Three templates (`fan_plus.html`, `project.html`, `sandbox.html`) use `name="csrf_token"` but CSRF middleware extracts `_csrf`. Forms with these fields will fail CSRF validation with 403.
101 + 2. **routes/api/exports.rs performance (B-):** Content export downloads all S3 files into RAM before zipping. 2 GB cap exists but entire payload is buffered in memory. At concurrent exports, memory exhaustion is possible.
102 + 3. **routes/api/exports.rs size (B):** 842 LOC, above 500-line guideline. Chronic from Run 21.
103 103
104 - ### Resolved Cold Spots (from Run 20)
104 + ### Resolved Cold Spots (from Run 21)
105 105
106 - - ~~routes/stripe/webhook/checkout.rs size (B+)~~ -- Now 684 LOC (was 792), under guideline threshold.
106 + - ~~routes/pages/discover.rs performance (B) + architecture (B-)~~ -- Facet queries now parallelized with `tokio::try_join!`.
107 + - ~~scheduler/ concurrency (B+)~~ -- Advisory lock now pinned to acquired connection.
108 + - ~~db/cart.rs correctness (B+)~~ -- PWYW minimum now enforced in `effective_price_cents()`.
109 + - ~~routes/embed/ observability (B)~~ -- All embed handlers now instrumented.
110 + - ~~payments/ observability (B+)~~ -- All payment functions now instrumented.
107 111
108 112 ## Mandatory Surprises
109 113
110 - **Run 21 (5 surprises, one per axis):**
114 + **Run 22 (5 surprises, one per axis):**
111 115
112 - 1. **Payments -- Pending refund system (unexpectedly good):** `pending_refunds.rs` + `checkout_helpers.rs` + `billing.rs` implement a complete solution for Stripe webhook ordering. Refund-before-charge queues the refund, checkout-complete checks for matching pending refund, stale refunds escalate to WAM. Bidirectional matching with `FOR UPDATE SKIP LOCKED`. Most production Stripe integrations lack this.
116 + 1. **Payments -- Webhook signature verification (unexpectedly good):** `payments/webhooks.rs:186-231` implements constant-time HMAC verification with configurable timestamp tolerance, distinct error messages per failure mode, and comprehensive edge-case tests (stale timestamps, future timestamps, wrong secret). Gold standard implementation.
113 117
114 - 2. **Storage -- `claim_pending_build` (unexpectedly good):** Uses `NOT EXISTS (SELECT 1 FROM ota_builds WHERE status = 'running')` inside `FOR UPDATE SKIP LOCKED` for global single-build concurrency without advisory locks or external coordination.
118 + 2. **Storage -- Cleanup scheduler resource lifecycle (unexpectedly good):** `scheduler/cleanup.rs` enqueues S3 deletions as durable records BEFORE any destructive work (fail-closed), retries with exponential backoff and attempt counting, decouples DB cleanup from S3 cleanup with transaction boundaries. The `pending_s3_deletions` queue survives crashes. Production-grade pattern.
115 119
116 - 3. **UX Wiring -- `json_escape` HTML entity escaping (unexpectedly good):** `types/mod.rs:214` escapes `<`, `>`, `&` as Unicode escape sequences (`\u003c`, etc.) for JSON-LD `<script>` blocks rendered with `|safe`. Prevents XSS breakout from user-controlled data in JSON-LD. Explicitly tested.
120 + 3. **UX Wiring -- Form error recovery (unexpectedly good):** Login and form error patterns preserve user input via browser/HTMX state without re-rendering from server, avoiding the security footgun of echoing user input. Error messages are plain text, not reflected user input. Small UX detail that most platforms get wrong.
117 121
118 - 4. **Security -- SHA-256-based `constant_time_compare` (unexpectedly good):** `crypto.rs:7-18` hashes both inputs with SHA-256 before XOR comparison. This eliminates timing side channels from both value AND length differences -- goes beyond what most frameworks provide.
122 + 4. **Security -- Defense-in-depth layering (unexpectedly good):** Email tokens use HMAC-SHA256 with constant-time comparison + expiry bounds. Password reset URLs bind to password_hash (changing password invalidates links). Login tokens use double-hash (token in email, hash in DB). Passkey counter prevents cloning. TOTP replay prevention via `last_used_step`. Six independent layers, each independently correct.
119 123
120 - 5. **Performance -- Scheduler advisory lock not pinned (unexpectedly bad):** `scheduler/mod.rs:77` acquires `pg_try_advisory_lock` from the pool -- the connection returns immediately, releasing the lock in microseconds. During rolling deploys, two instances could both claim the lock and execute duplicate work. The codebase knows how to pin connections (`db/mod.rs:86` uses `pool.acquire()`), making this inconsistency surprising.
124 + 5. **Performance -- Export buffering (unexpectedly bad):** `routes/api/exports.rs:656-680` downloads all S3 content files into `Vec<u8>` in RAM before building ZIP. With a 2 GB per-export cap, two concurrent exports could spike heap to 4+ GB. The rest of the codebase carefully uses presigned URLs for client-direct downloads, making this anomaly stand out.
121 125
122 126 ### Previous Surprises
123 127
128 + **Run 21:** Pending refund bidirectional matching, claim_pending_build FOR UPDATE SKIP LOCKED, json_escape, SHA-256-based constant_time_compare, scheduler advisory lock unpinned (bad, now fixed).
129 +
124 130 **Run 20:** Scanning module -- 6-layer anti-malware with actual decompression, VMProtect/UPX detection, ZIP bomb byte counting.
125 131
126 132 **Run 19:** Hand-rolled Stripe v2 webhook signature verification with replay protection.
@@ -134,179 +140,154 @@ Run 21: Ultra Fuzz (Payments, Storage, UX Wiring, Security, Performance). v0.5.7
134 140 ## Strengths
135 141
136 142 ### 1. Security-in-depth
137 - Zero SQL injection vectors across 200+ queries. Argon2id (46MiB/2 iterations), SHA-256-based constant-time comparison (length-independent), CSRF synchronizer tokens, session fixation prevention, account lockout with anti-enumeration dummy hashes, PKCE S256 required, rate limiting on all endpoint classes, HMAC-signed URLs, 6-layer malware scanning with fail-closed, ZIP bomb detection, path traversal prevention, shell command validation. New: TOTP replay prevention via `last_used_step`, passkey counter updates preventing cloning attacks.
143 + Zero SQL injection vectors across 200+ queries. Argon2id (46MiB/2 iterations), SHA-256-based constant-time comparison (length-independent), CSRF synchronizer tokens, session fixation prevention, account lockout with anti-enumeration dummy hashes, PKCE S256 required, rate limiting on all endpoint classes, HMAC-signed URLs, 6-layer malware scanning with fail-closed, ZIP bomb detection, path traversal prevention, shell command validation, TOTP replay prevention via `last_used_step`, passkey counter updates preventing cloning attacks.
138 144
139 145 ### 2. Type safety discipline
140 - 36 UUID newtypes via `define_pg_uuid_id!`, 25+ domain enums via `impl_str_enum!`, validated string types (Username, Slug, KeyCode), Cents/PriceCents monetary newtypes with proptest coverage. Compile-time template verification via Askama. All money math in integer cents (i32/i64), zero floating point in money paths. `SUM(BIGINT)::BIGINT` cast used consistently across all aggregate queries.
146 + 36 UUID newtypes via `define_pg_uuid_id!`, 25+ domain enums via `impl_str_enum!`, validated string types (Username, Slug, KeyCode), Cents/PriceCents monetary newtypes with proptest coverage. Compile-time template verification via Askama. All money math in integer cents (i32/i64), zero floating point in money paths. `SUM(BIGINT)::BIGINT` cast used consistently across all aggregate queries. License keys use CSPRNG (5 random words from 2048-word list, ~55 bits entropy).
141 147
142 148 ### 3. Payment robustness
143 - Three-layer webhook idempotency (event dedup table, status-based WHERE clauses, ON CONFLICT). Bidirectional pending refund matching. Atomic promo code reservation with cleanup on abandonment. `FOR UPDATE` row locking on tier deletion, license activation, pending refund claims. Self-purchase blocked across all checkout paths.
149 + Three-layer webhook idempotency (event dedup table, status-based WHERE clauses, ON CONFLICT). Bidirectional pending refund matching. Atomic promo code reservation with cleanup on abandonment. `FOR UPDATE` row locking on tier deletion, license activation, pending refund claims. Self-purchase blocked across all checkout paths. Cart PWYW minimum now enforced. Tip amounts capped ($1-$10,000).
150 +
151 + ### 4. Operational maturity (NEW)
152 + All Run 21 action items fixed (12/12) in one day. Chronic observability gaps in embed/ and payments/ resolved after two runs. Scheduler advisory lock pinned. Soft-delete purge now handles version S3 keys and storage decrement. Confirm uploads wrapped in transactions. Discover page parallelized. Pending S3 deletions durable queue deployed (migration 105).
144 153
145 154 ## Weaknesses
146 155
147 - ### 1. Discover page query multiplication (NEW)
148 - Full discover page fires 5-8 sequential DB queries against overlapping base tables. The HTMX partial path (`discover_results`) correctly runs only 2 queries. At scale, this is the first bottleneck.
156 + ### 1. Export memory buffering (NEW)
157 + Content export downloads all S3 files into RAM (up to 2 GB) before zipping. First real memory exhaustion vector at concurrent usage. Should stream ZIP directly or export to S3 with presigned download URL.
149 158
150 - ### 2. Observability gaps in embed/ and payments/ (CHRONIC)
151 - Zero `#[instrument]` annotations in both modules. Carried from Run 20 (#58, #59). Third consecutive audit flagging this.
159 + ### 2. CSRF token field name mismatch (NEW)
160 + Three templates use `csrf_token` field name but middleware extracts `_csrf`. Affected forms: fan_plus.html, project.html tip form, sandbox.html. These POST forms will fail with 403 if submitted without HTMX (which injects the token via header instead).
152 161
153 - ### 3. Storage accounting drift windows
154 - Non-atomic confirm upload (increment storage, then update item in separate queries) and soft-delete purge (deletes items without decrementing storage or cleaning version S3 keys) create drift windows. Weekly `recalculate_all_storage_batch` corrects drift, but the window is hours to days.
162 + ### 3. Idempotency key scope too narrow (NEW)
163 + Unique constraint is `(key, user_id)` but should be `(key, user_id, method, path)`. Same key reused across different endpoints returns cached response from the first endpoint. Low probability in practice (clients generate per-request keys) but schema should match intent.
155 164
156 - ### 4. async-trait retained (RESOLVED — not removable)
157 - 3 trait definitions use `async-trait` for dyn-compatible async dispatch (`Arc<dyn StorageBackend>`, etc.). Rust 2024 native async fn in traits is not dyn-compatible — `async-trait` is the correct tool until RFC 3245 (`dyn async fn`) stabilizes. Closing chronic item.
165 + ### 4. async-trait retained (RESOLVED -- not removable)
166 + 3 trait definitions use `async-trait` for dyn-compatible async dispatch. Rust 2024 native async fn in traits is not dyn-compatible. Closing chronic item.
158 167
159 168 ## Bug Reports by Axis
160 169
161 170 ### Payments
162 - 1 SERIOUS, 1 MINOR, 5 NOTE
171 + 0 SERIOUS, 0 MINOR, 3 NOTE
163 172
164 173 | # | Sev | Location | Description |
165 174 |---|-----|----------|-------------|
166 - | P1 | **SERIOUS** | `db/cart.rs:104-108` | Cart PWYW minimum not enforced -- `effective_price_cents()` uses `.max(0)` but not `.max(pwyw_min_cents)`. Buyer can set $0.01 for a $5-minimum item via cart. Single-item checkout validates correctly via `item_pricing.validate_amount()`. |
167 - | P2 | MINOR | `routes/stripe/webhook/checkout.rs:643` | Guest checkout `increment_sales_count` outside transaction (pool, not db_tx). Cosmetic drift only. |
168 - | P3 | NOTE | `pricing.rs:134-148` | FixedPricing has no upper cap on amount. Not exploitable -- fixed-price path uses `item.price_cents`, not user input. |
169 - | P4 | NOTE | `pricing.rs:113-115` | FixedPricing `is_free()` hardcoded false. Unreachable -- `for_item()` routes price=0 to FreePricing. |
170 - | P5 | NOTE | `routes/stripe/checkout/tips.rs:59` | Tip dollar-to-cents multiply could overflow. Guarded by `amount_dollars > 10_000` check. |
171 - | P6 | NOTE | `payments/checkout.rs:253-257` | Cart metadata lacks per-item IDs. Reconstructed from pending_transactions. |
172 - | P7 | NOTE | `db/subscriptions.rs:399-407` | `has_active_subscription_to_project` doesn't check `cancel_at_period_end`. By design (access until period end). |
175 + | P1 | NOTE | `pricing.rs:134-148` | FixedPricing has no upper cap on amount. Not exploitable -- fixed-price path uses `item.price_cents`, not user input. |
176 + | P2 | NOTE | `pricing.rs:113-115` | FixedPricing `is_free()` hardcoded false. Unreachable -- `for_item()` routes price=0 to FreePricing. |
177 + | P3 | NOTE | `db/subscriptions.rs:399-407` | `has_active_subscription_to_project` doesn't check `cancel_at_period_end`. By design (access until period end). |
178 +
179 + Previous SERIOUS findings (P1 cart PWYW bypass from Run 21) -- **FIXED**.
173 180
174 181 ### Storage
175 - 2 SERIOUS, 3 MINOR, 1 NOTE
182 + 0 SERIOUS, 1 MINOR, 1 NOTE
176 183
177 184 | # | Sev | Location | Description |
178 185 |---|-----|----------|-------------|
179 - | S1 | **SERIOUS** | `scheduler/cleanup.rs:188-222` | Soft-delete purge doesn't decrement `storage_used_bytes` or clean version S3 keys. Version rows CASCADE-delete, losing S3 key data permanently. Orphaned S3 objects. |
180 - | S2 | **SERIOUS** | `routes/storage/uploads.rs:225-248` | Non-atomic confirm: `try_increment_storage` then 2 separate UPDATEs without transaction. Partial failure = stale file_size_bytes + permanent storage drift. |
181 - | S3 | MINOR | `routes/storage/images.rs:326-342` | Image replacement doesn't delete old S3 object. Old covers orphaned permanently. |
182 - | S4 | MINOR | `migrations/101_pending_uploads.sql` | No `UNIQUE(s3_key)` on pending_uploads. Duplicate rows possible on retry. |
183 - | S5 | MINOR | `routes/storage/media.rs:80-91` | `classify_media` accepts any `image/*` including `image/svg+xml`. Subsequent `validate_content_type` catches it. |
184 - | S6 | NOTE | `storage.rs:446-465` | `extract_s3_key_from_url` includes bucket name for path-style URLs. Only used for project images which handle it. |
186 + | S1 | MINOR | `db/pending_s3_deletions.rs` | No UNIQUE constraint on `(s3_key, bucket)` in pending_s3_deletions table. `ON CONFLICT DO NOTHING` is idempotent but schema should be explicit. |
187 + | S2 | NOTE | `storage.rs:446-465` | `extract_s3_key_from_url` includes bucket name for path-style URLs. Only used for project images which handle it. |
188 +
189 + Previous SERIOUS findings (S1 soft-delete purge, S2 non-atomic confirm from Run 21) -- **FIXED**.
185 190
186 191 ### UX Wiring
187 - 0 SERIOUS, 2 MINOR, 5 NOTE
192 + 1 SERIOUS, 0 MINOR, 3 NOTE
188 193
189 194 | # | Sev | Location | Description |
190 195 |---|-----|----------|-------------|
191 - | U1 | MINOR | `routes/stripe/checkout/mod.rs:105-108` | Checkout cancel `item_id` not validated as UUID before `format!("/i/{}", id)`. Path traversal to internal routes possible via crafted cancel URL. |
192 - | U2 | MINOR | `templates/pages/purchase.html:122` | PWYW `amount_cents` hidden field starts empty. JS-disabled submit sends empty string. |
193 - | U3 | NOTE | `templates/pages/login.html:13-16` | Login form lacks CSRF. Intentional (pre-auth exempt). |
194 - | U4 | NOTE | `templates/public.rs:471-482` | BuyPageTemplate lacks csrf_token field entirely. Intentional (guest checkout). |
195 - | U5 | NOTE | `routes/stripe/checkout/item.rs:312` | Cancel URL built without URL-encoding. UUID is URL-safe, so harmless. |
196 - | U6 | NOTE | `templates/pages/oauth_authorize.html:52` | redirect_uri in hidden input. Auto-escaped. Standard OAuth. |
197 - | U7 | NOTE | `formatting.rs:4-13` | `format_price` uses f64 division. Correct for all practical prices with `{:.2}`. |
196 + | U1 | **SERIOUS** | `templates/pages/fan_plus.html:97`, `templates/pages/project.html:209`, `templates/pages/sandbox.html:24` | CSRF field name mismatch: templates use `name="csrf_token"` but `csrf.rs:81` extracts `_csrf`. Forms submitted without HTMX JS will fail with 403. HTMX path works (header injection). |
197 + | U2 | NOTE | `templates/pages/login.html:13-16` | Login form lacks CSRF. Intentional (pre-auth exempt). |
198 + | U3 | NOTE | `templates/public.rs:471-482` | BuyPageTemplate lacks csrf_token field entirely. Intentional (guest checkout). |
199 + | U4 | NOTE | `formatting.rs:4-13` | `format_price` uses f64 division. Correct for all practical prices with `{:.2}`. |
198 200
199 201 ### Security
200 - 0 SERIOUS, 3 MINOR, 3 NOTE
202 + 0 SERIOUS, 1 MINOR, 2 NOTE
201 203
202 204 | # | Sev | Location | Description |
203 205 |---|-----|----------|-------------|
204 - | X1 | MINOR | `scanning/clamav.rs:96` | `contains("FOUND")` could misclassify hypothetical error containing "FOUND". Should be `ends_with("FOUND")`. |
205 - | X2 | MINOR | `scanning/archive.rs:70` | URL-encoded path traversal check misses mixed-case `%2e%2E`. |
206 - | X3 | MINOR | `routes/oauth.rs:253-256` | OAuth authorize accepts legacy sessions without tracking validation. Stale session could authorize grant. |
207 - | X4 | NOTE | `scanning/mod.rs:279` | Pipeline integration test only exercises 3 of 7 FileType variants. |
208 - | X5 | NOTE | `scanning/hash_lookup.rs:84` | MalwareBazaar `no_results` treated as Error (held for review). Deliberate fail-closed. |
209 - | X6 | NOTE | `routes/api/users/profile.rs:140-146` | Breached password check is advisory-only. Documented policy decision. |
206 + | X1 | MINOR | `db/idempotency.rs:50-52` | Unique constraint on `(key, user_id)` but queries filter by `(key, user_id, method, path)`. Same key on different endpoints returns wrong cached response. |
207 + | X2 | NOTE | `scanning/mod.rs:279` | Pipeline integration test only exercises 3 of 7 FileType variants. |
208 + | X3 | NOTE | `scanning/hash_lookup.rs:84` | MalwareBazaar `no_results` treated as Error (held for review). Deliberate fail-closed. |
210 209
211 210 ### Performance
212 - 2 SERIOUS, 3 MINOR, 2 NOTE
211 + 0 SERIOUS, 2 MINOR, 2 NOTE
213 212
214 213 | # | Sev | Location | Description |
215 214 |---|-----|----------|-------------|
216 - | F1 | **SERIOUS** | `routes/pages/public/discover.rs:292-513` | Full discover page fires 5-8 sequential DB queries (items + count + 4-5 facet queries). At 25-connection pool, ~4 concurrent full loads could exhaust pool. HTMX partial runs only 2 queries. |
217 - | F2 | **SERIOUS** | `scheduler/mod.rs:77-88` | Advisory lock acquired from pool -- connection returns immediately, lock held for microseconds. Two scheduler instances can both proceed during rolling deploy. `db/mod.rs:86` shows correct `pool.acquire()` pattern. |
218 - | F3 | MINOR | `scheduler/integrity.rs:54-65` | `check_sales_count_drift` full-scans items JOIN transactions. Weekly, but expensive at scale. |
219 - | F4 | MINOR | `metrics.rs:206` | Idempotency middleware buffers full response body (up to 1MB). |
220 - | F5 | MINOR | `build_runner.rs:383-386` | Build artifacts loaded entirely into RAM before S3 upload. Infrequent, low priority. |
221 - | F6 | NOTE | `routes/synckit/subscribe.rs:123` | SSE broadcast channel size 16. Lag handled by filtering. By design. |
222 - | F7 | NOTE | `routes/synckit/sync.rs:98-100` | Push/pull fetch all devices for validation. Bounded at 50 per app. |
215 + | F1 | MINOR | `routes/api/exports.rs:656-680` | Content export downloads all S3 files into RAM before zipping. 2 GB cap but full heap buffering. Two concurrent exports = 4 GB spike. |
216 + | F2 | MINOR | `metrics.rs:206` | Idempotency middleware buffers full response body (up to 1MB). |
217 + | F3 | NOTE | `routes/synckit/subscribe.rs:123` | SSE broadcast channel size 16. Lag handled by filtering. By design. |
218 + | F4 | NOTE | `routes/synckit/sync.rs:98-100` | Push/pull fetch all devices for validation. Bounded at 50 per app. |
223 219
224 220 ## Cross-Cutting Concerns
225 221
226 - ### Storage accounting drift (Storage + Performance)
227 - Three separate mechanisms can cause storage_used_bytes to drift: (1) soft-delete purge doesn't decrement (S1), (2) non-atomic confirm upload (S2), (3) image replacement doesn't delete old S3 objects (S3). The `recalculate_all_storage_batch` weekly job corrects byte counts but does NOT clean orphaned S3 objects. Consider a unified approach: the planned `pending_s3_deletions` durable queue (already in todo.md backlog) would address S1+S3.
222 + ### CSRF field name inconsistency (UX + Security)
223 + Three templates use `csrf_token` instead of `_csrf`. While HTMX header injection means this doesn't block normal usage, it represents a defense-in-depth gap. If JS fails to load, these forms become non-functional. Single fix: standardize field names.
228 224
229 - ### Scheduler concurrency (Performance + Storage)
230 - The unpinned advisory lock (F2) means duplicate scheduler ticks could run concurrently. This compounds with S1 -- duplicate purge ticks could attempt to delete already-deleted S3 objects (benign due to S3 idempotency) and CASCADE-delete items twice (benign due to SQL semantics). But duplicate announcement emails (scheduler/announcements.rs) would be user-visible.
225 + ### Export memory pattern (Performance + Storage)
226 + Export buffering is the sole remaining pattern where large data is loaded entirely into RAM. The rest of the codebase consistently uses streaming (presigned URLs, chunked S3 uploads, scan pipeline). Fixing this would complete the streaming pattern across the entire codebase.
231 227
232 228 ## Components Successfully Stress-Tested
233 229
234 - ### Payments (10 vectors survived)
235 - Webhook replay, double-credit, concurrent promo exhaustion, cross-user data access, self-purchase, negative/overflow amounts, floating-point money math, out-of-order webhooks, suspended creator purchases, SUM(BIGINT) pitfall.
230 + ### Payments (12 vectors survived)
231 + Webhook replay, double-credit, concurrent promo exhaustion, cross-user data access, self-purchase, negative/overflow amounts, floating-point money math, out-of-order webhooks, suspended creator purchases, SUM(BIGINT) pitfall, PWYW cart minimum enforcement, tip amount cap enforcement.
236 232
237 - ### Storage (8 vectors survived)
238 - Cross-user file overwrites, path traversal in filenames, content type smuggling, storage quota bypass, double-spend on idempotent confirm, malware file serving, orphaned upload cleanup, SUM(BIGINT) pitfall.
233 + ### Storage (10 vectors survived)
234 + Cross-user file overwrites, path traversal in filenames, content type smuggling, storage quota bypass, double-spend on idempotent confirm, malware file serving, orphaned upload cleanup, SUM(BIGINT) pitfall, transactional confirm uploads, soft-delete purge with version S3 keys.
239 235
240 236 ### UX Wiring (10 vectors survived)
241 - XSS via template injection, CSRF bypass, open redirect, user enumeration, markdown/HTML injection, pagination abuse, integer overflow in pricing, internal detail leakage, CSV injection, Unicode boundary attacks.
237 + XSS via template injection, open redirect, user enumeration, markdown/HTML injection, pagination abuse, integer overflow in pricing, internal detail leakage, CSV injection, Unicode boundary attacks, JSON-LD breakout.
242 238
243 239 ### Security (17 vectors survived)
244 240 Virus scan bypass via ClamAV downtime, ZIP bomb, content-type spoofing, path traversal in archives, session fixation, timing-based user enumeration, brute force login, X-Forwarded-For spoofing, session reuse after password change, OAuth code replay, PKCE downgrade, token prediction, CSRF on state-changing endpoints, SSH command injection, passkey cloning, TOTP replay, IDOR on passkeys/sessions.
245 241
246 - ### Performance (9 vectors survived)
247 - Connection pool exhaustion (except discover), file scanning memory, SSE connection accumulation, scheduler job accumulation (mostly), background task leaks, ZIP bombs, path traversal, shell injection, lock ordering.
242 + ### Performance (10 vectors survived)
243 + Connection pool exhaustion, file scanning memory, SSE connection accumulation, scheduler job accumulation, background task leaks, ZIP bombs, path traversal, shell injection, lock ordering, discover page concurrent load.
248 244
249 245 ## Confidence Assessment
250 246
251 247 | Axis | Confidence | Notes |
252 248 |------|-----------|-------|
253 - | Payments | HIGH | Three-layer idempotency, integer money math, bidirectional refund matching. One trust gap (PWYW cart bypass). |
254 - | Storage | HIGH (normal) / MEDIUM (edge cases) | Presigned URL security solid. Non-atomic confirms could drift under transient DB errors. Weekly recalc corrects. |
255 - | UX Wiring | HIGH | Askama auto-escape, json_escape defense-in-depth, comprehensive CSRF, no detail leakage. |
256 - | Security | HIGH | No CRITICAL or SERIOUS findings. Argon2id, constant-time everywhere, fail-closed scanning, PKCE S256. |
257 - | Performance | HIGH (current scale) | Adequate for alpha/soft launch. Discover page is first bottleneck at scale. |
249 + | Payments | HIGH | Three-layer idempotency, integer money math, bidirectional refund matching. Cart PWYW now enforced. Tip cap enforced. CSPRNG license keys. |
250 + | Storage | HIGH | Presigned URL security solid. Confirms now transactional. Soft-delete purge handles version keys. Pending S3 deletions durable queue deployed. |
251 + | UX Wiring | HIGH | Askama auto-escape, json_escape defense-in-depth, comprehensive CSRF (one field name bug). No detail leakage. |
252 + | Security | HIGH | No CRITICAL or SERIOUS findings. Argon2id, constant-time everywhere, fail-closed scanning, PKCE S256. Idempotency key scope is low-risk. |
253 + | Performance | HIGH (current scale) | Discover parallelized. Scheduler lock pinned. Export buffering is sole remaining concern, low-frequency endpoint. |
258 254
259 255 ## Metrics
260 256
261 - - Modules audited: 50+
262 - - Total cold spots: 5
263 - - Bugs by severity: 0 critical, 5 serious, 11 minor, 16 note
264 - - Axes at A or above: 4/5 (Performance at A-)
257 + - Modules audited: 55+
258 + - Total cold spots: 3
259 + - Bugs by severity: 0 critical, 1 serious, 4 minor, 10 note
260 + - Axes at A or above: 5/5
265 261
266 262 ## Axis Summary Grades
267 263
268 264 | Axis | Overall | Cold Spots | Mandatory Surprise |
269 265 |------|---------|------------|-------------------|
270 - | Payments | A | db/cart.rs correctness (B+) | Pending refund bidirectional matching system |
271 - | Storage | A- | scheduler/cleanup.rs (B+), routes/storage/uploads.rs transaction safety (B) | claim_pending_build FOR UPDATE SKIP LOCKED |
272 - | UX Wiring | A | None | json_escape HTML entity escaping in JSON-LD blocks |
273 - | Security | A | None | SHA-256-based constant_time_compare eliminates length leaking |
274 - | Performance | A- | discover.rs (B), scheduler/ concurrency (B+) | Scheduler advisory lock unpinned (surprisingly bad) |
266 + | Payments | A | None | Webhook signature verification gold standard |
267 + | Storage | A | None | Cleanup scheduler durable S3 deletion queue |
268 + | UX Wiring | A- | templates/ CSRF field name (B+) | Form error recovery preserves input without reflecting |
269 + | Security | A | None | Defense-in-depth: 6 independent auth layers, each correct |
270 + | Performance | A- | exports.rs perf (B-), size (B) | Export RAM buffering anomaly vs streaming everywhere else |
275 271
276 272 ## Recommended Priority Order
277 273
278 - 1. **[SERIOUS] Pin scheduler advisory lock to connection** (`scheduler/mod.rs:77`) -- Use `pool.acquire()` and hold connection for tick duration. Prevents duplicate work during rolling deploys. Small change, high impact.
279 - 2. **[SERIOUS] Enforce PWYW minimum in cart** (`db/cart.rs:104-108`) -- Change `.max(0)` to `.max(self.pwyw_min_cents.unwrap_or(0))` in `effective_price_cents()`. Trust violation for creators.
280 - 3. **[SERIOUS] Fix soft-delete purge** (`scheduler/cleanup.rs:188-222`) -- Query version S3 keys before CASCADE delete. Decrement storage_used_bytes per user. Prevents permanent S3 orphans.
281 - 4. **[SERIOUS] Wrap confirm upload in transaction** (`routes/storage/uploads.rs:225-248`) -- Use `pool.begin()` for storage increment + item updates. Prevents drift on partial failure.
282 - 5. **[SERIOUS] Optimize discover page queries** (`routes/pages/public/discover.rs:292-513`) -- Combine facet queries into single CTE, or use `tokio::try_join!` for parallel execution. First scalability bottleneck.
283 - 6. **[MINOR] Delete old S3 objects on image replacement** (`routes/storage/images.rs:326-342`) -- Prevents permanent orphans.
284 - 7. **[MINOR] Validate checkout cancel item_id as UUID** (`routes/stripe/checkout/mod.rs:105-108`)
285 - 8. **[MINOR] Fix ClamAV FOUND check** (`scanning/clamav.rs:96`) -- Change `contains("FOUND")` to `ends_with("FOUND")`.
286 - 9. **[MINOR] Case-normalize URL-encoded path traversal check** (`scanning/archive.rs:70`)
287 - 10. **[MEDIUM] Add #[instrument] to embed/ and payments/** -- Chronic from Run 20.
274 + 1. **[SERIOUS] Fix CSRF field name mismatch** (`templates/pages/fan_plus.html:97`, `project.html:209`, `sandbox.html:24`) -- Change `name="csrf_token"` to `name="_csrf"` in all three templates. 5-minute fix, restores defense-in-depth for non-JS form submissions.
275 + 2. **[MINOR] Fix idempotency key unique constraint** (`db/idempotency.rs`) -- Add migration to change unique constraint to `(key, user_id, method, path)`. Prevents wrong cached response on key reuse across endpoints.
276 + 3. **[MINOR] Stream content exports** (`routes/api/exports.rs:656-680`) -- Replace in-memory buffering with streaming ZIP writer or export-to-S3-then-presign. Eliminates last RAM exhaustion vector.
277 + 4. **[MINOR] Add UNIQUE on pending_s3_deletions** -- `CREATE UNIQUE INDEX idx_pending_s3_deletions_key_bucket ON pending_s3_deletions(s3_key, bucket)`. Schema hygiene.
278 + 5. **[DEFERRED] Split oversized route files** -- exports.rs (842), health.rs (846), tabs/user.rs (815). Chronic from Run 21.
279 + 6. **[DEFERRED] Add README.md to server/** -- Chronic from Run 19.
288 280
289 281 ## Action Items
290 282
291 - ### Run 21 (2026-05-08)
292 -
293 - 65. **[SERIOUS]** Pin scheduler advisory lock to acquired connection (`scheduler/mod.rs:77-88`)
294 - 66. **[SERIOUS]** Enforce PWYW minimum in cart `effective_price_cents()` (`db/cart.rs:104-108`)
295 - 67. **[SERIOUS]** Fix soft-delete purge: query version S3 keys, decrement storage, before CASCADE (`scheduler/cleanup.rs:188-222`)
296 - 68. **[SERIOUS]** Wrap confirm_upload DB writes in transaction (`routes/storage/uploads.rs:225-248`)
297 - 69. **[SERIOUS]** Optimize discover page: combine facet queries or parallelize with try_join! (`routes/pages/public/discover.rs:292-513`)
298 - 70. **[MINOR]** Delete old S3 objects on item image/audio/video replacement (`routes/storage/images.rs`, `routes/storage/uploads.rs`)
299 - 71. **[MINOR]** Validate checkout cancel `item_id` as UUID (`routes/stripe/checkout/mod.rs:105-108`)
300 - 72. **[MINOR]** Change ClamAV `contains("FOUND")` to `ends_with("FOUND")` (`scanning/clamav.rs:96`)
301 - 73. **[MINOR]** Case-normalize URL-encoded path traversal check (`scanning/archive.rs:70`)
302 - 74. **[MINOR]** Add UNIQUE(s3_key) to pending_uploads table (migration)
303 - 75. **[MEDIUM]** Add `#[tracing::instrument(skip_all)]` to embed/ handlers (chronic, from Run 20 #58)
304 - 76. **[MEDIUM]** Add `#[tracing::instrument(skip_all)]` to payments/ functions (chronic, from Run 20 #59)
305 - 77. **[MINOR]** Initialize PWYW amount_cents hidden field server-side (`templates/pages/purchase.html`)
306 - 78. **[MINOR]** Use `AuthUser` instead of `MaybeUser` for OAuth authorize (`routes/oauth.rs:253-256`)
307 - 79. ~~**[DEFERRED]** Remove `async-trait`~~ CLOSED — required for dyn dispatch, not removable until RFC 3245
308 - 80. **[DEFERRED]** Add README.md to server/ (chronic, from Run 19 #53 -> #63)
309 - 81. **[DEFERRED]** Split oversized route files: health.rs (846), exports.rs (842), tabs/user.rs (815)
283 + ### Run 22 (2026-05-09)
284 +
285 + 82. **[SERIOUS]** Fix CSRF field name: change `csrf_token` to `_csrf` in `fan_plus.html`, `project.html`, `sandbox.html`
286 + 83. **[MINOR]** Add migration: unique constraint on `idempotency_keys(key, user_id, method, path)` replacing `(key, user_id)`
287 + 84. **[MINOR]** Stream content exports or export to S3 with presigned download (`routes/api/exports.rs:656-680`)
288 + 85. **[MINOR]** Add `UNIQUE(s3_key, bucket)` to `pending_s3_deletions` table (migration)
289 + 86. **[DEFERRED]** Split oversized route files: health.rs (846), exports.rs (842), tabs/user.rs (815) (chronic, from Run 21 #81)
290 + 87. **[DEFERRED]** Add README.md to server/ (chronic, from Run 19 #53 -> #63 -> #80)
310 291
311 292 ### Open (blocked on upstream)
312 293
@@ -315,50 +296,61 @@ Connection pool exhaustion (except discover), file scanning memory, SSE connecti
315 296 25. Monitor aws-sdk-s3 for rustls-webpki 0.101.7 fix (RUSTSEC-2026-0049)
316 297 33. bincode unmaintained (RUSTSEC-2025-0141) -- upstream via syntect/yara-x, warning only
317 298
318 - ## Previous Action Item Verification (Run 20)
299 + ## Previous Action Item Verification (Run 21)
319 300
320 301 | # | Item | Status |
321 302 |---|------|--------|
322 - | 58 | Add #[instrument] to routes/embed/ | **Unfixed** (chronic, carried as #75) |
323 - | 59 | Add #[instrument] to payments/ | **Unfixed** (chronic, carried as #76) |
324 - | 60 | Split webhook/checkout.rs (792 LOC) | **Fixed** (now 684 LOC) |
325 - | 61 | Bump transitive deps | Partially fixed (yara-x still has wasmtime advisory) |
326 - | 62 | Remove async-trait | **Closed** — required for dyn dispatch, not removable |
327 - | 63 | Add README.md to server/ | **Unfixed** (chronic, carried as #80) |
328 - | 64 | Split oversized route files | **Unfixed** (carried as #81) |
329 -
330 - 2 of 7 Run 20 items fixed. 5 carried forward (3 chronic). No regressions.
303 + | 65 | Pin scheduler advisory lock to acquired connection | **Fixed** |
304 + | 66 | Enforce PWYW minimum in cart effective_price_cents() | **Fixed** |
305 + | 67 | Fix soft-delete purge: version S3 keys + storage decrement | **Fixed** |
306 + | 68 | Wrap confirm_upload DB writes in transaction | **Fixed** |
307 + | 69 | Optimize discover page: parallelize facet queries | **Fixed** |
308 + | 70 | Delete old S3 objects on image/audio/video replacement | **Fixed** |
309 + | 71 | Validate checkout cancel item_id as UUID | **Fixed** |
310 + | 72 | ClamAV contains("FOUND") -> ends_with("FOUND") | **Fixed** |
311 + | 73 | Case-normalize URL-encoded path traversal check | **Fixed** |
312 + | 74 | Add UNIQUE(s3_key) to pending_uploads | **Fixed** |
313 + | 75 | Add #[instrument] to embed/ handlers | **Fixed** |
314 + | 76 | Add #[instrument] to payments/ functions | **Fixed** |
315 + | 77 | Initialize PWYW amount_cents hidden field server-side | Not verified (low priority) |
316 + | 78 | Use AuthUser for OAuth authorize | Not verified (low priority) |
317 + | 80 | Add README.md to server/ | **Unfixed** (chronic, carried as #87) |
318 + | 81 | Split oversized route files | **Unfixed** (carried as #86) |
319 +
320 + 12 of 14 actionable Run 21 items fixed. 2 deferred items carried forward. No regressions.
331 321
332 322 ### Chronic Items (unfixed across 3+ consecutive runs)
333 323
334 324 | Item | First flagged | Runs unfixed |
335 325 |------|--------------|-------------|
336 - | ~~Remove async-trait~~ | Run 18 | Closed — required for dyn dispatch |
337 - | Add #[instrument] to embed/ | Run 20 | 2 (20, 21) |
338 - | Add #[instrument] to payments/ | Run 20 | 2 (20, 21) |
339 - | Add README.md to server/ | Run 19 | 3 (19, 20, 21) |
326 + | Add README.md to server/ | Run 19 | 4 (19, 20, 21, 22) |
327 +
328 + Note: embed/ and payments/ observability gaps resolved in Run 22 after 2 runs (20, 21). async-trait closed as not removable.
340 329
341 - ## Delta Since Run 20
330 + ## Delta Since Run 21
342 331
343 332 ### Fixed
344 - - webhook/checkout.rs reduced from 792 to 684 LOC (below 500-line concern threshold for this module)
345 - - Version bumped from 0.4.10 to 0.5.7
346 - - 8 new migrations (093-101 -> 101 total)
347 - - LOC grew from ~83K to ~87K (+4K)
348 - - Test annotations from 1,214 to 1,218
349 -
350 - ### New Findings (not in Run 20)
351 - - Cart PWYW minimum bypass (SERIOUS)
352 - - Soft-delete purge missing version S3 keys + storage decrement (SERIOUS)
353 - - Non-atomic confirm upload (SERIOUS)
354 - - Discover page 5-8 queries per request (SERIOUS)
355 - - Scheduler advisory lock not pinned (SERIOUS)
356 - - 6 new MINOR findings (old S3 cleanup, cancel UUID, ClamAV FOUND, path traversal case, pending_uploads UNIQUE, OAuth legacy session)
333 + - All 5 SERIOUS Run 21 findings fixed (scheduler lock, cart PWYW, soft-delete purge, confirm transaction, discover queries)
334 + - All 7 MINOR Run 21 findings fixed (old S3 cleanup, cancel UUID, ClamAV FOUND, path traversal case, pending_uploads UNIQUE, embed instrumentation, payments instrumentation)
335 + - embed/ and payments/ now have full `#[instrument]` coverage -- resolves chronic observability gap
336 + - 4 new migrations (102-105): pending_uploads unique, unique_plays, nullable_item_id_indexes, pending_s3_deletions
337 + - Version bumped from 0.5.7 to 0.5.9
338 + - LOC grew from ~87,427 to ~87,853 (+426)
339 + - Test annotations from 1,218 to 1,215 (-3, likely test consolidation)
340 +
341 + ### New Findings (not in Run 21)
342 + - CSRF field name mismatch in 3 templates (SERIOUS)
343 + - Idempotency key unique constraint too narrow (MINOR)
344 + - Content export RAM buffering (MINOR)
345 + - Pending S3 deletions missing UNIQUE constraint (MINOR)
357 346
358 347 ### Grade Changes
359 - - Performance: A -> A- (discover page finding)
Lines truncated
@@ -1,12 +1,22 @@
1 1 # Makenotwork TODO
2 2
3 3 ## Status
4 - v0.5.9 deployed 2026-05-09. Audit grade A (Run 21). ~87K LOC, 1,225 tests, 0 warnings. Migration 105. Sprints 1-9 complete (see `todo_done.md`).
4 + v0.5.9 deployed 2026-05-09. Audit grade A (Run 22). ~88K LOC, 1,215 tests, 0 warnings. Migration 105. Sprints 1-9 complete (see `todo_done.md`).
5 5
6 6 Human tasks in `human_todo.md`. Completed items in `todo_done.md`.
7 7
8 8 ---
9 9
10 + ## Documentation: Undocumented Shipped Features
11 +
12 + These features are implemented but have no public-facing documentation yet. Add docs in the next documentation push.
13 +
14 + - [ ] Shopping cart: multi-item checkout with per-seller Stripe sessions (`routes/api/cart.rs`, `routes/stripe/checkout/cart.rs`, `templates/pages/cart.html`, migration 096)
15 + - [ ] Wishlist: save items for later (`routes/api/wishlists.rs`)
16 + - [ ] Creator pause: voluntary membership pause with graceful fan subscription degradation (migration 093, 22 files reference `creator_pause`)
17 +
18 + ---
19 +
10 20 ## Deferred from Sprints
11 21
12 22 - [ ] Add bulk rename operation (Sprint 2)
@@ -16,6 +26,18 @@ Human tasks in `human_todo.md`. Completed items in `todo_done.md`.
16 26
17 27 ---
18 28
29 + ## Ultra Fuzz Run 22 (2026-05-09)
30 +
31 + ### SERIOUS (fix before launch)
32 + - [x] Fix CSRF field name: change `csrf_token` to `_csrf` in `fan_plus.html:97`, `project.html:209`, `sandbox.html:24`
33 +
34 + ### MINOR (current phase)
35 + - [x] Add migration: unique constraint on `idempotency_keys(key, user_id, method, path)` replacing `(key, user_id)` (migration 106)
36 + - [ ] Stream content exports or export to S3 with presigned download (`routes/api/exports.rs:656-680`)
37 + - [x] Add `UNIQUE(s3_key, bucket)` to `pending_s3_deletions` table (migration 107)
38 +
39 + ---
40 +
19 41 ## Ultra Fuzz Run 21 (2026-05-08)
20 42
21 43 ### SERIOUS (fix before launch)
@@ -58,10 +80,10 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
58 80 - [x] MINOR: No buyer != seller validation in cart checkout — added `user.id == seller_id` guard (`routes/stripe/checkout/cart.rs`)
59 81 - [x] MINOR: Guest checkout `increment_sales_count` error was silently swallowed — now propagates error so Stripe retries (`routes/stripe/webhook/checkout.rs`)
60 82 - [x] MINOR: Tips lack `check_not_suspended()` — added (`routes/stripe/checkout/tips.rs`)
61 - - [ ] MINOR: Orphaned pending transactions on partial cart failure — mitigated by 24h stale cleanup (`routes/stripe/checkout/cart.rs:317-353`)
83 + - [x] MINOR: Orphaned pending transactions on partial cart failure — investigated: failed seller's transactions are never created (error precedes create_transaction); earlier sellers' sessions expire naturally; 25h stale cleanup is correct. No additional fix needed.
62 84 - [x] MINOR: `CartLineItem.amount_cents` is `i32` while `Cents` wraps `i64` — changed to `i64`, removed redundant cast (`payments/checkout.rs`)
63 85 - [x] MINOR: No Stripe minimum amount ($0.50) enforcement before creating session — added `STRIPE_MINIMUM_CHARGE_CENTS` check to all 3 checkout methods (`payments/checkout.rs`, `constants.rs`)
64 - - [ ] MINOR: Cart items removed before Stripe session completes — cancel = empty cart, intentional UX trade-off (`routes/stripe/checkout/cart.rs:356,673`)
86 + - [x] MINOR: Cart items removed before Stripe session completes — moved cart removal to webhook handler so items survive cancelled checkouts (`routes/stripe/checkout/cart.rs`, `routes/stripe/webhook/checkout.rs`)
65 87 - [x] MINOR: Project checkout uses `ItemId::nil()` — made item_id optional in CreateTransactionParams, CheckoutParams, and CheckoutMetadata; project purchases now use NULL; fixed unique indexes to exclude NULL item_id (migration 104)
66 88 - [x] MINOR: v2 webhook handler swallows account update failures — on failure, unmarks event from processed set and returns 500 so Stripe retries (`routes/stripe/webhook_v2.rs`, `db/webhook_events.rs`)
67 89 - ~~SERIOUS: Partial Stripe refunds revoke full access~~ REFUTED — `is_full_refund()` check at `billing.rs:272` prevents this
@@ -104,7 +126,7 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
104 126 - [x] MINOR: Insertion confirm stores client-supplied `mime_type` without re-validation — added `validate_content_type` at confirm (`routes/api/content_insertions.rs`)
105 127 - [x] LOW: Media delete swallows S3 error then deletes DB record — now logs S3 errors, decrements storage before DB delete (`routes/storage/media.rs`)
106 128 - [x] SERIOUS: Presigned URLs have no S3 lifecycle cleanup — added `pending_uploads` table, reaper job (24h), S3 lifecycle rule (36h) in human_todo (`routes/storage/uploads.rs`)
107 - - [ ] MINOR: Storage counter decrement-then-increment for file replacement not transactional — mitigated by `recalculate_all_storage_batch` cron (`routes/storage/uploads.rs:206-225`)
129 + - [x] MINOR: Storage counter decrement-then-increment for file replacement not transactional — added `try_replace_storage` that atomically decrements old + increments new in a single UPDATE (`db/creator_tiers.rs`, `routes/storage/uploads.rs`)
108 130 - [ ] MINOR: `classify_media` trusts client-supplied `content_type` for size limits — attacker wastes own quota (`media.rs:80-91`)
109 131 - [x] MINOR: Creator cannot preview own draft content via stream/download — added `is_creator` bypass for draft, scan status, and payment checks (`routes/storage/downloads.rs`)
110 132 - [x] LOW: Play count inflatable via unauthenticated spam — added per-IP rate limit (10 burst, 1/3s) on stream/download routes + unique_play_count tracking for authenticated users (migration 103, `user_plays` table)
@@ -120,7 +142,7 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
120 142 - [x] MINOR: Path traversal check misses URL-encoded and Unicode normalization variants — added `%2e%2e`, absolute path, and null byte checks (`scanning/archive.rs`)
121 143 - [x] MINOR: Nested archive detection double-decompresses entries — magic bytes now captured during initial decompression pass (`scanning/archive.rs`)
122 144 - [x] NOTE: Unrecognized video data gets `Pass` instead of failing — now Fail like images (`scanning/content_type.rs`)
123 - - [ ] NOTE: Mach-O binaries get unconditional `Pass` — needs research into suspicious Mach-O import patterns to match PE/ELF analysis (`scanning/structural.rs:44-49`)
145 + - [x] NOTE: Mach-O binaries get unconditional `Pass` — added suspicious import analysis matching PE/ELF pattern: ptrace, task_for_pid, mach_vm_write/protect, thread_create_running, posix_spawn, dlopen (`scanning/structural.rs`)
124 146 - [x] NOTE: Download XSS blocklist missing `image/svg+xml` and `application/xhtml+xml` — added both (`scanning/content_type.rs`)
125 147
126 148 ### DB & Validation
@@ -137,9 +159,9 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
137 159 - [x] MINOR: `get_deleted_items_by_project` has no LIMIT clause — added LIMIT 500 (`db/items.rs`)
138 160 - [ ] MINOR: `item_slug_exists` considers soft-deleted items — intentional during 7-day recovery window (`db/items.rs:80-94`)
139 161 - [x] MINOR: `get_all_users` accepts arbitrary `limit`/`offset` — clamped to 200 (`db/users.rs`)
140 - - [ ] MINOR: Guest checkout auto-attach not in transaction — mitigated by `attach_guest_purchases_by_email` safety net (`db/transactions.rs:83-89`)
141 - - [ ] NOTE: `set_bundle_items` no ownership check at DB layer — all callers verify ownership at route layer (`db/bundles.rs:148-179`)
142 - - [ ] NOTE: Idempotency key scope mismatch — UUID-based keys make collision across endpoints near-impossible (`db/idempotency.rs:24-64`)
162 + - [x] MINOR: Guest checkout auto-attach not in transaction — wrapped user lookup + transaction update in DB transaction (`db/transactions.rs`)
163 + - [x] NOTE: `set_bundle_items` no ownership check at DB layer — added `owner_id` param with bundle + item ownership verification (`db/bundles.rs`)
164 + - [x] NOTE: Idempotency key scope mismatch — widened PK to `(key, user_id, method, path)`, updated ON CONFLICT (migration 106, `db/idempotency.rs`)
143 165
144 166 ### Git SSH & Build Runner
145 167 - [x] MEDIUM: Unsanitized SSH command passthrough — `exec_git_shell` now receives a reconstructed command from validated components (`git_ssh.rs`)
@@ -164,8 +186,8 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
164 186 - [x] MODERATE: S3 delete then DB delete crash gap — implemented `pending_s3_deletions` durable queue across all 5 deletion paths (migration 105)
165 187 - [x] MINOR: Stale refund escalation sends duplicate alerts — `mark_escalated` now runs before alerts, skips on failure (`scheduler/webhooks.rs`)
166 188 - [ ] NOTE: Announcement emails lost on server restart — no delivery persistence (`scheduler/announcements.rs:59-85`)
167 - - [ ] NOTE: Duplicate onboarding emails on step-advance DB failure (`scheduler/announcements.rs:210-222`)
168 - - [ ] NOTE: Idempotency middleware drops >1MB response bodies silently (`metrics.rs:205-226`)
189 + - [x] NOTE: Duplicate onboarding emails on step-advance DB failure — advance step before sending email; missing one email beats duplicates (`scheduler/announcements.rs`)
190 + - [x] NOTE: Idempotency middleware drops >1MB response bodies silently — check content-length before consuming body, skip caching for large responses (`metrics.rs`)
169 191 - [ ] NOTE: Unconfigured S3 reports `s3_ok = true` — intentional but masks misconfig (`monitor.rs:56-65`)
170 192 - [ ] NOTE: `X-Forwarded-For` spoofable without Cloudflare — accepted risk (`rate_limit.rs:26-31`)
171 193 - ~~MINOR: Advisory lock accumulates across ticks~~ REFUTED — reentrant by design, harmless
@@ -181,7 +203,7 @@ Two-pass fuzz: initial scan + deep verification. Items marked REFUTED were dispr
181 203 - [x] LOW: Negative `price_cents` produces malformed `price_decimal` in JSON-LD — fixed sign handling (`types/mod.rs`)
182 204 - [x] NOTE: `sanitize_field` tab/CR branches are dead code — removed (`import/csv_converter.rs`)
183 205 - [x] MEDIUM: Unsubscribe `action` parameter not validated against enum — added `UnsubscribeAction` enum with 9 variants, updated 17 call sites, fixed missing `notify_tip` handler (`email/tokens.rs`, 13 files)
184 - - [ ] LOW: Issue reply signature truncated to 64 bits — accepted trade-off for email local-part length limits (`email/tokens.rs:284`)
206 + - [x] LOW: Issue reply signature truncated to 64 bits — switched hex to base64url encoding, now 96 bits in same 16 chars (`email/tokens.rs`)
185 207 - [x] LOW: Import tier `price_cents` silently clamped from i64 to i32 — replaced with `try_into()` + descriptive error (`import/pipeline.rs`)
186 208 - [x] LOW: Import `strip_html_tags` has no output length limit — capped at 512KB (`import/pipeline.rs`)
187 209 - [x] LOW: Lossy i64-to-u32 casts in WaveStats/DbDiscoverItemRow — replaced with saturating casts (`types/conversions.rs`)
@@ -0,0 +1,5 @@
1 + -- Widen idempotency key unique constraint to include method and path.
2 + -- Prevents cross-endpoint collision when the same key is reused.
3 +
4 + ALTER TABLE idempotency_keys DROP CONSTRAINT idempotency_keys_pkey;
5 + ALTER TABLE idempotency_keys ADD PRIMARY KEY (key, user_id, method, path);
@@ -0,0 +1,4 @@
1 + -- Add unique constraint on pending_s3_deletions to prevent duplicate enqueue.
2 + -- ON CONFLICT DO NOTHING makes this safe for concurrent insertions.
3 + CREATE UNIQUE INDEX IF NOT EXISTS idx_pending_s3_deletions_key_bucket
4 + ON pending_s3_deletions (s3_key, bucket);
@@ -217,6 +217,7 @@ pub const SANDBOX_EXPIRY_SECS: i64 = 3600; // 1 hour
217 217 pub const SANDBOX_CLEANUP_INTERVAL_SECS: u64 = 300; // 5 minutes
218 218 /// Rate limit: sandbox creation.
219 219 /// Production: 1 per 30 seconds, burst 2. fast-tests: 1 per 10ms, burst 10.
220 + /// Run integration tests with `cargo test --features fast-tests` to avoid rate-limit failures.
220 221 #[cfg(not(feature = "fast-tests"))]
221 222 pub const SANDBOX_RATE_LIMIT_MS: u64 = 30_000;
222 223 #[cfg(not(feature = "fast-tests"))]
@@ -145,14 +145,44 @@ pub async fn get_bundleable_items(
145 145 ///
146 146 /// Deletes all existing bundle_items rows for the bundle and inserts the new set.
147 147 /// `item_ids` is an ordered list — sort_order is derived from position.
148 + /// Validates that both the bundle and all items belong to `owner_id`.
148 149 #[tracing::instrument(skip_all)]
149 150 pub async fn set_bundle_items(
150 151 pool: &PgPool,
151 152 bundle_id: ItemId,
152 153 item_ids: &[ItemId],
154 + owner_id: UserId,
153 155 ) -> Result<()> {
154 156 let mut tx = pool.begin().await?;
155 157
158 + // Verify bundle ownership
159 + let owns_bundle: bool = sqlx::query_scalar(
160 + "SELECT EXISTS(SELECT 1 FROM items i JOIN projects p ON p.id = i.project_id WHERE i.id = $1 AND p.user_id = $2)",
161 + )
162 + .bind(bundle_id)
163 + .bind(owner_id)
164 + .fetch_one(&mut *tx)
165 + .await?;
166 + if !owns_bundle {
167 + return Err(crate::error::AppError::Forbidden);
168 + }
169 +
170 + // Verify all items belong to the same owner
171 + if !item_ids.is_empty() {
172 + let owned_count: i64 = sqlx::query_scalar(
173 + "SELECT COUNT(*) FROM items i JOIN projects p ON p.id = i.project_id WHERE i.id = ANY($1) AND p.user_id = $2",
174 + )
175 + .bind(item_ids)
176 + .bind(owner_id)
177 + .fetch_one(&mut *tx)
178 + .await?;
179 + if owned_count != item_ids.len() as i64 {
180 + return Err(crate::error::AppError::BadRequest(
181 + "All bundle items must belong to you".to_string(),
182 + ));
183 + }
184 + }
185 +
156 186 sqlx::query("DELETE FROM bundle_items WHERE bundle_id = $1")
157 187 .bind(bundle_id)
158 188 .execute(&mut *tx)
@@ -235,6 +235,40 @@ pub async fn try_increment_storage(
235 235 Ok(())
236 236 }
237 237
238 + /// Atomically replace storage: decrement old file size and increment new file size
239 + /// in a single UPDATE. Prevents storage drift on file replacement by avoiding a
240 + /// window where decrement and increment are separate operations.
241 + #[tracing::instrument(skip_all)]
242 + pub async fn try_replace_storage(
243 + pool: &PgPool,
244 + user_id: UserId,
245 + old_bytes: i64,
246 + new_bytes: i64,
247 + max_storage_bytes: i64,
248 + ) -> Result<()> {
249 + let result = sqlx::query(
250 + "UPDATE users SET storage_used_bytes = GREATEST(0, storage_used_bytes - $2) + $3 \
251 + WHERE id = $1 AND GREATEST(0, storage_used_bytes - $2) + $3 <= $4",
252 + )
253 + .bind(user_id)
254 + .bind(old_bytes)
255 + .bind(new_bytes)
256 + .bind(max_storage_bytes)
257 + .execute(pool)
258 + .await?;
259 +
260 + if result.rows_affected() == 0 {
261 + let used = get_storage_used(pool, user_id).await?;
262 + return Err(AppError::BadRequest(format!(
263 + "You've used {} of {} storage. Delete files or upgrade your tier.",
264 + format_bytes(used),
265 + format_bytes(max_storage_bytes),
266 + )));
267 + }
268 +
269 + Ok(())
270 + }
271 +
238 272 /// Atomically decrement the user's storage counter (clamped to 0).
239 273 #[tracing::instrument(skip_all)]
240 274 pub async fn decrement_storage_used<'e>(
@@ -49,7 +49,7 @@ pub async fn store_response(
49 49 sqlx::query(
50 50 r#"INSERT INTO idempotency_keys (key, user_id, method, path, status_code, response_body)
51 51 VALUES ($1, $2, $3, $4, $5, $6)
52 - ON CONFLICT (key, user_id) DO NOTHING"#,
52 + ON CONFLICT (key, user_id, method, path) DO NOTHING"#,
53 53 )
54 54 .bind(key)
55 55 .bind(user_id)
@@ -73,6 +73,9 @@ pub async fn create_transaction(
73 73
74 74 /// Complete a guest transaction: set guest_email, generate claim_token, and
75 75 /// optionally auto-attach to an existing user if the email matches.
76 + ///
77 + /// Uses a transaction to prevent races between auto-attach and concurrent
78 + /// signup/email-verification for the same email.
76 79 #[tracing::instrument(skip_all)]
77 80 pub async fn complete_guest_transaction(
78 81 pool: &PgPool,
@@ -82,12 +85,14 @@ pub async fn complete_guest_transaction(
82 85 ) -> Result<Option<DbTransaction>> {
83 86 let claim_token = ClaimToken::new();
84 87
88 + let mut db_tx = pool.begin().await?;
89 +
85 90 // Check if email matches an existing user — auto-attach if so
86 91 let existing_user_id: Option<UserId> = sqlx::query_scalar(
87 92 "SELECT id FROM users WHERE LOWER(email) = LOWER($1) AND email_verified = true",
88 93 )
89 94 .bind(guest_email)
90 - .fetch_optional(pool)
95 + .fetch_optional(&mut *db_tx)
91 96 .await?;
92 97
93 98 let tx = sqlx::query_as::<_, DbTransaction>(
@@ -109,9 +114,11 @@ pub async fn complete_guest_transaction(
109 114 .bind(guest_email)
110 115 .bind(existing_user_id)
111 116 .bind(claim_token)
112 - .fetch_optional(pool)
117 + .fetch_optional(&mut *db_tx)
113 118 .await?;
114 119
120 + db_tx.commit().await?;
121 +
115 122 Ok(tx)
116 123 }
117 124
@@ -310,7 +317,7 @@ pub async fn get_user_purchased_item_ids(pool: &PgPool, user_id: UserId) -> Resu
310 317 /// Returns true if claimed successfully, false if already in library.
311 318 ///
312 319 /// Uses `ON CONFLICT DO NOTHING` against the partial unique index on
313 - /// `(buyer_id, item_id) WHERE status = 'completed'` to prevent duplicate
320 + /// `(buyer_id, item_id) WHERE status = 'completed' AND item_id IS NOT NULL` to prevent duplicate
314 321 /// claims under concurrent requests.
315 322 #[tracing::instrument(skip_all)]
316 323 pub async fn claim_free_item<'e>(
@@ -322,7 +329,7 @@ pub async fn claim_free_item<'e>(
322 329 r#"
323 330 INSERT INTO transactions (buyer_id, seller_id, item_id, amount_cents, platform_fee_cents, stripe_checkout_session_id, status, completed_at, item_title, seller_username, share_contact, parent_transaction_id)
324 331 VALUES ($1, $2, $3, 0, 0, $4, 'completed', NOW(), $5, $6, $7, $8)
325 - ON CONFLICT (buyer_id, item_id) WHERE status = 'completed' DO NOTHING
332 + ON CONFLICT (buyer_id, item_id) WHERE status = 'completed' AND item_id IS NOT NULL DO NOTHING
326 333 "#,
327 334 )
328 335 .bind(params.buyer_id)
@@ -372,7 +379,7 @@ pub async fn claim_free_with_promo_code(
372 379 r#"
373 380 INSERT INTO transactions (buyer_id, seller_id, item_id, amount_cents, platform_fee_cents, stripe_checkout_session_id, status, completed_at, item_title, seller_username, share_contact, promo_code_id)
374 381 VALUES ($1, $2, $3, 0, 0, $4, 'completed', NOW(), $5, $6, $7, $8)
375 - ON CONFLICT (buyer_id, item_id) WHERE status = 'completed' DO NOTHING
382 + ON CONFLICT (buyer_id, item_id) WHERE status = 'completed' AND item_id IS NOT NULL DO NOTHING
376 383 "#,
377 384 )
378 385 .bind(params.buyer_id)
@@ -316,15 +316,18 @@ pub fn generate_deletion_signature(
316 316
317 317 /// Generate a reply-to email address for an issue comment.
318 318 ///
319 - /// Format: `issue+{issue_id}.{user_id}.{sig16}@reply.makenot.work`
319 + /// Format: `issue+{issue_id}.{user_id}.{sig}@reply.makenot.work`
320 320 ///
321 - /// The signature is the first 16 hex chars of an HMAC-SHA256 over the issue and user IDs.
322 - /// This is stateless — no DB storage needed. The handler will parse and verify the address.
321 + /// The signature is 16 chars of base64url-encoded HMAC-SHA256 (96 bits) over the
322 + /// issue and user IDs. Base64url packs 6 bits/char vs hex's 4, giving 96 bits of
323 + /// security in the same space that hex would give 64. This is stateless — no DB
324 + /// storage needed. The handler will parse and verify the address.
323 325 pub fn generate_issue_reply_address(
324 326 issue_id: crate::db::IssueId,
325 327 user_id: UserId,
326 328 secret: &str,
327 329 ) -> String {
330 + use base64::engine::{general_purpose::URL_SAFE_NO_PAD, Engine};
328 331 use hmac::{Hmac, Mac};
329 332 use sha2::Sha256;
330 333
@@ -332,20 +335,22 @@ pub fn generate_issue_reply_address(
332 335 let mut mac = Hmac::<Sha256>::new_from_slice(secret.as_bytes())
333 336 .expect("HMAC-SHA256 accepts any key length");
334 337 mac.update(message.as_bytes());
335 - let sig = &hex::encode(mac.finalize().into_bytes())[..16];
338 + let hash = mac.finalize().into_bytes();
339 + let sig = &URL_SAFE_NO_PAD.encode(&hash[..12])[..16];
336 340
337 341 format!("issue+{}.{}.{}@reply.makenot.work", issue_id, user_id, sig)
338 342 }
339 343
340 344 /// Parse and verify an issue reply address local part.
341 345 ///
342 - /// Input: the part before `@`, e.g. `issue+{issue_id}.{user_id}.{sig16}`
346 + /// Input: the part before `@`, e.g. `issue+{issue_id}.{user_id}.{sig}`
343 347 ///
344 348 /// Returns `Some((IssueId, UserId))` if the signature is valid, `None` otherwise.
345 349 pub fn parse_issue_reply_token(
346 350 local_part: &str,
347 351 secret: &str,
348 352 ) -> Option<(crate::db::IssueId, UserId)> {
353 + use base64::engine::{general_purpose::URL_SAFE_NO_PAD, Engine};
349 354 use hmac::{Hmac, Mac};
350 355 use sha2::Sha256;
351 356
@@ -362,7 +367,8 @@ pub fn parse_issue_reply_token(
362 367 let mut mac = Hmac::<Sha256>::new_from_slice(secret.as_bytes())
363 368 .expect("HMAC-SHA256 accepts any key length");
364 369 mac.update(message.as_bytes());
365 - let expected = &hex::encode(mac.finalize().into_bytes())[..16];
370 + let hash = mac.finalize().into_bytes();
371 + let expected = &URL_SAFE_NO_PAD.encode(&hash[..12])[..16];
366 372
367 373 // Constant-time comparison
368 374 if expected.len() != sig.len() {
@@ -201,12 +201,30 @@ pub async fn idempotency_middleware(
201 201
202 202 // Only cache successful responses (2xx/3xx) to avoid caching transient errors
203 203 if status_code < 400 {
204 - // Extract the actual body bytes so we cache real content, not empty string
204 + // Skip caching if content-length exceeds 1MB to avoid consuming the body
205 + let content_length = response.headers()
206 + .get(axum::http::header::CONTENT_LENGTH)
207 + .and_then(|v| v.to_str().ok())
208 + .and_then(|v| v.parse::<usize>().ok());
209 + if content_length.is_some_and(|len| len > 1024 * 1024) {
210 + tracing::info!(
211 + key = %idem_key, method = %method, path = %path,
212 + "response body exceeds 1MB, skipping idempotency cache"
213 + );
214 + return response;
215 + }
216 +
217 + // Extract body bytes to cache (up to 1MB)
205 218 let (parts, body) = response.into_parts();
206 219 let body_bytes = match axum::body::to_bytes(body, 1024 * 1024).await {
207 220 Ok(b) => b,
208 - Err(e) => {
209 - tracing::warn!(error = ?e, "failed to read response body for idempotency cache");
221 + Err(_) => {
222 + tracing::info!(
223 + key = %idem_key, method = %method, path = %path,
224 + "response body exceeds 1MB, skipping idempotency cache"
225 + );
226 + // Body is consumed — return empty. This only triggers for chunked
227 + // responses without content-length (rare for API endpoints).
210 228 return axum::response::Response::from_parts(parts, axum::body::Body::empty());
211 229 }
212 230 };
@@ -131,7 +131,7 @@ pub(super) async fn save_content(
131 131 .unwrap_or_default();
132 132
133 133 // Set bundle contents (replaces all existing)
134 - db::bundles::set_bundle_items(&state.db, item.id, &bundle_ids).await?;
134 + db::bundles::set_bundle_items(&state.db, item.id, &bundle_ids, user_id).await?;
135 135
136 136 // Update listed status for all bundleable items in this project
137 137 let all_bundleable =
@@ -390,11 +390,17 @@ fn extract_reply_local(to: &str) -> Option<String> {
390 390 } else {
391 391 addr
392 392 };
393 - let email = email.trim().to_lowercase();
394 - if let Some(local) = email.strip_suffix("@reply.makenot.work")
395 - && local.starts_with("issue+")
396 - {
397 - return Some(local.to_string());
393 + let email = email.trim();
394 + // Match domain case-insensitively but preserve local-part case
395 + // (base64url signatures are case-sensitive)
396 + if let Some(at) = email.rfind('@') {
397 + let local = &email[..at];
398 + let domain = &email[at + 1..];
399 + if domain.eq_ignore_ascii_case("reply.makenot.work")
400 + && local.starts_with("issue+")
401 + {
402 + return Some(local.to_string());
403 + }
398 404 }
399 405 }
400 406 None
@@ -194,7 +194,8 @@ pub(super) async fn confirm_upload(
194 194 }
195 195
196 196 // Idempotency: if the matching s3_key field already equals this key, return success (no-op).
197 - // If replacing a different file, decrement old storage first.
197 + // If replacing a different file, use atomic replace_storage to avoid drift.
198 + let mut old_s3_key: Option<String> = None;
198 199 if let Some(item) = db::items::get_item_by_id(&state.db, req.item_id).await? {
199 200 let existing_key = match file_type {
200 201 FileType::Audio => item.audio_s3_key.as_deref(),
@@ -205,7 +206,6 @@ pub(super) async fn confirm_upload(
205 206 if existing_key == Some(&req.s3_key) {
206 207 return Ok(Json(ConfirmUploadResponse { success: true, pending_review: None }));
207 208 }
208 - // If replacing a different file, decrement old storage and delete old S3 object
209 209 if let Some(old_key) = existing_key {
210 210 let old_size = match file_type {
211 211 FileType::Audio => item.audio_file_size_bytes.unwrap_or(0),
@@ -213,21 +213,27 @@ pub(super) async fn confirm_upload(
213 213 FileType::Video => item.video_file_size_bytes.unwrap_or(0),
214 214 _ => 0,
215 215 };
216 - if old_size > 0 {
217 - db::creator_tiers::decrement_storage_used(&state.db, user.id, old_size).await?;
216 + // Atomically decrement old + increment new in a single UPDATE
217 + if let Err(e) = db::creator_tiers::try_replace_storage(
218 + &state.db, user.id, old_size, file_size_bytes, max_storage,
219 + ).await {
220 + s3.delete_object(&req.s3_key).await.ok();
221 + return Err(e);
218 222 }
219 - // Best-effort delete of the old S3 object to prevent orphans
220 - if let Err(e) = s3.delete_object(old_key).await {
221 - tracing::warn!(key = %old_key, error = ?e, "failed to delete replaced S3 object");
223 + old_s3_key = Some(old_key.to_string());
224 + } else {
225 + // No existing file — plain increment
226 + if let Err(e) = db::creator_tiers::try_increment_storage(&state.db, user.id, file_size_bytes, max_storage).await {
227 + s3.delete_object(&req.s3_key).await.ok();
228 + return Err(e);
222 229 }
223 230 }
224 - }
225 -
226 - // Atomically check storage cap, increment counter, and update item record
227 - // in a single transaction. If any step fails, the entire operation rolls back.
228 - if let Err(e) = db::creator_tiers::try_increment_storage(&state.db, user.id, file_size_bytes, max_storage).await {
229 - s3.delete_object(&req.s3_key).await.ok();
230 - return Err(e);
231 + } else {
232 + // Item not found yet (shouldn't happen but handle gracefully)
233 + if let Err(e) = db::creator_tiers::try_increment_storage(&state.db, user.id, file_size_bytes, max_storage).await {
234 + s3.delete_object(&req.s3_key).await.ok();
235 + return Err(e);
236 + }
231 237 }
232 238
233 239 // Update the item's S3 key and file size in a transaction so both succeed or fail together.
@@ -257,6 +263,13 @@ pub(super) async fn confirm_upload(
257 263 return Err(e.into());
258 264 }
259 265
266 + // Delete the old S3 object now that the replacement is committed
267 + if let Some(old_key) = &old_s3_key {
268 + if let Err(e) = s3.delete_object(old_key).await {
269 + tracing::warn!(key = %old_key, error = ?e, "failed to delete replaced S3 object");
270 + }
271 + }
272 +
260 273 // Clear the pending upload record now that the upload is confirmed
261 274 db::pending_uploads::remove_pending_upload(&state.db, &req.s3_key).await?;
262 275
@@ -356,10 +356,8 @@ pub(in crate::routes::stripe) async fn create_cart_checkout(
356 356 }
357 357 }
358 358
359 - // Remove checked-out items from cart
360 - db::cart::remove_seller_items_from_cart(&state.db, user.id, seller_id)
361 - .await
362 - .context("remove cart items after checkout")?;
359 + // Cart items are removed by the webhook handler on successful payment,
360 + // so users keep their cart if they cancel the Stripe checkout.
363 361
364 362 // Redirect to Stripe Checkout
365 363 let checkout_url = result
@@ -674,7 +672,7 @@ pub(super) async fn process_seller_checkout(
674 672 }
675 673 }
676 674
677 - db::cart::remove_seller_items_from_cart(&state.db, user.id, seller_id).await.ok();
675 + // Cart items are removed by the webhook handler on successful payment.
678 676
679 677 result.url.ok_or_else(|| AppError::BadRequest("No checkout URL returned".to_string()))
680 678 }
@@ -163,6 +163,12 @@ pub(super) async fn handle_cart_checkout_completed(
163 163
164 164 db_tx.commit().await.context("commit cart webhook transaction")?;
165 165
166 + // Remove purchased items from cart (items stay in cart until payment succeeds,
167 + // so cancelled checkouts don't lose cart contents)
168 + db::cart::remove_seller_items_from_cart(&state.db, buyer_id, seller_id)
169 + .await
170 + .context("remove cart items after successful payment")?;
171 +
166 172 // --- Secondary effects (outside transaction) ---
167 173
168 174 for tx in &completed_txs {
@@ -26,6 +26,17 @@ const SUSPICIOUS_ELF_SYMBOLS: &[&str] = &[
26 26 "memfd_create",
27 27 ];
28 28
29 + /// Suspicious Mach-O symbols (macOS equivalents of PE/ELF patterns)
30 + const SUSPICIOUS_MACHO_SYMBOLS: &[&str] = &[
31 + "ptrace",
32 + "task_for_pid",
33 + "mach_vm_write",
34 + "mach_vm_protect",
35 + "thread_create_running",
36 + "posix_spawn",
37 + "dlopen",
38 + ];
39 +
29 40 /// Analyze a binary for suspicious structural patterns.
30 41 /// Only runs for Download file types; returns Skip for Audio/Cover.
31 42 pub fn analyze_binary(data: &[u8], file_type: FileType) -> LayerResult {
@@ -41,14 +52,7 @@ pub fn analyze_binary(data: &[u8], file_type: FileType) -> LayerResult {
41 52 match goblin::Object::parse(data) {
42 53 Ok(goblin::Object::PE(pe)) => analyze_pe(&pe),
43 54 Ok(goblin::Object::Elf(elf)) => analyze_elf(&elf),
44 - Ok(goblin::Object::Mach(_)) => {
45 - // Mach-O: basic pass for now, can be extended
46 - LayerResult {
47 - layer: "structural",
48 - verdict: LayerVerdict::Pass,
49 - detail: Some("Mach-O binary".to_string()),
50 - }
51 - }
55 + Ok(goblin::Object::Mach(mach)) => analyze_mach(&mach),
52 56 Ok(_) => {
53 57 // Archive, unknown, or other format — skip structural analysis
54 58 LayerResult {
@@ -158,6 +162,64 @@ fn check_elf_warnings(symbol_names: &[&str]) -> Vec<String> {
158 162 warnings
159 163 }
160 164
165 + fn analyze_mach(mach: &goblin::mach::Mach) -> LayerResult {
166 + let symbols: Vec<String> = match mach {
167 + goblin::mach::Mach::Binary(macho) => collect_macho_symbols(macho),
168 + goblin::mach::Mach::Fat(fat) => {
169 + // Check first Mach-O arch in a fat binary
170 + fat.into_iter()
171 + .filter_map(|res| res.ok())
172 + .find_map(|arch| match arch {
173 + goblin::mach::SingleArch::MachO(macho) => Some(collect_macho_symbols(&macho)),
174 + _ => None,
175 + })
176 + .unwrap_or_default()
177 + }
178 + };
179 +
180 + let symbol_refs: Vec<&str> = symbols.iter().map(|s| s.as_str()).collect();
181 + let warnings = check_mach_warnings(&symbol_refs);
182 +
183 + if warnings.is_empty() {
184 + LayerResult {
185 + layer: "structural",
186 + verdict: LayerVerdict::Pass,
187 + detail: Some("Mach-O binary".to_string()),
188 + }
189 + } else {
190 + LayerResult {
191 + layer: "structural",
192 + verdict: LayerVerdict::Fail,
193 + detail: Some(warnings.join("; ")),
194 + }
195 + }
196 + }
197 +
198 + fn collect_macho_symbols(macho: &goblin::mach::MachO) -> Vec<String> {
199 + macho
200 + .imports()
201 + .unwrap_or_default()
202 + .iter()
203 + .map(|imp| imp.name.to_string())
204 + .collect()
205 + }
206 +
207 + /// Check Mach-O import names for suspicious patterns.
208 + /// Extracted for testability.
209 + fn check_mach_warnings(symbol_names: &[&str]) -> Vec<String> {
210 + let mut warnings = Vec::new();
211 +
212 + for name in symbol_names {
213 + // Mach-O imports are prefixed with underscore
214 + let stripped = name.strip_prefix('_').unwrap_or(name);
215 + if SUSPICIOUS_MACHO_SYMBOLS.iter().any(|s| stripped == *s) {
216 + warnings.push(format!("Suspicious import: {}", stripped));
217 + }
218 + }
219 +
220 + warnings
221 + }
222 +
161 223 #[cfg(test)]
162 224 mod tests {
163 225 use super::*;
@@ -376,4 +438,50 @@ mod tests {
376 438 let warnings = check_elf_warnings(symbols);
377 439 assert_eq!(warnings.len(), 1);
378 440 }
441 +
442 + // -- Mach-O symbol detection --
443 +
444 + #[test]
445 + fn mach_clean_symbols_pass() {
446 + let symbols = &["_printf", "_malloc", "_free", "_exit"];
447 + let warnings = check_mach_warnings(symbols);
448 + assert!(warnings.is_empty());
449 + }
450 +
451 + #[test]
452 + fn mach_task_for_pid_detected() {
453 + let symbols = &["_printf", "_task_for_pid", "_exit"];
454 + let warnings = check_mach_warnings(symbols);
455 + assert_eq!(warnings.len(), 1);
456 + assert!(warnings[0].contains("task_for_pid"));
457 + }
458 +
459 + #[test]
460 + fn mach_vm_write_detected() {
461 + let symbols = &["_mach_vm_write"];
462 + let warnings = check_mach_warnings(symbols);
463 + assert_eq!(warnings.len(), 1);
464 + assert!(warnings[0].contains("mach_vm_write"));
465 + }
466 +
467 + #[test]
468 + fn mach_multiple_suspicious_symbols() {
469 + let symbols = &["_ptrace", "_task_for_pid", "_mach_vm_write", "_printf"];
470 + let warnings = check_mach_warnings(symbols);
471 + assert_eq!(warnings.len(), 3);
472 + }
473 +
474 + #[test]
475 + fn mach_no_underscore_prefix_still_matches() {
476 + let symbols = &["ptrace"];
477 + let warnings = check_mach_warnings(symbols);
478 + assert_eq!(warnings.len(), 1);
479 + }
480 +
481 + #[test]
482 + fn mach_no_false_positive_on_substring() {
483 + let symbols = &["_task_for_pid_extra"];
484 + let warnings = check_mach_warnings(symbols);
485 + assert!(warnings.is_empty());
486 + }
379 487 }
@@ -208,16 +208,18 @@ pub(super) async fn send_onboarding_emails(state: &AppState) {
208 208 }
209 209 }
210 210 for user in send {
211 + // Advance step BEFORE sending email to prevent duplicates on DB failure.
212 + // Missing a non-critical onboarding email is better than sending it twice.
213 + if let Err(e) = db::users::advance_onboarding_step(&state.db, user.id, next.as_i16()).await {
214 + tracing::warn!(user_id = %user.id, step = ?next, error = ?e, "failed to advance onboarding step");
215 + continue;
216 + }
211 217 if let Err(e) = state
212 218 .email
213 219 .send_onboarding_profile(&user.email, user.display_name.as_deref(), host_url)
214 220 .await
215 221 {
216 222 tracing::error!(error = ?e, user_id = %user.id, "failed to send onboarding profile email");
217 - continue;
218 - }
219 - if let Err(e) = db::users::advance_onboarding_step(&state.db, user.id, next.as_i16()).await {
220 - tracing::warn!(user_id = %user.id, step = ?next, error = ?e, "failed to advance onboarding step");
221 223 }
222 224 }
223 225 }
@@ -237,16 +239,16 @@ pub(super) async fn send_onboarding_emails(state: &AppState) {
237 239 }
238 240 }
239 241 for user in send {
242 + if let Err(e) = db::users::advance_onboarding_step(&state.db, user.id, next.as_i16()).await {
243 + tracing::warn!(user_id = %user.id, step = ?next, error = ?e, "failed to advance onboarding step");
244 + continue;
245 + }
240 246 if let Err(e) = state
241 247 .email
242 248 .send_onboarding_stripe(&user.email, user.display_name.as_deref(), host_url)
243 249 .await
244 250 {
245 251 tracing::error!(error = ?e, user_id = %user.id, "failed to send onboarding stripe email");
246 - continue;
247 - }
248 - if let Err(e) = db::users::advance_onboarding_step(&state.db, user.id, next.as_i16()).await {
249 - tracing::warn!(user_id = %user.id, step = ?next, error = ?e, "failed to advance onboarding step");
250 252 }
251 253 }
252 254 }