diff --git a/README.md b/README.md index ce29578..04b6056 100644 --- a/README.md +++ b/README.md @@ -224,12 +224,13 @@ origin a browser sends, and is refused at boot rather than accepted as a list that allows nothing. A browser reads a cross-origin response only if the service names the requesting origin back, so an origin missing from this list fails in the page with an opaque network error. What reaches the service -differs by route: the three `GET` routes are simple requests, delivered and -answered in full with only the browser withholding the body from the page, -while every `POST` route carries a JSON body and is therefore preflighted, and -a refused preflight ends the call before the `POST` is ever sent. Neither -leaves anything behind that names the origin, because the service logs no -requests at all; the list it accepted is on its startup line instead. +differs by method: a `GET` is a simple request, delivered and answered in full +with only the browser withholding the body from the page, while every `POST` +route carries a JSON body and is therefore preflighted, and a refused +preflight ends the call before the `POST` is ever sent. Neither leaves +anything behind that names the origin: the only line a request ever writes is +the one the error handler logs on a 5xx, and it carries the method and the +path but no origin. The list it accepted is on its startup line instead. Each entry is written as a browser computes an origin, `http(s)://host` with a port only when it is not the scheme's default and with no path, query or trailing slash; the service refuses to start on anything else, since the diff --git a/RUNBOOK.md b/RUNBOOK.md index 72136c0..4178b1f 100644 --- a/RUNBOOK.md +++ b/RUNBOOK.md @@ -287,8 +287,9 @@ reason the seed script looks the way it does. of `*` anywhere in the list means any origin, so there is no value that allows none. A refused origin leaves nothing here to find: a simple request is served in full and only the browser withholds the body, a preflighted one - never arrives at all, and the service logs no requests either way. The list - it accepted is on the startup line instead. + never arrives at all, and no log line names the origin either way, since the + only line a request ever writes is the error handler's on a 5xx, carrying the + method and the path. The list it accepted is on the startup line instead. - `LEDGER_USER_ID` has no effect on the running service, which submits nothing. The seed prints it as a record of the user it submitted under, not as an input the service reads back. Setting it in `registry/.env` changes nothing at all: diff --git a/SPEC.md b/SPEC.md index be32d20..c43f99f 100644 --- a/SPEC.md +++ b/SPEC.md @@ -53,7 +53,7 @@ All three suites were re-run against the tree this document ships with, exit 0: | Suite | Result | Needs | |---|---|---| | Daml Script | **80 scenarios**, 12 modules | nothing, runs in-process | -| Registry unit | **245 tests**, 11 files | nothing, in-process server with a stub ledger | +| Registry unit | **246 tests**, 11 files | nothing, in-process server with a stub ledger | | End-to-end | **18 tests**, 4 files | a live participant, verified against Canton 3.5.12 | The end-to-end suite drives both transfer paths against a real participant: it @@ -63,8 +63,8 @@ resulting exercise itself over the JSON Ledger API, forwarding the service's ### Size and status -976 lines of production Daml, 2508 lines of Daml tests, 1840 lines of -TypeScript service, 4665 lines of TypeScript tests, each figure a +976 lines of production Daml, 2508 lines of Daml tests, 1846 lines of +TypeScript service, 4693 lines of TypeScript tests, each figure a `find -name '*.daml'` (or `'*.ts'`) `| xargs wc -l` count over `daml/canton-token-forge/daml`, `daml/canton-token-forge-test/daml`, `registry/src` and `registry/test` respectively. The two Daml paths name the @@ -449,7 +449,7 @@ exist. | Level | What it covers | |---|---| | Daml Script, 80 scenarios | Every choice and both factory paths, including negative cases: wrong `expectedAdmin`, a batch transfer routed through another instrument of the same admin, non-positive amounts, duplicate and locked inputs, cross-instrument spending, an escrow that does not back the transfer it settles, both sides of every deadline instant, missing authority, the `decimals` bound, and the batch transfer's own refusals: outputs whose total exceeds the inputs and a lock output already past its expiry | -| Registry unit, 245 tests | Every route against an in-process server with a stub ledger: response shapes, error schemas, 404 and 409 behaviour, context and disclosure contents, the state an escrow lookup has to be in before a context may report a reclaim, config validation, that a configured browser origin is answered and an unconfigured one is not, on rejections as well as on successes, that an entry no browser could ever send is refused at boot, a pattern and a scheme a browser sends no Origin in included, rather than accepted as a list that allows nothing, that no response allows credentials under either origin mode, that a simple request from an unconfigured origin is served in full regardless and refused only in the browser, a preflighted one being stopped in the browser before it is sent, that a path the service does not route answers a preflight all the same, and that each request is validated against the one spec that describes it, whichever form its request target arrives in and even when it carries a fragment, which is no form at all | +| Registry unit, 246 tests | Every route against an in-process server with a stub ledger: response shapes, error schemas, 404 and 409 behaviour, context and disclosure contents, the state an escrow lookup has to be in before a context may report a reclaim, config validation, that a configured browser origin is answered and an unconfigured one is not, on rejections as well as on successes, that an entry no browser could ever send is refused at boot, a pattern and a scheme a browser sends no Origin in included, rather than accepted as a list that allows nothing, that no response allows credentials under either origin mode, that a simple request from an unconfigured origin is served in full regardless and refused only in the browser, a preflighted one being stopped in the browser before it is sent, that a path the service does not route answers a preflight all the same, that the only line a request leaves in the log names its method and path and never the origin it came from, and that each request is validated against the one spec that describes it, whichever form its request target arrives in and even when it carries a fragment, which is no form at all | | End-to-end, 18 tests | Both transfer paths and the faucet against a live participant, submitting real exercises built from the service's own answers, including a misconfigured escrow template id that must not produce a reclaim report | The end-to-end suite allocates its own parties and instrument per run, so it @@ -467,7 +467,7 @@ instrument, then prints a ready-to-paste service configuration. ```bash npm run setup # vendors the Splice interface DARs into deps/ npm test # builds the production DAR, runs 80 Daml scenarios -cd registry && npm install && npm test # 245 unit tests, no ledger needed +cd registry && npm install && npm test # 246 unit tests, no ledger needed npm run sandbox # a local Canton sandbox with the JSON Ledger API npm run seed # an admin, demo users, one instrument diff --git a/registry/src/config.ts b/registry/src/config.ts index 5c225b5..0470f65 100644 --- a/registry/src/config.ts +++ b/registry/src/config.ts @@ -41,7 +41,12 @@ const DEFAULT_DIRECT_TRANSFER_MARGIN_MS = 30_000 const MAX_DIRECT_TRANSFER_MARGIN_MS = 3_600_000 // The dApp dev server the CORS report was filed from, so the reported case -// works with no configuration. Any real deployment sets CORS_ORIGINS itself. +// works with no configuration. Any real deployment sets CORS_ORIGINS itself, +// and a deployment that leaves the default in place hands a dev origin nothing +// it could not already have: the service authenticates no client, so this list +// decides which pages a browser will hand a response to, never who may ask for +// one. A permissive default is therefore a usability choice here rather than an +// access-control one, which is also why no value of this variable allows none. const DEFAULT_CORS_ORIGINS = 'http://localhost:3012' export function loadConfig(env: NodeJS.ProcessEnv): Config { diff --git a/registry/src/index.ts b/registry/src/index.ts index 1359934..83d499a 100644 --- a/registry/src/index.ts +++ b/registry/src/index.ts @@ -32,9 +32,10 @@ const server = app.listen(config.port, () => { // service side. A refused simple request is delivered and answered in full, // with only the browser withholding the body afterwards, while a refused // preflight ends its POST before it is sent, so what arrives differs but - // neither leaves a trace: nothing here is logged per request. Recording what - // was accepted at boot is what lets an operator tell a rejected origin from - // an unreachable service. + // neither leaves a trace that names the origin: the only line a request ever + // writes is the error handler's, on a 5xx, and it carries the method and the + // path and no origin at all. Recording what was accepted at boot is what lets + // an operator tell a rejected origin from an unreachable service. logger.info( { port: config.port, corsOrigins: config.corsOrigins }, 'canton-token-forge registry listening', diff --git a/registry/test/cors.test.ts b/registry/test/cors.test.ts index b935099..64e05fb 100644 --- a/registry/test/cors.test.ts +++ b/registry/test/cors.test.ts @@ -1,7 +1,7 @@ import request from 'supertest' import { describe, expect, it } from 'vitest' import { createServer } from '../src/server' -import { cfgEntry, config, ledgerFrom } from './helpers/fixtures' +import { cfgEntry, config, ledgerFrom, recordingLogger } from './helpers/fixtures' const ALLOWED_ORIGIN = config.corsOrigins[0] const DISALLOWED_ORIGIN = 'http://not-allowed.example' @@ -164,6 +164,34 @@ describe('cors', () => { expect(preflight.headers['access-control-allow-credentials']).toBeUndefined() }) + // The service side of a refused origin is documented as leaving nothing that + // names it, and the only line any request writes is this one, on a 5xx. What + // has to hold is therefore not that nothing is logged but that what is logged + // identifies the request without identifying where it came from, which is the + // difference between an operator who can find the request and one who could + // tell an allowed caller from a refused one. + it('logs a 5xx by method and path, naming no origin', async () => { + const { logger, entries, errors } = recordingLogger() + const base = ledgerFrom({}) + const ledger = { + ...base, + activeContracts: () => Promise.reject(new Error('ledger down')), + } + const res = await request(createServer({ ledger, config, logger })) + .get('/registry/metadata/v1/instruments') + .set('Origin', DISALLOWED_ORIGIN) + expect(res.status).toBe(500) + expect(res.headers['access-control-allow-origin']).toBeUndefined() + expect(errors).toEqual(['request failed']) + expect(entries).toHaveLength(1) + expect(entries[0]).toMatchObject({ + status: 500, + method: 'GET', + path: '/registry/metadata/v1/instruments', + }) + expect(JSON.stringify(entries[0])).not.toContain(DISALLOWED_ORIGIN) + }) + it('reflects whatever origin asks when corsOrigins is ["*"]', async () => { const app = createServer({ ledger: ledgerFrom({}), config: { ...config, corsOrigins: ['*'] } }) const res = await request(app).get('/healthz').set('Origin', 'http://anything.example')