Skip to content

Commit fd33d24

Browse files
committed
fix(sdk): reject scalar str/bytes for get_many ids and attributes
`{"InfraDevice": {"ids": "dev-1"}}` and `{"InfraDevice": {"ids": ["dev-1"], "attributes": "name"}}` are easy mistakes — a caller with one id or one attribute reaches for the bare value rather than wrapping it in a list. Strings and bytes are iterable in Python, so the compiler used to walk them character-by-character: the first form sent seven one-character GraphQL ids, and the second built a selection set of `n { value } a { value } m { value } e { value }` because each character passed the GraphQL-identifier regex. Reject `str`/`bytes` explicitly for both fields and surface a descriptive `ValidationError` instead. The original list-of-id / list-of-attribute path is unchanged. Reported by the cubic AI code reviewer on #1108.
1 parent e52ccca commit fd33d24

2 files changed

Lines changed: 34 additions & 2 deletions

File tree

infrahub_sdk/client.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -203,10 +203,18 @@ def compile_get_many_query(
203203
problems.append(f"{kind}: entry must be a mapping with an 'ids' key")
204204
continue
205205
ids = entry.get("ids")
206-
if not ids:
207-
problems.append(f"{kind}: 'ids' must be a non-empty list")
206+
if not ids or isinstance(ids, (str, bytes)):
207+
# A bare string passed for ``ids`` would iterate character-by-character
208+
# below and turn one id into N one-character GraphQL ids — silently
209+
# wrong. Reject up front.
210+
problems.append(f"{kind}: 'ids' must be a non-empty list of id strings")
208211
continue
209212
attributes = entry.get("attributes") or []
213+
if isinstance(attributes, (str, bytes)):
214+
# Same trap as ``ids``: a bare string would split into single-char
215+
# field names that each pass the identifier regex.
216+
problems.append(f"{kind}: 'attributes' must be a list of attribute names")
217+
continue
210218
bad_attrs = [a for a in attributes if not isinstance(a, str) or not _GRAPHQL_IDENTIFIER_RE.match(a)]
211219
if bad_attrs:
212220
problems.append(f"{kind}: invalid attribute names {bad_attrs!r}")

tests/unit/sdk/test_get_many.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,30 @@ def test_invalid_attribute_name_raises() -> None:
121121
compile_get_many_query({"InfraDevice": {"ids": ["dev-1"], "attributes": ["name", "bad name"]}})
122122

123123

124+
def test_string_ids_rejected_not_iterated_as_chars() -> None:
125+
# Passing a bare string would silently iterate character-by-character and
126+
# send N one-character GraphQL ids. Reject up front.
127+
with pytest.raises(ValidationError, match="'ids' must be a non-empty list"):
128+
compile_get_many_query({"InfraDevice": {"ids": "dev-1"}}) # type: ignore[dict-item]
129+
130+
131+
def test_bytes_ids_rejected() -> None:
132+
with pytest.raises(ValidationError, match="'ids' must be a non-empty list"):
133+
compile_get_many_query({"InfraDevice": {"ids": b"dev-1"}}) # type: ignore[dict-item]
134+
135+
136+
def test_string_attributes_rejected_not_iterated_as_chars() -> None:
137+
# Same trap as ``ids``: a bare string would split into single-character
138+
# field names that each pass the GraphQL identifier regex.
139+
with pytest.raises(ValidationError, match="'attributes' must be a list"):
140+
compile_get_many_query({"InfraDevice": {"ids": ["dev-1"], "attributes": "name"}}) # type: ignore[dict-item]
141+
142+
143+
def test_bytes_attributes_rejected() -> None:
144+
with pytest.raises(ValidationError, match="'attributes' must be a list"):
145+
compile_get_many_query({"InfraDevice": {"ids": ["dev-1"], "attributes": b"name"}}) # type: ignore[dict-item]
146+
147+
124148
def test_multiple_problems_are_collected() -> None:
125149
with pytest.raises(ValidationError) as excinfo:
126150
compile_get_many_query(

0 commit comments

Comments
 (0)