Claude/repo potential review 25d776 - #1
Merged
Conversation
Security fixes:
- storage: contain all file paths within the bucket dir and drop the
double-decode, closing arbitrary read/write/delete via path traversal
- crud/external-api: validate every dynamic column name against the real
table columns (pragma table_info), closing SQL injection through JSON
body keys and query filter/select/order fields
- auth: require team:write for user creation and settings:write for API
key creation; ignore a forged user_id unless the caller has team:write
- auth: replace passwordless first-login (account takeover) with a
single-use, 72h invite-token flow (/auth/accept-invite)
- middleware: re-check is_active on every request so disabling a user
takes effect immediately instead of at JWT expiry
- index: refuse to boot in production with a placeholder JWT secret,
warn on wildcard CORS, and rate-limit login/accept-invite
- Dockerfile: remove insecure baked-in JWT_SECRET and wildcard CORS
Bug fix:
- auth: datetime("now") used double quotes; better-sqlite3 runs with
SQLITE_DQS=0 so the statement threw and execute() swallowed it, meaning
change-password and accept-invite silently never persisted. Now
datetime('now').
Frontend:
- rework SignUp into invite acceptance (reads ?token=, calls accept-invite)
- make ForgotPassword honest (no fake "email sent"); relabel Login link
- add auth.acceptInvite to the API client
Tests + CI:
- Vitest + supertest integration tests over a temp SQLite (17 tests)
covering each security invariant with positive controls
- GitHub Actions CI: gates on server typecheck, tests, frontend build,
and docker build; lint/frontend-typecheck run non-blocking
- fix tsconfig.server.json CommonJS/ESM mismatch and Express param typing
so the server typechecks cleanly
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Invoice/job numbers were generated three inconsistent ways: the external API used COUNT(*)+1 (which reuses a number after a delete and uses a different format), while the frontend and recurring generator did a read-modify-write of the company_settings counter across separate steps (racy: two concurrent creates get the same number and the second fails the UNIQUE constraint). Server: - add server/utils/numbering.ts: allocateInvoiceNumber/allocateJobNumber do an atomic read-increment-write of the settings counters inside a transaction (better-sqlite3 is synchronous, so this is race-free) - external-api: allocate numbers via the shared helper instead of COUNT(*) - functions.ts: recurring generation uses the allocator; add POST /api/functions/allocate-invoice-number and /allocate-job-number for the frontend to reserve a number atomically - schema: add job_prefix/job_next_number to company_settings; migration adds them to existing DBs and creates UNIQUE indexes on invoice_number and job_number only when no duplicates exist (skips with a warning otherwise so boot never breaks) - also fixes a latent double-dash format bug (prefix 'INV-' + '-' number) Frontend: - InvoiceDetail/JobDetail reserve the number via the atomic endpoint at save time and stop writing the counter back client-side; a user-typed custom invoice number is still respected on the create path - preview format now matches the server allocator output Tests: add numbering.test.ts (6 tests) — sequential allocation, no reuse after delete, distinct numbers via the endpoint and external API, and the UNIQUE backstop. Full suite 23 passing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
App.tsx statically imported all ~35 pages, so the login screen downloaded the entire app (a single 1.4 MB / 370 kB gzip JS chunk). Convert every page to React.lazy behind a Suspense boundary (Login stays eager as the common entry point). Initial entry chunk drops to ~388 kB / 123 kB gzip, with each route loaded on demand; the >500 kB build warning is gone. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.