Skip to content

Commit 288459f

Browse files
committed
Harden recursive trust metadata validation
1 parent 904ca61 commit 288459f

5 files changed

Lines changed: 89 additions & 11 deletions

File tree

CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ GovEngine follows conservative pre-1.0 versioning while the API boundary is stil
66

77
## Unreleased
88

9+
- Applies the shared bounded-JSON limits to key-resolution and trust-store
10+
adapter records, and rejects forbidden trust-material keys recursively across
11+
nested mappings, lists and tuples.
912
- Removes the unconsumed `govengine.contracts.execution` package and the
1013
runtime-owned `govengine.execution.command_shape` helper. RExecOp now owns
1114
argv normalization at its connector boundary, and clean-install/public-truth

SECURITY.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,4 +42,6 @@ New safety-sensitive code should be deterministic by default, testable without l
4242
- approved vs prepared execution shape;
4343
- dry-run/local/mock/live truth;
4444
- receipt/evidence non-claims;
45+
- bounded, recursively filtered metadata at key-resolution and trust-store
46+
adapter boundaries;
4547
- owner-review boundaries.

docs/API_STABILITY_MATRIX.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ or host-specific Ravenclaw/Tecrax runtime behavior.
6363
| experimental | govengine.runtime_shell | `GovControlAction`, `GovQueueLane`, `GovQueueSnapshot`, `GovRuntimeSnapshot`, `GovSchedulerTick`, `control_action_from_host_action`, `queue_snapshot_from_lanes`, `validate_control_action`, `validate_queue_snapshot`, `validate_runtime_snapshot`, `validate_scheduler_tick` | Host-provided runtime shell projection; no scheduler/storage/live-execution authority. |
6464
| adapter | govengine.sclite_contracts lazy exports | `GovSCLiteLifecycleVerifier`, `review_bundle_state`, `review_bundle_transition_decision`, `review_sclite_bundle`, `verify_lifecycle_manifest` | Lazy SCLite bridge exports; SCLite owns lifecycle and review verification. |
6565
| adapter | govengine.scope_ports | `FunctionalScopePort`, `GovScopePort` | Host-neutral scope port protocols/helpers. |
66-
| adapter | govengine.signing | `KeyResolutionRequest`, `KeyResolutionResult`, `KeyResolverPort`, `SignatureEnvelope`, `SignedArtifact`, `SigningPolicy`, `TrustPolicy`, `TrustStoreDecision`, `TrustStorePort`, `VerificationResult`, `canonical_govengine_record`, `govengine_record_digest`, `signed_artifact_from_record`, `signature_transition_decision`, `verify_signed_govengine_record` | Host-provided signer/verifier/key-resolver/trust-store decision records plus deterministic serialization/digest, signed-envelope helpers, and signature transition decisioning for GovEngine-owned records only; no SCLite canonicalization, PKI/KMS, or key-store ownership. |
66+
| adapter | govengine.signing | `KeyResolutionRequest`, `KeyResolutionResult`, `KeyResolverPort`, `SignatureEnvelope`, `SignedArtifact`, `SigningPolicy`, `TrustPolicy`, `TrustStoreDecision`, `TrustStorePort`, `VerificationResult`, `canonical_govengine_record`, `govengine_record_digest`, `signed_artifact_from_record`, `signature_transition_decision`, `verify_signed_govengine_record` | Host-provided signer/verifier/key-resolver/trust-store decision records plus deterministic serialization/digest, signed-envelope helpers, and signature transition decisioning for GovEngine-owned records only. Key-resolution and trust-store metadata is bounded JSON and rejects trust-material keys recursively through mappings and collections; GovEngine still owns no SCLite canonicalization, PKI/KMS, or key store. |
6767
| fixture | govengine.signing demo helpers | `DemoDigestSigner`, `DemoDigestVerifier`, `demo_sign_and_verify`, `demo_sign_govengine_record` | Deterministic demo-only signer/verifier helpers; not cryptographic identity proof. |
6868
| experimental | govengine.state_index | `ArtifactStateIndex` | Lightweight artifact state summary helper. |
6969
| experimental | govengine.state_machine | `GovRunState`, `StateTransition`, `apply_state_transition`, `validate_run_state`, `validate_state_transition` | Neutral run-state transitions; no persistence/scheduler/live-execution authority. |

govengine/signing.py

Lines changed: 26 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
from string import hexdigits
99
from typing import Any, Mapping, Protocol
1010

11+
from govengine._json_boundary import bounded_json_copy
1112
from govengine.api import GovApiError, require_mapping
1213
from govengine.core import ArtifactDescriptor, GovernanceContext, ReasonCode, TransitionDecision
1314

@@ -146,8 +147,9 @@ def __post_init__(self) -> None:
146147

