🚀 Benmore Framework — Production-Readiness Push is live (11 PRs in review)

[ArkashJ]

🚀 Benmore Framework — Production-Readiness Push is live

We're hardening the Benmore framework for production, and the work is happening in the open. 11 pull requests are in review right now, spanning durable jobs, the database backend, the SDK surface, static-asset security, runtime contracts, and the dev experience.

We'd love eyes, feedback, and contributors. Here's the full slate:

🛡️ Reliability & durable jobs

• Harden durable job transactions #6 — Harden durable job transactions — tighten transaction boundaries around job state changes.
• Add durable job lease recovery #9 — Add durable job lease recovery — recover orphaned jobs whose worker leases expired.
• [codex] Add durable job unique keys #10 — Add durable job unique keys — dedupe job enqueues with unique keys.
• [codex] Run cron through durable jobs #11 — Run cron through durable jobs — route scheduled work through the durable-job engine.

🗄️ Data layer & SDK

• Add database backend frontier #5 — Add database backend frontier — pluggable database backend (pgx).
• Add SDK store and query primitives #4 — Add SDK store and query primitives — first-class store/query building blocks for app code.
• Add runtime OpenAPI contract surface #2 — Add runtime OpenAPI contract surface — runtime-introspectable API contracts.
• Add generated type contract fixture #7 — Add generated type contract fixture — golden fixture locking down generated types.

🔒 Security & DX

• [codex] Harden static asset serving #12 — Harden static asset serving — close path-traversal and static-serving gaps.
• Add dev browser hot reload #3 — Add dev browser hot reload — instant browser reload in dev.
• Document production readiness contribution loop #8 — Document production readiness contribution loop — how to contribute to this push.

How to get involved

• 👀 Review a PR — every PR has a thorough automated review attached; add your take.
• 💡 Share ideas — drop suggestions in the Ideas category.
• ❓ Ask questions — Q&A is open.

A master tracking issue links all of these together — follow it to watch the whole push land.

Let's make Benmore the most pleasant way to ship production apps. 🙌

[alextitov19]

Benmore Framework — Security & Correctness Deep Dive

