From cbc4d863e52163c77f63aee82807a7df8a5d53b1 Mon Sep 17 00:00:00 2001 From: Arsalan Shahid Date: Mon, 21 Sep 2026 16:10:36 +0100 Subject: [PATCH] Sweep SPECIFICATION 15.1 against the code, and correct the descriptor schema #138 asked for a sweep of 15 for the pattern the profile gate exposed, once #136 was settled. This is it. 15.1 listed eight flat obligations on the Coordinator. Three were not that. Signature verification and step-up are turned on by a profile, and 6.5 now binds advertising to enforcing so a descriptor cannot understate them. TLS and the filtering of shadow-mode delivery belong to the deployment: a Coordinator answers the caller who asked it, and no reference has a delivery layer to filter in, which is why 11.3 carried the same unmet MUST. A required_scope is declared per method in the catalogue and enforced by neither reference, so the text says so rather than claiming a check that is not there. What is left is grouped by who does it, and each unconditional claim is now held to the code by a test in each reference, the way 8.1 is: the chain ordered by acceptance, every accepted operation recorded and no read recorded, the mode ceiling refusing with -32040 and recording the change, the role checks, and random ids outside test mode. The workspace descriptor schema described a descriptor neither reference sends. It required name, coordinator and evidence_count, which neither sends at all, and evidence_head, which a workspace with no chain cannot have. It declared none of profiles, audit_count, task_count, override_count or routing_policy_uri, which both send. Nothing validated against it at runtime, so it drifted unchecked; the same test now holds the schema and the coordinator together. --- CHANGELOG.md | 15 ++ SPECIFICATION.md | 62 +++++--- .../tests/test_spec_mandatory_protections.py | 141 ++++++++++++++++++ .../tests/spec_mandatory_protections.test.ts | 137 +++++++++++++++++ schemas/core/chap-workspace.schema.json | 28 +++- 5 files changed, 361 insertions(+), 22 deletions(-) create mode 100644 packages/coordinator-py/tests/test_spec_mandatory_protections.py create mode 100644 packages/coordinator/tests/spec_mandatory_protections.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 0f5628e..45a9093 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,14 @@ participant that sends twice in one millisecond. Neither code was allocated in either reference. The chain is ordered by arrival and `prev_hash`, not by the sender's clock, and 15.1 now says so. +**Specification.** 15.1 is regrouped by who has to do the work. It read as +eight obligations on the Coordinator and three of them were not that: +signature verification and step-up are turned on by a profile, TLS and +delivery filtering belong to the deployment, and a `required_scope` is +declared per method and enforced by neither reference, which the text now +says. 11.3's shadow-observer MUST moves with it: a Coordinator answers the +caller who asked, and no reference has a delivery layer to filter. + ### Added - **One catalogue, generated into both references.** Which profile owns which @@ -65,6 +73,13 @@ sender's clock, and 15.1 now says so. ### Fixed +- **The workspace descriptor schema describes the descriptor.** It required + `name`, `coordinator` and `evidence_count`, none of which either reference + sends, and `evidence_head`, which a workspace with no chain cannot have. It + declared none of `profiles`, `audit_count`, `task_count`, `override_count` + or `routing_policy_uri`, all of which both references send. Nothing + validated a descriptor against it at runtime, so it had drifted unchecked; + a test in each reference now holds the two together. - **`workspace.describe` answers the same on both references.** Python sent `evidence_head: null` where TypeScript omitted the key, so the same call returned different JSON. A workspace with no chain has no head, and both now diff --git a/SPECIFICATION.md b/SPECIFICATION.md index 62b985b..a9dc97c 100644 --- a/SPECIFICATION.md +++ b/SPECIFICATION.md @@ -1234,11 +1234,13 @@ The Coordinator MUST: - Reject any `task.create` or `control.supersede` whose mode exceeds the workspace's ceiling with error `-32040` (`mode_ceiling_exceeded`). -- Refuse to dispatch shadow-mode artefacts to participants not on - the workspace's `shadow_observers` list. - Record every mode change as a first-class evidence entry. - Reject privileged mode transitions without valid step-up auth with - error `-32402` (`step_up_required`). + error `-32402` (`step_up_required`), where `identity-oidc/1.0` is in force. + +Delivery of shadow-mode output to the workspace's `shadow_observers` alone is +a requirement on the deployment's delivery layer rather than on the +Coordinator, which answers the caller who asked it. See §15.1. ### 11.4 Per-task overrides @@ -1544,22 +1546,48 @@ section summarises requirements normative to the specification. ### 15.1 Mandatory protections -Conformant implementations MUST: +This section is grouped by who has to do the work, because the list read as +eight flat obligations on the Coordinator and three of them were not that. +A requirement the reference implementations do not meet is worse than no +requirement, since the references are what conformance is measured against. + +**A conformant Coordinator MUST:** -1. Verify every signature before accepting any message into the - evidence chain. -2. Order the chain by acceptance rather than by the sender's clock: - record arrival, assign a sequence, and link each entry to the - previous one. A sender-declared timestamp that goes backwards is an - operational signal, described in +1. Order the chain by acceptance rather than by the sender's clock: record + arrival, assign a sequence, and link each entry to the one before. A + sender-declared timestamp that goes backwards is an operational signal, + described in [SECURITY.md](./SECURITY.md#sender-declared-timestamps). -3. Reject messages whose `prev_hash` does not match the current - chain head. -4. Enforce role/method/scope checks before dispatching. -5. Enforce the mode ceiling and shadow-observer routing rules. -6. Require step-up authentication for privileged methods. -7. Use TLS 1.3+ for all production transports. -8. Use cryptographically random ULIDs for `id` generation. +2. Record every accepted operation on the chain. The reads named in §6.5 are + the exception and are recorded nowhere, because appending on read would + grow and re-link the chain each time it was inspected. +3. Refuse a method whose owning profile the workspace does not advertise + (§15.4), and refuse a `workspace.create` whose descriptor understates what + the Coordinator enforces (§6.5). +4. Enforce the authorisation the workspace holds: membership where a profile + requires it, and the role checks the method defines. A `required_scope` is + declared per method in the catalogue and is not yet enforced by either + reference; treat it as descriptive until it is. +5. Enforce the mode ceiling, refusing a `task.create` or `control.supersede` + above it with `-32040`, and record every mode change on the chain. +6. Generate `id` values as cryptographically random ULIDs outside test mode. + +**A profile turns these on, and §6.5 binds advertising to enforcing, so a +descriptor cannot understate them:** + +7. `security-signed/1.0`: verify every signature before accepting a message + into the chain, and refuse a message that does not verify. +8. `identity-oidc/1.0`: require step-up authentication, within the workspace's + window, for the privileged methods. + +**The deployment MUST**, because a Coordinator library has no transport or +delivery layer of its own to do it in: + +9. Use TLS 1.3 or later for every production transport. +10. Filter delivery of shadow-mode output to the workspace's + `shadow_observers`. A Coordinator answers the caller who asked; which + participants are notified of what is the delivery layer's decision, and + no reference implements a delivery layer. ### 15.2 Recommended protections diff --git a/packages/coordinator-py/tests/test_spec_mandatory_protections.py b/packages/coordinator-py/tests/test_spec_mandatory_protections.py new file mode 100644 index 0000000..9fd1e2a --- /dev/null +++ b/packages/coordinator-py/tests/test_spec_mandatory_protections.py @@ -0,0 +1,141 @@ +""" +SPECIFICATION 15.1 is checked against the coordinator, the way 8.1 is. + +The section listed eight flat obligations on the Coordinator, and three of them +were not that: signature verification and step-up are turned on by a profile, +TLS and delivery filtering belong to the deployment, and a scope check is +declared per method and enforced by neither reference. A requirement the +references do not meet is worse than none, because the references are what +conformance is measured against. + +This holds each remaining unconditional claim to the code, and holds the +descriptor schema to what the coordinator sends. The mirror is +packages/coordinator/tests/spec_mandatory_protections.test.ts. +""" +from __future__ import annotations + +import json +import re +from pathlib import Path + +from chap_coordinator import Coordinator, CoordinatorOptions + +ROOT = Path(__file__).resolve().parents[3] +SPEC = (ROOT / "SPECIFICATION.md").read_text(encoding="utf-8") +WORKSPACE_SCHEMA = json.loads( + (ROOT / "schemas/core/chap-workspace.schema.json").read_text(encoding="utf-8")) + +PROFILES = ["core/1.0", "review/1.0", "modes/1.0", "control/1.0"] + + +def _ready(**options): + coord = Coordinator(CoordinatorOptions( + deterministic_ids=True, deterministic_clock=True, + default_profiles=PROFILES, **options)) + + def send(method, sender="human:a", **params): + return coord.dispatch({"jsonrpc": "2.0", "id": method, "method": method, + "params": {"workspace": "w", "from": sender, **params}}) + + send("workspace.create", profiles=PROFILES) + send("participant.join", sender="human:a", type="human", role="admin") + send("participant.join", sender="agent:b", type="agent", role="drafter") + return coord, send + + +def _section(heading: str, until: str) -> str: + """The section with its line wrapping collapsed, so a phrase can be sought.""" + return " ".join(SPEC[SPEC.index(heading):SPEC.index(until)].split()) + + +# ------------------------------------------------ the section says what it says + +def test_the_section_separates_who_has_to_do_the_work(): + # A flat list of MUSTs on the Coordinator is what let three requirements + # sit there unmet. The grouping is the fix, so it is held in place. + body = _section("### 15.1 Mandatory protections", "### 15.2") + assert "A conformant Coordinator MUST" in body + assert "A profile turns these on" in body + assert "The deployment MUST" in body + + +def test_the_section_claims_no_scope_enforcement(): + # Declared per method in the catalogue, enforced by neither reference. + body = _section("### 15.1 Mandatory protections", "### 15.2") + assert "not yet enforced by either reference" in body + + +# --------------------------------------------- the unconditional ones are true + +def test_the_chain_is_ordered_by_acceptance_not_by_the_sender_clock(): + coord, send = _ready(enable_chain=True) + for backwards in ("2030-01-01T00:00:00.000Z", "2020-01-01T00:00:00.000Z"): + send("task.create", kind="k", input={}, assignee="agent:b", ts=backwards) + audit = coord.get_workspace("w").audit + assert [e.seq for e in audit] == sorted(e.seq for e in audit) + assert all(a.arrived <= b.arrived for a, b in zip(audit, audit[1:])) + + +def test_every_accepted_operation_is_recorded_and_the_reads_are_not(): + coord, send = _ready() + ws = coord.get_workspace("w") + before = len(ws.audit) + send("task.create", kind="k", input={}, assignee="agent:b") + assert len(ws.audit) == before + 1 + for read in ("workspace.describe", "audit.read"): + send(read) + assert len(ws.audit) == before + 1 + + +def test_the_mode_ceiling_is_enforced_and_the_change_is_recorded(): + coord, send = _ready() + ws = coord.get_workspace("w") + before = len(ws.audit) + assert "error" not in send("control.set_mode_ceiling", new_ceiling="trial") + assert len(ws.audit) == before + 1, "a mode change is a first-class entry" + + refused = send("task.create", kind="k", input={}, assignee="agent:b", mode="production") + assert refused["error"]["code"] == -32040 + + +def test_a_role_check_the_method_defines_is_enforced(): + coord, send = _ready() + assert "error" not in send("workspace.set_profiles", profiles=PROFILES) + refused = send("workspace.set_profiles", sender="agent:b", profiles=PROFILES) + assert "error" in refused and "admin" in refused["error"]["message"] + + +def test_ids_are_random_outside_test_mode(): + live = Coordinator(CoordinatorOptions()) + minted = {live.ids.task_id() for _ in range(64)} + assert len(minted) == 64 + assert all(re.fullmatch(r"tsk_[0-9A-HJKMNP-TV-Z]{26}", t) for t in minted) + + +# ------------------------------------------- the conditional ones are optional + +def test_signature_verification_is_what_the_profile_turns_on(): + _, send = _ready() + assert "error" not in send("task.create", kind="k", input={}, assignee="agent:b"), \ + "an unsigned envelope is accepted where the profile is not in force" + + signed = Coordinator(CoordinatorOptions(require_signatures=True)) + signed.dispatch({"jsonrpc": "2.0", "id": "c", "method": "workspace.create", + "params": {"workspace": "w", "profiles": ["core/1.0"]}}) + # 6.5: enforcement on adds the profile, so the descriptor cannot understate. + assert "security-signed/1.0" in signed.get_workspace("w").profiles + + +# ------------------------------------------ the descriptor matches its schema + +def test_the_descriptor_carries_what_its_schema_requires(): + for chained in (False, True): + coord, send = _ready(enable_chain=chained) + descriptor = send("workspace.describe")["result"] + for field in WORKSPACE_SCHEMA["required"]: + assert field in descriptor, f"{field} is required and not sent (chain={chained})" + for field in descriptor: + assert field in WORKSPACE_SCHEMA["properties"], \ + f"{field} is sent and not declared (chain={chained})" + # The head exists only where there is a chain, which is why it is optional. + assert "evidence_head" not in WORKSPACE_SCHEMA["required"] diff --git a/packages/coordinator/tests/spec_mandatory_protections.test.ts b/packages/coordinator/tests/spec_mandatory_protections.test.ts new file mode 100644 index 0000000..0d2a691 --- /dev/null +++ b/packages/coordinator/tests/spec_mandatory_protections.test.ts @@ -0,0 +1,137 @@ +/** + * SPECIFICATION 15.1 is checked against the coordinator, the way 8.1 is. + * + * The section listed eight flat obligations on the Coordinator, and three of + * them were not that: signature verification and step-up are turned on by a + * profile, TLS and delivery filtering belong to the deployment, and a scope + * check is declared per method and enforced by neither reference. A + * requirement the references do not meet is worse than none, because the + * references are what conformance is measured against. + * + * This holds each remaining unconditional claim to the code, and holds the + * descriptor schema to what the coordinator sends. The mirror is + * packages/coordinator-py/tests/test_spec_mandatory_protections.py. + */ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { Coordinator } from "../src/coordinator.ts"; + +const SPEC = readFileSync(new URL("../../../SPECIFICATION.md", import.meta.url), "utf8"); +const WORKSPACE_SCHEMA = JSON.parse(readFileSync( + new URL("../../../schemas/core/chap-workspace.schema.json", import.meta.url), "utf8")); + +const PROFILES = ["core/1.0", "review/1.0", "modes/1.0", "control/1.0"]; + +function ready(options: Record = {}) { + const c = new Coordinator({ deterministicIds: true, deterministicClock: true, + defaultProfiles: PROFILES, ...options } as never); + const send = (method: string, params: Record = {}, from = "human:a"): any => + c.dispatch({ jsonrpc: "2.0", id: method, method, + params: { workspace: "w", from, ...params } } as never); + send("workspace.create", { profiles: PROFILES }); + send("participant.join", { type: "human", role: "admin" }, "human:a"); + send("participant.join", { type: "agent", role: "drafter" }, "agent:b"); + return { c, send }; +} + +/** The section with its line wrapping collapsed, so a phrase can be sought. */ +function section(heading: string, until: string): string { + return SPEC.slice(SPEC.indexOf(heading), SPEC.indexOf(until)).split(/\s+/).join(" "); +} + +// ------------------------------------------------ the section says what it says + +test("the section separates who has to do the work", () => { + // A flat list of MUSTs on the Coordinator is what let three requirements sit + // there unmet. The grouping is the fix, so it is held in place. + const body = section("### 15.1 Mandatory protections", "### 15.2"); + assert.ok(body.includes("A conformant Coordinator MUST")); + assert.ok(body.includes("A profile turns these on")); + assert.ok(body.includes("The deployment MUST")); +}); + +test("the section claims no scope enforcement", () => { + // Declared per method in the catalogue, enforced by neither reference. + const body = section("### 15.1 Mandatory protections", "### 15.2"); + assert.ok(body.includes("not yet enforced by either reference")); +}); + +// --------------------------------------------- the unconditional ones are true + +test("the chain is ordered by acceptance, not by the sender's clock", () => { + const { c, send } = ready({ enableChain: true }); + for (const ts of ["2030-01-01T00:00:00.000Z", "2020-01-01T00:00:00.000Z"]) { + send("task.create", { kind: "k", input: {}, assignee: "agent:b", ts }); + } + const audit = (c.workspaces.get("w") as any).audit as Array<{ seq: number; arrived: string }>; + assert.deepEqual(audit.map(e => e.seq), [...audit.map(e => e.seq)].sort((a, b) => a - b)); + for (let i = 1; i < audit.length; i++) assert.ok(audit[i - 1].arrived <= audit[i].arrived); +}); + +test("every accepted operation is recorded and the reads are not", () => { + const { c, send } = ready(); + const ws = c.workspaces.get("w") as any; + const before = ws.audit.length; + send("task.create", { kind: "k", input: {}, assignee: "agent:b" }); + assert.equal(ws.audit.length, before + 1); + for (const read of ["workspace.describe", "audit.read"]) send(read); + assert.equal(ws.audit.length, before + 1); +}); + +test("the mode ceiling is enforced and the change is recorded", () => { + const { c, send } = ready(); + const ws = c.workspaces.get("w") as any; + const before = ws.audit.length; + assert.equal(send("control.set_mode_ceiling", { new_ceiling: "trial" }).error, undefined); + assert.equal(ws.audit.length, before + 1, "a mode change is a first-class entry"); + + const refused = send("task.create", { kind: "k", input: {}, assignee: "agent:b", mode: "production" }); + assert.equal(refused.error.code, -32040); +}); + +test("a role check the method defines is enforced", () => { + const { send } = ready(); + assert.equal(send("workspace.set_profiles", { profiles: PROFILES }).error, undefined); + const refused = send("workspace.set_profiles", { profiles: PROFILES }, "agent:b"); + assert.match(refused.error.message, /admin/); +}); + +test("ids are random outside test mode", () => { + const live = new Coordinator({} as never); + const minted = new Set(Array.from({ length: 64 }, () => (live as any).ids.taskId())); + assert.equal(minted.size, 64); + for (const id of minted) assert.match(id as string, /^tsk_[0-9A-HJKMNP-TV-Z]{26}$/); +}); + +// ------------------------------------------- the conditional ones are optional + +test("signature verification is what the profile turns on", () => { + const { send } = ready(); + assert.equal(send("task.create", { kind: "k", input: {}, assignee: "agent:b" }).error, undefined, + "an unsigned envelope is accepted where the profile is not in force"); + + const signed = new Coordinator({ requireSignatures: true } as never); + signed.dispatch({ jsonrpc: "2.0", id: "c", method: "workspace.create", + params: { workspace: "w", profiles: ["core/1.0"] } } as never); + // 6.5: enforcement on adds the profile, so the descriptor cannot understate. + assert.ok((signed.workspaces.get("w") as any).profiles.includes("security-signed/1.0")); +}); + +// ------------------------------------------ the descriptor matches its schema + +test("the descriptor carries what its schema requires", () => { + for (const chained of [false, true]) { + const { send } = ready({ enableChain: chained }); + const descriptor = send("workspace.describe").result; + for (const field of WORKSPACE_SCHEMA.required) { + assert.ok(field in descriptor, `${field} is required and not sent (chain=${chained})`); + } + for (const field of Object.keys(descriptor)) { + assert.ok(field in WORKSPACE_SCHEMA.properties, + `${field} is sent and not declared (chain=${chained})`); + } + } + // The head exists only where there is a chain, which is why it is optional. + assert.ok(!WORKSPACE_SCHEMA.required.includes("evidence_head")); +}); diff --git a/schemas/core/chap-workspace.schema.json b/schemas/core/chap-workspace.schema.json index 7aa4f47..66b76e0 100644 --- a/schemas/core/chap-workspace.schema.json +++ b/schemas/core/chap-workspace.schema.json @@ -5,8 +5,7 @@ "description": "Descriptor for a CHAP workspace.", "type": "object", "required": [ - "id", "name", "created", "state", "mode", "mode_ceiling", - "coordinator", "members", "evidence_head", "evidence_count" + "id", "created", "state", "mode", "mode_ceiling", "members", "profiles" ], "properties": { "id": { @@ -106,12 +105,31 @@ "evidence_head": { "type": "string", "pattern": "^sha256:[0-9a-f]{64}$", - "description": "Hash of the current chain head." + "description": "Hash of the current chain head. Omitted where the workspace has no chain, so it is not required." }, - "evidence_count": { + "audit_count": { "type": "integer", "minimum": 0, - "description": "Number of entries currently in the chain." + "description": "Number of entries currently in the log. Named evidence_count in 0.1, which neither reference ever sent." + }, + "task_count": { + "type": "integer", + "minimum": 0, + "description": "Number of tasks the workspace holds." + }, + "override_count": { + "type": "integer", + "minimum": 0, + "description": "Number of override artefacts the workspace holds." + }, + "profiles": { + "type": "array", + "items": { "type": "string" }, + "description": "The advertised profile set. Normative: SPECIFICATION 6.5 says what each entry means, and 15.4 refuses a method whose owning profile is absent." + }, + "routing_policy_uri": { + "type": "string", + "description": "Where the routing policy is fetched from, when routing/1.0 is in force." }, "anchors": { "type": "array",