147148
@classmethod
148149
def from_mapping(cls, value: Mapping[str, Any]) -> "KeyResolutionResult":
149-
raw = require_mapping(value, reason_code="invalid_key_resolution_result")
150-
_reject_forbidden_trust_material(raw)
150+
raw = _bounded_trust_mapping(
151+
require_mapping(value, reason_code="invalid_key_resolution_result")
152+
)
151153
metadata = raw.get("metadata") if isinstance(raw.get("metadata"), Mapping) else {}
152154
return cls(
153155
status=str(raw.get("status") or ""),
@@ -195,8 +197,9 @@ def __post_init__(self) -> None:
195197

196198
@classmethod
197199
def from_mapping(cls, value: Mapping[str, Any]) -> "TrustStoreDecision":
198-
raw = require_mapping(value, reason_code="invalid_trust_store_decision")
199-
_reject_forbidden_trust_material(raw)
200+
raw = _bounded_trust_mapping(
201+
require_mapping(value, reason_code="invalid_trust_store_decision")
202+
)
200203
metadata = raw.get("metadata") if isinstance(raw.get("metadata"), Mapping) else {}
201204
return cls(
202205
status=str(raw.get("status") or raw.get("trust_status") or ""),
@@ -435,14 +438,27 @@ def _validate_govengine_record_digest(record_digest: str) -> str:
435438

436439
def _bounded_trust_metadata(value: Mapping[str, Any] | None) -> dict[str, Any]:
437440
metadata = value if isinstance(value, Mapping) else {}
438-
_reject_forbidden_trust_material(metadata)
439-
return dict(metadata)
441+
return _bounded_trust_mapping(metadata)
442+
440443

444+
def _bounded_trust_mapping(value: Mapping[str, Any]) -> dict[str, Any]:
445+
copied = bounded_json_copy(value)
446+
if not isinstance(copied, dict):
447+
raise GovApiError("invalid_trust_metadata")
448+
_reject_forbidden_trust_material(copied)
449+
return copied
441450

442-
def _reject_forbidden_trust_material(value: Mapping[str, Any]) -> None:
443-
for key in value:
444-
if str(key).lower() in FORBIDDEN_TRUST_MATERIAL_KEYS:
445-
raise GovApiError("forbidden_trust_material")
451+
452+
def _reject_forbidden_trust_material(value: Any) -> None:
453+
if isinstance(value, Mapping):
454+
for key, nested in value.items():
455+
if key.strip().lower() in FORBIDDEN_TRUST_MATERIAL_KEYS:
456+
raise GovApiError("forbidden_trust_material")
457+
_reject_forbidden_trust_material(nested)
458+
return
459+
if isinstance(value, (list, tuple)):
460+
for nested in value:
461+
_reject_forbidden_trust_material(nested)
446462

447463

448464
def _canonical_record_value(value: Any) -> Any:

tests/test_signing_bridge.py

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,63 @@ def test_key_resolution_request_rejects_api_key_metadata() -> None:
327327
KeyResolutionRequest(signer_id="owner-demo", metadata={"api_key": "must-not-cross-boundary"})
328328

329329

330+
@pytest.mark.parametrize(
331+
"record",
332+
[
333+
lambda metadata: KeyResolutionRequest(
334+
signer_id="owner-demo",
335+
metadata=metadata,
336+
),
337+
lambda metadata: KeyResolutionResult.from_mapping({
338+
"status": "resolved",
339+
"signer_id": "owner-demo",
340+
"key_ref": "host-key://owner-demo/current",
341+
"metadata": metadata,
342+
}),
343+
lambda metadata: TrustStoreDecision.from_mapping({
344+
"status": "trusted",
345+
"signer_id": "owner-demo",
346+
"trust_anchor_ref": "host-trust://anchors/demo",
347+
"metadata": metadata,
348+
}),
349+
],
350+
)
351+
def test_trust_records_reject_forbidden_keys_inside_nested_collections(record) -> None:
352+
with pytest.raises(GovApiError, match="forbidden_trust_material"):
353+
record({"items": [{"nested": ({"password": "must-not-cross-boundary"},)}]})
354+
355+
356+
def test_trust_records_normalize_forbidden_key_spelling() -> None:
357+
with pytest.raises(GovApiError, match="forbidden_trust_material"):
358+
TrustStoreDecision.from_mapping({
359+
"status": "trusted",
360+
"signer_id": "owner-demo",
361+
"trust_anchor_ref": "host-trust://anchors/demo",
362+
"metadata": {"nested": {" PASSWORD ": "must-not-cross-boundary"}},
363+
})
364+
365+
366+
def test_trust_metadata_uses_shared_json_depth_limit() -> None:
367+
metadata = {}
368+
cursor = metadata
369+
for _ in range(34):
370+
cursor["nested"] = {}
371+
cursor = cursor["nested"]
372+
373+
with pytest.raises(GovApiError, match="json_boundary_max_depth"):
374+
KeyResolutionRequest(signer_id="owner-demo", metadata=metadata)
375+
376+
377+
def test_trust_metadata_rejects_non_finite_numbers() -> None:
378+
with pytest.raises(GovApiError, match="json_boundary_non_finite_number"):
379+
KeyResolutionResult.from_mapping({
380+
"status": "resolved",
381+
"signer_id": "owner-demo",
382+
"key_ref": "host-key://owner-demo/current",
383+
"metadata": {"confidence": nan},
384+
})
385+
386+
330387
def test_trust_store_decision_unknown_signer_is_not_trusted() -> None:
331388
decision = TrustStoreDecision.from_mapping({
332389
"status": "unknown",

0 commit comments

Comments
 (0)