mirror of
https://github.com/mauriceboe/TREK.git
synced 2026-10-18 21:16:47 +00:00
test(server): pin the SQL strings in the files exempt from the SQL text rule
MaintenanceRepository, DemoRepository and six seeders are exempt from the SQL string selectors as whole files, so new SQL text added to any of them passed lint unseen, and nothing noticed when a file no longer needed its exemption. The comment promised a list that can only shrink, but nothing held it to that. The selector test now works out which files under src/db/repositories and src/db/seeders the config really exempts and compares that set with a pinned table, then runs the SQL selectors over each exempt file's source and compares the hits with the count pinned for it (23 today). One more statement fails, one fewer asks for the pin to come down, and a file at zero has to leave the ignores. Per-line disables would have done the same job but add suppressions the codebase is meant to lose.
This commit is contained in:
+1
-1
@@ -45,7 +45,7 @@ Nest owns everything. Every domain is a DI module under `src/nest/<domain>/` (`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<string, unknown>` + 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).
|
||||
|
||||
@@ -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',
|
||||
|
||||
@@ -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<unknown>; 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<string, number> = {
|
||||
'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<string, number> = {};
|
||||
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);
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user