From 6f6b47a87d9dcf2a089fc804979c92161d5a1e11 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 31 Jul 2026 11:03:06 +0500 Subject: [PATCH] fix(workflows): guard a non-string overlay edit 'operation' MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_parse_edit` reads `operation` straight from hand-edited YAML and then does `if operation not in VALID_OPERATIONS`. `VALID_OPERATIONS` is a frozenset, so that membership test hashes the value — and an unhashable one raises: operation={'insert_after': 'a'} -> TypeError: unhashable type: 'dict' operation=['insert_after'] -> TypeError: unhashable type: 'list' `validate_overlay_yaml`'s docstring promises "validation never raises", and nothing upstream catches TypeError (layer_sources wraps only YAMLError/OSError/UnicodeDecodeError; _commands catches only ValueError), so the CLI dies with a raw traceback instead of reporting the error. The trigger is an ordinary authoring mistake: nesting the recommended shorthand form under the explicit key. Every other field in the same function is isinstance-guarded first (`anchor`, `step`, `step["id"]`); `operation` was the outlier. Check the type first and return the message the function already uses for `operation: None` / `operation: 7`. Co-Authored-By: Claude Opus 5 (1M context) --- src/specify_cli/workflows/overlays/schema.py | 7 +++- tests/workflows/test_overlay_schema.py | 38 ++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/src/specify_cli/workflows/overlays/schema.py b/src/specify_cli/workflows/overlays/schema.py index 221d2fe8e5..0a018b7af0 100644 --- a/src/specify_cli/workflows/overlays/schema.py +++ b/src/specify_cli/workflows/overlays/schema.py @@ -87,7 +87,12 @@ def _parse_edit(edit_raw: dict[str, Any], idx: int) -> tuple[OverlayEdit | None, else: return None, f"Edit at index {idx} has no operation; expected one of {sorted(VALID_OPERATIONS)}." - if operation not in VALID_OPERATIONS: + # ``operation`` comes straight from hand-edited YAML, so it may be an + # unhashable mapping/sequence (``operation: {insert_after: a}`` when the + # shorthand form is nested by mistake). Membership-testing an unhashable + # value against the frozenset raises TypeError, which would escape this + # never-raising validator; check the type first, like 'anchor' below. + if not isinstance(operation, str) or operation not in VALID_OPERATIONS: return None, f"Edit at index {idx} has invalid operation {operation!r}." if not isinstance(anchor, str) or not anchor: diff --git a/tests/workflows/test_overlay_schema.py b/tests/workflows/test_overlay_schema.py index 08813f853b..77e0432eca 100644 --- a/tests/workflows/test_overlay_schema.py +++ b/tests/workflows/test_overlay_schema.py @@ -136,6 +136,44 @@ def test_invalid_operation_field_rejected(self): assert overlay is None assert any("operation" in e.lower() for e in errors), errors + @pytest.mark.parametrize( + "operation", + [ + {"insert_after": "a"}, + ["insert_after"], + ], + ) + def test_non_string_operation_rejected_without_raising(self, operation): + """An unhashable 'operation' must be reported, not raised. + + `VALID_OPERATIONS` is a frozenset, so `operation not in ...` hashes the + value. Nesting the shorthand form under the explicit key by mistake + (`operation: {insert_after: a}`) therefore raised + `TypeError: unhashable type: 'dict'` out of a validator whose docstring + promises "validation never raises" — and nothing upstream catches + TypeError, so the CLI died with a raw traceback. + """ + overlay, errors = validate_overlay_yaml( + { + "id": "ov", + "extends": "wf", + "priority": 10, + "edits": [ + { + "operation": operation, + "anchor": "a", + "step": { + "id": "b", + "type": "command", + "command": "echo", + }, + } + ], + } + ) + assert overlay is None + assert any("invalid operation" in err for err in errors), errors + def test_shorthand_and_explicit_mixed_list(self): overlay, errors = validate_overlay_yaml( {