fix(smoke): report sanitized operation failure diagnostics
This commit is contained in:
@@ -21,10 +21,17 @@ class SmokeError(RuntimeError):
|
||||
pass
|
||||
|
||||
|
||||
def safe_error(stage: str, code: str, status: int | None = None) -> SmokeError:
|
||||
def safe_error(
|
||||
stage: str,
|
||||
code: str,
|
||||
status: int | None = None,
|
||||
trace_id: str | None = None,
|
||||
) -> SmokeError:
|
||||
message = f"stage={stage[:64]} code={code[:96]}"
|
||||
if status is not None:
|
||||
message += f" status={status}"
|
||||
if trace_id is not None:
|
||||
message += f" trace_id={trace_id}"
|
||||
return SmokeError(message[:256])
|
||||
|
||||
|
||||
@@ -286,6 +293,29 @@ def create_operation(
|
||||
raise safe_error("operation_create", "invalid_response") from error
|
||||
|
||||
|
||||
def is_safe_diagnostic_identifier(value: Any) -> bool:
|
||||
return (
|
||||
isinstance(value, str)
|
||||
and 1 <= len(value) <= 64
|
||||
and value[0].islower()
|
||||
and value[0].isascii()
|
||||
and all(
|
||||
character.isascii()
|
||||
and (character.islower() or character.isdigit() or character == "_")
|
||||
for character in value
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def is_safe_trace_id(value: Any) -> bool:
|
||||
return (
|
||||
isinstance(value, str)
|
||||
and len(value) == 32
|
||||
and value != "0" * 32
|
||||
and all(character in "0123456789abcdef" for character in value)
|
||||
)
|
||||
|
||||
|
||||
def run_operation_test(
|
||||
client: Client,
|
||||
workspace_id: str,
|
||||
@@ -297,9 +327,24 @@ def run_operation_test(
|
||||
admin_path(workspace_id, f"/operations/{operation_id}/test-runs"),
|
||||
{"version": operation_version, "input": {"probe": "ok"}},
|
||||
).body
|
||||
if not isinstance(result, dict) or result.get("ok") is not True:
|
||||
if isinstance(result, dict) and result.get("ok") is True:
|
||||
return
|
||||
if not isinstance(result, dict) or result.get("ok") is not False:
|
||||
raise safe_error("operation_test", "outcome_not_ok")
|
||||
|
||||
errors = result.get("errors")
|
||||
failure = errors[0] if isinstance(errors, list) and errors else None
|
||||
code = failure.get("code") if isinstance(failure, dict) else None
|
||||
stage = failure.get("stage") if isinstance(failure, dict) else None
|
||||
trace_id = result.get("trace_id")
|
||||
if (
|
||||
is_safe_diagnostic_identifier(code)
|
||||
and is_safe_diagnostic_identifier(stage)
|
||||
and is_safe_trace_id(trace_id)
|
||||
):
|
||||
raise safe_error(stage, code, trace_id=trace_id)
|
||||
raise safe_error("operation_test", "outcome_not_ok")
|
||||
|
||||
|
||||
def operation_etag(client: Client, workspace_id: str, operation_id: str) -> str:
|
||||
response = client.request_json(
|
||||
|
||||
@@ -207,8 +207,9 @@ class AuthenticatedProductSmokeTests(unittest.TestCase):
|
||||
publish = next(request for request in client.requests if request[1].endswith("/operations/op_safe/publish"))
|
||||
self.assertEqual(publish[3]["headers"], {"If-Match": '"safe-etag"'})
|
||||
|
||||
def test_admin_test_run_uses_created_version_and_rejects_failed_outcome(self) -> None:
|
||||
def test_admin_test_run_uses_created_version_and_reports_safe_typed_failure(self) -> None:
|
||||
smoke = load_smoke_module()
|
||||
trace_id = "0123456789abcdef0123456789abcdef"
|
||||
|
||||
class FakeClient:
|
||||
def __init__(self, ok):
|
||||
@@ -217,7 +218,22 @@ class AuthenticatedProductSmokeTests(unittest.TestCase):
|
||||
|
||||
def request_json(self, method, path, payload=None, **kwargs):
|
||||
self.payload = payload
|
||||
return smoke.JsonResponse(200, {}, {"ok": self.ok, "errors": [{"message": "secret-canary"}]})
|
||||
return smoke.JsonResponse(
|
||||
200,
|
||||
{},
|
||||
{
|
||||
"ok": self.ok,
|
||||
"trace_id": trace_id,
|
||||
"errors": [
|
||||
{
|
||||
"code": "outbound_target_rejected",
|
||||
"stage": "adapter",
|
||||
"message": "secret-canary",
|
||||
"context": {"url": "https://private.invalid/token-canary"},
|
||||
}
|
||||
],
|
||||
},
|
||||
)
|
||||
|
||||
passing = FakeClient(True)
|
||||
smoke.run_operation_test(passing, "ws", "op", 9)
|
||||
@@ -225,7 +241,188 @@ class AuthenticatedProductSmokeTests(unittest.TestCase):
|
||||
|
||||
with self.assertRaises(smoke.SmokeError) as raised:
|
||||
smoke.run_operation_test(FakeClient(False), "ws", "op", 9)
|
||||
self.assertNotIn("secret-canary", str(raised.exception))
|
||||
self.assertEqual(
|
||||
str(raised.exception),
|
||||
"stage=adapter code=outbound_target_rejected trace_id=0123456789abcdef0123456789abcdef",
|
||||
)
|
||||
for forbidden in ("secret-canary", "private.invalid", "token-canary", "context"):
|
||||
self.assertNotIn(forbidden, str(raised.exception))
|
||||
|
||||
def test_admin_test_run_fails_closed_for_malformed_diagnostics(self) -> None:
|
||||
smoke = load_smoke_module()
|
||||
valid_trace_id = "0123456789abcdef0123456789abcdef"
|
||||
valid_failure = {"code": "upstream_timeout", "stage": "upstream"}
|
||||
malformed_results = (
|
||||
None,
|
||||
[],
|
||||
"secret-canary",
|
||||
{"trace_id": valid_trace_id, "errors": [valid_failure]},
|
||||
{"ok": None, "trace_id": valid_trace_id, "errors": [valid_failure]},
|
||||
{"ok": 0, "trace_id": valid_trace_id, "errors": [valid_failure]},
|
||||
{"ok": "false", "trace_id": valid_trace_id, "errors": [valid_failure]},
|
||||
{"ok": False, "errors": []},
|
||||
{"ok": False, "errors": {}},
|
||||
{"ok": False, "trace_id": valid_trace_id, "errors": [None]},
|
||||
{"ok": False, "errors": ["secret-canary"]},
|
||||
{"ok": False, "errors": [valid_failure]},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id[:-1],
|
||||
"errors": [valid_failure],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id + "0",
|
||||
"errors": [valid_failure],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": "0" * 32,
|
||||
"errors": [valid_failure],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id.upper(),
|
||||
"errors": [valid_failure],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": "a" * 31 + "١",
|
||||
"errors": [valid_failure],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": "a" * 31 + "\n",
|
||||
"errors": [valid_failure],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": 42,
|
||||
"errors": [valid_failure],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "", "stage": "upstream"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "a" * 65, "stage": "upstream"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "Upstream_timeout", "stage": "upstream"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "upstream_таймаут", "stage": "upstream"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": 42, "stage": "upstream"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "upstream_timeout", "stage": ""}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "upstream_timeout", "stage": "a" * 65}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "upstream_timeout", "stage": "Upstream"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "upstream_timeout", "stage": "вверх"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "upstream_timeout", "stage": 42}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "unsafe-value://secret-canary", "stage": "upstream"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": valid_trace_id,
|
||||
"errors": [{"code": "upstream_timeout", "stage": "unsafe stage secret-canary"}],
|
||||
},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": "not-a-trace-id-secret-canary",
|
||||
"errors": [{"code": "upstream_timeout", "stage": "upstream"}],
|
||||
},
|
||||
)
|
||||
|
||||
class FakeClient:
|
||||
def __init__(self, result):
|
||||
self.result = result
|
||||
|
||||
def request_json(self, *args, **kwargs):
|
||||
return smoke.JsonResponse(200, {}, self.result)
|
||||
|
||||
for result in malformed_results:
|
||||
with self.subTest(result=result), self.assertRaises(smoke.SmokeError) as raised:
|
||||
smoke.run_operation_test(FakeClient(result), "ws", "op", 9)
|
||||
self.assertEqual(str(raised.exception), "stage=operation_test code=outcome_not_ok")
|
||||
self.assertNotIn("secret-canary", str(raised.exception))
|
||||
|
||||
def test_admin_test_run_accepts_maximum_length_safe_diagnostics(self) -> None:
|
||||
smoke = load_smoke_module()
|
||||
identifier = "a" * 64
|
||||
trace_id = "0123456789abcdef0123456789abcdef"
|
||||
|
||||
class FakeClient:
|
||||
def request_json(self, *args, **kwargs):
|
||||
return smoke.JsonResponse(
|
||||
200,
|
||||
{},
|
||||
{
|
||||
"ok": False,
|
||||
"trace_id": trace_id,
|
||||
"request_preview": {"credential": "secret-canary"},
|
||||
"response_preview": {"token": "secret-canary"},
|
||||
"errors": [{"code": identifier, "stage": identifier}],
|
||||
},
|
||||
)
|
||||
|
||||
with self.assertRaises(smoke.SmokeError) as raised:
|
||||
smoke.run_operation_test(FakeClient(), "ws", "op", 9)
|
||||
rendered = str(raised.exception)
|
||||
self.assertEqual(rendered, f"stage={identifier} code={identifier} trace_id={trace_id}")
|
||||
self.assertLessEqual(len(rendered.encode("ascii")), 256)
|
||||
self.assertNotIn("secret-canary", rendered)
|
||||
|
||||
def test_admin_test_run_success_ignores_malformed_diagnostics(self) -> None:
|
||||
smoke = load_smoke_module()
|
||||
|
||||
class FakeClient:
|
||||
def request_json(self, *args, **kwargs):
|
||||
return smoke.JsonResponse(
|
||||
200,
|
||||
{},
|
||||
{
|
||||
"ok": True,
|
||||
"trace_id": "secret-canary",
|
||||
"errors": "secret-canary",
|
||||
"request_preview": {"token": "secret-canary"},
|
||||
},
|
||||
)
|
||||
|
||||
smoke.run_operation_test(FakeClient(), "ws", "op", 9)
|
||||
|
||||
def test_edit_and_archive_use_fresh_operation_preconditions(self) -> None:
|
||||
smoke = load_smoke_module()
|
||||
|
||||
Reference in New Issue
Block a user