diff --git a/CHANGELOG.md b/CHANGELOG.md index a8e4e0a..072cdf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,13 @@ When preparing a new release: ### Added +- Monotonic, auditable archive/unarchive lifecycle for transfers: each + archive or unarchive appends an immutable `archiveHistory` event with + actor, reason, and a non-decreasing timestamp; `lastArchivedAt` retains + the prior archive instant after unarchive so reconciliation is not lost. + Optional `expectedUpdatedAt` (body or `If-Match`) rejects stale concurrent + commands with `409 STALE_ARCHIVE_COMMAND`. Audit log entries + `transfer.archived` / `transfer.unarchived` carry the same actor/reason. - Cursor pagination for `GET /api/transfers` and `GET /api/audit`. Pass `?cursor=` (with optional `?order=asc|desc`) to page by an indexed position instead of a row offset; responses carry a `pageInfo` block with @@ -55,6 +62,21 @@ When preparing a new release: ### Fixed +- Archive and unarchive HTTP requests preserve supplied non-string + `expectedUpdatedAt` values when there is no nonblank `If-Match` fallback. + The existing service check rejects those requests with + `409 STALE_ARCHIVE_COMMAND` instead of treating them as unversioned writes. + Body/header selection, omitted/null/blank optional values, actor attribution, + timestamp ordering, and no-token retries keep their existing behavior. + +- Archive and unarchive HTTP requests now store the existing keyed actor + fingerprint in lifecycle history and audit entries, keeping bearer credentials + out of transfer and audit responses while preserving attribution across cycles. + Fingerprints share the pagination signing secret and the process-local store's + lifetime; trusted service callers retain their explicit actor labels. Existing + event timestamps, reasons, request IDs, retries, and stale-command checks are + unchanged. + - Offset pagination over transfer and audit history repeated or skipped rows when records were written while a client was paging, because the window was defined by a row count rather than a position. Cursor pagination anchors to diff --git a/docs/validation/archive-reason-20261004.md b/docs/validation/archive-reason-20261004.md new file mode 100644 index 0000000..e9655a0 --- /dev/null +++ b/docs/validation/archive-reason-20261004.md @@ -0,0 +1,43 @@ +# Archive reason input validation — 2026-10-04 + +PR: https://github.com/RemitFlow/RemitFlow-Backend/pull/144 + +## Change + +Reject object and array `reason` values with a 400 validation error before archive or unarchive looks up a transfer. The previous `String(reason)` conversion could throw for valid JSON such as `{"toString":null}`; the application error handler then rendered an internal-error envelope (500). Plain objects and arrays could also become meaningless audit-reason strings. + +The three-line guard preserves the existing normalization of omitted, null, empty-string, string, number, and boolean reasons. This change does not alter archive history, timestamp ordering, or concurrency checks. + +## Source pins + +| Input | Git object | +| --- | --- | +| Baseline branch head | `a9d97a6d386119a8fe01d7cc867c70b09bd76355` | +| Original `src/services/transferService.js` | `0d421aeeac2cd3d7e9f131918c74d1cc086269bf` | +| Candidate `src/services/transferService.js` | `dc550d29e0fc0463de352ac834a835acac0c9b88` | +| `test/archiveReasonValidation.test.js` | `fd295a1489615673eaf11f7b7d2e44d280ddd94a` | +| Unchanged `src/utils/ApiError.js` | `6b586f60d991ab4e5174e0af4e0d94cf1e3ceb58` | +| Unchanged `src/middleware/errorHandler.js` | `e75a516bde4ec4052d39bea396298e9f2ac5c981` | +| Unchanged `src/services/errorTrackingService.js` | `205391bbd86d48478edc854bf4d5e4a4813596ac` | + +## Focused execution + +Node.js v24.19.0; the same three `node:test` cases executed once against each service version. + +| Case | Original | Candidate | +| --- | --- | --- | +| Reject structured reasons before transfer lookup | Failed: reached missing-transfer 404 | Passed: 400 for both actions | +| Preserve scalar normalization | Passed: retained missing-transfer 404 | Passed: retained missing-transfer 404 | +| Render malformed JSON reason through the application error handler | Failed: 500 instead of 400 | Passed: 400 validation envelope and request ID | + +Original: **1 passed, 2 failed**, exit 1. Candidate: **3 passed, 0 failed**, exit 0. Both executions had zero skipped or cancelled cases. + +The execution used `node --test-reporter=tap` with a CommonJS source loader in memory. It loaded the complete service, ApiError, error handler, and error-tracking source at the pins above, plus the unchanged regression-test source in both phases. Explicit collaborators were an empty transfer map, a logging sink, error tracking disabled in config, and empty unused service dependencies. The cases stop at reason normalization or missing-transfer lookup. + +This establishes the source and error-envelope behavior. It does not establish full Express routing, authentication, successful archive mutations, history integration, or full-suite acceptance. No dependency install or full-suite run was performed for this change. + +The maintained regression file can be run in a normal repository checkout with dependencies installed: + +```bash +node --test test/archiveReasonValidation.test.js +``` diff --git a/docs/validation/rfb144-body-precondition-20261004.md b/docs/validation/rfb144-body-precondition-20261004.md new file mode 100644 index 0000000..e0de050 --- /dev/null +++ b/docs/validation/rfb144-body-precondition-20261004.md @@ -0,0 +1,81 @@ +# PR 144 body-precondition acceptance — 4 October 2026 + +## Result + +The existing archive acceptance suites passed **46 tests, with 0 failures, skips, +or cancellations**, on source commit +`45d0bd4e8f1ad71a22ee3d759a84388ae2a9f968`. + +This completes the previously unexecuted native validation of the body-precondition +continuation on [RemitFlow-Backend PR 144](https://github.com/RemitFlow/RemitFlow-Backend/pull/144). +That continuation was published while its execution environment was unavailable. + +[Successful GitHub Actions run 37191775463](https://github.com/woahwhattheheck/RemitFlow-Backend/actions/runs/37191775463) +contains the complete command output. The acceptance job is `111405306802`. +It ran on the public contributor fork and checked out the exact source commit +above before installing or executing anything. + +## Execution + +Runtime: **Node.js v22.23.3**, **npm 10.9.9**, Linux x64, GitHub-hosted +`ubuntu-latest`. The existing repository CI also selects Node.js 22. + +The job used the unchanged manifest and lockfile with `npm ci`, followed by: + +```bash +node --test --test-concurrency=1 \ + test/transferArchive.test.js \ + test/transferArchiveLifecycle.test.js +``` + +The test runner reported a total duration of 589.94016 ms. This is the duration +of that suite invocation, excluding checkout and dependency installation; it is +not a backend throughput or latency benchmark. + +The existing HTTP cases execute the application through a local listener and +use its in-memory store and mock settlement adapter. The ten pending additions +all passed: + +- Eight malformed body-precondition cases cover number, boolean, array, and + object values for both archive and unarchive. They require + `409 STALE_ARCHIVE_COMMAND` and unchanged transfer history and audit records. +- Two compatibility groups cover optional values, trimmed body precedence, + bare and weak-quoted headers, valid header fallback for a non-string body, + and a stale header fallback. The existing successful and stale-command + behavior is preserved. + +All 36 earlier tests in these two suites also passed. No source correction was +needed after execution. + +## Source and artifact identities + +| Item | Identity | +| --- | --- | +| Tested repository | `woahwhattheheck/RemitFlow-Backend` | +| Tested source commit | `45d0bd4e8f1ad71a22ee3d759a84388ae2a9f968` | +| Validation workflow commit | `8ab10b252c90b28d161ad804b65fd0a22e1f95bb` | +| Package manifest Git blob | `a1bf7b6726b103f58de2e8cf6f2bab71fca6883b` | +| Package lock Git blob | `be3ad84b812121d2ec4f47541df3a55816bb210c` | +| Package lock SHA-256 | `74077a5a97f6548a4049aa9756e14dfbc04beab1c82b57923942ff0d99524894` | +| Locked dependencies | 76 | +| Actions artifact | `11299062132` / `rfb144-body-precondition-a993` | +| Artifact ZIP SHA-256 | `523eb61c2ef23e0f865b7387bef0dad14931688b2f7939995b9de88f5cfb8a8f` | +| Dependency tar SHA-256 | `0ae2e8dba5fb52549442edcae73b55a93d4e7f5d5141c842431373c565f09680` | + +The artifact contains `acceptance.tap`, `runtime-metadata.json`, and the +`node_modules.tar.gz` produced by that same lockfile installation. It expires +on **11 October 2026 at 09:19:31 UTC**. Its downloaded ZIP and dependency-tar +digests were verified. Reuse of these dependencies requires the matching +lockfile; the artifact does not establish execution of another source revision. + +## Limits + +Only the two named maintained suites ran in this continuation. There is no new +full-suite result, live-provider settlement result, deployment, or upstream +maintainer acceptance claim. The successful fork job does not replace upstream +CI approval. + +The original contribution remains on `latch/remitflow-126-archive-timestamps`. +This receipt is an additive documentation change; its evidence refers to the +tested source commit above. The separate validation workflow stays on +`validation/rfb144-body-precondition-a993`. diff --git a/src/controllers/transferController.js b/src/controllers/transferController.js index 4fb2e72..bdb91b5 100644 --- a/src/controllers/transferController.js +++ b/src/controllers/transferController.js @@ -2,6 +2,7 @@ const transferService = require('../services/transferService'); const { buildHistoryPage } = require('../utils/historyPage'); +const { actorFingerprint } = require('../utils/cursor'); const idempotencyService = require('../services/idempotencyService'); const ApiError = require('../utils/ApiError'); @@ -148,21 +149,64 @@ function cancelTransfer(req, res) { res.json(transfer); } +/** + * Read optional optimistic-concurrency token for archive mutations. + * Prefers body.expectedUpdatedAt; falls back to a bare If-Match header value. + * @param {import('express').Request} req + * @returns {string|undefined} + */ +function readExpectedUpdatedAt(req) { + const fromBody = req.body && req.body.expectedUpdatedAt; + if (typeof fromBody === 'string' && fromBody.trim() !== '') { + return fromBody.trim(); + } + const ifMatch = req.get('If-Match'); + if (typeof ifMatch === 'string' && ifMatch.trim() !== '') { + return ifMatch.trim().replace(/^W\//, '').replace(/^"|"$/g, ''); + } + // Preserve supplied non-string tokens for the service's strict freshness check. + return fromBody != null && typeof fromBody !== 'string' ? fromBody : undefined; +} + +/** + * Shared options for archive / unarchive mutations. + * @param {import('express').Request} req + * @returns {object} + */ +function archiveMutationOptions(req) { + const body = req.body || {}; + return { + requestId: req.id, + // History and audit responses must identify the caller without storing credentials. + actor: req.token ? actorFingerprint(req) : null, + reason: body.reason, + expectedUpdatedAt: readExpectedUpdatedAt(req), + }; +} + /** * POST /api/transfers/:id/archive * Archive a transfer, hiding it from default list results. + * Body may include `reason` and `expectedUpdatedAt` (optimistic concurrency). */ function archiveTransfer(req, res) { - const transfer = transferService.archiveTransfer(req.params.id); + const transfer = transferService.archiveTransfer( + req.params.id, + archiveMutationOptions(req) + ); res.json(transfer); } /** * POST /api/transfers/:id/unarchive * Unarchive a transfer, restoring it to default list results. + * Body may include `reason` and `expectedUpdatedAt` (optimistic concurrency). */ function unarchiveTransfer(req, res) { - const transfer = transferService.unarchiveTransfer(req.params.id); + const transfer = transferService.unarchiveTransfer( + req.params.id, + archiveMutationOptions(req) + ); res.json(transfer); } diff --git a/src/services/transferService.js b/src/services/transferService.js index 1517f62..941667b 100644 --- a/src/services/transferService.js +++ b/src/services/transferService.js @@ -12,10 +12,12 @@ const config = require('../config'); // Keep lifecycle timestamps strictly increasing even when multiple operations // happen within the same millisecond (common in tests and API batches). -function nextTimestamp(previous) { - const now = Date.now(); - const previousMs = previous ? Date.parse(previous) : NaN; - return new Date(Math.max(now, Number.isFinite(previousMs) ? previousMs + 1 : now)).toISOString(); +function nextTimestamp(...previous) { + const next = previous.reduce((latest, value) => { + const previousMs = value ? Date.parse(value) : NaN; + return Number.isFinite(previousMs) ? Math.max(latest, previousMs + 1) : latest; + }, Date.now()); + return new Date(next).toISOString(); } /** @@ -264,6 +266,10 @@ function createTransferUnchecked(data, requestId, idempotency) { createdAt: new Date().toISOString(), updatedAt: null, archivedAt: null, + lastArchivedAt: null, + // Append-only archive/unarchive events. Timestamps here are immutable once + // written so repeated archive cycles never erase earlier lifecycle order. + archiveHistory: [], }; transfer.updatedAt = nextTimestamp(transfer.createdAt); @@ -354,35 +360,197 @@ function cancelTransfer(id, requestId) { return transfer; } +/** + * Normalise archive/unarchive options. Accepts either an options object or a + * legacy bare requestId string so existing call sites keep working. + * @param {string|object} [optionsOrRequestId] + * @returns {{ requestId: string|undefined, actor: string|null, reason: string|null, expectedUpdatedAt: string|undefined }} + */ +function normaliseArchiveOptions(optionsOrRequestId) { + if (optionsOrRequestId == null) { + return { requestId: undefined, actor: null, reason: null, expectedUpdatedAt: undefined }; + } + if (typeof optionsOrRequestId === 'string') { + return { + requestId: optionsOrRequestId, + actor: null, + reason: null, + expectedUpdatedAt: undefined, + }; + } + const reason = optionsOrRequestId.reason; + if (reason !== null && typeof reason === 'object') { + throw ApiError.badRequest('reason must not be an object or array'); + } + return { + requestId: optionsOrRequestId.requestId, + actor: optionsOrRequestId.actor == null ? null : String(optionsOrRequestId.actor), + reason: reason == null || reason === '' ? null : String(reason), + expectedUpdatedAt: optionsOrRequestId.expectedUpdatedAt, + }; +} + +/** + * Reject a command whose caller observed a stale `updatedAt`. + * Omitting expectedUpdatedAt preserves the previous unversioned behaviour. + * @param {object} transfer + * @param {string|undefined} expectedUpdatedAt + */ +function assertFreshArchiveCommand(transfer, expectedUpdatedAt) { + if (expectedUpdatedAt == null) return; + if (transfer.updatedAt !== expectedUpdatedAt) { + throw ApiError.conflict( + `Stale archive command for transfer ${transfer.id}`, + { + code: 'STALE_ARCHIVE_COMMAND', + expectedUpdatedAt, + actualUpdatedAt: transfer.updatedAt, + archivedAt: transfer.archivedAt, + } + ); + } +} + +/** + * Latest immutable archive-history timestamp, used as a monotonic floor. + * @param {object} transfer + * @returns {string|null} + */ +function lastArchiveHistoryAt(transfer) { + const history = transfer.archiveHistory; + if (!Array.isArray(history) || history.length === 0) return null; + return history[history.length - 1].at; +} + +/** + * Append an immutable archive lifecycle event and return it. + * @param {object} transfer + * @param {object} event + * @returns {object} + */ +function appendArchiveHistory(transfer, event) { + if (!Array.isArray(transfer.archiveHistory)) { + transfer.archiveHistory = []; + } + const frozen = Object.freeze({ + action: event.action, + at: event.at, + actor: event.actor, + reason: event.reason, + requestId: event.requestId, + }); + transfer.archiveHistory.push(frozen); + return frozen; +} + /** * Archive a transfer. An archived transfer is hidden from default list results - * but remains queryable. Archiving is idempotent and orthogonal to the transfer - * lifecycle status. + * but remains queryable. Archiving is idempotent for retries that observe the + * current archived state, orthogonal to the transfer lifecycle status, and + * records an immutable, auditable event with actor and reason. + * + * Pass `expectedUpdatedAt` to opt into optimistic concurrency: a mismatched + * value is rejected as a stale command so concurrent archive/unarchive cycles + * cannot silently overwrite each other. + * * @param {string} id + * @param {string|object} [optionsOrRequestId] * @returns {object} */ -function archiveTransfer(id) { +function archiveTransfer(id, optionsOrRequestId) { + const options = normaliseArchiveOptions(optionsOrRequestId); const transfer = getTransferOrThrow(id); - if (!transfer.archivedAt) { - const timestamp = nextTimestamp(transfer.updatedAt); - transfer.archivedAt = timestamp; - transfer.updatedAt = timestamp; + assertFreshArchiveCommand(transfer, options.expectedUpdatedAt); + + // Already archived: idempotent retry. Do not move archivedAt / updatedAt + // backwards or rewrite history — that was the original failure mode. + if (transfer.archivedAt) { + return transfer; } + + const timestamp = nextTimestamp( + transfer.updatedAt, + lastArchiveHistoryAt(transfer) + ); + // Keep the current state and concurrency token intact if audit append fails. + auditService.addEntry({ + action: 'transfer.archived', + resourceId: transfer.id, + payload: { + archivedAt: timestamp, + actor: options.actor, + reason: options.reason, + }, + requestId: options.requestId, + }); + + transfer.archivedAt = timestamp; + transfer.lastArchivedAt = timestamp; + transfer.updatedAt = timestamp; + + appendArchiveHistory(transfer, { + action: 'archive', + at: timestamp, + actor: options.actor, + reason: options.reason, + requestId: options.requestId || null, + }); + return transfer; } /** * Unarchive a previously archived transfer, restoring it to default list results. + * Clears the current-state `archivedAt` flag but retains `lastArchivedAt` and + * an immutable history entry so the lifecycle order stays reconcilable. + * * @param {string} id + * @param {string|object} [optionsOrRequestId] * @returns {object} */ -function unarchiveTransfer(id) { +function unarchiveTransfer(id, optionsOrRequestId) { + const options = normaliseArchiveOptions(optionsOrRequestId); const transfer = getTransferOrThrow(id); + assertFreshArchiveCommand(transfer, options.expectedUpdatedAt); + if (!transfer.archivedAt) { - throw ApiError.conflict(`Transfer is not archived: ${id}`); + throw ApiError.conflict(`Transfer is not archived: ${id}`, { + code: 'NOT_ARCHIVED', + }); } + + const previousArchivedAt = transfer.archivedAt; + const timestamp = nextTimestamp( + transfer.updatedAt, + lastArchiveHistoryAt(transfer), + previousArchivedAt + ); + + // Keep the current state and concurrency token intact if audit append fails. + auditService.addEntry({ + action: 'transfer.unarchived', + resourceId: transfer.id, + payload: { + previousArchivedAt, + unarchivedAt: timestamp, + actor: options.actor, + reason: options.reason, + }, + requestId: options.requestId, + }); + transfer.archivedAt = null; - transfer.updatedAt = nextTimestamp(transfer.updatedAt); + transfer.lastArchivedAt = previousArchivedAt; + transfer.updatedAt = timestamp; + + appendArchiveHistory(transfer, { + action: 'unarchive', + at: timestamp, + actor: options.actor, + reason: options.reason, + requestId: options.requestId || null, + }); + return transfer; } diff --git a/test/archiveAuditRetry.test.js b/test/archiveAuditRetry.test.js new file mode 100644 index 0000000..d03c6f0 --- /dev/null +++ b/test/archiveAuditRetry.test.js @@ -0,0 +1,94 @@ +'use strict'; + +process.env.NODE_ENV = 'test'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const { once } = require('node:events'); +const createApp = require('../src/app'); +const config = require('../src/config'); +const { reset } = require('../src/store'); +const transferService = require('../src/services/transferService'); +const auditService = require('../src/services/auditService'); + +for (const action of ['archive', 'unarchive']) { + test(`HTTP ${action} preserves state and token after audit failure, then retries once`, async (t) => { + reset(); + const transfer = transferService.createTransfer({ + senderName: 'Alice', recipientName: 'Bob', amount: 100, from: 'USD', to: 'EUR', + }); + if (action === 'unarchive') transferService.archiveTransfer(transfer.id); + const before = JSON.parse(JSON.stringify(transfer)); + const beforeAudit = JSON.parse(JSON.stringify(auditService.getEntriesForResource(transfer.id))); + const originalTokens = config.apiTokens; + const token = 'archive-audit-invented-regression-token'; + config.apiTokens = { [token]: ['transfers:read', 'transfers:write'] }; + const server = createApp().listen(0, '127.0.0.1'); + await once(server, 'listening'); + const url = `http://127.0.0.1:${server.address().port}/api/transfers/${transfer.id}`; + const reason = `retry ${action} after audit recovery`; + const requestId = `audit-retry-${action}`; + const headers = { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json', + 'X-Request-Id': requestId, + Connection: 'close', + }; + const command = () => fetch(`${url}/${action}`, { + method: 'POST', headers, + body: JSON.stringify({ expectedUpdatedAt: before.updatedAt, reason }), + }); + const auditAction = action === 'archive' ? 'transfer.archived' : 'transfer.unarchived'; + let rejectedWrites = 0; + let rejectAudit = t.mock.method(auditService, 'addEntry', () => { + rejectedWrites += 1; + throw new Error('synthetic audit append unavailable'); + }); + try { + const rejected = await command(); + await rejected.json(); + assert.equal(rejected.status, 500); + assert.equal(rejectedWrites, 1); + const readback = await fetch(url, { headers }); + assert.equal(readback.status, 200); + assert.deepEqual(await readback.json(), before, + 'failed audit append must leave transfer state and history unchanged'); + assert.deepEqual(auditService.getEntriesForResource(transfer.id), beforeAudit); + + rejectAudit.mock.restore(); + rejectAudit = null; + const retried = await command(); + const after = await retried.json(); + assert.equal(retried.status, 200, 'the unchanged concurrency token must remain retryable'); + assert.ok(after.updatedAt > before.updatedAt); + assert.deepEqual(after.archiveHistory.slice(0, -1), before.archiveHistory); + assert.equal(after.archiveHistory.length, before.archiveHistory.length + 1); + const event = after.archiveHistory.at(-1); + assert.equal(event.action, action); + assert.equal(event.at, after.updatedAt); + assert.equal(event.reason, reason); + assert.equal(event.requestId, requestId); + if (action === 'archive') { + assert.equal(after.archivedAt, after.updatedAt); + assert.equal(after.lastArchivedAt, after.updatedAt); + } else { + assert.equal(after.archivedAt, null); + assert.equal(after.lastArchivedAt, before.archivedAt); + } + const entries = auditService.getEntriesForResource(transfer.id); + assert.equal(entries.length, beforeAudit.length + 1); + const matching = entries.filter((entry) => entry.action === auditAction); + assert.equal(matching.length, 1, 'successful retry must append exactly one audit entry'); + assert.equal(matching[0].requestId, requestId); + assert.equal(matching[0].payload.reason, reason); + assert.equal(matching[0].payload.actor, event.actor); + assert.equal(matching[0].payload[action === 'archive' ? 'archivedAt' : 'unarchivedAt'], after.updatedAt); + } finally { + if (rejectAudit) rejectAudit.mock.restore(); + config.apiTokens = originalTokens; + server.closeAllConnections(); + await new Promise((resolve) => server.close(resolve)); + reset(); + } + }); +} diff --git a/test/archiveReasonValidation.test.js b/test/archiveReasonValidation.test.js new file mode 100644 index 0000000..fd295a1 --- /dev/null +++ b/test/archiveReasonValidation.test.js @@ -0,0 +1,61 @@ +'use strict'; + +const { test } = require('node:test'); +const assert = require('node:assert/strict'); +const transferService = require('../src/services/transferService'); +const errorHandler = require('../src/middleware/errorHandler'); +const ApiError = require('../src/utils/ApiError'); + +const absentTransferId = 'archive-reason-validation-absent-transfer'; +const actions = ['archiveTransfer', 'unarchiveTransfer']; + +test('archive and unarchive reject structured reasons before transfer lookup', () => { + for (const action of actions) { + for (const reason of [{}, [], ['retention'], { toString: null }, { toString: [], valueOf: false }]) { + assert.throws( + () => transferService[action](absentTransferId, { reason }), + error => error instanceof ApiError && error.statusCode === 400 && /reason/.test(error.message), + action + ': structured reason must be an input error', + ); + } + } +}); + +test('existing scalar reasons continue through normalization to transfer lookup', () => { + for (const action of actions) { + for (const reason of [undefined, null, '', 'retention review', 0, 7, false, true]) { + assert.throws( + () => transferService[action](absentTransferId, { reason }), + error => error instanceof ApiError && error.statusCode === 404, + action + ': scalar reason must retain the missing-transfer result', + ); + } + } +}); + +test('malformed JSON reason renders a validation envelope instead of an internal error', () => { + for (const action of actions) { + let error; + try { + transferService[action](absentTransferId, { reason: JSON.parse('{"toString":null}') }); + } catch (caught) { + error = caught; + } + assert.ok(error); + let status; + let body; + const response = { + status(value) { status = value; return this; }, + json(value) { body = value; return this; }, + }; + errorHandler(error, { + id: 'archive-reason-regression', + method: 'POST', + originalUrl: '/api/transfers/' + absentTransferId + '/' + action, + }, response, () => assert.fail('error handler must finish the response')); + assert.equal(status, 400); + assert.equal(body.error.status, 400); + assert.equal(body.error.requestId, 'archive-reason-regression'); + assert.match(body.error.message, /reason/); + } +}); diff --git a/test/transferArchiveLifecycle.test.js b/test/transferArchiveLifecycle.test.js new file mode 100644 index 0000000..e377852 --- /dev/null +++ b/test/transferArchiveLifecycle.test.js @@ -0,0 +1,654 @@ +'use strict'; + +const { test, beforeEach } = require('node:test'); +const assert = require('node:assert/strict'); +const { once } = require('node:events'); + +process.env.NODE_ENV = 'test'; + +const { reset } = require('../src/store'); +const transferService = require('../src/services/transferService'); +const auditService = require('../src/services/auditService'); +const ApiError = require('../src/utils/ApiError'); +const config = require('../src/config'); +const createApp = require('../src/app'); + +beforeEach(() => { + reset(); +}); + +function createSample() { + return transferService.createTransfer({ + senderName: 'Alice', + recipientName: 'Bob', + amount: 100, + from: 'USD', + to: 'EUR', + }); +} + +function assertStrictlyIncreasing(timestamps) { + for (let i = 1; i < timestamps.length; i += 1) { + assert.ok( + timestamps[i] > timestamps[i - 1], + `expected ${timestamps[i]} > ${timestamps[i - 1]} at index ${i}` + ); + } +} + +const archiveWriterA = 'archive-lifecycle-invented-local-writer-a'; +const archiveWriterB = 'archive-lifecycle-invented-local-writer-b'; +const archiveReader = 'archive-lifecycle-invented-local-reader'; + +async function startArchiveHttp(t) { + const originalTokens = config.apiTokens; + config.apiTokens = { + [archiveWriterA]: ['transfers:write'], + [archiveWriterB]: ['transfers:write'], + [archiveReader]: ['transfers:read', 'audit:read'], + }; + const server = createApp().listen(0, '127.0.0.1'); + t.after(async () => { + config.apiTokens = originalTokens; + server.closeAllConnections(); + await new Promise((resolve) => server.close(resolve)); + }); + await once(server, 'listening'); + const baseUrl = `http://127.0.0.1:${server.address().port}/api`; + + return async (path, { token, method = 'GET', body, requestId, ifMatch } = {}) => { + const headers = { Authorization: `Bearer ${token}`, Connection: 'close' }; + if (body) headers['Content-Type'] = 'application/json'; + if (requestId) headers['X-Request-Id'] = requestId; + if (ifMatch !== undefined) headers['If-Match'] = ifMatch; + const response = await fetch(`${baseUrl}${path}`, { + method, + headers, + body: body ? JSON.stringify(body) : undefined, + }); + return { status: response.status, body: await response.json() }; + }; +} + +function assertNoArchiveCredentials(value) { + const serialized = JSON.stringify(value); + for (const token of [archiveWriterA, archiveWriterB, archiveReader]) { + assert.equal(serialized.includes(token), false, 'stored and readable records must omit bearer credentials'); + } +} + +// ============================================================================ +// State machine +// ============================================================================ + +test('state-machine: active → archived → active is the only archive cycle', () => { + const transfer = createSample(); + assert.equal(transfer.archivedAt, null); + assert.deepEqual(transfer.archiveHistory, []); + + const archived = transferService.archiveTransfer(transfer.id, { + actor: 'ops-token', + reason: 'retention-hold', + requestId: 'req-1', + }); + assert.ok(archived.archivedAt); + assert.equal(archived.archiveHistory.length, 1); + assert.equal(archived.archiveHistory[0].action, 'archive'); + + const active = transferService.unarchiveTransfer(transfer.id, { + actor: 'ops-token', + reason: 'hold-lifted', + requestId: 'req-2', + }); + assert.equal(active.archivedAt, null); + assert.equal(active.archiveHistory.length, 2); + assert.equal(active.archiveHistory[1].action, 'unarchive'); + + assert.throws( + () => transferService.unarchiveTransfer(transfer.id), + (err) => err instanceof ApiError && err.statusCode === 409 + ); +}); + +test('state-machine: archive is orthogonal to claim/cancel status', () => { + const claimed = createSample(); + transferService.claimTransfer(claimed.id); + transferService.archiveTransfer(claimed.id, { actor: 'a', reason: 'cleanup' }); + assert.equal(claimed.status, 'claimed'); + assert.ok(claimed.archivedAt); + + const cancelled = createSample(); + transferService.cancelTransfer(cancelled.id); + transferService.archiveTransfer(cancelled.id, { actor: 'a', reason: 'cleanup' }); + assert.equal(cancelled.status, 'cancelled'); + assert.ok(cancelled.archivedAt); +}); + +// ============================================================================ +// Stale-command rejection +// ============================================================================ + +test('stale-command: archive rejects mismatched expectedUpdatedAt', () => { + const transfer = createSample(); + const stale = transfer.updatedAt; + + // Advance updatedAt via an intervening claim so the original token is stale. + transferService.claimTransfer(transfer.id); + assert.notEqual(transfer.updatedAt, stale); + + assert.throws( + () => transferService.archiveTransfer(transfer.id, { + expectedUpdatedAt: stale, + actor: 'ops', + reason: 'stale-try', + }), + (err) => ( + err instanceof ApiError && + err.statusCode === 409 && + err.details && + err.details.code === 'STALE_ARCHIVE_COMMAND' && + err.details.expectedUpdatedAt === stale && + err.details.actualUpdatedAt === transfer.updatedAt + ) + ); + + assert.equal(transfer.archivedAt, null); + assert.equal(transfer.archiveHistory.length, 0); +}); + +test('stale-command: unarchive rejects mismatched expectedUpdatedAt', () => { + const transfer = createSample(); + transferService.archiveTransfer(transfer.id, { actor: 'ops', reason: 'park' }); + const fresh = transfer.updatedAt; + + assert.throws( + () => transferService.unarchiveTransfer(transfer.id, { + expectedUpdatedAt: '1999-01-01T00:00:00.000Z', + actor: 'ops', + reason: 'stale-unarchive', + }), + (err) => ( + err instanceof ApiError && + err.statusCode === 409 && + err.details.code === 'STALE_ARCHIVE_COMMAND' + ) + ); + + // Matching token still succeeds. + const result = transferService.unarchiveTransfer(transfer.id, { + expectedUpdatedAt: fresh, + actor: 'ops', + reason: 'release', + }); + assert.equal(result.archivedAt, null); +}); + +// ============================================================================ +// Race / optimistic concurrency +// ============================================================================ + +test('race: second concurrent archive with the same expectedUpdatedAt loses', () => { + const transfer = createSample(); + const observed = transfer.updatedAt; + + const winner = transferService.archiveTransfer(transfer.id, { + expectedUpdatedAt: observed, + actor: 'worker-a', + reason: 'race-win', + requestId: 'r-a', + }); + assert.ok(winner.archivedAt); + + assert.throws( + () => transferService.archiveTransfer(transfer.id, { + expectedUpdatedAt: observed, + actor: 'worker-b', + reason: 'race-lose', + requestId: 'r-b', + }), + (err) => ( + err instanceof ApiError && + err.statusCode === 409 && + err.details.code === 'STALE_ARCHIVE_COMMAND' + ) + ); + + // Winner's timestamp and single history event survive the loser. + assert.equal(transfer.archivedAt, winner.archivedAt); + assert.equal(transfer.archiveHistory.length, 1); + assert.equal(transfer.archiveHistory[0].actor, 'worker-a'); +}); + +test('race: archive then unarchive with stale token cannot reorder history', () => { + const transfer = createSample(); + const beforeArchive = transfer.updatedAt; + + transferService.archiveTransfer(transfer.id, { + expectedUpdatedAt: beforeArchive, + actor: 'a', + reason: 'first', + }); + const afterArchive = transfer.updatedAt; + + transferService.unarchiveTransfer(transfer.id, { + expectedUpdatedAt: afterArchive, + actor: 'a', + reason: 'second', + }); + + assert.throws( + () => transferService.archiveTransfer(transfer.id, { + expectedUpdatedAt: beforeArchive, + actor: 'b', + reason: 'stale-reorder', + }), + (err) => err instanceof ApiError && err.details.code === 'STALE_ARCHIVE_COMMAND' + ); + + assert.equal(transfer.archiveHistory.length, 2); + assert.equal(transfer.archivedAt, null); +}); + +// ============================================================================ +// Retry / idempotency +// ============================================================================ + +test('retry: re-archive without expectedUpdatedAt does not overwrite archivedAt', () => { + const transfer = createSample(); + const first = transferService.archiveTransfer(transfer.id, { + actor: 'ops', + reason: 'initial', + requestId: 'req-1', + }); + const archivedAt = first.archivedAt; + const updatedAt = first.updatedAt; + const historyLen = first.archiveHistory.length; + + const retry = transferService.archiveTransfer(transfer.id, { + actor: 'ops', + reason: 'retry', + requestId: 'req-1-retry', + }); + + assert.equal(retry.archivedAt, archivedAt); + assert.equal(retry.updatedAt, updatedAt); + assert.equal(retry.archiveHistory.length, historyLen); +}); + +test('retry: matching expectedUpdatedAt on already-archived transfer is a no-op', () => { + const transfer = createSample(); + transferService.archiveTransfer(transfer.id, { actor: 'ops', reason: 'park' }); + const token = transfer.updatedAt; + const archivedAt = transfer.archivedAt; + + const retry = transferService.archiveTransfer(transfer.id, { + expectedUpdatedAt: token, + actor: 'ops', + reason: 'retry-same', + }); + + assert.equal(retry.archivedAt, archivedAt); + assert.equal(retry.updatedAt, token); + assert.equal(retry.archiveHistory.length, 1); +}); + +// ============================================================================ +// Event order / monotonic timestamps +// ============================================================================ + +test('event-order: archive → unarchive → archive keeps strictly increasing timestamps', () => { + const transfer = createSample(); + + transferService.archiveTransfer(transfer.id, { + actor: 'ops', + reason: 'cycle-1', + requestId: 'e1', + }); + const firstArchivedAt = transfer.archivedAt; + + transferService.unarchiveTransfer(transfer.id, { + actor: 'ops', + reason: 'cycle-1-lift', + requestId: 'e2', + }); + assert.equal(transfer.lastArchivedAt, firstArchivedAt); + + transferService.archiveTransfer(transfer.id, { + actor: 'ops', + reason: 'cycle-2', + requestId: 'e3', + }); + + assert.equal(transfer.archiveHistory.length, 3); + const ats = transfer.archiveHistory.map((e) => e.at); + assertStrictlyIncreasing(ats); + assert.ok(transfer.archivedAt > firstArchivedAt); + assert.equal(transfer.archiveHistory[0].at, firstArchivedAt); + // Original archive timestamp is preserved in history (not overwritten). + assert.equal(transfer.archiveHistory[0].action, 'archive'); + assert.equal(transfer.archiveHistory[1].action, 'unarchive'); + assert.equal(transfer.archiveHistory[2].action, 'archive'); +}); + +test('event-order: history event timestamps are immutable after write', () => { + const transfer = createSample(); + transferService.archiveTransfer(transfer.id, { actor: 'ops', reason: 'lock' }); + const event = transfer.archiveHistory[0]; + const originalAt = event.at; + + assert.throws(() => { + event.at = '1999-01-01T00:00:00.000Z'; + }, TypeError); + + assert.equal(event.at, originalAt); +}); + +test('event-order: updatedAt never moves backward across archive lifecycle', () => { + const transfer = createSample(); + const stamps = [transfer.createdAt, transfer.updatedAt]; + + transferService.archiveTransfer(transfer.id, { actor: 'a', reason: 'r1' }); + stamps.push(transfer.updatedAt); + transferService.unarchiveTransfer(transfer.id, { actor: 'a', reason: 'r2' }); + stamps.push(transfer.updatedAt); + transferService.archiveTransfer(transfer.id, { actor: 'a', reason: 'r3' }); + stamps.push(transfer.updatedAt); + + assertStrictlyIncreasing(stamps); +}); + +// ============================================================================ +// Auditability (actor + reason) +// ============================================================================ + +test('audit: archive and unarchive record actor and reason', () => { + const transfer = createSample(); + + transferService.archiveTransfer(transfer.id, { + actor: 'auditor-token', + reason: 'compliance-review', + requestId: 'req-arch', + }); + transferService.unarchiveTransfer(transfer.id, { + actor: 'auditor-token', + reason: 'review-complete', + requestId: 'req-unarch', + }); + + const entries = auditService.getEntriesForResource(transfer.id); + const archived = entries.find((e) => e.action === 'transfer.archived'); + const unarchived = entries.find((e) => e.action === 'transfer.unarchived'); + + assert.ok(archived); + assert.equal(archived.payload.actor, 'auditor-token'); + assert.equal(archived.payload.reason, 'compliance-review'); + assert.equal(archived.requestId, 'req-arch'); + + assert.ok(unarchived); + assert.equal(unarchived.payload.actor, 'auditor-token'); + assert.equal(unarchived.payload.reason, 'review-complete'); + assert.ok(unarchived.payload.previousArchivedAt); + + assert.equal(transfer.archiveHistory[0].actor, 'auditor-token'); + assert.equal(transfer.archiveHistory[0].reason, 'compliance-review'); + assert.equal(transfer.archiveHistory[1].reason, 'review-complete'); +}); + +test('HTTP archive cycles retain distinct, stable actors without exposing writer credentials', async (t) => { + const request = await startArchiveHttp(t); + const transfer = createSample(); + let version = transfer.updatedAt; + const commands = [ + { action: 'archive', token: archiveWriterA, reason: 'review' }, + { action: 'unarchive', token: archiveWriterA, reason: 'review-complete' }, + { action: 'archive', token: archiveWriterB, reason: 'second-review' }, + ]; + + for (const [index, command] of commands.entries()) { + const requestId = `archive-http-${index}`; + const response = await request(`/transfers/${transfer.id}/${command.action}`, { + token: command.token, + method: 'POST', + body: { reason: command.reason, expectedUpdatedAt: version }, + requestId, + }); + assert.equal(response.status, 200); + const event = response.body.archiveHistory[index]; + assert.match(event.actor, /^[a-f0-9]{16}$/); + assert.equal(event.reason, command.reason); + assert.equal(event.requestId, requestId); + assert.equal(event.at, response.body.updatedAt); + assert.ok(response.body.updatedAt > version); + assertNoArchiveCredentials(response.body); + version = response.body.updatedAt; + } + + const actors = transfer.archiveHistory.map((event) => event.actor); + assert.equal(actors[0], actors[1], 'the same writer retains one actor across both endpoints'); + assert.notEqual(actors[0], actors[2], 'different writers remain distinguishable'); + const entries = auditService.getEntriesForResource(transfer.id) + .filter((entry) => entry.action !== 'transfer.created').reverse(); + assert.deepEqual(entries.map((entry) => entry.payload.actor), actors); + assert.deepEqual(entries.map((entry) => entry.payload.reason), commands.map((command) => command.reason)); + assert.deepEqual(entries.map((entry) => entry.requestId), ['archive-http-0', 'archive-http-1', 'archive-http-2']); + assertNoArchiveCredentials(transfer); + assertNoArchiveCredentials(entries); + + for (const path of [ + `/transfers/${transfer.id}`, + '/transfers?archived=all', + `/audit?resourceId=${transfer.id}`, + ]) { + const response = await request(path, { token: archiveReader }); + assert.equal(response.status, 200); + assertNoArchiveCredentials(response.body); + } +}); + +test('HTTP unarchive fingerprints its writer and preserves legacy service actor labels', async (t) => { + const request = await startArchiveHttp(t); + const transfer = createSample(); + transferService.archiveTransfer(transfer.id, { + actor: 'backoffice-job', + reason: 'legacy-review', + requestId: 'legacy-request', + }); + const originalEvent = { ...transfer.archiveHistory[0] }; + + const response = await request(`/transfers/${transfer.id}/unarchive`, { + token: archiveWriterB, + method: 'POST', + body: { reason: 'restored-over-http', expectedUpdatedAt: transfer.updatedAt }, + requestId: 'http-restore', + }); + assert.equal(response.status, 200); + assert.equal(response.body.archivedAt, null); + assert.deepEqual(response.body.archiveHistory[0], originalEvent); + const event = response.body.archiveHistory[1]; + assert.match(event.actor, /^[a-f0-9]{16}$/); + assert.equal(event.reason, 'restored-over-http'); + assert.equal(event.requestId, 'http-restore'); + assertNoArchiveCredentials(response.body); + + const entries = auditService.getEntriesForResource(transfer.id); + assert.equal(entries.find((entry) => entry.action === 'transfer.archived').payload.actor, 'backoffice-job'); + assert.equal(entries.find((entry) => entry.action === 'transfer.unarchived').payload.actor, event.actor); + assertNoArchiveCredentials(transfer); + assertNoArchiveCredentials(entries); +}); + +// ============================================================================ +// Regression: original failure mode +// ============================================================================ + +test('regression: interleaved status and archive commands cannot reuse a version token', (t) => { + const now = Date.now(); + t.mock.method(Date, 'now', () => now); + + for (const changeStatus of ['claimTransfer', 'cancelTransfer']) { + for (const initiallyArchived of [true, false]) { + const transfer = createSample(); + transferService.archiveTransfer(transfer.id); + if (!initiallyArchived) transferService.unarchiveTransfer(transfer.id); + + transferService[changeStatus](transfer.id); + const observed = transfer.updatedAt; + const action = initiallyArchived ? 'unarchiveTransfer' : 'archiveTransfer'; + const staleAction = initiallyArchived ? 'archiveTransfer' : 'unarchiveTransfer'; + + transferService[action](transfer.id, { expectedUpdatedAt: observed }); + assert.ok( + transfer.updatedAt > observed, + `${changeStatus} followed by ${action} must advance updatedAt` + ); + const historyLength = transfer.archiveHistory.length; + const updatedAt = transfer.updatedAt; + + assert.throws( + () => transferService[staleAction](transfer.id, { expectedUpdatedAt: observed }), + (err) => err instanceof ApiError && err.details.code === 'STALE_ARCHIVE_COMMAND' + ); + assert.equal(transfer.updatedAt, updatedAt); + assert.equal(transfer.archiveHistory.length, historyLength); + } + } +}); + +test('regression: archive transitions respect history and legacy archive timestamp floors', (t) => { + const now = Date.now(); + t.mock.method(Date, 'now', () => now - 60_000); + + const transfer = createSample(); + transferService.archiveTransfer(transfer.id); + transferService.unarchiveTransfer(transfer.id); + const historyAt = transfer.archiveHistory.at(-1).at; + transfer.updatedAt = transfer.createdAt; + transferService.archiveTransfer(transfer.id); + assert.ok(transfer.updatedAt > historyAt); + + const legacy = createSample(); + legacy.archivedAt = new Date(now + 60_000).toISOString(); + const previousArchivedAt = legacy.archivedAt; + transferService.unarchiveTransfer(legacy.id); + assert.ok(legacy.updatedAt > previousArchivedAt); + assert.equal(legacy.lastArchivedAt, previousArchivedAt); + assert.equal(legacy.archiveHistory[0].at, legacy.updatedAt); +}); + +test('regression: repeated archive/unarchive does not hide earlier lifecycle timestamps', () => { + const transfer = createSample(); + + transferService.archiveTransfer(transfer.id, { + actor: 'ops', + reason: 'first-archive', + }); + const firstAt = transfer.archivedAt; + + transferService.unarchiveTransfer(transfer.id, { + actor: 'ops', + reason: 'temporary-restore', + }); + // Current flag is cleared, but lastArchivedAt and history retain the order. + assert.equal(transfer.archivedAt, null); + assert.equal(transfer.lastArchivedAt, firstAt); + assert.equal(transfer.archiveHistory[0].at, firstAt); + + transferService.archiveTransfer(transfer.id, { + actor: 'ops', + reason: 'second-archive', + }); + const secondAt = transfer.archivedAt; + + assert.ok(secondAt > firstAt); + // History still starts with the first archive — not overwritten. + assert.equal(transfer.archiveHistory[0].at, firstAt); + assert.equal(transfer.archiveHistory[0].reason, 'first-archive'); + assert.equal(transfer.archiveHistory[2].at, secondAt); + assert.equal(transfer.archiveHistory[2].reason, 'second-archive'); + assertStrictlyIncreasing(transfer.archiveHistory.map((e) => e.at)); +}); + +// ============================================================================ +// HTTP optimistic-concurrency token preservation +// ============================================================================ + +for (const action of ['archive', 'unarchive']) { + for (const [kind, value] of [ + ['number', 0], + ['boolean', false], + ['array', []], + ['object', {}], + ]) { + test(`HTTP concurrency token: ${action} rejects a supplied ${kind} without changing history`, async (t) => { + const request = await startArchiveHttp(t); + const transfer = createSample(); + if (action === 'unarchive') transferService.archiveTransfer(transfer.id); + const before = JSON.parse(JSON.stringify(transfer)); + const beforeAudit = JSON.parse(JSON.stringify(auditService.getEntriesForResource(transfer.id))); + + const response = await request(`/transfers/${transfer.id}/${action}`, { + token: archiveWriterA, + method: 'POST', + body: { expectedUpdatedAt: value, reason: 'stale-type-should-not-write' }, + }); + + assert.equal(response.status, 409); + assert.equal(response.body.error.details.code, 'STALE_ARCHIVE_COMMAND'); + assert.deepEqual(response.body.error.details.expectedUpdatedAt, value); + assert.deepEqual(JSON.parse(JSON.stringify(transferService.getTransferOrThrow(transfer.id))), before); + assert.deepEqual(JSON.parse(JSON.stringify(auditService.getEntriesForResource(transfer.id))), beforeAudit); + const readback = await request(`/transfers/${transfer.id}`, { token: archiveReader }); + assert.equal(readback.status, 200); + assert.deepEqual(readback.body, before); + }); + } + + test(`HTTP concurrency token: ${action} preserves optional values and body/header selection`, async (t) => { + const request = await startArchiveHttp(t); + const stale = '1999-01-01T00:00:00.000Z'; + const cases = [ + { name: 'omitted', options: () => ({ body: { reason: 'optional-token' } }) }, + { name: 'null', options: () => ({ body: { expectedUpdatedAt: null } }) }, + { name: 'empty', options: () => ({ body: { expectedUpdatedAt: '' } }) }, + { name: 'whitespace', options: () => ({ body: { expectedUpdatedAt: ' ' } }) }, + { name: 'trimmed body wins', options: version => ({ + body: { expectedUpdatedAt: ` ${version} ` }, ifMatch: stale, + }) }, + { name: 'bare header', options: version => ({ ifMatch: version }) }, + { name: 'weak quoted header', options: version => ({ ifMatch: `W/"${version}"` }) }, + { name: 'header fallback with non-string body', options: version => ({ + body: { expectedUpdatedAt: false }, ifMatch: version, + }) }, + ]; + + for (const scenario of cases) { + const transfer = createSample(); + if (action === 'unarchive') transferService.archiveTransfer(transfer.id); + const historyLength = transfer.archiveHistory.length; + const response = await request(`/transfers/${transfer.id}/${action}`, { + token: archiveWriterA, + method: 'POST', + ...scenario.options(transfer.updatedAt), + }); + assert.equal(response.status, 200, scenario.name); + assert.equal(response.body.archiveHistory.length, historyLength + 1, scenario.name); + assert.equal(response.body.archiveHistory.at(-1).action, action, scenario.name); + } + + const transfer = createSample(); + if (action === 'unarchive') transferService.archiveTransfer(transfer.id); + const before = JSON.parse(JSON.stringify(transfer)); + const beforeAudit = JSON.parse(JSON.stringify(auditService.getEntriesForResource(transfer.id))); + const response = await request(`/transfers/${transfer.id}/${action}`, { + token: archiveWriterA, + method: 'POST', + body: { expectedUpdatedAt: 0 }, + ifMatch: stale, + }); + assert.equal(response.status, 409); + assert.equal(response.body.error.details.code, 'STALE_ARCHIVE_COMMAND'); + assert.equal(response.body.error.details.expectedUpdatedAt, stale); + assert.deepEqual(JSON.parse(JSON.stringify(transferService.getTransferOrThrow(transfer.id))), before); + assert.deepEqual(JSON.parse(JSON.stringify(auditService.getEntriesForResource(transfer.id))), beforeAudit); + }); +}