From 5891d8f20cd0ef35f962c966522dcc00d28b8e79 Mon Sep 17 00:00:00 2001 From: Vishal Katyal Date: Sat, 12 Sep 2026 06:38:56 -0400 Subject: [PATCH] fix: keep conditional rules on branches that carry a title `EcKeysCarryCrvXYSchema`, the published signing key type, accepted `{ kid, kty: "EC" }` with no curve and no coordinates, and the OKP equivalent. profile.json states both as conditional branches on jwk_public_key. Two things in the recorder combined to drop them. `describeConditionalRule` rejected any branch carrying a key other than `if` or `then`. Every jwk_public_key branch carries a `title`, so all five were judged unmodellable even though two are plain `required` consequences the recorder already knows how to express. Annotation keywords assert nothing, so they are now ignored when the branch shape is checked. `recordConditionalRules` then discarded the whole rule list when any branch was unmodellable, which would still have dropped the two expressible branches because the three that pair a curve with an algorithm use a `properties` consequence the recorder does not model. Unmodellable branches are now skipped individually. Every `if`/`then` only adds constraints, so keeping the expressible subset can only enforce more, never reject something the schema permits. Still not enforced, and out of scope here: the three curve to algorithm pairings, because their consequence shape is not modelled. A P-256 key declaring ES384 is accepted as before. Testing. Four cases, of which two fail before this change and pass after; reverting either half alone reproduces the two failures. Suite 151 passed, from 147. Regeneration against release/2026-08-25 is idempotent and the exported name set is unchanged. Only `EcKeysCarryCrvXYSchema` changes, one hunk. All seven JSON Web Keys the specification publishes, and the profile response scaffold, still parse, so the regression surface is empty. --- scripts/inject-schema-constraints.mjs | 31 +++++++- src/spec_generated.ts | 101 ++++++++++++++++++++++--- tests/profile-key-conditionals.test.js | 65 ++++++++++++++++ 3 files changed, 185 insertions(+), 12 deletions(-) create mode 100644 tests/profile-key-conditionals.test.js diff --git a/scripts/inject-schema-constraints.mjs b/scripts/inject-schema-constraints.mjs index 88d6afd..a448344 100644 --- a/scripts/inject-schema-constraints.mjs +++ b/scripts/inject-schema-constraints.mjs @@ -664,13 +664,31 @@ function numericBounds(node) { return Object.keys(descriptor).length ? descriptor : null; } +// Annotation keywords carry no assertion, so a branch that merely labels itself +// is still a branch this recorder can model. profile.json titles every +// jwk_public_key branch ("EC keys carry crv, x, y" and so on), and treating the +// title as an unmodellable extra keyword disqualified all five. +const CONDITIONAL_BRANCH_ANNOTATIONS = new Set([ + "title", + "description", + "$comment", + "examples", + "default", + "deprecated", +]); + function describeConditionalRule(branch, properties) { const condition = branch?.if; const consequence = branch?.then; if ( !condition || !consequence || - Object.keys(branch).some((key) => key !== "if" && key !== "then") || + Object.keys(branch).some( + (key) => + key !== "if" && + key !== "then" && + !CONDITIONAL_BRANCH_ANNOTATIONS.has(key) + ) || Object.keys(condition).some( (key) => key !== "properties" && key !== "required" ) || @@ -796,12 +814,19 @@ function recordConditionalRules(node, properties) { for (const branch of branches) { const rule = describeConditionalRule(branch, properties); if (!rule) { + // A branch this recorder cannot model is skipped, not fatal. Dropping the + // whole list because one branch is unmodellable discarded the branches + // that ARE expressible: profile.json's jwk_public_key states five, two of + // them plain `required` consequences, and the three `alg` const branches + // took those two down with them. Every `if`/`then` only adds constraints, + // so keeping the expressible subset can only enforce more, never reject + // something the schema permits. unsupported = true; - break; + continue; } rules.push(rule); } - if (unsupported) rules.length = 0; + void unsupported; rules.sort((left, right) => JSON.stringify(left).localeCompare(JSON.stringify(right)) ); diff --git a/src/spec_generated.ts b/src/spec_generated.ts index 3f1224f..ff8f19b 100644 --- a/src/spec_generated.ts +++ b/src/spec_generated.ts @@ -1280,15 +1280,98 @@ export const McpToolCallSchema = z.object({ }); export type McpToolCall = z.infer; -export const EcKeysCarryCrvXYSchema = z.object({ - alg: z.string().optional(), - crv: z.string().optional(), - kid: z.string(), - kty: z.string(), - use: z.string().optional(), - x: z.string().optional(), - y: z.string().optional(), -}); +export const EcKeysCarryCrvXYSchema = z + .object({ + alg: z.string().optional(), + crv: z.string().optional(), + kid: z.string(), + kty: z.string(), + use: z.string().optional(), + x: z.string().optional(), + y: z.string().optional(), + }) + .superRefine((value, ctx) => { + for (const rule of [ + { + kind: "required", + discriminator: "kty", + values: ["EC"], + negated: false, + required: ["crv", "x", "y"], + field: null, + format: null, + target: null, + minimum: null, + maximum: null, + exclusiveMinimum: null, + exclusiveMaximum: null, + }, + { + kind: "required", + discriminator: "kty", + values: ["OKP"], + negated: false, + required: ["crv", "x"], + field: null, + format: null, + target: null, + minimum: null, + maximum: null, + exclusiveMinimum: null, + exclusiveMaximum: null, + }, + ]) { + const record = value as Record; + const discriminatorVal = record[rule.discriminator]; + if (discriminatorVal === undefined) continue; + const matches = (rule.values as readonly unknown[]).includes( + discriminatorVal + ); + if (rule.negated ? matches : !matches) continue; + if (rule.kind === "required") { + for (const field of rule.required) { + if (!(field in record)) + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [field], + message: "Field is required by a conditional constraint", + }); + } + continue; + } + if (rule.kind === "format") { + const field = rule.field; + const fieldValue = field === null ? undefined : record[field]; + if (rule.format === "uri" && typeof fieldValue === "string") { + try { + new URL(fieldValue); + } catch { + if (field !== null) + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [field], + message: "Value must be a valid URI", + }); + } + } + continue; + } + if (rule.target === null) continue; + const target = record[rule.target]; + if (typeof target !== "number") continue; + const invalid = + (rule.minimum !== null && target < rule.minimum) || + (rule.maximum !== null && target > rule.maximum) || + (rule.exclusiveMinimum !== null && target <= rule.exclusiveMinimum) || + (rule.exclusiveMaximum !== null && target >= rule.exclusiveMaximum); + if (invalid) + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: [rule.target], + message: "Value violates a conditional numeric constraint", + }); + } + }); export type EcKeysCarryCrvXY = z.infer; export const AdjustmentLineItemClassSchema = z.object({ diff --git a/tests/profile-key-conditionals.test.js b/tests/profile-key-conditionals.test.js new file mode 100644 index 0000000..e36acb2 --- /dev/null +++ b/tests/profile-key-conditionals.test.js @@ -0,0 +1,65 @@ +// Fidelity tests for the conditional rules on a published signing key. +// +// profile.json states five conditional branches on `jwk_public_key`. Two are +// plain `required` consequences that this repo can already model: an EC key +// carries crv, x and y, and an OKP key carries crv and x. Every branch also +// carries a `title`, and the branch shape check treated that annotation as an +// unmodellable extra keyword, so all five were discarded and a key declaring +// `kty: "EC"` with no curve and no coordinates parsed cleanly. +// +// The remaining three branches pair a curve with an algorithm through a +// `properties` consequence, which the recorder does not model. Those are still +// not enforced, so a P-256 key declaring ES384 is still accepted here. + +const { test } = require("node:test"); +const assert = require("node:assert/strict"); + +const { EcKeysCarryCrvXYSchema } = require("./.dist/spec_generated.js"); + +const accepts = (schema, value) => schema.safeParse(value).success === true; +const rejects = (schema, value) => schema.safeParse(value).success === false; + +const EC = { + kid: "key-1", + kty: "EC", + crv: "P-256", + alg: "ES256", + x: "f83OJ3D2xF1Bg8vub9tLe1gHMzV76e8Tus9uPHvRVEU", + y: "x_FEzRu9m36HLN_tue659LNpXW6pCyStikYjKIWI5a0", +}; +const OKP = { + kid: "key-2", + kty: "OKP", + crv: "Ed25519", + alg: "EdDSA", + x: "11qY", +}; + +test("an EC key must carry crv, x and y", () => { + assert.ok(accepts(EcKeysCarryCrvXYSchema, EC)); + assert.ok(rejects(EcKeysCarryCrvXYSchema, { kid: "key-1", kty: "EC" })); + const { crv, ...noCrv } = EC; + assert.ok(rejects(EcKeysCarryCrvXYSchema, noCrv)); + const { y, ...noY } = EC; + assert.ok(rejects(EcKeysCarryCrvXYSchema, noY)); +}); + +test("an OKP key must carry crv and x", () => { + assert.ok(accepts(EcKeysCarryCrvXYSchema, OKP)); + assert.ok(rejects(EcKeysCarryCrvXYSchema, { kid: "key-2", kty: "OKP" })); + const { x, ...noX } = OKP; + assert.ok(rejects(EcKeysCarryCrvXYSchema, noX)); +}); + +test("the conditional applies only to the matching key type", () => { + // An unrelated key type carries neither obligation, so the rule must not + // fire and turn every other key into an error. + assert.ok( + accepts(EcKeysCarryCrvXYSchema, { kid: "key-3", kty: "oct", alg: "HS256" }) + ); +}); + +test("the required members the base schema already states still hold", () => { + assert.ok(rejects(EcKeysCarryCrvXYSchema, { kid: "key-1" })); + assert.ok(rejects(EcKeysCarryCrvXYSchema, { kty: "EC" })); +});