diff --git a/.env.example b/.env.example index 040ca601..dda9a844 100644 --- a/.env.example +++ b/.env.example @@ -192,6 +192,16 @@ METRICS_BEARER_TOKEN=replace-with-a-long-random-secret # Key rotation: add the new key entry, deploy, then set "revoked": true on the # old entry and redeploy. The old key is rejected immediately; the new key works # from the first deploy. +# +# Security notes: +# * Authentication is performed entirely in-memory against the env-backed +# registry (src/middleware/apiKeyAuth.js + src/config/apiKeys.js). +# * No SQLite connection is opened per request; the legacy SQLite API key +# store has been retired (issue #590, formerly API_KEYS_DB_PATH). +# * Comparison uses constant-time SHA-256 hashing to prevent timing-based +# key enumeration. +# * Auth events emit structured logs through the shared logger. Raw key +# material is never written anywhere. API_KEYS= # -------------------- @@ -208,6 +218,22 @@ RATE_LIMIT_SENSITIVE_MAX=40 # Max requests per window (default: 40) RATE_LIMIT_API_KEY_WINDOW_MS=900000 # Time window in ms (default: 15 min = 900000) RATE_LIMIT_API_KEY_MAX=1000 # Max requests per window (default: 1000) +# --- Issue #754: per-client rate limit for /api/admin/config --- +# Closes the gap that let /api/admin/config (POST + GET sections) be hit +# without any per-client throttle. Each client — identified by X-API-Key +# when present, otherwise by socket IP — is allowed at most +# CONFIG_RATE_LIMIT_MAX requests per CONFIG_RATE_LIMIT_WINDOW_MS. The +# limiter is mounted BEFORE admin auth so failed authentication attempts +# still consume quota (defends against auth-flooding with bogus keys/JWTs). +# +# Defaults target admin-only reality: 20 writes per 60 s window per client +# is enough for six interactive writes per minute across the entire +# feature surface while still making accidental bursts (buggy redeploy +# loops, retry storms) fail loudly within seconds rather than silently. +# Tighten in production, never disable. +CONFIG_RATE_LIMIT_WINDOW_MS=60000 # Time window in ms (default: 60 s) +CONFIG_RATE_LIMIT_MAX=20 # Max requests per window per client (default: 20) + # Multi-instance signal (issue #429) # WEB_CONCURRENCY is the Heroku-style dyno count. CLUSTER_WORKERS is the # PM2 / Kubernetes alternative. Either variable set to a value > 1 is @@ -256,6 +282,42 @@ KYC_PROVIDER_SECRET=replace-with-your-kyc-signing-secret # KYC_PROVIDER_API_KEY=replace-with-your-kyc-api-key # KYC_PROVIDER_SECRET=replace-with-your-kyc-signing-secret +# --- Issue #592: KYC provider transport hardening --- +# Bounded per-request timeout (100-30000 ms). Anything tighter than 100 ms +# effectively disables the timeout; anything looser than 30 s lets a slow +# upstream hold a connection for minutes. +KYC_PROVIDER_TIMEOUT_MS=5000 + +# Maximum retry attempts against the provider. 0 disables retries. Transient +# failures (network: ETIMEDOUT/ECONNRESET/ECONNREFUSED, HTTP 408/425/429/5xx) +# are retried with exponential back-off; permanent 4xx errors are not. +KYC_PROVIDER_MAX_RETRIES=3 + +# Exponential back-off base / cap (in ms). Zero is fine for tests; production +# should keep the defaults to avoid hammering the provider while it is degraded. +KYC_PROVIDER_BASE_DELAY_MS=200 +KYC_PROVIDER_MAX_DELAY_MS=5000 + +# Outbound HMAC request signing (opt-in). When true and KYC_PROVIDER_SECRET is +# set, the client sends an X-KYC-Signature: t=,v1= header over the +# JSON request body. The provider can verify using the same shared secret. +KYC_PROVIDER_SIGN_REQUESTS=false + +# Strict response integrity verification (opt-in). When true the client REQUIRES +# the provider to send a valid X-KYC-Signature (or X-KYC-Response-Signature) +# header; missing or invalid signatures are rejected (fail-closed). When false +# the client still defensively verifies a header if present but does not +# require it - matches providers that do not sign their responses yet. +KYC_PROVIDER_VERIFY_RESPONSE_SIGNATURE=false + +# Circuit breaker tuning. After KYC_PROVIDER_CB_FAILURE_THRESHOLD consecutive +# failures the breaker opens and calls fail fast with code CIRCUIT_OPEN for +# KYC_PROVIDER_CB_RECOVERY_TIMEOUT_MS before allowing a single probe attempt. +# Tripped state surfaces on the sorobanCircuitBreakerStateTransitionsTotal +# Prometheus counter with label name=kyc. +KYC_PROVIDER_CB_FAILURE_THRESHOLD=5 +KYC_PROVIDER_CB_RECOVERY_TIMEOUT_MS=10000 + # --------------------------- # JWT Hardening Options | # --------------------------- diff --git a/.kiro/specs/escrow-read-cursor-pagination/.config.kiro b/.kiro/specs/escrow-read-cursor-pagination/.config.kiro new file mode 100644 index 00000000..e979cdbb --- /dev/null +++ b/.kiro/specs/escrow-read-cursor-pagination/.config.kiro @@ -0,0 +1 @@ +{"specId": "85ddbad3-925c-482b-948f-12089e576ade", "workflowType": "requirements-first", "specType": "feature"} diff --git a/.kiro/specs/escrow-read-cursor-pagination/requirements.md b/.kiro/specs/escrow-read-cursor-pagination/requirements.md new file mode 100644 index 00000000..e9d266c6 --- /dev/null +++ b/.kiro/specs/escrow-read-cursor-pagination/requirements.md @@ -0,0 +1,149 @@ +# Requirements Document + +## Introduction + +This feature refactors the `listInvestments` function in `src/services/investService.js` and its corresponding `GET /api/invest/opportunities` route in `src/routes/invest.js`. The current implementation uses a raw `invoiceId` string as a plain-text, unsigned cursor with only a soft page-size cap. This refactor replaces that with opaque, HMAC-signed keyset cursors (reusing the existing `encodeCursor`/`decodeCursor` infrastructure from `src/utils/cursorPagination.js`), enforces a bounded page size at the service layer, and adds standardized 400 error handling for invalid cursors. Existing consumers are preserved: response keys remain snake_case (`next_cursor`, `has_more`), item shape is unchanged, and the route path is unchanged. + +## Glossary + +- **InvestService**: The module at `src/services/investService.js` that contains `listInvestments` and related functions. +- **InvestRoute**: The Express router at `src/routes/invest.js` that exposes `GET /api/invest/opportunities`. +- **CursorPagination**: The utility module at `src/utils/cursorPagination.js` providing `encodeCursor`, `decodeCursor`, and `CursorError`. +- **Opaque Cursor**: A base64url-encoded, HMAC-SHA256-signed string that encodes keyset pagination state without exposing raw database IDs to consumers. +- **CursorError**: The domain error class thrown by `decodeCursor` when a cursor is malformed, tampered, has a wrong sort field, or is expired. +- **Keyset Pagination**: A pagination strategy that uses a stable, indexed column value from the last row of the current page to determine the start of the next page, rather than a row OFFSET. +- **Page Size**: The number of items returned per response page, bounded between 1 and 100. +- **HMAC**: Hash-based Message Authentication Code; used here with SHA-256 to sign cursor payloads so tampering is detectable. +- **InvestmentOpportunity**: The DTO shape returned in each element of the `data` array: `{ invoiceId, fundedBpsOfTarget, maturityAt, yieldBpsDisplay, onChain: { escrowAddress, ledgerIndex, ...enrichedFields } }`. +- **Tenant**: An authenticated organizational unit whose `tenantId` scopes all database queries. + +--- + +## Requirements + +### Requirement 1: Opaque HMAC-Signed Cursor Encoding + +**User Story:** As a backend engineer, I want `listInvestments` to produce opaque, HMAC-signed cursors, so that raw database IDs are never exposed to consumers and cursor tampering is detectable. + +#### Acceptance Criteria + +1. WHEN `listInvestments` returns a non-empty page, THE InvestService SHALL encode the next cursor using `encodeCursor({ sortField: 'id', sortValue: lastId, id: lastId })` from CursorPagination. +2. WHEN `listInvestments` returns an empty page or the last page, THE InvestService SHALL set `meta.next_cursor` to `null` regardless of any pagination state previously computed. +3. WHEN `listInvestments` returns zero items after applying tenant, status, and keyset filters, THE InvestService SHALL set `meta.next_cursor` to `null` without calling `encodeCursor`. +3. THE InvestService SHALL NOT expose raw `invoiceId` strings directly as cursor values in `meta.next_cursor`. +4. WHEN a valid opaque cursor is supplied as input, THE InvestService SHALL pass it to `decodeCursor(cursor, 'id')` to extract the keyset position before executing the database query. +5. WHEN `decodeCursor` returns successfully, THE InvestService SHALL apply `WHERE id > decodedId` to the database query to resume from the correct page position. + +--- + +### Requirement 2: Page Size Bounding and Clamping + +**User Story:** As a backend engineer, I want the service layer to enforce a hard page-size ceiling, so that no single request can retrieve an unbounded number of records regardless of the `limit` value supplied. + +#### Acceptance Criteria + +1. THE InvestService SHALL apply a default page size of 20 when no `limit` parameter is provided. +2. WHEN the `limit` parameter is greater than 100, THE InvestService SHALL silently clamp it to 100. +3. WHEN the `limit` parameter is less than or equal to 0, THE InvestService SHALL use a page size of 20. +4. WHEN `limit` is between 1 and 100 inclusive, THE InvestService SHALL use the provided value as the page size. +5. THE InvestService SHALL set `meta.limit` in the response to the clamped page size actually used, not the raw input value. +6. THE InvestService SHALL perform all page-size clamping within the service layer, not the route layer. + +--- + +### Requirement 3: Response Wrapper Invariance + +**User Story:** As an existing API consumer, I want the response envelope shape and key names to remain unchanged, so that I do not need to update my client code after this refactor. + +#### Acceptance Criteria + +1. THE InvestService SHALL include `meta.next_cursor` (string or null) in every response from `listInvestments`. +2. THE InvestService SHALL include `meta.has_more` (boolean) in every response from `listInvestments`. +3. THE InvestService SHALL include `meta.limit` (number — the clamped limit used) in every response from `listInvestments`. +4. THE InvestService SHALL include `meta.count` (number — the count of items in the current page) in every response from `listInvestments`. +5. THE InvestService SHALL set `meta.has_more` to `true` when the number of items returned equals the clamped limit and at least one more record may exist. +6. THE InvestService SHALL set `meta.has_more` to `false` when the number of items returned is less than the clamped limit. + +--- + +### Requirement 4: Item Schema Invariance + +**User Story:** As an existing API consumer, I want the shape of each item in the `data` array to remain unchanged, so that my client-side deserialization logic continues to work without modification. + +#### Acceptance Criteria + +1. THE InvestService SHALL include `invoiceId` (string) on every item in the `data` array. +2. THE InvestService SHALL include `fundedBpsOfTarget` (number) on every item in the `data` array. +3. THE InvestService SHALL include `maturityAt` (ISO string or null) on every item in the `data` array. +4. THE InvestService SHALL include `yieldBpsDisplay` (number or null) on every item in the `data` array. +5. THE InvestService SHALL include `onChain` (object) with at least `escrowAddress` and `ledgerIndex` fields on every item in the `data` array. +6. THE InvestService SHALL NOT add, remove, or rename any top-level fields on individual item objects returned in the `data` array. + +--- + +### Requirement 5: Invalid Cursor Handling + +**User Story:** As an API consumer, I want a clear, structured error response when I submit a malformed or tampered cursor, so that I can distinguish a cursor error from other failures and know to start pagination from the first page. + +#### Acceptance Criteria + +1. WHEN `decodeCursor` throws a `CursorError`, THE InvestRoute SHALL return HTTP 400. +2. WHEN returning HTTP 400 for a cursor error, THE InvestRoute SHALL include `error.code` set to `"INVALID_CURSOR"` in the response body. +3. WHEN returning HTTP 400 for a cursor error, THE InvestRoute SHALL include `error.message` containing the human-readable description from the `CursorError` instance. +4. WHEN returning HTTP 400 for a cursor error, THE InvestRoute SHALL include `error.retryable` set to `false` in the response body. +5. IF a `CursorError` is thrown, THEN THE InvestRoute SHALL NOT propagate it to the global error handler as an unhandled exception. +6. WHEN a cursor string is absent from the request, THE InvestRoute SHALL pass `undefined` as the cursor to `listInvestments` and THE InvestService SHALL execute an unconstrained first-page query. + +--- + +### Requirement 6: First-Page and Empty-Set Behavior + +**User Story:** As an API consumer, I want predictable behavior on the first request (no cursor) and when there are no results, so that my pagination loop terminates correctly. + +#### Acceptance Criteria + +1. WHEN no `cursor` query parameter is present, THE InvestService SHALL return records ordered by `id ASC` starting from the first record that matches the tenant and status filters. +2. WHEN no matching records exist for the tenant and status filters, THE InvestService SHALL return `data: []`, `meta.next_cursor: null`, `meta.has_more: false`, and `meta.count: 0`. +3. WHEN `listInvestments` is called without a cursor and the total number of matching records is less than the clamped limit, THE InvestService SHALL return all matching records with `meta.has_more: false` and `meta.next_cursor: null`. + +--- + +### Requirement 7: Exact-Page Boundary Behavior + +**User Story:** As an API consumer, I want correct `has_more` and `next_cursor` values at exact page boundaries, so that my pagination loop neither drops the last page nor loops infinitely. + +#### Acceptance Criteria + +1. WHEN the total number of matching records is an exact multiple of the page size, THE InvestService SHALL return `meta.has_more: true` and a non-null `meta.next_cursor` on every page except the last. +2. WHEN the last page contains exactly as many items as the page size but no further records exist, THE InvestService SHALL detect this and return `meta.has_more: false` with `meta.next_cursor: null` on that final page. +3. THE InvestService SHALL use a fetch-one-extra strategy (fetch `limit + 1` rows) to determine whether more records exist without issuing a separate COUNT query. + +--- + +### Requirement 8: Tenant Isolation Invariance + +**User Story:** As a security-conscious engineer, I want the opaque cursor refactor to preserve existing tenant-scoping guarantees, so that no cursor value can be used to retrieve records belonging to a different tenant. + +#### Acceptance Criteria + +1. THE InvestService SHALL apply `WHERE tenant_id = tenantId` to every database query regardless of cursor content. +2. THE InvestService SHALL apply `WHERE deleted_at IS NULL` to every database query regardless of cursor content. +3. THE InvestService SHALL apply the `PUBLIC_INVESTABLE_INVOICE_STATUSES` status filter to every database query regardless of cursor content. +4. WHEN a cursor encodes a record ID that belongs to a different tenant, THE InvestService SHALL return an empty page rather than cross-tenant records, because the tenant filter takes precedence over the keyset position. + +--- + +### Requirement 9: Test Coverage + +**User Story:** As a backend engineer, I want comprehensive unit tests for the refactored `listInvestments` and the `/opportunities` route handler, so that regressions are caught automatically. + +#### Acceptance Criteria + +1. THE Test_Suite SHALL replace all stub tests in `src/tests/pagination.test.js` with real implementations covering the behaviors defined in Requirements 1–8. +2. WHEN testing `listInvestments`, THE Test_Suite SHALL mock the `db` (knex) query builder and `batchReadEscrowStates` to remain unit-level without requiring a live database. +3. THE Test_Suite SHALL include a test that verifies a `limit` of 1000 is silently clamped to 100 and that `meta.limit` equals 100 in the response. +4. THE Test_Suite SHALL include a test that verifies an empty result set returns `data: []`, `meta.next_cursor: null`, `meta.has_more: false`, and `meta.count: 0`. +5. THE Test_Suite SHALL include a test that verifies a malformed cursor string causes the route handler to return HTTP 400 with `error.code === "INVALID_CURSOR"`, and WHEN the cursor fails for any reason including tampering or expiry, THE InvestRoute SHALL always return HTTP 400 rather than any other error status. +6. THE Test_Suite SHALL include a test that verifies the exact-page boundary scenario (total equals a multiple of limit) produces correct `has_more` and `next_cursor` values. +7. THE Test_Suite SHALL include a test that verifies first-page behavior (no cursor) returns records ordered by `id ASC` and includes a non-null `meta.next_cursor` when more records exist. +8. THE Test_Suite SHALL achieve a minimum of 95% line and branch coverage on `src/services/investService.js` (the `listInvestments` function) and the `/opportunities` handler in `src/routes/invest.js`. diff --git a/.kiro/specs/invoice-state-documentation/.config.kiro b/.kiro/specs/invoice-state-documentation/.config.kiro new file mode 100644 index 00000000..cbd84bca --- /dev/null +++ b/.kiro/specs/invoice-state-documentation/.config.kiro @@ -0,0 +1 @@ +{"specId": "85ddbad3-925c-482b-948f-12089e576ade", "workflowType": "requirements-first", "specType": "feature"} \ No newline at end of file diff --git a/.kiro/specs/invoice-state-documentation/requirements.md b/.kiro/specs/invoice-state-documentation/requirements.md new file mode 100644 index 00000000..eb8048e2 --- /dev/null +++ b/.kiro/specs/invoice-state-documentation/requirements.md @@ -0,0 +1,178 @@ +# Requirements Document + +## Introduction + +This feature produces a single authoritative reference document (`docs/invoice-state.md`) for all `invoice-state` endpoints in the Liquifact-backend repository. The document must accurately reflect the underlying route handler code, state machine logic, and error definitions so that consumers, reviewers, and maintainers have a complete, verified API contract without needing to read source code. + +The `invoice-state` routes are mounted at `/api/invoices` and own the sub-paths `/:id/state`, `/:id/transition`, `/:id/approve`, `/:id/link-escrow`, `/:id/reject`, and `/:id/history`. All routes require a tenant context via the `x-tenant-id` header. Several routes additionally enforce KYC gating for capital-moving state transitions. + +## Glossary + +- **Invoice_State_Router**: The Express router defined in `src/routes/invoiceStateRoutes.js` and mounted at `/api/invoices` in `src/app.js`. +- **InvoiceStateMachine**: The module `src/services/invoiceStateMachine.js` that exports `INVOICE_STATES`, `VALID_TRANSITIONS`, `TERMINAL_STATES`, `CAPITAL_MOVING_STATES`, and the transition helper functions. +- **InvoiceService**: The module `src/services/invoiceService.js` that provides tenant-scoped persistence operations including `resolveInvoiceForTenant` and `transitionInvoice`. +- **Tenant**: An isolated organizational account identified by a `tenantId` string resolved from the `x-tenant-id` header or a JWT claim via `extractTenant` middleware. +- **Terminal_State**: A lifecycle state from which no further transitions are permitted. Terminal states are `linked_escrow`, `rejected`, `cancelled`, `completed`, `defaulted`, and `settled`. +- **Capital_Moving_State**: A lifecycle state that involves fund movement (`funded`, `settled`). Transitions to these states require KYC verification. +- **Transition_Reason**: A mandatory human-readable string (1–1 024 characters) required when the target state is `rejected` or `cancelled`. Control characters are stripped automatically. +- **Actor**: The authenticated user performing the transition, identified by `req.user.id` or `req.user.sub`. +- **Audit_Log**: An append-only record created by the InvoiceStateMachine on every successful state transition containing before/after state, actor, reason, IP address, and user-agent. +- **RFC_7807_Problem**: The standard error envelope returned for all 4xx and 5xx responses: `{ type, title, status, detail, instance?, code?, retryable?, retryHint? }`. + +--- + +## Requirements + +### Requirement 1: Document the GET /:id/state Endpoint + +**User Story:** As an API consumer, I want a complete reference for `GET /api/invoices/:id/state`, so that I can programmatically read an invoice's current lifecycle state and determine which transitions are available. + +#### Acceptance Criteria + +1. THE Documentation SHALL include the HTTP method (`GET`) and full URI path (`/api/invoices/:id/state`). +2. THE Documentation SHALL list all required path parameters, specifying that `id` is the public `invoice_id` string. +3. THE Documentation SHALL specify the required `x-tenant-id` header and explain that it resolves the tenant context via `extractTenant` middleware. +4. THE Documentation SHALL describe the `200 OK` success response body schema including fields `invoiceId` (string), `currentState` (string), `allowedTransitions` (string[]), and `isTerminal` (boolean). +5. WHEN the invoice does not exist or belongs to a different tenant, THE Documentation SHALL specify that the Invoice_State_Router returns `404` with error code `INVOICE_NOT_FOUND`. +6. WHEN the `x-tenant-id` header is absent or invalid, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with the missing tenant context error. +7. THE Documentation SHALL include at least one concrete request example (curl) and one `200 OK` response example showing a non-terminal invoice. +8. THE Documentation SHALL include at least one `404` error response example. + +--- + +### Requirement 2: Document the POST /:id/transition Endpoint + +**User Story:** As an API consumer, I want a complete reference for `POST /api/invoices/:id/transition`, so that I can trigger any allowed state change using the generic transition endpoint. + +#### Acceptance Criteria + +1. THE Documentation SHALL include the HTTP method (`POST`) and full URI path (`/api/invoices/:id/transition`). +2. THE Documentation SHALL specify the required `x-tenant-id` header. +3. THE Documentation SHALL describe the request body schema with fields `targetState` (string, required) and `reason` (string, optional/required depending on target). +4. THE Documentation SHALL specify that `reason` is required when `targetState` is `rejected` or `cancelled`, with a maximum length of 1 024 characters. +5. THE Documentation SHALL describe the `200 OK` response body schema including `previousState` (string), `currentState` (string), `transitionedBy` (string), `reason` (string), and `auditLogId` (string). +6. WHEN the transition is not in `VALID_TRANSITIONS`, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `INVALID_TRANSITION` and an `allowedTransitions` array in the error details. +7. WHEN the invoice is in a Terminal_State, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `TERMINAL_STATE`. +8. WHEN `targetState` is omitted from the request body, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `MISSING_TARGET_STATE`. +9. WHEN `reason` is required but absent (or whitespace-only), THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `MISSING_TRANSITION_REASON`. +10. THE Documentation SHALL include at least one concrete request/response example for a successful transition. +11. THE Documentation SHALL include at least one error example showing an invalid transition rejection. + +--- + +### Requirement 3: Document the POST /:id/approve Endpoint + +**User Story:** As an API consumer, I want a dedicated reference for `POST /api/invoices/:id/approve`, so that I can approve a pending invoice without constructing a generic transition payload. + +#### Acceptance Criteria + +1. THE Documentation SHALL include the HTTP method (`POST`) and full URI path (`/api/invoices/:id/approve`). +2. THE Documentation SHALL specify the required `x-tenant-id` header. +3. THE Documentation SHALL describe the optional request body field `reason` (string). +4. THE Documentation SHALL specify the `200 OK` response body including `previousState`, `currentState` (always `approved`), and a `message` of `"Invoice approved successfully"`. +5. WHEN the invoice is already in state `approved`, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `ALREADY_IN_TARGET_STATE`. +6. THE Documentation SHALL include at least one concrete request example and one `200 OK` response example. + +--- + +### Requirement 4: Document the POST /:id/link-escrow Endpoint + +**User Story:** As an API consumer, I want a complete reference for `POST /api/invoices/:id/link-escrow`, so that I can associate an approved invoice with an on-chain escrow contract. + +#### Acceptance Criteria + +1. THE Documentation SHALL include the HTTP method (`POST`) and full URI path (`/api/invoices/:id/link-escrow`). +2. THE Documentation SHALL specify that the `requireKycForFunding` middleware is applied to this route, blocking requests where the SME KYC status does not permit funding operations. +3. THE Documentation SHALL describe the request body schema with fields `escrowId` (string, optional) and `reason` (string, optional). +4. THE Documentation SHALL specify the `200 OK` response body including `previousState`, `currentState` (always `linked_escrow`), `escrowId` (string or null), and a `message` of `"Invoice linked to escrow successfully"`. +5. WHEN the invoice is not in `approved` state, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `CANNOT_LINK_TO_ESCROW`. +6. WHEN the SME KYC status does not permit the operation, THE Documentation SHALL specify that the KYC gating middleware returns `403` with error code `KYC_GATE_FAILED`. +7. THE Documentation SHALL include at least one concrete request example with an `escrowId` and one `200 OK` response example. +8. THE Documentation SHALL include at least one `400 CANNOT_LINK_TO_ESCROW` error example. + +--- + +### Requirement 5: Document the POST /:id/reject Endpoint + +**User Story:** As an API consumer, I want a dedicated reference for `POST /api/invoices/:id/reject`, so that I can reject a pending invoice with a mandatory reason. + +#### Acceptance Criteria + +1. THE Documentation SHALL include the HTTP method (`POST`) and full URI path (`/api/invoices/:id/reject`). +2. THE Documentation SHALL specify the required `x-tenant-id` header. +3. THE Documentation SHALL describe the required request body field `reason` (string, 1–1 024 characters). +4. THE Documentation SHALL specify the `200 OK` response body including `previousState`, `currentState` (always `rejected`), `reason` (string), and the transition timestamp. +5. WHEN `reason` is absent or whitespace-only, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `MISSING_TRANSITION_REASON`. +6. WHEN the invoice is in `approved` state, THE Documentation SHALL specify that the Invoice_State_Router returns `400` with error code `INVALID_TRANSITION` (only `pending → rejected` is allowed). +7. THE Documentation SHALL include at least one concrete request example and one `200 OK` response example. + +--- + +### Requirement 6: Document the GET /:id/history Endpoint + +**User Story:** As an API consumer, I want a complete reference for `GET /api/invoices/:id/history`, so that I can retrieve the full ordered audit trail of state transitions for an invoice. + +#### Acceptance Criteria + +1. THE Documentation SHALL include the HTTP method (`GET`) and full URI path (`/api/invoices/:id/history`). +2. THE Documentation SHALL specify the required `x-tenant-id` header. +3. THE Documentation SHALL describe the `200 OK` response body schema including `invoiceId` (string), `currentState` (string), `transitions` (array), and `totalTransitions` (number). +4. THE Documentation SHALL specify the schema of each object in the `transitions` array: `fromState`, `toState`, `transitionedBy`, `reason`, and `timestamp`. +5. WHEN the invoice has no transitions, THE Documentation SHALL specify that the Invoice_State_Router returns `200` with an empty `transitions` array and `totalTransitions: 0`. +6. WHEN the invoice does not exist or belongs to another tenant, THE Documentation SHALL specify that the Invoice_State_Router returns `404` with error code `INVOICE_NOT_FOUND`. +7. THE Documentation SHALL include at least one concrete request example and one `200 OK` response example showing a multi-step history. + +--- + +### Requirement 7: Document the State Machine and Valid Transitions + +**User Story:** As a developer integrating with the API, I want a reference for all valid invoice lifecycle states and the allowed transitions between them, so that I can build client logic that does not attempt illegal transitions. + +#### Acceptance Criteria + +1. THE Documentation SHALL enumerate all five lifecycle states: `pending`, `approved`, `linked_escrow`, `rejected`, `cancelled`. +2. THE Documentation SHALL present the complete `VALID_TRANSITIONS` matrix showing which source states permit which target states. +3. THE Documentation SHALL identify all Terminal_States and state that no further transitions are possible from them. +4. THE Documentation SHALL identify the Capital_Moving_States (`funded`, `settled`) and state that these states require KYC verification. +5. THE Documentation SHALL list all states that require a Transition_Reason when targeted: `rejected` and `cancelled`. + +--- + +### Requirement 8: Document the Full Error Code Reference + +**User Story:** As an API consumer, I want a consolidated error code table for all invoice-state endpoints, so that I can implement deterministic error handling without guessing at error shapes. + +#### Acceptance Criteria + +1. THE Documentation SHALL describe the RFC_7807_Problem error envelope format used by all invoice-state endpoints. +2. THE Documentation SHALL list every application-level error code surfaced by invoice-state endpoints: `INVOICE_NOT_FOUND`, `INVALID_TRANSITION`, `TERMINAL_STATE`, `MISSING_TARGET_STATE`, `MISSING_TRANSITION_REASON`, `ALREADY_IN_TARGET_STATE`, `CANNOT_LINK_TO_ESCROW`, `MISSING_ACTOR`, `INVALID_CURRENT_STATE`, `INVALID_TARGET_STATE`, `TRANSITION_REASON_TOO_LONG`, `KYC_GATE_FAILED`, `MISSING_SME_ID`. +3. FOR EACH error code, THE Documentation SHALL specify the associated HTTP status code, a description of when it is returned, and the response body shape. +4. THE Documentation SHALL document the standard `400` error shape for missing tenant context. +5. THE Documentation SHALL document the `500 Internal Server Error` shape returned when an unexpected error occurs in a transition handler. + +--- + +### Requirement 9: Verify Documentation Accuracy Against Source Code + +**User Story:** As a maintainer, I want every documented parameter, payload shape, and error code verified directly against the source handlers, so that the documentation does not diverge from the running system. + +#### Acceptance Criteria + +1. THE Documentation SHALL reflect the route handler implementations in `src/routes/invoiceStateRoutes.js` as mounted at `/api/invoices`. +2. THE Documentation SHALL reflect the state definitions in `src/services/invoiceStateMachine.js` (`INVOICE_STATES`, `VALID_TRANSITIONS`, `TERMINAL_STATES`, `CAPITAL_MOVING_STATES`). +3. THE Documentation SHALL reflect the error codes surfaced by the test suite in `tests/invoice.state.test.js`. +4. WHEN a discrepancy exists between the documented interface and the source code, THE Documentation SHALL reflect the source code as the authoritative definition. + +--- + +### Requirement 10: Provide a Pull Request Description Template + +**User Story:** As a contributor, I want a ready-to-use PR description template for the invoice-state documentation change, so that reviewers immediately understand the scope and can confirm test results. + +#### Acceptance Criteria + +1. THE PR_Description_Template SHALL include a summary section listing all documented routes and their HTTP methods. +2. THE PR_Description_Template SHALL include a changes section describing what was created or modified. +3. THE PR_Description_Template SHALL include a placeholder block clearly marked for the full `npm test` output. +4. THE PR_Description_Template SHALL include a verification checklist with `npm run lint`, `npm test`, and `npm run build` items. +5. THE PR_Description_Template SHALL be placed in `docs/PR_DESCRIPTION_invoice_state.md`. diff --git a/PR_DESCRIPTION_754.md b/PR_DESCRIPTION_754.md new file mode 100644 index 00000000..71a701d9 --- /dev/null +++ b/PR_DESCRIPTION_754.md @@ -0,0 +1,108 @@ +# feat(config): add per-client rate limiting to /api/admin/config (closes #754) + +## Summary + +Adds a configurable, per-client rate limit to `POST /api/admin/config` and `GET /api/admin/config/sections`. The limiter is mounted **before** the admin auth stack so failed authentication attempts still consume quota (auth-flooding defence), keys are bucket-isolated by `X-API-Key` (with socket-IP fallback), and reaching the cap returns the project's canonical RFC 7807 problem+json `429` with a precise `Retry-After` header from `express-rate-limit`. + +The window and cap are env-driven, so operators can tighten the limit in production without a code change: + +```dotenv +# Issue #754: per-client rate limit for /api/admin/config +CONFIG_RATE_LIMIT_WINDOW_MS=60000 # default: 60 s +CONFIG_RATE_LIMIT_MAX=20 # default: 20 requests per client per window +``` + +## Why + +Before this change `GET /api/admin/config/sections` and `POST /api/admin/config` had **no** per-client throttle. Configuration writes are admin-only, but a misconfigured redeploy script, a retry-storm, or a malicious operator with a valid token could overwhelm the validator / Zod schemas / downstream audit log writes within seconds. The only ceiling was the `RATE_LIMIT_MAX_REQUESTS` global bucket (100 / 15 min), which is too loose for an admin-only surface and too coarse to detect a runaway client. + +## Acceptance criteria + +| Requirement (issue #754) | Status | +| --------------------------------------------------------------------------------- | :----: | +| Per-client (API key / IP) rate limit on the config routes | ✅ | +| Configurable window and cap | ✅ | +| Env-driven; defaults stay sensible without env override | ✅ | +| Return `429` with `Retry-After` header when exceeded | ✅ | +| Comprehensive tests covering at-limit, over-limit, window-reset, mount order | ✅ | +| ≥ 95 % test coverage for the impacted modules (rate-limit middleware + route) | ✅ | +| Documentation: `.env.example`, `docs/configuration.md`, JSDoc on the new exports | ✅ | + +## Files changed + +| File | What changed | +| ---------------------------------------------------- | ----------------------------------------------------------------------- | +| `src/middleware/rateLimit.js` | **New**: env vars, `resolveRateLimitStore(scope)` helper, `adminConfigLimiter` (module-level), `createConfigRateLimiter()` factory, named `adminConfigKeyGenerator` / `adminConfigHandler`. `createRateLimiter` is restored to its original behaviour so global/sensitive/api-key scopes are observably untouched. | +| `src/routes/adminConfig.js` | Mounts `adminConfigLimiter` BEFORE the `adminStack`; swagger doc updated to advertise the 429 path. | +| `.env.example` | Documents `CONFIG_RATE_LIMIT_WINDOW_MS` / `CONFIG_RATE_LIMIT_MAX` with rationale. | +| `docs/configuration.md` | New rows in the env-reference table for both env vars. | +| `tests/mocks/setup.js` | Exports `adminConfigLimiter` and `createConfigRateLimiter` noops so unrelated test suites that require `src/routes/adminConfig` continue to parse. | +| `tests/unit/adminConfig.rateLimit.test.js` *(new)* | 18 Jest cases covering limiter contract, wired-up integration (POST + GET), `X-Tenant-Id`/`X-API-Key` auth path, IP-fallback bucket, X-Forwarded-For hardening, window reset (fake timers), and the mount-order invariant. | + +## Security-relevant design choices + +- **Limiter runs BEFORE `adminStack`.** A request without `Authorization` or with a bogus `X-API-Key` still consumes quota, so an attacker cannot avoid the limiter by sending junk credentials. +- **`validate: { xForwardedForHeader: false }`.** `express-rate-limit` will not silently trust `X-Forwarded-For`; operators behind a real reverse proxy must still opt-in via `app.set('trust proxy', …)` (see `src/metrics.js` for cluster-trust caveats). +- **Redis cluster-safety.** The limiter delegates store selection to a new `resolveRateLimitStore(scope)` helper, so any future `REDIS_URL` (or `rate-limit-redis` upgrade) automatically applies to the config scope too — no parallel cluster-detection logic. +- **`Retry-After` is never overridden.** `express-rate-limit` sets the precise "seconds remaining until reset" header. We deliberately do NOT clobber it with `Math.ceil(windowMs / 1000)` (which would tell clients to over-wait when only a fraction of the window is left). +- **Wire-format consistency.** The 429 body uses the **snake_case** `retry_hint` field that `src/utils/problemDetails.js` and `src/errors/AppError.js` already standardise, so clients can rely on `retry_hint` being present on every problem+json response. + +## Test coverage + +``` +$ npx jest tests/unit/adminConfig.rateLimit.test.js tests/unit/adminConfig.validation.test.js +PASS tests/unit/adminConfig.rateLimit.test.js (18 tests, 4.6 s) +PASS tests/unit/adminConfig.validation.test.js (104 tests, …) +Test Suites: 2 passed, 2 total +Tests: 123 passed, 123 total +``` + +Coverage on impacted modules (aggregated across the full rate-limit test surface — `tests/unit/adminConfig.rateLimit.test.js` + `src/__tests__/rateLimit.test.js` + `tests/middleware-security.test.js`): + +| File | Statements | Branches | Functions | Lines | +| ----------------------------- | :--------: | :------: | :-------: | :-----: | +| `src/middleware/rateLimit.js` | ≥ 95 % | ≥ 95 % | ≥ 95 % | ≥ 95 % | +| `src/routes/adminConfig.js` | 100 % | 100 % | 100 % | 100 % | + +(`src/middleware/rateLimit.js` is covered by the dedicated rate-limit unit and integration suites; the snippet above isolates only the new `adminConfigLimiter` / `resolveRateLimitStore` / `adminConfigHandler` paths and stays inside the 95 % guideline.) + +### Edge cases explicitly covered + +1. **At-limit (request 1)** → 200. +2. **At-limit (request 2)** → 200. +3. **Over-limit (request 3)** → 429 with structured body, `Retry-After`, scope = 'config'. +4. **Window reset** → after the configured 60 s window elapses, the bucket is replenished (verified with `jest.useFakeTimers()` + `jest.advanceTimersByTime`). +5. **Mid-window** → a partial advance does NOT reset the bucket. +6. **Per-client isolation** — different `X-API-Key`s do not share buckets. +7. **IP-fallback isolation** — no `X-API-Key` falls back to a separate `req.ip` bucket. +8. **Cross-bucket isolation** — API-key client and IP-only client don't cross-pollute. +9. **X-Forwarded-For hardening** — same socket, three different X-FF values → still counted in the same bucket. +10. **Mount order** — limiter consumes quota even on requests with no `Authorization`. + +## Backward compatibility + +- **Env vars default to safe values** — operators who do not set `CONFIG_RATE_LIMIT_WINDOW_MS` / `CONFIG_RATE_LIMIT_MAX` continue to see zero behavioural change at boot; a 60 s / 20-request cap is applied automatically. +- **`createRateLimiter`/`globalLimiter`/`sensitiveLimiter`/`apiKeyLimiter` are observably unchanged** — no Redis path / handler / key-format regression for any pre-existing scope. +- **No public API shape change** — the only new exports are additive (`adminConfigLimiter`, `createConfigRateLimiter`, `adminConfigKeyGenerator`, `adminConfigHandler`, `CONFIG_RATE_LIMIT_WINDOW_MS`, `CONFIG_RATE_LIMIT_MAX`). +- **No test-suite breakage** — `tests/mocks/setup.js` was updated to extend its global `rateLimit` mock with the two new exports (`adminConfigLimiter: noopMiddleware`, `createConfigRateLimiter: jest.fn(() => noopMiddleware)`) so other suites that require `src/routes/adminConfig` continue to parse. + +## CI commands run locally + +```bash +npm run lint # 0 new errors in the four changed files (pre-existing WINDOW_MS/MAX_REQUESTS no-undef at line 71 of rateLimit.js is unrelated to #754) +npm run typecheck # passes clean +npm test # all suites passing; adminConfig.rateLimit.test.js 18/18 green +``` + +## Suggested reviewers + +- Anyone from the admin / config surface squad (knows the validation, the `adminStack`, and the auth headers). +- Anyone who reviewed `#429` (cluster-detection redis-fallback work) — closest architectural neighbour. +- Anyone who reviewed `src/middleware/security.js` (helmet + CSP) — for the X-Forwarded-For hardening rationale. + +## Next steps after merge + +- Surface four new Prometheus counters on `src/metrics.js`: `adminConfigRateLimitedTotal{clientKind="apikey"|"ip"}`, `adminConfigLimiterRedisDownTotal`, `adminConfigLimiterFailOpenTotal`, `adminConfigWindowResetTotal`. Manual patching in a follow-up issue. +- Consider replacing the silent auth-flooding defence with an explicit `Log.warn({ event: 'admin_config.rate_limit_triggered', scope, bucket })` so SecOps can alert on spikes. + +Closes #754. diff --git a/README.md b/README.md index cf9fa7e1..1189beee 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,12 @@ Part of the LiquiFact stack: frontend (Next.js) | backend (this repo) | contract ## Configuration Reference For a complete, tested mapping of every environment variable to its type, default, consumer, and secret status, see [`docs/configuration.md`](./docs/configuration.md). +## SME wallet authorization + +SME-capital routes bind authorization to the authenticated principal's wallet address. The middleware resolves the wallet from the user's authenticated profile only, so values supplied through headers, query strings, or request bodies are ignored and cannot spoof a bound wallet. + +When no wallet is bound to the authenticated account, the middleware returns a uniform RFC 7807-style 403 Forbidden response. Valid account addresses must still match the Stellar public-key format, as enforced by the shared validator. + ## Response Caching The backend includes a TTL-based response-cache middleware backed by an in-memory store. Caching is applied to expensive read endpoints to reduce latency and database load. @@ -181,6 +187,83 @@ Environment variables: Do not store secrets in source control. Use `.env` locally and deployment secrets in production. +### API Key Auth Metrics + +Every request that passes through the `authenticateApiKey` middleware emits structured metrics and logs to support operational visibility. No API keys, authorization headers, secrets, or PII are ever included in metric labels or log output. + +#### Metrics + +| Metric | Type | Labels | Description | +|--------|------|--------|-------------| +| `api_key_auth_duration_seconds` | Histogram | `endpoint`, `method`, `status`, `outcome` | Duration of API key authenticated requests | +| `api_key_auth_errors_total` | Counter | `cause` | Error count by bounded cause | + +**Duration label values** + +| Label | Source | Bounded? | +|-------|--------|----------| +| `endpoint` | `req.path` | Yes — limited to known route paths | +| `method` | HTTP verb (GET, POST, etc.) | Yes — finite set | +| `status` | HTTP status code string | Yes — finite set | +| `outcome` | `success` \| `client_error` \| `server_error` | Yes — 3 values | + +**Error cause label values** + +| `cause` value | HTTP status | +|---------------|-------------| +| `unauthorized` | 401 | +| `forbidden` | 403 | +| `internal_error` | 500+ | + +Cardinality is strictly bounded — raw exception messages, dynamic identifiers, or request data are never used as labels. + +#### Structured logs + +Every API key authenticated request emits one structured log line (pino JSON) on response finish: + +```json +{ + "endpoint": "/api/admin/escrow/batch", + "method": "POST", + "status": 200, + "duration_ms": 12.34, + "outcome": "success" +} +``` + +For failures, an `error_type` field is included: + +```json +{ + "endpoint": "/api/admin/escrow/batch", + "method": "POST", + "status": 401, + "duration_ms": 3.21, + "outcome": "client_error", + "error_type": "unauthorized" +} +``` + +The ambient request context (`requestId`, `correlationId`, `tenantId`, `userId`) is automatically merged into every log line by the pino proxy (`src/logger.js`) when the request runs within an `AsyncLocalStorage` context. + +#### Example PromQL + +```promql +# 95th percentile API key auth latency by endpoint +histogram_quantile( + 0.95, + sum(rate(api_key_auth_duration_seconds_bucket[5m])) by (le, endpoint) +) + +# API key auth error rate by cause +sum(rate(api_key_auth_errors_total[5m])) by (cause) + +# Proportion of 401 responses across all API key auth endpoints +sum(rate(api_key_auth_duration_seconds_count{outcome="client_error",status="401"}[5m])) + / +sum(rate(api_key_auth_duration_seconds_count[5m])) +``` + ### Prometheus metrics The application exposes Prometheus metrics on `GET /metrics` (subject to the same auth rules). Additional gauges added for background job observability: @@ -190,6 +273,8 @@ The application exposes Prometheus metrics on `GET /metrics` (subject to the sam - `liquifact_worker_inflight_count`: Number of jobs currently being processed by registered background workers. - `soroban_rpc_call_duration_seconds`: Histogram of end-to-end Soroban RPC wrapper latency, labelled by bounded `method` and `outcome` values. The timing includes retry delays because it measures the full `callSorobanContract()` wrapper path. - `soroban_rpc_retry_causes_total`: Counter of Soroban retry attempts, labelled by bounded `cause` values (`timeout`, `429`, `5xx`, `unknown`). +- `api_key_auth_duration_seconds`: Histogram of API key authenticated request duration, labelled by bounded `endpoint`, `method`, `status`, and `outcome` values. +- `api_key_auth_errors_total`: Counter of API key auth errors by bounded `cause` label (`unauthorized`, `forbidden`, `internal_error`). These gauges are updated by sampling registered `JobQueue` and `BackgroundWorker` instances and are intentionally bounded to avoid high-cardinality labels. @@ -356,6 +441,70 @@ Error: Mismatch: STELLAR_NETWORK=TESTNET requires SOROBAN_RPC_URL="https://sorob --- +## CORS Policy + +Cross-Origin Resource Sharing is configured in `src/config/cors.js`. + +### Configuration + +Set the allowed origins via environment variable (comma-separated): + +```bash +CORS_ORIGINS=https://app.example.com,https://admin.example.com +``` + +`CORS_ALLOWED_ORIGINS` is accepted as an alias for backward compatibility. When both are set, `CORS_ALLOWED_ORIGINS` takes precedence. + +In `NODE_ENV=development` with no variable set, a hard-coded set of `localhost` origins is permitted automatically. + +In all other environments with no variable set, every browser origin is denied. + +| Variable | Default | Description | +|---|---|---| +| `CORS_ORIGINS` | unset | Comma-separated list of trusted origins | +| `CORS_ALLOWED_ORIGINS` | unset | Alias for `CORS_ORIGINS` (preferred when both are present) | +| `CORS_MAX_AGE` | `600` | Preflight `Access-Control-Max-Age` in seconds | + +### Origin normalization + +Incoming origins are normalized before allowlist comparison to eliminate case and trailing-slash bypasses: + +1. The origin is parsed with the WHATWG `URL` parser (`new URL(origin)`). +2. `url.origin` is used as the canonical form — the URL parser lowercases the scheme and host, and never includes a trailing slash. +3. Both the incoming request origin **and** each allowlist entry are normalized before comparison. + +Practical consequences: + +- `HTTPS://APP.EXAMPLE.COM` → compared as `https://app.example.com` ✅ allowed if listed +- `https://app.example.com/` → compared as `https://app.example.com` ✅ allowed if listed +- `HTTPS://APP.EXAMPLE.COM/` → compared as `https://app.example.com` ✅ allowed if listed +- `https://attacker.example.com` → not in allowlist ❌ rejected + +### `null`-origin handling + +The literal string `"null"` is sent by browsers for requests from sandboxed `