Date: 2026-06-19
Version reviewed: Benmore framework v2.7.177 (c3c01cb)
Scope: Full subsystem audit (auth, RBAC/multi-tenancy, data layer/SQL, uploads/SSRF, encryption, flows/compute/webhooks, concurrency/jobs/realtime, server/config/prod) cross-referenced against the 11 open production-hardening PRs (#2–#12).
Method: 7 parallel deep reviews + direct source spot-verification of the top findings (marked ✓ verified).

---

⚠️ Correction / re-verification (2026-06-19, post-publish)

This audit ran against an earlier revision of the PRs. The 11 PRs were all updated later the same day, so I re-verified every finding against the current diffs. Net changes:

Now resolved by the updated PRs (strike these):

• H-2 ("fixed 5-min lease has no heartbeat") — RESOLVED by PR Add durable job lease recovery #9. It now heartbeats the lease every window/3 while the worker is alive, scoped to a lease_owner token, with all terminal writes guarded by that token. A slow-but-alive job is never reclaimed.
• H-4 ("parallel: step shares one *sql.Tx") — RESOLVED by PR Harden durable job transactions #6. It adds FlowContext.TxMu and serializes every *sql.Tx access in execStepSQL / execStepEnqueue on the transactional path. (My original audit flagged this as "not covered" — it was fixed in a later push.)
• Cron same-minute re-fire after completion (a LOW item) — RESOLVED by PR [codex] Run cron through durable jobs #11 (the unique_key dedup now also matches completed rows).

Narrowed / reframed:

• C-1 — downgraded from Critical to High and reframed. PR Add durable job lease recovery #9's heartbeat means double-execution no longer happens for slow jobs, only for a genuine worker crash in the gap between an external side-effect (email/webhook sent) and the status='completed' write — the fundamental at-least-once limit. PR [codex] Run cron through durable jobs #11 also defaults cron jobs to max_attempts=1 (at-most-once), reducing cron double-fire. Remaining fix: exactly-once for DB-only work (fold the completion write into the flow transaction, extending PR Harden durable job transactions #6) + deterministic job:<id>:<step> idempotency keys threaded into the email/webhook senders.
• H-5 (nested-private upload) — narrowed by PR [codex] Harden static asset serving #12 (now catches a symlink resolving into top-level private/), but a literal nested uploads/<x>/private/ dir still disagrees with isPrivateUploadPath. Partial.

Re-confirmed STILL OPEN (no current PR touches these files): H-1, H-3, H-6, H-7, H-8, H-9 (verified — 3 PRs edit server.go, none add timeouts/body-limit), H-10–H-17 (H-14's pubsub.go eventPoller leak is distinct from the cron-poller lifecycle PR #11 did fix), and all Mediums (M-1–M-7). These live in auth.go, crud.go, uploads_files.go, image_transform.go, fetch_async.go, pubsub.go, scheduling.go, websocket.go, ratelimit.go, encryption.go, webhooks.go, production.go — the subsystems slated for the separate security-hardening PR.

The rest of the report stands as written.

---

TL;DR

The framework is unusually well-built for its surface area — SQL is parameterized with allowlisted identifiers throughout, multi-tenant scoping is enforced at the SQL layer everywhere checked, crypto uses crypto/rand + AES-GCM + constant-time compares, and the JS sandbox (goja) is genuinely locked down. The 11 open PRs are real, focused production-hardening and should be merged — but they touch a narrow slice (durable jobs, cron, static-file traversal, db-backend, OpenAPI, hot-reload, SDK). The highest-severity issues are in subsystems no PR touches: webhooks, flows, uploads, auth, and the realtime/rate-limit layer.

---

Part 1 — What the 11 PRs fix (merge these)

PR	What it fixes	Verdict
#6 transactional job enqueue	Flow rolled back but job still queued; worker flows ignored transaction:true; also fixes H-4 (parallel: + *sql.Tx race via FlowContext.TxMu)	✅ Correct fix. Merge.
#9 job lease recovery	Jobs stuck in running forever after a worker crash	✅ Good — lease + heartbeat + lease_owner-guarded terminal writes. Residual at-least-once on genuine crash only (see C-1)
#10 job unique keys	Duplicate work on double-click/retry/webhook-replay	✅ De-dupes enqueue. Relies on fragile error-string matching
#11 cron via durable jobs	Cron fire lost on crash (fire-and-forget goroutine)	✅ Real improvement; flips semantics to at-least-once
#12 static asset hardening	Path traversal / symlink escape on /static, /uploads, TSX, clean-URLs	✅ Solid EvalSymlinks+filepath.Rel resolver. Does not cover the nested-private/ bypass (H-5)
#2 /api/_openapi	—	✅ Correctly auth-gated; filters _benmore_* tables & sensitive columns
#3 dev hot-reload	—	✅ Prod-gated, but injection/SSE-auth predicates are inconsistent for features.testing apps (cosmetic)
#4/#7/#8 SDK store/query, type fixtures, docs	—	✅ Additive, low-risk

Gaps within the PR set: even after #6/#9/#10/#11 land, job/hook execution is still at-least-once with no idempotency, so lease recovery (#9) can double-send emails/webhooks (C-1). And #6's tx-aware enqueue needs to be extended to the data-event and scheduler paths.

---

Part 2 — Critical / High issues NO PR addresses

🟠 HIGH (ranked)

C-1 — External side effects aren't idempotent → double-send on a genuine worker crash · jobs.go:148-170, 286-297 (downgraded from Critical — see correction banner)
The job claim is atomic and PR #9's heartbeat keeps a live job's lease fresh, so slow jobs are no longer falsely reclaimed. The residual: the job body runs outside any transaction that includes the completion write, so if the process dies between an external side effect (email/webhook sent) and status='completed', recovery requeues and a second worker re-sends. This is the fundamental at-least-once limit — unclosable at the queue layer.
→ DB-only work: fold the completion write into the flow transaction (extends PR #6) = true exactly-once. External work: thread a deterministic job:<id>:<step> idempotency key into the email/webhook senders (provider Idempotency-Key header / outbound webhook event-ID) so replays are no-ops.

H-1 — Webhook signature verifier fails OPEN on empty secret ✓ verified · flows.go:3109-3111

secret := InterpolateEnv(flow.Secret, appDir)
if secret == "" { return true }   // accepts ANY payload as authentic

Found independently by two reviewers. Any verify: stripe|github|slack|... flow whose secret env var is unset/typo'd/mid-rotation accepts forged webhooks → drives whatever the flow does (mark invoice paid, trigger privileged flow). The newer verify_recipe.go:92 path correctly returns an error on empty secret — the two paths disagree on the most security-critical case.
→ Return false and log loudly when Verify != "" but secret is empty.

H-2 — Fixed 5-min job lease has no heartbeat RESOLVED in current PR #9 · jobs.go
Any job exceeding 5 min has its lease expire while running → concurrent re-execution. The live PR #9 adds startLeaseHeartbeat (renews every window/3, scoped to lease_owner) plus lease_owner-guarded terminal writes and an env-tunable window. A slow-but-alive job is never reclaimed. No action needed — strike this finding.

H-3 — HTTP-triggered flows enforce no CSRF and use single-role checks · flows.go:466-619
Unlike CRUD/workflow handlers, flow HTTP handlers never call validateCSRF. A POST /api/checkout flow with auth: required is triggerable cross-site on a logged-in cookie session. Also compares only session.Role (ignores the multi-role session.Roles the rest of the framework uses), and applies no tenant row-scoping to flow SQL. → Add the CSRF+Bearer gate; use the shared role resolver.

H-4 — parallel: step shares one *sql.Tx across goroutines RESOLVED in current PR #6 · flows.go
In a transaction:true flow, parallel children all call flowDB() → the same non-concurrency-safe *sql.Tx. PR #6 adds FlowContext.TxMu and serializes all *sql.Tx access in execStepSQL / execStepEnqueue on the transactional path. No action needed — strike this finding.

H-5 — Nested */private/* uploads served with no signature check · server.go:646 (not fixed by PR #12)
The serving gate checks only a top-level private/ prefix, but isPrivateUploadPath (used by the signer everywhere else) treats any /private/ segment as private. So GET /uploads/userdocs/private/ssn.pdf with no ?sig= returns 200. Classifier and gate disagree. → Gate serving with the same isPrivateUploadPath predicate.

H-6 — Image-transform endpoint = unauthenticated decompression bomb ✓ verified · image_transform.go:100
No auth gate; calls image.Decode(f) on the full source with no pixel bound before decoding. A 50 MB PNG → multi-GB RGBA in memory; a few concurrent requests OOM-kill the process. → image.DecodeConfig first, reject W*H > ~40MP, add a concurrency semaphore.

H-7 — Upload size cap is ineffective; multipart spills to disk unbounded ✓ verified · uploads_files.go:51
r.ParseMultipartForm(maxUploadSize) is called with no prior http.MaxBytesReader on r.Body — the arg is only the in-memory threshold; larger bodies stream to a temp file with no ceiling. POST 10 GB → disk exhaustion. → Wrap r.Body = http.MaxBytesReader(w, r.Body, max) before parsing. (Same root cause as H-9.)

H-8 — /_internal/fetch forwards cookies & skips SSRF for r.Host-derived URLs ✓ verified · fetch_async.go:40-49, 78-86
Relative URL → proto+r.Host+url, SSRF check skipped, caller cookies forwarded. r.Host is attacker-controllable absent a strict proxy → confused-deputy SSRF-to-self replaying the victim's cookies to internal endpoints. Endpoint has no auth/CSRF gate. → Use a configured canonical origin; never forward cookies to untrusted-derived destinations.

H-9 — No global request body size limit & missing ReadHeaderTimeout/MaxHeaderBytes · server.go:824-830
No http.MaxBytesReader in the middleware chain; handlers json.Decode(r.Body) directly → memory-exhaustion DoS. Server has ReadTimeout but no ReadHeaderTimeout (Slowloris) or MaxHeaderBytes. → Add a body-size middleware + the two server fields.

H-10 — Member can self-escalate in-tenant role via the configurable RoleField · crud.go:1106-1113, 2135
The protected map hard-codes "role" but not app.Group.RoleField (e.g. member_role). A tenant member can PATCH their own membership row to member_role: company_admin; on next request session.Roles includes it and every role:company_admin gate passes. (Cannot cross tenants — GlobalAdmin ignores session.Roles — but defeats in-tenant role gates.) → Add RoleField to the protected map for the membership table.

H-11 — CSRF token has no session binding ✓ verified · auth.go:316-324
Token is HMAC(serverSecret, nonce|ts) with zero user/session component, and /api/_csrf is unauthenticated. One token validates for every user; validateCSRF doesn't also check Origin. Current CSRF safety rests entirely on SameSite=Lax cookies — a single point of failure (breaks if SameSite is relaxed, or under same-site subdomain attacks). → Bind the token to the session ID.

H-12 — OTP keyed by loginValue on send but email on verify · auth.go:1136 vs 1262
For identifier: username|phone, the OTP row is stored under username/phone but looked up by email → every verification returns "expired," locking out all non-email-identifier users (functional break + brute-force surface if a stale row exists). → Key send and verify on the same value.

H-13 — Impersonation-end accepts any valid admin token · auth.go:758-800
/end-impersonation validates that the supplied admin_token is some admin session, not the one that started the impersonation, and the start response echoes admin_token in JSON. A leaked impersonation response + any admin token pivots the browser into that admin session. → Bind the originating admin session ID to the impersonation row and require a match.

H-14 — Cluster event-poller leaks goroutines per reload & is panic-unguarded · pubsub.go:60-137, server.go:280
StartEventPoller return value discarded; its two goroutines listen only on their own stopCh, never app.Stop → every SIGHUP leaks 2 goroutines + a 1s DB poller; raw go (not safeGo) means a panic crashes the process. (Also scheduling.go:282 sweeper uses raw go.) → Wire to app.Stop + safeGo.

H-15 — Scheduled-task sweeper has no atomic claim · scheduling.go:296-325
SELECT pending … → run flow → UPDATE done with no AND status='pending' guard. Overlapping sweepers (SIGHUP / slow tick) double-fire reminders & flows. The jobs queue learned this; scheduling.go didn't. → Mirror processNextJob's atomic claim.

H-16 — WebSocket connection cap bypassable by anon clients; no global cap · websocket.go:307-369
The per-user cap keys on userID; all anon clients share userID==0, and when the app has no auth the cap block is skipped entirely → unbounded connections, each spawning a write goroutine + hijacked FD. → Add a global cap + per-IP cap for anon.

H-17 — Rate-limiter keys on attacker-supplied raw Bearer/session value · ratelimit.go:204-226
Bucket key is "user:"+<raw bearer> before validating the token. Sending a random Authorization: Bearer xxx per request mints a fresh bucket each time → completely bypasses the anon IP limit. Combined with the unbounded visitor map (5-min sweep), also a memory-growth DoS. → Key on the resolved principal only; bound the map.

---

Part 3 — Notable Medium issues

• M-1 sql_dynamic interpolates request params as raw SQL text (flows.go:1188, interpolateCtx:2730) — a comment says "never funnel user input here" but nothing enforces it; textbook SQLi if any app author does. → Forbid request-sourced refs or gate behind trusted: true.
• M-2 Field encryption silently returns plaintext on nil key (encryption.go:712) — Go helper fails open while the SQL benmore_encrypt fails closed. → Return an error.
• M-3 GCM uses no AAD (encryption.go:728) — ciphertexts aren't bound to table/column/row; a DB-writer can cut-and-paste an encrypted value between columns and it decrypts cleanly (shared app key). → Add table||column as AAD (v2 format).
• M-4 Encryption key = single SHA-256 of operator string, no KDF/salt (encryption.go:173) — human passphrases are offline-brute-forceable; same passphrase → same key across tenants. → Require hex/base64 32-byte material or Argon2id+salt.
• M-5 selectFields not intersected with schema/sensitive denylist (crud.go:725) — ?fields=password_hash projection isn't masked the way write-backs are. → Intersect with colAllowlist + sensitive denylist.
• M-6 Webhook subscriber secrets stored plaintext (webhooks.go:21) — masked in API but cleartext on disk, while OAuth tokens are encrypted. → Encrypt at rest.
• M-7 applyAPIFilters variadic allowlist is a fail-open footgun (production.go:402,527) — when no allowlist is passed, ORDER BY <user>/filter on arbitrary real columns (info-disclosure oracle). Only tests hit it today; one careless caller away from live. → Make the allowlist mandatory + fail-closed.
• M-8 SVG/XML not in upload MIME blocklist + disposition keyed on extension not sniffed type (upload.go:413,629) — stored-XSS risk if ever served inline; compounds with the 'unsafe-inline' CSP.
• M-9 CSP relies on 'unsafe-inline' script-src + 4 default CDNs; no object-src/base-uri (server.go:890) — documented punt; object-src 'none' + base-uri 'self' are zero-risk to add now.
• M-10 No memory cap on compute (compute.go:225) — wall-clock timeout exists (good) but a single huge allocation OOMs before the statement-boundary interrupt; ReDoS not bounded (goja regex isn't RE2).

Lower-severity items

• Job-retry backoff integer overflow at high max_attempts (jobs.go:196) — 1<<attempts wraps negative ~attempt 58 → immediate hot-loop retry.
• Negative skew_seconds disables webhook replay protection in prod (verify_recipe.go:64).
• Synchronous flow errors leak raw SQL/step errors to clients on 500 (flows.go:691).
• OAuth lacks PKCE and OIDC nonce (oauth.go:173) — defense-in-depth gap.
• Per-account lockout has no per-IP limit on the web login path (auth.go:496); clientIP trusts forwarded headers (platform_shims.go:35).
• CloudFront signed URLs use RSA-SHA1 with up to 24h TTL (media_cdn.go:308, signed_urls.go:141).
• SSE has no read-side timeout / connection cap (sse.go:144) — slow-loris-style per-connection hold.
• OAuth token refresh uses http.PostForm not the SSRF-safe client (oauth_tokens.go:117).
• Email-verification is decorative unless require_verified is also set; signup issues a full session immediately (auth.go:1498-1553).
• Advisory lock acquire relies on string-matching the driver's UNIQUE-constraint error (locks.go:165) — fragile across driver versions.
• Cron same-minute dedup has a completed-then-re-tick edge (unique index only covers pending/running) (PR [codex] Run cron through durable jobs #11 / cron.go:457).
• JobStats builds SQL via fmt.Sprintf on a status literal (jobs.go:491) — not user-reachable today, but a SQLi-shaped pattern.

---

Part 4 — Verified solid (don't waste time here)

• Auth tokens all crypto/rand; bcrypt cost 12.
• Reset tokens SHA-256-hashed + single-use + all-sessions-revoked; account-enumeration-safe forgot-password.
• OAuth email-verification-takeover closed (provider email_verified enforced before linking).
• Multi-tenant isolation holds across CRUD/batch/includes/FTS/count/cursor/query — single rowScopeClause source of truth, act-as-aware, fully parameterized. No bypass found.
• GCM nonces random per-encryption (no reuse, no deterministic-IV path); HKDF-separated blind-index key (full-length, no truncation).
• safeHTTPClientStrict validates the post-resolution dial IP (defeats DNS-rebind to IMDS 169.254.169.254) and re-checks redirects.
• Admin routes uniformly adminAuth-gated; /metrics dark unless tokened (constant-time compare); no pprof//debug endpoints.
• Panic-recovery middleware is outermost and leaks no stack trace to clients.
• Request log / app logs redact secrets, tokens, passwords, PII; sensitiveGETParams strips secrets from logged URLs.
• The goja compute sandbox has no fs/net/env/db access, eval/Function stripped, traversal-guarded module resolution, and a real wall-clock timeout + interrupt.
• Job claim is atomic against double-claim (the execution is the gap, see C-1).
• WebSocket upgrade enforces same-origin (CSWSH closed); SSE/WS broadcast is non-blocking-drop (no head-of-line stall).
• PR Add runtime OpenAPI contract surface #2 OpenAPI surface is correctly auth-gated and filters internal tables + sensitive columns.

---

Recommended next actions

1. Merge the 11 PRs — they're net-positive. PR Add durable job lease recovery #9's heartbeat already resolves H-2; pair it with the C-1 idempotency work (exactly-once for DB jobs via the flow transaction + deterministic idempotency keys for external sends) to close the genuine-crash double-send.
2. Fix the one-line auth bypasses first: H-1 (webhook fail-open) and H-12 (OTP keying) are quick, high-impact, and currently breaking/bypassable.
3. Add the two DoS guards: H-7/H-9 (body size limit middleware) and H-6 (image pixel bound) — small diffs, real exposure.
4. Then the authz hardening: H-3 (flow CSRF), H-10 (RoleField), H-11 (CSRF binding), H-13 (impersonation).

---

Findings marked ✓ verified were confirmed by direct source inspection. All other findings cite exact file:line and were produced by focused subsystem review of v2.7.177 at commit c3c01cb.

[ArkashJ]

@alextitov19 Addressing those issues! Updating Master ticket: #14

[ArkashJ]

📈 Progress update — review findings cleared, security hardening underway

Quick status on the production-readiness push:

• ✅ All 11 PRs (Add runtime OpenAPI contract surface #2–[codex] Harden static asset serving #12) reviewed — a thorough automated review is attached to each.
• ✅ Every review finding addressed & pushed — 72+ findings fixed across the branches, all building green. See the running tally in the master tracking issue 📋 [Master] Production-Readiness Push — Tracking 11 PRs (#2–#12) #14.
• 🔒 Security deep-dive complete — a full v2.7.177 audit (auth, RBAC/multi-tenancy, SQL, uploads/SSRF, crypto, flows, jobs/realtime) found the framework unusually well-built (parameterized SQL throughout, tenant isolation holds, AES-GCM + crypto/rand, a genuinely locked-down JS sandbox). The remaining sharp edges live in subsystems the 11 PRs don't touch — those are now being fixed in a dedicated security-hardening PR.

Want to help? Pick a PR and add your review, or drop thoughts in Ideas. Every bit of scrutiny makes the framework more trustworthy to build on. 🙌

[ArkashJ]

🔒 Security-hardening PR is up — and the whole board is green

Milestone update on the production-readiness push:

• 🔒 Security-hardening PR 🔒 [codex] Security & correctness hardening (v2.7.177 audit — 40 findings) #15 opened — 40 findings from the v2.7.177 deep-dive, across 9 subsystems (auth, flows/webhooks, durable jobs, uploads/SSRF, server/DoS, CRUD/RBAC, crypto, realtime/rate-limit, OAuth). Draft, building & testing green, open for review.
• 🛑 Durable-job double-send gap closed — crash-mid-send no longer risks a duplicate email/webhook; lease recovery now fails closed (C-1).
• 🔀 Stacked PRs de-conflicted — the durable-jobs stack (Harden durable job transactions #6→Add durable job lease recovery #9→[codex] Add durable job unique keys #10→[codex] Run cron through durable jobs #11) is merged clean.
• ✅ All 12 PRs are now MERGEABLE / CLEAN.

This is a great moment to review: every PR builds, tests pass under go test -tags sqlite_fts5, and the security PR has inline comments explaining each invariant. Jump into a PR, or share ideas in Ideas. 🙌

[alextitov19]

🔍 Round 2 — Ultra-deep audit of main (post-#15…#18, commit c5e4318)

A second full-codebase pass after the hardening landed. 8 parallel deep reviews across subsystems, focused on (a) security-critical files that got zero hardening (rbac/access/scopes, migrations, templating, comms), and (b) the high-churn files (#15's auth/encryption rewrites + the hand-reconciled #16–#18 jobs/cron/static). Items marked ✓ were verified directly in source.

Calibration (credit where due)

#15 genuinely fixed most round-1 findings — independently re-confirmed correct: M-2 (nil-key fail-closed), M-6 (webhook secret encrypted at rest), H-2 (lease heartbeat), H-4 (parallel-tx reject), H-13 (impersonation binding), H-16 (WS global cap), dev-reload gating, and the strict-client SSRF fix. The issues below are new, or cases where a fix is incomplete/incorrect — including two regressions introduced by the reconciliation PRs (#16–#18), which were mine.

---

🔴 CRITICAL

1. ✓ Prisma auto-migrate DROPs tables on app boot; backup is taken after the destructive migration — prisma_migrate.go:102,258 + app_loader.go:32 (migration) vs :46 (backup). Removing or renaming a model in schema.prisma (a rename = drop+add at the diff layer) silently DROP TABLEs on the next restart, and the "pre-migrate" backup is captured post-drop — so there is no recovery path. Column-drops are already demoted to warnings; table-drops are not. → Demote table-drops to warnings / require explicit prune, and back up before LoadSchema when the diff is destructive.

🟠 HIGH

2. ✓ CSRF session-binding (the H-11 "fix") is neutralized by an unconditional unbound-token fallback — auth.go:519-523. validateCSRF accepts the bound or the legacy unbound MAC, always; /api/_csrf and the injected <meta>/form tokens are still minted unbound, so one unbound token validates for every session. Protection now rests entirely on the Origin check (auth.go:505), which treats absent/null Origin as same-origin (auth.go:418). → When a session exists, require the bound signature; keep the unbound path only for genuine pre-auth POSTs.

3. ✓ Nested-private/ symlink bypass serves private uploads unsigned (regression — my #17 code) — server.go:775. The URL gate uses isPrivateUploadPath (matches any /private/ segment) but the symlink-resolved re-check matches top-level private/ only. A symlink uploads/public.png → uploads/x/private/secret passes both checks → served with no signed URL. → Re-check with isPrivateUploadPath("uploads/"+rel).

4. ✓ Stored XSS in the feedback admin detail modal → admin session compromise (found by 2 reviewers) — feedback_ui.go:1347-1354. Visitor-submitted body/page_url/element_selector/screenshot/visitor_*/comment fields are concatenated into el.innerHTML with no escaping (the list view escapes; the modal regressed). Any demo-token visitor plants <img src=x onerror=…> that runs in the app owner's admin session. → Escape every field client-side (escH() already exists) or build with textContent.

5. Host-header poisoning of password-reset / verify-email links → pre-auth account takeover — auth.go:2047-2054,2111 + platform_shims.go:83. With seo.url unset, baseURL is built from attacker-controlled r.Host/X-Forwarded-Proto; the genuine reset email then contains https://attacker/reset?token=<valid>. → Require an operator-configured canonical origin for security links.

6. ✓ executeFlowJob (+ scheduler sweeper) read app.Flows without app.mu (regression — my #18 fix missed the busiest reader) — jobs.go:500, scheduling.go:332. #18 locked the write side + findCronJob/runCronJob, but not executeFlowJob; a SIGHUP reload racing a running flow job is a real data race. Also app.Stop reassigned unlocked (server.go:101-102) and app.Design/app.Tables/app.UUIDTables published outside the #18 lock (server.go:138-141,170-171). → Snapshot under RLock; move all config publishes inside the lock.

7. ✓ H-10 reopened via the /api/_transaction path — txUpdate (crud.go ~2765) omits the app.Group.RoleField guard that handleUpdate (crud.go:1173) and handleBatchUpdate (crud.go:2208) have. A tenant member can POST /api/_transaction to set their own member_role: admin → in-tenant privilege escalation. The create paths (handleCreate/handleBatchCreate/handleIngest/txInsert) are also unprotected (Medium). → Add the guard to all write paths; factor into one shared helper.

8. Backup rollback / auto-restore selects the WRONG (stale) backup — migrate.go:632-650,408-417. sort.Strings over mixed pre-migrate-* + scheduled-* filenames orders by prefix, not timestamp, so "newest" can be a days-old scheduled backup → restore silently discards recent data. → Sort by parsed timestamp suffix.

9. Scheduled/manual backups still use WAL-unsafe raw file copy — migrate.go:526-572. backupDatabase was fixed to use VACUUM INTO, but bm db backup + the hourly worker still raw-copy data.db+-wal separately → inconsistent snapshots. → Route through sqliteVacuumInto.

10. File-based migrations are non-atomic and can wedge — migrate_advanced.go:271-329. Statements run via bare db.Exec (no tx); a mid-file failure commits the prefix but never records the migration, so it re-runs from the top forever ("duplicate column"). → Wrap each file in one tx; record bookkeeping in the same tx.

11. ✓ Crypto: M-3 AAD column-binding is dead code on reads, and v1-downgrade strips it — encryption.go:765-770,1017-1031. The only read path (DecryptRowFields) passes expectColumn="", so the column-move check never runs where data is returned; and v1 ciphertext is accepted unconditionally, so an attacker can re-wrap v2→v1 to skip the AAD. The column-move protection M-3 claims to add isn't actually enforced. → Thread the real column into the read path; gate v1 acceptance off post-migration.

12. Scheduled tasks have no orphan recovery — scheduling.go:314-321. The H-15 atomic claim flips pending→running, but there's no lease/heartbeat/recovery sweep (unlike jobs) and executeFlowJob runs unguarded — a crash mid-fire strands the task in running forever (lost reminder/flow). → Add a stale-running requeue sweep with a claimed-at TTL.

🟡 MEDIUM (grouped)

• Authz: self-elevation via membership-row create paths (RoleField unprotected on create/ingest/txInsert); GET /api/_auth/roles leaks the full role/scope catalogue to any authenticated user (rbac.go:373).
• Auth residuals: |-delimited signed cookies (OTP id|email|otpkey, MFA-setup, signToken) over a never-format-validated email → field-injection/scope-confusion (auth.go:1326, tokens.go:21); per-IP lockout bypass via spoofable X-Benmore-App marker in the self-hosted build + fail-open on empty IP (platform_shims.go:44); impersonate response echoes the raw admin_token (auth.go:870).
• Migrations/data: Prisma @@map/@map identifiers interpolated into DDL with no validation → injection/breakage (prisma_schema.go, prisma_migrate.go:254); new NOT NULL column back-filled with '' even for INTEGER/DATETIME → silent corruption (prisma_migrate.go:281); PrunePrismaDrift drops without a backup.
• Crypto: AAD lacks a row-id component → row-to-row ciphertext swap within a column (encryption.go:431); signed-URL key silently becomes an ephemeral per-process random if serverSecret is empty at first call (signed_urls.go:64); webhook v0 body-only signature emitted by default (replayable; webhooks.go:360).
• Comms/SSRF: email opts.From not CRLF-sanitized (the one header field left unstripped, email.go:66); DNS-rebinding on the non-strict safeHTTPClient redirect check + isPrivateURL blind to alt-IP encodings (decimal/0x/IPv4-mapped-IPv6), and webhook registration uses the non-strict check (fetch.go:197-262, webhooks.go:94); notification title/body stored unescaped (stored XSS if the app renders via innerHTML).
• Realtime: cross-tenant presence enumeration via unscoped GET /api/_presence?slug= (presence.go:135); WS per-user/per-anon-IP caps are racy (only the global cap is re-checked under the write lock, websocket.go:398).
• Concurrency/perf: rate-limiter evictStalestLocked is O(n) under the global lock on every new key once capped → the H-17 flood becomes a CPU/lock-contention DoS (ratelimit.go:126); shared rate-limiter cleanup goroutine leaks one per hot reload (ratelimit.go:88); cron scheduler double-started per reload (server.go:224+:510).

🟢 LOW / cleanup

• Stray empty jobs_transaction_test.go.tmp committed to the repo (an artifact from the reconciliation — should be git rm'd).
• XLSX formula injection in WriteXLSX/buildSheet (latent — endpoint advertised but unwired; reuse csvSafeCell); XLSX XML control-char passthrough.
• Cron DOM/DOW AND'd instead of POSIX-OR'd (cron.go:626); every Nm drift from minute-truncated lastFire; Tx-dedup returns job_id:0 → broken status_url (jobs.go:221).
• PDF render uses --no-sandbox + leaks Chrome stderr in dev (pdf.go:223,237); SMS no E.164 validation / rate-limit bypass via format variants.
• Argon2id accepts below-floor params (encryption.go:848); rotation comments say sha256 but code uses the versioned derivation; versioning history unbounded + partial-NULL revert; drift detector blind to nullability/default changes; scopes.go empty-Scopes fail-open (contained by the _none_:read backstop).

---

Bottom line

1 Critical (data loss on a schema edit) and ~11 High. The standouts are the cases where a prior "fix" is incomplete — #2 (CSRF), #7 (txUpdate H-10), #11 (AAD) — plus two regressions the reconciliation introduced (#3 symlink, #6 flow race), which were mine. The most directly attacker-exploitable from outside are the feedback stored XSS (#4) and reset-link host-header poisoning (#5). Suggested fix order: #1 + the two self-introduced regressions (#3, #6), then the CSRF/authz cluster (#2, #7), then data-integrity (#8–#10).

Methodology: 8 subsystem reviews against c5e4318; every High was confirmed by reading the cited code. Round-1 findings already fixed by #15 were excluded unless found to be incompletely fixed.

[alextitov19]

✅ Round-2 findings fixed — 5 batched PRs merged (#19–#23)

All round-2 audit findings are now fixed on main (HEAD 6bc3156). Shipped as 5 themed PRs, each build + go vet + go test -tags sqlite_fts5 ./... green and -race clean:

PR	Theme	Highlights
#19	Migrations & data integrity	CRITICAL #1: back up before destructive Prisma migrations + prune (rollback can now actually recover). #8 timestamp-aware backup sort. #9 VACUUM INTO scheduled backups. #10 atomic file migrations. NOT-NULL backfill uses type-appropriate defaults.
#20	Concurrency regressions (the two I introduced)	#3 nested-private/ symlink bypass. #6 executeFlowJob/scheduler app.Flows race + app.Stop/app.Design/app.Tables under lock + capture-once across all 10 reload-restarted goroutines. #12 scheduled-task orphan recovery. Rate-limiter leak + O(1) eviction. cron double-start. stray .tmp removed.
#21	CSRF & authz	#2 CSRF origin gate hardened with Sec-Fetch-Site (unbound token no longer replayable cross-site). #5 reset/verify links via securityLinkBaseURL (no host-header poisoning). #7 RoleField guard on all write paths (incl. /api/_transaction). |-injection guard, loopback-only X-Benmore-App, no admin_token echo, admin-only roles catalogue.
#22	Crypto & webhooks	Webhook v0 (replayable) off by default. Signed-URL key no longer permanently caches an ephemeral key. Argon2 floor warning. Rotation comment fix. (M-3 AAD/row-id format changes deliberately deferred — DB-write threat, and strict read-path enforcement would false-reject aliased reads / risk data-inaccessibility.)
#23	XSS, comms, realtime	#4 (HIGH) feedback admin stored-XSS escaped. Email From CRLF strip. XLSX formula-injection. WebSocket per-class cap race. Cross-tenant presence namespacing. SMS E.164 validation.

Deliberately not changed (with rationale in each PR): crypto AAD format / row-id binding (data-loss risk for a DB-write-only threat; benmore_decrypt still enforces where the column is known); isPrivateURL alt-IP encodings (already fail-closed + Go handles IPv4-mapped IPv6); PDF --no-sandbox (environment-dependent, error echo is dev-only).

Final state: go build ✅ · go vet ✅ · go test -tags sqlite_fts5 ./... ✅ · -race ✅ · 0 open PRs.