feat: validate workflow contract references
This commit is contained in:
@@ -0,0 +1,52 @@
|
||||
# Task 2 Report
|
||||
|
||||
## Scope
|
||||
|
||||
Implemented fail-closed contract and `$ref` validation for
|
||||
`wf_contract_manifest.manifest_from_openrpc()` without changing the Task 1
|
||||
public interface. JSON Schema keywords remain opaque; validation is limited to
|
||||
the OpenRPC envelope, normalized operation/component values, and their `$ref`
|
||||
graph.
|
||||
|
||||
## TDD Evidence
|
||||
|
||||
- Added tests for unsupported OpenRPC versions, malformed envelopes, duplicate
|
||||
methods, malformed dotted names, invalid parameters, invalid result shapes,
|
||||
and non-component success results.
|
||||
- Added tests for external, unsupported-local, unsupported-component,
|
||||
dangling, and escaped component references.
|
||||
- Added a passing nested schema reference case while retaining the existing
|
||||
error-component reference assertion.
|
||||
- Ran the new tests before implementation: 10 cases failed for the intended
|
||||
missing validation behaviors; existing Task 1 coverage remained green.
|
||||
|
||||
## Implementation
|
||||
|
||||
- Pins `$.openrpc` to `1.2.6`.
|
||||
- Rejects duplicate method names at the later method path.
|
||||
- Uses the strict `malformed dotted method name` diagnostic.
|
||||
- Requires success results to be exactly a local `components/schemas` `$ref`
|
||||
object.
|
||||
- Walks every operation parameter/result/error and every normalized component,
|
||||
recursively inspecting only `$ref` values.
|
||||
- Rejects external refs, unsupported namespaces, escaped component keys, and
|
||||
dangling refs; forward refs are accepted after the complete manifest is
|
||||
assembled.
|
||||
|
||||
## Verification
|
||||
|
||||
```text
|
||||
39 passed in 0.10s
|
||||
ruff check: All checks passed!
|
||||
basedpyright --level error: 0 errors, 0 warnings, 0 notes
|
||||
git diff --check: passed (only Git line-ending warnings)
|
||||
```
|
||||
|
||||
## Commit
|
||||
|
||||
The Task 2 commit is recorded by the final response after the final verification
|
||||
and scope audit.
|
||||
|
||||
## Concerns
|
||||
|
||||
None identified.
|
||||
@@ -1,6 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Mapping
|
||||
from collections.abc import Iterator, Mapping
|
||||
|
||||
from .model import (
|
||||
ContractManifest,
|
||||
@@ -11,6 +11,8 @@ from .model import (
|
||||
ManifestParameter,
|
||||
)
|
||||
|
||||
type ComponentIndex = dict[str, set[str]]
|
||||
|
||||
|
||||
def _mapping(value: object, path: str) -> Mapping[str, object]:
|
||||
if not isinstance(value, Mapping):
|
||||
@@ -60,15 +62,82 @@ def _schema(value: object, path: str) -> JsonSchema:
|
||||
return normalized
|
||||
|
||||
|
||||
def _walk_references(value: JsonValue, path: str) -> Iterator[tuple[str, str]]:
|
||||
"""Yield every ``$ref`` while treating JSON Schema vocabulary as opaque."""
|
||||
if isinstance(value, dict):
|
||||
for key, child in value.items():
|
||||
child_path = f"{path}.{key}"
|
||||
if key == "$ref":
|
||||
if not isinstance(child, str):
|
||||
raise ManifestError(child_path, "expected a reference string")
|
||||
yield child_path, child
|
||||
else:
|
||||
yield from _walk_references(child, child_path)
|
||||
elif isinstance(value, list):
|
||||
for index, child in enumerate(value):
|
||||
yield from _walk_references(child, f"{path}[{index}]")
|
||||
|
||||
|
||||
def _validate_references(
|
||||
manifest: ContractManifest, component_index: ComponentIndex
|
||||
) -> None:
|
||||
values: list[tuple[str, JsonValue]] = []
|
||||
for operation_index, operation in enumerate(manifest["operations"]):
|
||||
operation_path = f"$.operations[{operation_index}]"
|
||||
for parameter_index, parameter in enumerate(operation["params"]):
|
||||
values.append(
|
||||
(
|
||||
f"{operation_path}.params[{parameter_index}].schema",
|
||||
parameter["schema"],
|
||||
)
|
||||
)
|
||||
values.append((f"{operation_path}.result.schema", operation["result"]["schema"]))
|
||||
for error_index, error in enumerate(operation["errors"]):
|
||||
values.append((f"{operation_path}.errors[{error_index}]", error))
|
||||
|
||||
for key, component in manifest["components"]["schemas"].items():
|
||||
values.append((f"$.components.schemas.{key}", component))
|
||||
for key, component in manifest["components"]["errors"].items():
|
||||
values.append((f"$.components.errors.{key}", component))
|
||||
|
||||
for value_path, value in values:
|
||||
for reference_path, reference in _walk_references(value, value_path):
|
||||
if not reference.startswith("#/"):
|
||||
raise ManifestError(reference_path, "external references are not supported")
|
||||
|
||||
parts = reference[2:].split("/")
|
||||
if len(parts) != 3 or parts[0] != "components":
|
||||
raise ManifestError(
|
||||
reference_path, "unsupported local reference namespace"
|
||||
)
|
||||
namespace, key = parts[1], parts[2]
|
||||
if namespace not in component_index:
|
||||
raise ManifestError(
|
||||
reference_path, "unsupported component reference namespace"
|
||||
)
|
||||
if "~0" in key or "~1" in key:
|
||||
raise ManifestError(
|
||||
reference_path, "unsupported escaped component reference"
|
||||
)
|
||||
if key not in component_index[namespace]:
|
||||
raise ManifestError(reference_path, "dangling local reference")
|
||||
|
||||
|
||||
def manifest_from_openrpc(document: Mapping[str, object]) -> ContractManifest:
|
||||
"""Normalize an OpenRPC document into the stable workflow contract shape."""
|
||||
openrpc_version = _string(document.get("openrpc"), "$.openrpc")
|
||||
if openrpc_version != "1.2.6":
|
||||
raise ManifestError(
|
||||
"$.openrpc",
|
||||
f"unsupported OpenRPC version '{openrpc_version}'; expected '1.2.6'",
|
||||
)
|
||||
methods = _list(document.get("methods"), "$.methods")
|
||||
components = _mapping(document.get("components"), "$.components")
|
||||
schemas = _mapping(components.get("schemas"), "$.components.schemas")
|
||||
component_errors = _mapping(components.get("errors"), "$.components.errors")
|
||||
|
||||
operations: list[ManifestOperation] = []
|
||||
seen_methods: set[str] = set()
|
||||
for method_index, method_value in enumerate(methods):
|
||||
method_path = f"$.methods[{method_index}]"
|
||||
method = _mapping(method_value, method_path)
|
||||
@@ -77,8 +146,13 @@ def manifest_from_openrpc(document: Mapping[str, object]) -> ContractManifest:
|
||||
if len(segments) < 2 or any(not segment for segment in segments):
|
||||
raise ManifestError(
|
||||
f"{method_path}.name",
|
||||
"expected a method name with non-empty dot-separated segments",
|
||||
"malformed dotted method name",
|
||||
)
|
||||
if method_name in seen_methods:
|
||||
raise ManifestError(
|
||||
f"{method_path}.name", f"duplicate method '{method_name}'"
|
||||
)
|
||||
seen_methods.add(method_name)
|
||||
|
||||
params: list[ManifestParameter] = []
|
||||
for parameter_index, parameter_value in enumerate(
|
||||
@@ -100,6 +174,17 @@ def manifest_from_openrpc(document: Mapping[str, object]) -> ContractManifest:
|
||||
|
||||
result_path = f"{method_path}.result"
|
||||
result = _mapping(method.get("result"), result_path)
|
||||
if "schema" not in result:
|
||||
raise ManifestError(f"{result_path}.schema", "expected an object")
|
||||
result_schema = _schema(result["schema"], f"{result_path}.schema")
|
||||
if set(result_schema) != {"$ref"} or not (
|
||||
isinstance(result_schema["$ref"], str)
|
||||
and result_schema["$ref"].startswith("#/components/schemas/")
|
||||
):
|
||||
raise ManifestError(
|
||||
f"{result_path}.schema",
|
||||
"success result must reference a named schema component",
|
||||
)
|
||||
errors: list[JsonSchema] = []
|
||||
for error_index, error_value in enumerate(
|
||||
_list(method.get("errors"), f"{method_path}.errors")
|
||||
@@ -114,7 +199,7 @@ def manifest_from_openrpc(document: Mapping[str, object]) -> ContractManifest:
|
||||
"namespace": segments[:-1],
|
||||
"action": segments[-1],
|
||||
"params": params,
|
||||
"result": {"schema": _schema(result.get("schema"), f"{result_path}.schema")},
|
||||
"result": {"schema": result_schema},
|
||||
"errors": errors,
|
||||
}
|
||||
)
|
||||
@@ -128,9 +213,15 @@ def manifest_from_openrpc(document: Mapping[str, object]) -> ContractManifest:
|
||||
for key in sorted(component_errors)
|
||||
}
|
||||
|
||||
return {
|
||||
manifest: ContractManifest = {
|
||||
"manifest_version": 1,
|
||||
"source": {"format": "openrpc", "openrpc_version": openrpc_version},
|
||||
"operations": sorted(operations, key=lambda operation: operation["method"]),
|
||||
"components": {"schemas": normalized_schemas, "errors": normalized_errors},
|
||||
}
|
||||
component_index = {
|
||||
"schemas": set(manifest["components"]["schemas"]),
|
||||
"errors": set(manifest["components"]["errors"]),
|
||||
}
|
||||
_validate_references(manifest, component_index)
|
||||
return manifest
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from copy import deepcopy
|
||||
from typing import Any
|
||||
|
||||
import pytest
|
||||
@@ -152,18 +153,18 @@ def test_rejects_invalid_or_missing_top_level_envelope_values(
|
||||
("name", "message"),
|
||||
[
|
||||
("", "expected a non-empty string"),
|
||||
("workflow", "expected a method name with non-empty dot-separated segments"),
|
||||
("workflow", "malformed dotted method name"),
|
||||
(
|
||||
"workflow..run",
|
||||
"expected a method name with non-empty dot-separated segments",
|
||||
"malformed dotted method name",
|
||||
),
|
||||
(
|
||||
".workflow.run",
|
||||
"expected a method name with non-empty dot-separated segments",
|
||||
"malformed dotted method name",
|
||||
),
|
||||
(
|
||||
"workflow.run.",
|
||||
"expected a method name with non-empty dot-separated segments",
|
||||
"malformed dotted method name",
|
||||
),
|
||||
],
|
||||
)
|
||||
@@ -222,3 +223,106 @@ def test_rejects_invalid_method_error_schema_shape() -> None:
|
||||
"$.methods[1].errors[0]",
|
||||
"expected a schema object",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("mutate", "path", "message"),
|
||||
[
|
||||
(
|
||||
lambda document: document.update({"openrpc": "2.0.0"}),
|
||||
"$.openrpc",
|
||||
"unsupported OpenRPC version '2.0.0'; expected '1.2.6'",
|
||||
),
|
||||
(lambda document: document.update({"methods": {}}), "$.methods", "expected an array"),
|
||||
(
|
||||
lambda document: document["methods"].append(
|
||||
deepcopy(document["methods"][0])
|
||||
),
|
||||
"$.methods[2].name",
|
||||
"duplicate method 'workflow.zeta.run'",
|
||||
),
|
||||
(
|
||||
lambda document: document["methods"][0].update({"name": "workflow..run"}),
|
||||
"$.methods[0].name",
|
||||
"malformed dotted method name",
|
||||
),
|
||||
(
|
||||
lambda document: document["methods"][0].update({"params": {}}),
|
||||
"$.methods[0].params",
|
||||
"expected an array",
|
||||
),
|
||||
(
|
||||
lambda document: document["methods"][0]["params"].append(
|
||||
{"name": "value", "required": "yes", "schema": {"type": "string"}}
|
||||
),
|
||||
"$.methods[0].params[0].required",
|
||||
"expected a boolean",
|
||||
),
|
||||
(
|
||||
lambda document: document["methods"][0].update({"result": {}}),
|
||||
"$.methods[0].result.schema",
|
||||
"expected an object",
|
||||
),
|
||||
(
|
||||
lambda document: document["methods"][0]["result"].update(
|
||||
{
|
||||
"schema": {
|
||||
"type": "object",
|
||||
"properties": {"ok": {"type": "boolean"}},
|
||||
}
|
||||
}
|
||||
),
|
||||
"$.methods[0].result.schema",
|
||||
"success result must reference a named schema component",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_rejects_malformed_openrpc_contracts(
|
||||
mutate: Any, path: str, message: str
|
||||
) -> None:
|
||||
document = synthetic_openrpc_document()
|
||||
mutate(document)
|
||||
|
||||
assert_manifest_error(document, path, message)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("reference", "message"),
|
||||
[
|
||||
("https://example.test/schema.json", "external references are not supported"),
|
||||
("#/definitions/Result", "unsupported local reference namespace"),
|
||||
(
|
||||
"#/components/parameters/Value",
|
||||
"unsupported component reference namespace",
|
||||
),
|
||||
("#/components/schemas/Missing", "dangling local reference"),
|
||||
(
|
||||
"#/components/schemas/A~0B",
|
||||
"unsupported escaped component reference",
|
||||
),
|
||||
],
|
||||
)
|
||||
def test_rejects_unsupported_or_dangling_references(
|
||||
reference: str, message: str
|
||||
) -> None:
|
||||
document = synthetic_openrpc_document()
|
||||
document["components"]["schemas"]["AlphaResult"]["properties"]["linked"] = {
|
||||
"$ref": reference
|
||||
}
|
||||
|
||||
with pytest.raises(ManifestError) as exc_info:
|
||||
manifest_from_openrpc(document)
|
||||
|
||||
assert exc_info.value.path.endswith(".properties.linked.$ref")
|
||||
assert exc_info.value.message == message
|
||||
|
||||
|
||||
def test_accepts_nested_schema_and_error_component_references() -> None:
|
||||
document = synthetic_openrpc_document()
|
||||
document["components"]["schemas"]["AlphaResult"]["properties"]["linked"] = {
|
||||
"$ref": "#/components/schemas/ZetaResult"
|
||||
}
|
||||
|
||||
manifest = manifest_from_openrpc(document)
|
||||
|
||||
assert manifest["operations"][0]["errors"] == [{"$ref": "#/components/errors/5000"}]
|
||||
|
||||
Reference in New Issue
Block a user