diff --git a/loopx/cli_commands/goal_channel_operation.py b/loopx/cli_commands/goal_channel_operation.py index cc8e70ef9e..ff5c2dd70a 100644 --- a/loopx/cli_commands/goal_channel_operation.py +++ b/loopx/cli_commands/goal_channel_operation.py @@ -245,28 +245,35 @@ def _operation_error_packet( return packet -def _safe_validation_reason(exc: BaseException) -> str: - """Return the operator-actionable validation reason without private paths. - - The typed-operation normalizer and proposal helpers raise code-authored - ValueErrors ("operation payload_digest does not match payload", "operation - executor binding is invalid", ...). Collapsing every one of them into - "the Goal Channel configuration is invalid" hides the real cause from the - operator and from the agent. Keep the message when it is clearly a - code-authored phrase, and fall back to the exception class otherwise. +#: Fixed, public-safe validation reasons the typed-operation owner can raise. +#: The packet publishes the typed code, never the free-text exception, so no +#: request-derived value can leak through this diagnostic field. +_VALIDATION_REASON_CODES = { + "operation executor binding is invalid": "operation_executor_binding_invalid", + "operation payload_digest does not match payload": "operation_payload_digest_mismatch", + "operation projection fields are unavailable": "operation_projection_fields_unavailable", + "operation projection is unavailable": "operation_projection_unavailable", + "operation review plan is unavailable": "operation_review_plan_unavailable", + "operation review frame is unavailable": "operation_review_frame_unavailable", + "typed operation proposal is unavailable": "typed_operation_proposal_unavailable", + "operation request JSON must be an object": "operation_request_not_an_object", + "the preparing Agent must be registered for this Goal": "preparing_agent_unregistered", +} +_UNCLASSIFIED_VALIDATION_REASON = "unclassified_validation_error" + + +def _validation_reason_code(exc: BaseException) -> str: + """Map a code-authored validator message to a fixed public-safe code. + + Collapsing every ValueError into "the Goal Channel configuration is + invalid" hides the real cause from the operator and from the agent, while + echoing the message would publish request-derived text. Publish a typed + code from a fixed allowlist and fall back for anything unknown. """ - text = str(exc).strip() - if ( - not text - or len(text) > 240 - or "/" in text - or "\\" in text - or "~" in text - or any(ord(character) < 32 for character in text) - ): - return type(exc).__name__ - return text + return _VALIDATION_REASON_CODES.get( + str(exc).strip(), _UNCLASSIFIED_VALIDATION_REASON + ) def run_goal_channel_operation( @@ -378,12 +385,13 @@ def run_goal_channel_operation( ) except ValueError as exc: # Keep the compatibility blocker, but never discard the real reason the - # typed-operation owner rejected this request. + # typed-operation owner rejected this request. Only a fixed, public-safe + # code is published; the message itself may carry request-derived text. return _operation_error_packet( request=request, blocker="invalid_configuration", summary="the Goal Channel configuration is invalid", - details={"validation_reason": _safe_validation_reason(exc)}, + details={"validation_reason_code": _validation_reason_code(exc)}, ) except Exception: return _operation_error_packet( diff --git a/tests/extensions/test_lark_goal_channel_operation.py b/tests/extensions/test_lark_goal_channel_operation.py index def1a9cfa0..a9a1a18928 100644 --- a/tests/extensions/test_lark_goal_channel_operation.py +++ b/tests/extensions/test_lark_goal_channel_operation.py @@ -19,7 +19,7 @@ from loopx.cli_commands.goal_channel_operation import ( GoalChannelOperationContext, _prepare_goal_channel_operation, - _safe_validation_reason, + _validation_reason_code, run_goal_channel_operation, ) from loopx.control_plane.collaboration.operation_handoff import agent_operation_action @@ -2408,24 +2408,85 @@ def failing_record( assert any("+messages-send" in call for call in calls) -def test_safe_validation_reason_keeps_actionable_code_messages() -> None: - """A rejected prepare must surface its real reason, not a generic line.""" +def test_validation_reason_code_publishes_only_fixed_codes() -> None: + """A rejected prepare must surface a typed reason, never free text.""" assert ( - _safe_validation_reason(ValueError("operation executor binding is invalid")) - == "operation executor binding is invalid" + _validation_reason_code(ValueError("operation executor binding is invalid")) + == "operation_executor_binding_invalid" ) assert ( - _safe_validation_reason( - ValueError("operation payload_digest does not match payload") - ) - == "operation payload_digest does not match payload" + _validation_reason_code(ValueError("operation payload_digest does not match payload")) + == "operation_payload_digest_mismatch" + ) + # Unknown text - including anything request-derived - falls back. + assert ( + _validation_reason_code(ValueError("private detail: /home/operator/secret")) + == "unclassified_validation_error" ) - # Anything that could carry a private path or control characters collapses - # to the exception class instead of reaching the operator packet. assert ( - _safe_validation_reason(ValueError("private detail: /home/operator/secret")) - == "ValueError" + _validation_reason_code(ValueError("asset=SYNTH request text")) + == "unclassified_validation_error" ) - assert _safe_validation_reason(ValueError("line\nbreak")) == "ValueError" - assert _safe_validation_reason(ValueError("")) == "ValueError" + assert _validation_reason_code(ValueError("")) == "unclassified_validation_error" + + +def test_prepare_operation_publishes_a_typed_validation_reason(tmp_path: Path) -> None: + """The CLI packet carries a fixed code and no request-derived text.""" + + store, registry, runtime, channel, _target = _fixture(tmp_path) + payload = {"schema_version": "synthetic_payload_v0", "marker": "synthetic-only"} + request_path = tmp_path / "request.json" + request_path.write_text( + json.dumps( + { + "schema_version": "loopx_operation_request_v0", + "goal_id": GOAL_ID, + "agent_id": AGENT_ID, + "domain": "finance", + "operation_kind": "finance.order.simulate", + "operation_schema": "synthetic_payload_v0", + "payload_ref": "private-input:synthetic-only", + "payload": payload, + # Deliberately wrong: the normalizer rejects this digest. + "payload_digest": "0" * 64, + "projection": { + "schema_version": "loopx_operation_projection_v0", + "title": "Synthetic validation fixture", + "subtitle": "Synthetic fixture ยท no venue call", + "focus": "Reason-code regression", + "fields": [{"label": "Order", "value": "synthetic"}], + "warning": "Synthetic only.", + "simulated": True, + }, + "destination_account_ref": "account:simulation", + "expires_at": ( + datetime.now(timezone.utc) + timedelta(minutes=30) + ).isoformat(), + "authorized_principals": [f"lark:{OPERATOR_ID}"], + } + ), + encoding="utf-8", + ) + args = Namespace( + goal_channel_command="prepare-operation", + goal_id=GOAL_ID, + agent_id=AGENT_ID, + summary="Synthetic reason-code fixture", + idempotency_key="validation-reason-code-fixture", + request_json=str(request_path), + execute=False, + target_path=None, + ) + packet = run_goal_channel_operation( + args, + context=GoalChannelOperationContext(runtime, registry, runtime, channel), + ) + assert packet["ok"] is False + assert packet["blocker"] == "invalid_configuration" + assert packet["details"] == { + "validation_reason_code": "operation_payload_digest_mismatch" + } + rendered = json.dumps(packet) + assert "does not match payload" not in rendered + assert "synthetic-only" not in rendered