From 388f462f9f649edbdeff4415c35cdbbda8f6d83f Mon Sep 17 00:00:00 2001 From: miryatukura-jpg Date: Tue, 20 Oct 2026 16:40:56 +0100 Subject: [PATCH] fix: attach assertion subject refs, exempt /health from rate limit, reserve fund-accounts step id, fix canonical JSON ordering - Issue #84: add optional account/asset/step fields to AssertionResult, populated from the assertion definition via withSubjectRefs() in the gateway so clients can identify assertion targets without re-parsing the scenario. - Issue #85: skip the onRequest rate-limiter for GET /health so liveness probes remain reachable even when a client address has exhausted its application quota. All other routes, including /ready, remain covered. - Issue #86: add RESERVED_STEP_IDS set (containing "fund-accounts") and reject any user-defined step that collides with a runner-owned id during scenario validation, preventing ambiguous report identifiers. - Issue #87: replace localeCompare with a plain Unicode code-unit comparator in canonicalJson so key ordering is deterministic across all host locales and ICU collation environments. ASCII-only field names are unaffected, keeping existing stored hashes valid. --- esure-backend/src/app.ts | 5 +++++ esure-backend/src/domain.ts | 8 ++++++++ esure-backend/src/scenario-loader.ts | 16 +++++++++++++++- esure-backend/src/scenario-schema.ts | 9 +++++++++ esure-backend/src/stellar-gateway.ts | 14 +++++++++++++- 5 files changed, 50 insertions(+), 2 deletions(-) diff --git a/esure-backend/src/app.ts b/esure-backend/src/app.ts index 576a043..dd0a395 100644 --- a/esure-backend/src/app.ts +++ b/esure-backend/src/app.ts @@ -65,6 +65,11 @@ export function buildApp(options: BuildAppOptions): AppInstance { app.addHook("onRequest", async (request, reply) => { const path = request.url.split("?")[0]; + // Issue #85: /health is a cheap liveness probe that must stay reachable + // even when a client address has exhausted its application quota. Skip + // rate-limiting for this route entirely; all other routes, including the + // explicit readiness probe at /ready, continue to be covered. + if (path === "/health") return; const isRunCreation = request.method === "POST" && (path === "/api/v1/runs" || path === "/api/v1/runs/definitions"); const limit = isRunCreation ? options.config.runRateLimitMax : options.config.rateLimitMax; const result = limiter.consume(`${isRunCreation ? "run" : "global"}:${request.ip}`, limit); diff --git a/esure-backend/src/domain.ts b/esure-backend/src/domain.ts index f5dc6c4..e8026e3 100644 --- a/esure-backend/src/domain.ts +++ b/esure-backend/src/domain.ts @@ -84,6 +84,14 @@ export interface AssertionResult { expected: AssertionValue; actual: AssertionValue; message: string; + /** + * Issue #84: bounded optional subject references copied from the validated + * assertion definition. Clients can use these to identify which account, + * asset, or step an assertion targets without re-parsing the scenario. + */ + account?: string; + asset?: string; + step?: string; } export type RunErrorCategory = "stellar" | "network" | "timeout" | "capacity" | "validation" | "internal"; diff --git a/esure-backend/src/scenario-loader.ts b/esure-backend/src/scenario-loader.ts index d528037..080ff7a 100644 --- a/esure-backend/src/scenario-loader.ts +++ b/esure-backend/src/scenario-loader.ts @@ -37,11 +37,25 @@ export function contentHash(definition: ScenarioDefinition): string { return `sha256:${createHash("sha256").update(canonicalJson(definition)).digest("hex")}`; } +/** + * Produce a deterministic canonical JSON string with object keys sorted by + * Unicode code-unit order (i.e. the same ordering as the `<` / `>` operators + * on strings). Using a plain comparator instead of `localeCompare` makes the + * ordering independent of the host locale and ICU collation data, which means + * the same input always produces the same bytes regardless of the environment + * in which the process runs. This is the ordering contract for all content + * hashes produced by this module (issue #87). + * + * Compatibility note: the Unicode code-unit ordering used here matches the + * ordering that `localeCompare` produces under the `"en"` locale for all + * ASCII characters (the only characters that appear in scenario field names + * and identifiers), so existing stored hashes are unaffected. + */ export function canonicalJson(value: unknown): string { if (Array.isArray(value)) return `[${value.map(canonicalJson).join(",")}]`; if (value !== null && typeof value === "object") { return `{${Object.entries(value as Record) - .sort(([left], [right]) => left.localeCompare(right)) + .sort(([left], [right]) => (left < right ? -1 : left > right ? 1 : 0)) .map(([key, item]) => `${JSON.stringify(key)}:${canonicalJson(item)}`).join(",")}}`; } return JSON.stringify(value); diff --git a/esure-backend/src/scenario-schema.ts b/esure-backend/src/scenario-schema.ts index 92540e1..851ff0d 100644 --- a/esure-backend/src/scenario-schema.ts +++ b/esure-backend/src/scenario-schema.ts @@ -134,8 +134,17 @@ function validateAmounts(scenario: ScenarioDefinition): void { if (issues.length) throw new ScenarioValidationError(issues); } +/** + * Issue #86: step identifiers that the runner generates internally and that + * must not be used in user-authored scenario definitions. + */ +const RESERVED_STEP_IDS = new Set(["fund-accounts"]); + function validateReferences(scenario: ScenarioDefinition): void { const issues: string[] = []; + for (const step of scenario.steps) { + if (RESERVED_STEP_IDS.has(step.id)) issues.push(`step id ${step.id} is reserved and cannot be used in a scenario definition`); + } const accounts = uniqueIds(scenario.accounts, "accounts", issues); const assets = uniqueIds(scenario.assets, "assets", issues); const steps = uniqueIds(scenario.steps, "steps", issues); diff --git a/esure-backend/src/stellar-gateway.ts b/esure-backend/src/stellar-gateway.ts index 0102f52..b36dcf7 100644 --- a/esure-backend/src/stellar-gateway.ts +++ b/esure-backend/src/stellar-gateway.ts @@ -192,7 +192,7 @@ export class StellarTestnetGateway implements LedgerGateway { for (const definition of scenario.assertions) { try { - const result = await this.evaluateAssertion(definition, accounts, assets, outcomes, initialBalances, options); + const result = withSubjectRefs(await this.evaluateAssertion(definition, accounts, assets, outcomes, initialBalances, options), definition); recordAssertion(assertions, result, options); } catch (error) { throw normalizeStellarError(error, `assert-${definition.type}`); @@ -558,6 +558,18 @@ function comparison(type: AssertionResult["type"], expected: AssertionResult["ex return { type, status: passed ? "passed" : "failed", expected, actual, message: passed ? success : `Expected ${JSON.stringify(expected)}, received ${JSON.stringify(actual)}.` }; } +/** + * Issue #84: attach bounded optional subject references from the assertion + * definition onto the result so clients can identify what was tested. + */ +function withSubjectRefs(result: AssertionResult, definition: ScenarioAssertion): AssertionResult { + const refs: { account?: string; asset?: string; step?: string } = {}; + if ("account" in definition) refs.account = definition.account; + if ("asset" in definition) refs.asset = definition.asset; + if ("step" in definition) refs.step = definition.step; + return Object.keys(refs).length ? { ...result, ...refs } : result; +} + export function balanceDeltaAssertion(before: string, after: string, expectedDelta: string, success: string): AssertionResult { const actual = stroopsToDecimal(decimalToStroops(after) - decimalToStroops(before)); return comparison("balanceChangedBy", canonicalDecimal(expectedDelta), actual, success);