Skip to content

Audit-plan fixes: password/cron/AI-limit security, submission race, perf, CI, tests#116

Open
Vijayabaskar56 wants to merge 2 commits into
devfrom
improve/audit-fixes
Open

Audit-plan fixes: password/cron/AI-limit security, submission race, perf, CI, tests#116
Vijayabaskar56 wants to merge 2 commits into
devfrom
improve/audit-fixes

Conversation

@Vijayabaskar56

Copy link
Copy Markdown
Member

Implements the /improve deep audit plans (10 of 12) plus the fixes from a multi-agent high-effort code review. Code only — all plan/doc .md files are intentionally kept local and excluded from this PR.

What's included

Security

  • Form passwords: constant-time compare + scrypt hashing. Hashing now happens on every write path — saveFormSettings and the updateForm/draftSettings flush the settings UI actually uses (a HIGH-severity plaintext-at-rest gap the review caught and this PR fixes).
  • Cron auth: CRON_SECRET bearer token is now authoritative; x-vercel-cron is honored only as a fallback on Vercel when no secret is set (closes the header-only bypass).
  • AI generation: per-org short-window rate limit before the LLM call (applies to Pro/Business too), with a distinct quota/ai-rate-limited code and a "slow down" client toast wired into both the theme and streaming paths.

Correctness / perf

  • Submission finalize is now a single atomic ON CONFLICT DO UPDATE on (formId, draftId) — kills the double-submit 500 race; the no-downgrade (completed→incomplete) guard is preserved.
  • Question-progress batch is one multi-row upsert (was up to 20 serial round-trips), with intra-batch de-dup.

Tech-debt / DX

  • App console.* routed through the structured logger (embed IIFE scripts + the evlog drain intentionally excluded).
  • Stale customization keys (coverFit) stripped at the read boundary + an idempotent scrub script.
  • CI now runs tsc --noEmit and the Vitest suite on every PR (previously neither ran in CI).
  • nitro pinned (was "latest"); better-auth/vite/vitest/esbuild bumped to clear the critical + 2 high advisories.

Tests added: password-hash, cron-auth, submissions upsert, AI rate-limit, batch analytics, number-format, scale values, customization migrate.

Verification

  • pnpm exec tsc --noEmit → clean (exit 0).
  • Pure-logic unit tests: 309 passing.
  • DB-dependent tests (submissions / AI rate-limit / analytics) are blocked on the new tables existing — see DDL below.

⚠️ Action required before this works in any environment

These need additive DDL applied via DIRECT_URL (NOT db:push/db:migrate — tracking is drifted):

CREATE TABLE IF NOT EXISTS public.ai_request_rate_limits (
  org_id text PRIMARY KEY,
  window_start timestamptz NOT NULL DEFAULT now(),
  count integer NOT NULL DEFAULT 0
);

And to scrub stale customization (optional, idempotent — scripts/scrub-stale-customization.ts):

UPDATE forms        SET customization = customization - 'coverFit' WHERE customization ? 'coverFit';
UPDATE form_versions SET customization = customization - 'coverFit' WHERE customization ? 'coverFit';

Also: the new CI test job needs DATABASE_URL + DIRECT_URL configured as GitHub Actions secrets.

Not in this PR (by design)

  • Plan 005 (analytics ingestion guards) — stopped: the "reject non-published forms" step contradicts a deliberate, test-pinned design (recorders write unconditionally; gating is display-only). Needs a rethink (rate-limit only).
  • Plan 009 transitive dep overrides — need pnpm-workspace.yaml (out of the plan's scope); some patches are inside the 7-day quarantine window.
  • Low UX follow-up: after hashing, the form-password field round-trips the scrypt hash (masked by default; only visible if the owner clicks reveal). Not fixed here because the settings onChange writes values wholesale, so blanking the field would risk wiping the password in active WIP — left as a deliberate follow-up.
  • All .md docs (README/CONTRIBUTING corrections + the plans/ directory).

Note on scope

The working tree had in-progress local WIP (dashboard/settings refactor) entangled in shared files with the fixes; it could not be cleanly separated (e.g. dashboard.tsx depends on new untracked components), so it rides along in this branch.

Vijayabaskar56 and others added 2 commits June 18, 2026 18:31
- Override the line tab variant border so dark mode uses the intended
  gray-200 rail instead of the brighter default soft border.
Implements the improve-audit plans (code only; plan/docs .md kept local):

- security: constant-time + scrypt-hashed form passwords, hashed on every
  write path (saveFormSettings AND updateForm/draftSettings)
- security: cron auth requires CRON_SECRET; x-vercel-cron no longer trusted alone
- security: per-org short-window rate limit on AI form generation (+ client
  "slow down" toast wired in both theme and streaming paths)
- bug: atomic ON CONFLICT upsert for submission finalize (kills double-submit race)
- perf: single multi-row upsert for question-progress batch (was N serial)
- tech-debt: app console.* routed through the structured logger (embed excluded)
- tech-debt: strip stale customization keys at read boundary + scrub script
- tests: password, cron, submissions, AI rate-limit, batch analytics,
  number-format, scale, customization
- ci: run tsc --noEmit and the vitest suite on every PR
- deps: pin nitro (was "latest"); bump better-auth/vite/vitest/esbuild CVEs

Also bundles in-progress local dashboard/settings WIP that shares files with the
above (could not be cleanly separated).

Co-authored-by: improve-skill workflow <noreply@anthropic.com>
@vercel

vercel Bot commented Jun 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
reform Error Error Jun 19, 2026 1:09pm

Request Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant