diff --git a/server/CLAUDE.md b/server/CLAUDE.md index 9ba6c318a..1d8f47bf2 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -45,7 +45,7 @@ Nest owns everything. Every domain is a DI module under `src/nest//` (`c - **Thin controller → injectable provider.** Never resurrect a plain function-module service; don't grow god services — split a domain into several modules when it outgrows one class (trips and auth already are). - **Inject, don't reach for module globals.** No imports of the global `db` proxy from `nest/` layers at all any more — the last production importer converted onto a repository, and the export now survives only so ~190 test files' `vi.mock('…/db/database', …)` factories keep typechecking; no new code depending on the `ws` singleton or event-sink globals. There is no `DatabaseService` to inject either — take the repository(ies) you need plus `UnitOfWork` for transactions. Modules must be importable without side effects. -- **Transactions are not optional.** Multi-statement writes go in `await this.uow.transactional(async () => …)` (`UnitOfWork` from `nest/database`, provided by the global `OrmModule`); never hand-roll `BEGIN`/`COMMIT` and never call `db.transaction()` (the Phase 1 async sweep removes it). Every DB-touching method is `async`; `@typescript-eslint/no-floating-promises` and `no-misused-promises` are errors, so an unawaited call or a `Promise` in a condition fails lint. Bind values with `?`; identifiers only via a literal allow-list. In `src/db/repositories/` and `src/db/seeders/`, ESLint refuses raw SQL in any spelling: `raw()`, the `sql` tag, and SQL text handed to `execute`/`run`/`all`/`get`. Build the statement with the query builder; `MaintenanceRepository`, `DemoRepository` and the six existing seeders are the named exceptions in `eslint.config.mjs`. +- **Transactions are not optional.** Multi-statement writes go in `await this.uow.transactional(async () => …)` (`UnitOfWork` from `nest/database`, provided by the global `OrmModule`); never hand-roll `BEGIN`/`COMMIT` and never call `db.transaction()` (the Phase 1 async sweep removes it). Every DB-touching method is `async`; `@typescript-eslint/no-floating-promises` and `no-misused-promises` are errors, so an unawaited call or a `Promise` in a condition fails lint. Bind values with `?`; identifiers only via a literal allow-list. In `src/db/repositories/` and `src/db/seeders/`, ESLint refuses raw SQL in any spelling: `raw()`, the `sql` tag, and SQL text handed to `execute`/`run`/`all`/`get`. Build the statement with the query builder; `MaintenanceRepository`, `DemoRepository` and the six existing seeders are the named exceptions in `eslint.config.mjs`, and `tests/unit/eslint-rules/restricted-syntax-selectors.test.ts` pins how many SQL strings each still holds, so they can only lose SQL, never gain it. - **Migrations are append-only.** A migration is identified by its class name in `mikro_orm_migrations`, and files sort by the timestamp in that name — never rename or delete one that has shipped, and never edit its body; add a new one. No env interpolation in SQL. (The old positional `schema_version` array and the hand-kept `db/migrations.ts` test builder are both gone; the numbered migrations are the only schema history, for tests too.) A new foreign key comes with an index that leads with its column(s), or deleting the parent scans the child table; `tests/unit/db/foreign-key-indexes.test.ts` fails on one without. - **Every write endpoint validates** through the Zod pipe with a schema from `@trek/shared` — no `Record` + ad-hoc checks. Also verify every referenced id exists and belongs to the same trip. - **Every outbound `fetch`** gets a timeout, a response-size cap and boundary validation — no `as`-casting provider responses, no silent `catch → null`. User-influenced URLs go through the SSRF guard. Never fall back to another user's API key. ESLint requires an init with a `signal` on `fetch`, `safeFetch`, `safeFetchFollow` and `safeFetchAdminConfigured` alike, or a literal `timeoutMs` in the options object of `safeFetch`/`safeFetchFollow`; an init spelled `undefined`, `null` or `void 0` counts as none (`safeFetchLlm` takes `LLM_TIMEOUT_MS` inside). Behind that, the guarded wrappers (`utils/ssrfGuard.ts`) cap every body at `DEFAULT_MAX_RESPONSE_BYTES` (64 MB) and, for a call without a signal, wait `DEFAULT_RESPONSE_TIMEOUT_MS` (30 s) for the headers. Pass a tighter `maxBytes` when you buffer a small answer, `timeoutMs` for a deadline over the whole exchange, and `maxBytes: null` only where the body is streamed on or counted by the caller (`pipeAsset`, the doc-sync transfers, the Ollama pull). diff --git a/server/eslint.config.mjs b/server/eslint.config.mjs index f6db0065f..89e15b440 100644 --- a/server/eslint.config.mjs +++ b/server/eslint.config.mjs @@ -348,6 +348,9 @@ export default tseslint.config( // documented escape hatch for the demo seed data, and every seeder but // DatabaseSeeder writes its rows as SQL text (four of them with the // SQLite-only INSERT OR IGNORE). A new seeder starts on the query builder. + // tests/unit/eslint-rules/restricted-syntax-selectors.test.ts pins how many + // SQL strings each of them holds, so new SQL text in an exempt file fails + // there, and a file with none left has to leave this list. files: ['src/db/repositories/**/*.ts', 'src/db/seeders/**/*.ts'], ignores: [ 'src/db/repositories/MaintenanceRepository.ts', diff --git a/server/tests/unit/eslint-rules/restricted-syntax-selectors.test.ts b/server/tests/unit/eslint-rules/restricted-syntax-selectors.test.ts index 208b5b0ae..614acfd9c 100644 --- a/server/tests/unit/eslint-rules/restricted-syntax-selectors.test.ts +++ b/server/tests/unit/eslint-rules/restricted-syntax-selectors.test.ts @@ -1,6 +1,7 @@ import tsParser from '@typescript-eslint/parser'; import { ESLint, Linter } from 'eslint'; +import fs from 'fs'; import path from 'path'; import { beforeAll, describe, expect, it } from 'vitest'; @@ -134,21 +135,57 @@ declare const repo: { run(sql: string, params?: unknown[]): Promise; ge expect(found.some((m) => m.startsWith('Dialect SQL goes through'))).toBe(true); }); - it('leaves the named adapters alone, and only them', async () => { - for (const file of [ - 'src/db/repositories/MaintenanceRepository.ts', - 'src/db/repositories/DemoRepository.ts', - 'src/db/seeders/AddonSeeder.ts', - 'src/db/seeders/AdminSeeder.ts', - 'src/db/seeders/CategorySeeder.ts', - 'src/db/seeders/DocumentProviderSeeder.ts', - 'src/db/seeders/PhotoProviderSeeder.ts', - 'src/db/seeders/SchemaVersionSeeder.ts', - ]) { + // The files eslint.config.mjs exempts from the SQL string rule, with the SQL + // strings each holds today. A count may only go down: a higher one is new SQL + // text slipping in under the exemption, a lower one means the pin (and at 0 + // the ignores entry) has to follow. + const EXEMPT_SQL_STRINGS: Record = { + 'src/db/repositories/MaintenanceRepository.ts': 4, + 'src/db/repositories/DemoRepository.ts': 9, + 'src/db/seeders/AddonSeeder.ts': 1, + 'src/db/seeders/AdminSeeder.ts': 2, + 'src/db/seeders/CategorySeeder.ts': 2, + 'src/db/seeders/DocumentProviderSeeder.ts': 2, + 'src/db/seeders/PhotoProviderSeeder.ts': 2, + 'src/db/seeders/SchemaVersionSeeder.ts': 1, + }; + const SQL_MESSAGE_PREFIX = 'A SQL string handed to the connection'; + + function filesUnder(dir: string): string[] { + return fs + .readdirSync(path.join(serverRoot, dir), { recursive: true, encoding: 'utf8' }) + .filter((name) => name.endsWith('.ts')) + .map((name) => `${dir}/${name.split(path.sep).join('/')}`); + } + + it('exempts exactly the pinned files and keeps the rest of the guard on them', async () => { + const exempt: string[] = []; + for (const file of [...filesUnder('src/db/repositories'), ...filesUnder('src/db/seeders')]) { + const options = (await selectorsFor(file)) as { message?: string }[]; + if (!options.some((o) => o.message?.startsWith(SQL_MESSAGE_PREFIX))) exempt.push(file); + } + expect(exempt.sort()).toEqual(Object.keys(EXEMPT_SQL_STRINGS).sort()); + for (const file of exempt) { const options = await selectorsFor(file); expect(hits(options, "void connection.execute('PRAGMA wal_checkpoint(TRUNCATE)');"), file).toEqual([]); - // The rest of the guard still applies to them. - expect(messages(options, `${PRELUDE}\nvoid safeFetch(url);`), file).toContain(TIMEOUT); + expect(messages(options, `${PRELUDE} +void safeFetch(url);`), file).toContain(TIMEOUT); + } + }); + + it('holds each exempt file to the SQL strings it has today', async () => { + // The selectors of a file the rule does apply to, run over the exempt file's source. + const options = await selectorsFor('src/db/repositories/Probe.repository.ts'); + const counts: Record = {}; + for (const file of Object.keys(EXEMPT_SQL_STRINGS)) { + const source = fs.readFileSync(path.join(serverRoot, file), 'utf8'); + counts[file] = messages(options, source).filter((m) => m.startsWith(SQL_MESSAGE_PREFIX)).length; + } + expect(counts, 'new SQL text in an exempt file fails; after removing some, lower its pin').toEqual( + EXEMPT_SQL_STRINGS, + ); + for (const [file, n] of Object.entries(counts)) { + expect(n, `${file} holds no SQL string any more: drop it from the ignores in eslint.config.mjs`).toBeGreaterThan(0); } });