diff --git a/server/CLAUDE.md b/server/CLAUDE.md index c0d1ef56a..9ba6c318a 100644 --- a/server/CLAUDE.md +++ b/server/CLAUDE.md @@ -48,7 +48,7 @@ Nest owns everything. Every domain is a DI module under `src/nest//` (`c - **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`. - **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 (`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). +- **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). - **Fail closed.** Unrecognized flag values mean OFF; sandbox/permission opt-outs refuse in production; never pick a less-safe path based on the *absence* of a file. GET handlers must not write. - **Don't fork canonical paths.** Auth verification is `verifyJwtAndLoadUser` (with the `password_version` gate) — reuse it. - **Money**: integer cents; largest-remainder splits (`BudgetService.splitEqualShares` is the reference); FX frozen at entry; explicit flags, never sentinel values; custom splits must reconcile to zero before persisting. diff --git a/server/eslint.config.mjs b/server/eslint.config.mjs index 6bfcd7bad..f6db0065f 100644 --- a/server/eslint.config.mjs +++ b/server/eslint.config.mjs @@ -46,6 +46,27 @@ const RAW_SQL_SELECTORS = [ }, ]; +// A guarded call whose options object (third argument) sets `timeoutMs` has a +// deadline over the whole exchange, so it does not need a signal in the init. +// esquery only resolves `:nth-child` inside `:has` when each step is its own +// nested `:has`, hence the shape. +const FETCH_CALL = `CallExpression[callee.name=${FETCH_CALLEE}]:not(:has(> ObjectExpression.arguments:nth-child(3):has(> Property[key.name='timeoutMs'])))`; + +// An argument that stands for "no init": undefined, null or void 0, also behind an `as` cast. +const NO_INIT = [ + "Identifier[name='undefined']", + "Literal[raw='null']", + "UnaryExpression[operator='void']", + // esquery compares String(value), so a missing `name` would read as 'undefined': + // the cast's inner node type is pinned first. + "TSAsExpression[expression.type='Identifier'][expression.name='undefined']", + "TSAsExpression[expression.type='Literal'][expression.raw='null']", + "TSAsExpression[expression.type='UnaryExpression'][expression.operator='void']", +].join(', '); + +const FETCH_TIMEOUT_MESSAGE = + 'Outbound fetch needs a timeout: pass { signal: AbortSignal.timeout(ms) }, or { timeoutMs } in the options of safeFetch/safeFetchFollow.'; + const FETCH_SELECTORS = [ { // Every outbound fetch needs a timeout (server/CLAUDE.md): a provider @@ -55,17 +76,17 @@ const FETCH_SELECTORS = [ // wrappers in utils/ssrfGuard.ts are held to the same rule: they hand // the platform a spread init, so the rule cannot see through them, and // their own fallback only bounds the wait for the headers. - selector: `CallExpression[callee.name=${FETCH_CALLEE}][arguments.length=1]`, - message: 'Outbound fetch needs a timeout: pass { signal: AbortSignal.timeout(ms) }.', + selector: `${FETCH_CALL}[arguments.length=1]`, + message: FETCH_TIMEOUT_MESSAGE, }, { - selector: `CallExpression[callee.name=${FETCH_CALLEE}] > ObjectExpression.arguments:nth-child(2):not(:has(Property[key.name='signal'])):not(:has(SpreadElement))`, - message: 'Outbound fetch needs a timeout: pass { signal: AbortSignal.timeout(ms) }.', + selector: `${FETCH_CALL} > ObjectExpression.arguments:nth-child(2):not(:has(Property[key.name='signal'])):not(:has(SpreadElement))`, + message: FETCH_TIMEOUT_MESSAGE, }, { // `safeFetch(url, undefined, options)` is the same call without an init. - selector: `CallExpression[callee.name=${FETCH_CALLEE}] > Identifier.arguments:nth-child(2)[name='undefined']`, - message: 'Outbound fetch needs a timeout: pass { signal: AbortSignal.timeout(ms) }.', + selector: `${FETCH_CALL} > :matches(${NO_INIT}).arguments:nth-child(2)`, + message: FETCH_TIMEOUT_MESSAGE, }, ]; 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 29737e788..208b5b0ae 100644 --- a/server/tests/unit/eslint-rules/restricted-syntax-selectors.test.ts +++ b/server/tests/unit/eslint-rules/restricted-syntax-selectors.test.ts @@ -51,7 +51,8 @@ declare const init: object; declare const signal: AbortSignal; `; -const TIMEOUT = 'Outbound fetch needs a timeout: pass { signal: AbortSignal.timeout(ms) }.'; +const TIMEOUT = + 'Outbound fetch needs a timeout: pass { signal: AbortSignal.timeout(ms) }, or { timeoutMs } in the options of safeFetch/safeFetchFollow.'; describe('outbound fetch timeout selectors', () => { let options: unknown[]; @@ -67,6 +68,14 @@ describe('outbound fetch timeout selectors', () => { ['safeFetchFollow(url, undefined, { bypassInternalIpAllowed: true })'], ["safeFetchFollow(url, { headers: { a: 'b' } })"], ['safeFetchAdminConfigured(url, {})'], + // Other spellings of "no init" must not slip past the undefined case. + ['safeFetch(url, void 0, { rejectUnauthorized: false })'], + ['safeFetch(url, null, { rejectUnauthorized: false })'], + ['safeFetch(url, null as never, { maxBytes: 1024 })'], + ['safeFetch(url, undefined as never, {})'], + // A timeoutMs nested in the init is not the options deadline. + ['safeFetch(url, { headers: { timeoutMs: 5 } })'], + ['safeFetch(url, undefined, { maxBytes: 1024 }, { timeoutMs: 5000 })'], ])('flags %s', (call) => { expect(messages(options, `${PRELUDE}\nvoid ${call};`)).toEqual([TIMEOUT]); }); @@ -81,6 +90,14 @@ describe('outbound fetch timeout selectors', () => { ['safeFetchFollow(url, { signal }, { bypassInternalIpAllowed: true })'], // safeFetchLlm takes its deadline from LLM_TIMEOUT_MS inside the wrapper. ["safeFetchLlm(url, { method: 'POST' })"], + // timeoutMs in the options is a deadline over the whole exchange. + ['safeFetch(url, undefined, { timeoutMs: 5000 })'], + ['safeFetch(url, void 0, { timeoutMs: 5000, maxBytes: 1024 })'], + ['safeFetch(url, { headers: {} }, { timeoutMs: 5000 })'], + ['safeFetchFollow(url, undefined, { timeoutMs: 5000 })'], + // A cast init is still an init: the "no init" casts must not match it. + ['fetch(url, { ...init, signal } as object)'], + ['safeFetch(url, init as object, { maxBytes: 1024 })'], ])('accepts %s', (call) => { expect(messages(options, `${PRELUDE}\nvoid ${call};`)).toEqual([]); });