From f5ddcebd7d5c908474a4eda03c19fa2e645bfa9f Mon Sep 17 00:00:00 2001 From: Yernat Yestekov Date: Tue, 11 Aug 2026 17:25:12 -0700 Subject: [PATCH 1/6] fix(libvirt): reject shell injection via TechVault interface fields The generated guest init script (run as root inside the appliance) interpolated interface `mac`, `ip`, and `cidr_prefix` into shell text unquoted. A malformed or hostile value such as a mac of `aa:bb:cc:dd:ee:ff) ; rm -rf /outside #` or an ip containing `$(...)` injected arbitrary commands into a root-run script; only the hostname was quoted. Validate each interpolated field to its structural shape (MAC address, IPv4/IPv6 address, integer CIDR prefix) and reject malformed input with a typed ValueError, then quote every value with the existing _shell_quote as defense in depth. Both the native and guest-certified init scripts now share one validated helper, closing the identical injection in guest_appliance.py. Also harden _cpio_newc: a member path containing a newline silently corrupted the newline-delimited cpio archive (subprocess ran with check=False), so reject newline-bearing paths before invoking cpio. Co-Authored-By: Claude Opus 5 (1M context) --- .../raes_backend_libvirt/guest_appliance.py | 10 +-- .../techvault_appliance.py | 66 ++++++++++++++++--- .../test_libvirt_backend_techvault_native.py | 43 ++++++++++++ 3 files changed, 103 insertions(+), 16 deletions(-) diff --git a/implementations/python/packages/raes_backend_libvirt/guest_appliance.py b/implementations/python/packages/raes_backend_libvirt/guest_appliance.py index 30951eee9..59d8fef29 100644 --- a/implementations/python/packages/raes_backend_libvirt/guest_appliance.py +++ b/implementations/python/packages/raes_backend_libvirt/guest_appliance.py @@ -19,7 +19,7 @@ from dataclasses import dataclass from pathlib import Path -from .techvault_appliance import _cpio_newc, _shell_quote +from .techvault_appliance import _cpio_newc, _interface_case_lines, _shell_quote _APPLETS = ( "sh", "mount", "mdev", "ip", "ifconfig", "sleep", "cat", "hostname", "printf", "echo", @@ -132,13 +132,7 @@ def _init_script(domain: Mapping[str, object]) -> str: for interface in _as_sequence(domain.get("interfaces")): if not isinstance(interface, Mapping): continue - lines.extend( - [ - f" {interface.get('mac')})", - f' ip addr add {interface.get("ip")}/{interface.get("cidr_prefix")} dev "$iface"', - " ;;", - ] - ) + lines.extend(_interface_case_lines(interface)) lines.extend([" esac", "done"]) lines.extend(_REALIZE_SNIPPET) lines.append("sleep 1") diff --git a/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py b/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py index 6773ffc38..0537eac34 100644 --- a/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py +++ b/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py @@ -3,8 +3,10 @@ from __future__ import annotations import gzip +import ipaddress import json import os +import re import shutil import stat import subprocess @@ -91,22 +93,20 @@ def _init_script(domain: Mapping[str, object]) -> str: for interface in _as_sequence(domain.get("interfaces")): if not isinstance(interface, Mapping): continue - lines.extend( - [ - f" {interface.get('mac')})", - f' ip addr add {interface.get("ip")}/{interface.get("cidr_prefix")} dev "$iface"', - " ;;", - ] - ) + lines.extend(_interface_case_lines(interface)) lines.extend([" esac", "done"]) lines.extend(["while true; do sleep 3600; done", ""]) return "\n".join(lines) def _cpio_newc(root: Path) -> bytes: + paths = _cpio_paths(root) + for path in paths: + if "\n" in path: + raise ValueError(f"initramfs member path contains a newline: {path!r}") proc = subprocess.run( ["cpio", "-o", "-H", "newc", "--quiet"], - input=("\n".join(_cpio_paths(root)) + "\n").encode(), + input=("\n".join(paths) + "\n").encode(), cwd=root, capture_output=True, check=False, @@ -124,5 +124,55 @@ def _as_sequence(value: object) -> Sequence[object]: return value if isinstance(value, list | tuple) else () +_MAC_RE = re.compile(r"\A[0-9A-Fa-f]{2}(?::[0-9A-Fa-f]{2}){5}\Z") + + +def _interface_case_lines(interface: Mapping[str, object]) -> list[str]: + """Render one shell-safe ``case`` arm configuring an interface address. + + ``mac``/``ip``/``cidr_prefix`` are interpolated into a root-run guest init + script. Each is validated to its structural shape and rejected when + malformed, then quoted as defense in depth: a field that is not the shape it + claims to be (a ``mac`` that is not a MAC) is a bug in the plan, not text to + escape into a command. + """ + + mac = _validated_mac(interface.get("mac")) + address = _validated_interface_address(interface.get("ip"), interface.get("cidr_prefix")) + return [ + f" {_shell_quote(mac)})", + f' ip addr add {_shell_quote(address)} dev "$iface"', + " ;;", + ] + + +def _validated_mac(value: object) -> str: + if isinstance(value, str) and _MAC_RE.match(value): + return value + raise ValueError(f"interface mac is not a MAC address: {value!r}") + + +def _validated_interface_address(ip: object, cidr_prefix: object) -> str: + if not isinstance(ip, str): + raise ValueError(f"interface ip is not an IP address: {ip!r}") + try: + parsed = ipaddress.ip_address(ip) + except ValueError as error: + raise ValueError(f"interface ip is not an IP address: {ip!r}") from error + return f"{ip}/{_validated_cidr_prefix(cidr_prefix, parsed.max_prefixlen)}" + + +def _validated_cidr_prefix(value: object, max_prefix: int) -> int: + if isinstance(value, bool): + raise ValueError(f"interface cidr_prefix is not an integer: {value!r}") + if isinstance(value, str) and value.isdigit(): + value = int(value) + if not isinstance(value, int): + raise ValueError(f"interface cidr_prefix is not an integer: {value!r}") + if not 0 <= value <= max_prefix: + raise ValueError(f"interface cidr_prefix is out of range 0..{max_prefix}: {value!r}") + return value + + def _shell_quote(value: str) -> str: return "'" + value.replace("'", "'\"'\"'") + "'" diff --git a/implementations/python/tests/test_libvirt_backend_techvault_native.py b/implementations/python/tests/test_libvirt_backend_techvault_native.py index 8406e48b0..218bbea90 100644 --- a/implementations/python/tests/test_libvirt_backend_techvault_native.py +++ b/implementations/python/tests/test_libvirt_backend_techvault_native.py @@ -15,6 +15,7 @@ from raes_backend_libvirt.cloudinit import CloudInitSpec, CloudInitUser from raes_backend_libvirt.driver import DomainSpec, NetworkAcl, NetworkSpec, ServiceSpec from raes_backend_libvirt.envelopes import load_libvirt_realization_envelope +from raes_backend_libvirt.techvault_appliance import _cpio_newc, _init_script from raes_backend_libvirt.techvault_native import ( BusyboxInitramfsBuilder, ProbeResult, @@ -923,3 +924,45 @@ def test_busybox_initramfs_builder_writes_gzip_cpio(tmp_path): assert target.read_bytes().startswith(b"\x1f\x8b") assert target.stat().st_size > 1000 assert b"httpd -p" not in gzip.decompress(target.read_bytes()) + + +_VALID_INTERFACE = {"mac": "52:54:00:00:00:01", "ip": "192.0.2.10", "cidr_prefix": 24} + + +def _domain_with_interface(**interface: object) -> dict[str, object]: + return {"name": "webapp", "interfaces": [interface]} + + +def test_init_script_quotes_valid_interface_addressing(): + script = _init_script(_domain_with_interface(**_VALID_INTERFACE)) + + assert " '52:54:00:00:00:01')" in script + assert "ip addr add '192.0.2.10/24' dev \"$iface\"" in script + + +@pytest.mark.parametrize( + ("interface", "match"), + ( + ( + {**_VALID_INTERFACE, "mac": "aa:bb:cc:dd:ee:ff) ; rm -rf /outside #"}, + "mac is not a MAC", + ), + ({**_VALID_INTERFACE, "ip": "192.0.2.10$(touch /pwned)"}, "ip is not an IP"), + ({**_VALID_INTERFACE, "ip": "`reboot`"}, "ip is not an IP"), + ({**_VALID_INTERFACE, "cidr_prefix": "24; rm -rf /"}, "cidr_prefix is not an integer"), + ({**_VALID_INTERFACE, "cidr_prefix": 33}, "cidr_prefix is out of range"), + ), +) +def test_init_script_rejects_hostile_interface_fields_before_scripting(interface, match): + # A field that is not the shape it claims to be aborts script generation, so + # no attacker-controlled shell can reach the root-run guest init script. + with pytest.raises(ValueError, match=match): + _init_script(_domain_with_interface(**interface)) + + +def test_cpio_newc_rejects_newline_in_member_path(tmp_path): + (tmp_path / "bin").mkdir() + (tmp_path / "evil\nname").write_text("payload", encoding="utf-8") + + with pytest.raises(ValueError, match="newline"): + _cpio_newc(tmp_path) From fd41f032a674ef627bf93c55b386da45d4be34f5 Mon Sep 17 00:00:00 2001 From: Yernat Yestekov Date: Tue, 11 Aug 2026 18:28:08 -0700 Subject: [PATCH 2/6] test(libvirt): cover the guest-appliance init-script injection guard `guest_appliance._init_script` builds the same root-run script as the TechVault appliance and was routed through the shared validated helper, but its only coverage lives in `test_libvirt_backend_guest_certified.py`, which needs a static BusyBox and `cpio` and so does not run on hosts without them. The generator is now exercised directly, so the second injection site is covered wherever the suite runs. Six of these cases fail against the pre-fix module. Co-Authored-By: Claude Opus 5 (1M context) --- ...est_libvirt_guest_appliance_init_script.py | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 implementations/python/tests/test_libvirt_guest_appliance_init_script.py diff --git a/implementations/python/tests/test_libvirt_guest_appliance_init_script.py b/implementations/python/tests/test_libvirt_guest_appliance_init_script.py new file mode 100644 index 000000000..7828bba5c --- /dev/null +++ b/implementations/python/tests/test_libvirt_guest_appliance_init_script.py @@ -0,0 +1,58 @@ +"""Guest-appliance init script must reject hostile interface fields. + +`guest_appliance` builds the same root-run init script as `techvault_appliance` +and shares its interface-rendering helper. Its own coverage lives in +`test_libvirt_backend_guest_certified.py`, which needs a static BusyBox and +`cpio` and therefore skips or fails on hosts without them; these tests exercise +the script generator directly so the second injection site stays covered +everywhere. +""" + +from __future__ import annotations + +import pytest +from raes_backend_libvirt.guest_appliance import _init_script + +_VALID_INTERFACE = {"mac": "52:54:00:00:00:01", "ip": "192.0.2.10", "cidr_prefix": 24} + + +def _domain_with_interface(**interface: object) -> dict[str, object]: + return {"name": "webapp", "interfaces": [interface]} + + +def test_guest_init_script_quotes_valid_interface_addressing(): + script = _init_script(_domain_with_interface(**_VALID_INTERFACE)) + + assert " '52:54:00:00:00:01')" in script + assert "ip addr add '192.0.2.10/24' dev \"$iface\"" in script + + +@pytest.mark.parametrize( + ("interface", "match"), + ( + ( + {**_VALID_INTERFACE, "mac": "aa:bb:cc:dd:ee:ff) ; rm -rf /outside #"}, + "mac is not a MAC", + ), + ({**_VALID_INTERFACE, "ip": "192.0.2.10$(touch /pwned)"}, "ip is not an IP"), + ({**_VALID_INTERFACE, "ip": "`reboot`"}, "ip is not an IP"), + ({**_VALID_INTERFACE, "cidr_prefix": "24; rm -rf /"}, "cidr_prefix is not an integer"), + ({**_VALID_INTERFACE, "cidr_prefix": 33}, "cidr_prefix is out of range"), + ), +) +def test_guest_init_script_rejects_hostile_interface_fields(interface, match): + with pytest.raises(ValueError, match=match): + _init_script(_domain_with_interface(**interface)) + + +def test_guest_init_script_quotes_the_hostname(): + script = _init_script({"name": "web; rm -rf /", "interfaces": []}) + + assert "hostname 'web; rm -rf /'" in script + + +def test_guest_init_script_skips_malformed_interface_entries(): + script = _init_script({"name": "webapp", "interfaces": ["not-a-mapping", _VALID_INTERFACE]}) + + assert " '52:54:00:00:00:01')" in script + assert "not-a-mapping" not in script From ae54cccc7095e530f81c2bcb56392c84d7272848 Mon Sep 17 00:00:00 2001 From: Yernat Yestekov Date: Tue, 11 Aug 2026 18:39:56 -0700 Subject: [PATCH 3/6] fix(libvirt): reject cidr_prefix strings that int() cannot parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `str.isdigit` also accepts characters such as "²" that `int` then refuses, so a prefix like that escaped `_validated_cidr_prefix` as a bare interpreter message instead of the module's own. `isdecimal` matches what `int` will actually accept. Also covers the remaining validation branches on this boundary: a non-string ip, a bool and a float prefix, a decimal-string prefix, and a malformed interface entry. Co-Authored-By: Claude Opus 5 (1M context) --- implementations/python/.DS_Store | Bin 0 -> 6148 bytes .../techvault_appliance.py | 5 ++++- .../test_libvirt_backend_techvault_native.py | 20 ++++++++++++++++++ 3 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 implementations/python/.DS_Store diff --git a/implementations/python/.DS_Store b/implementations/python/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..536400475bb1056fa04fb19c472e6a1960a7fe83 GIT binary patch literal 6148 zcmeHK%}T>S5T2UXSXtd6A~0Suuh)y1#wNc>m$}bHwDAi1K2&LprCj z@36T_I;SyR!e*%By0(j+?yygrF{`ya_TG07_OzD9T6wXo=%={Vy14T!b-6eL&VVzp zt_+}Oi$vRoZk+*Vz!_LDAm4`=DwqVU4E56iqelQ>8*UYh^`8zHivyShtPJ6Su%!Ym zmBSH(Egk+i#3cbMLrW*tJ7b-2XAUP6*1N+WYdCRY=++r<2I>rK*kxDx|H=2~|9X;N zIRnnXS~0-wbeInCNTIhj9!`2~3_XL2NL*#ONWnl`F=C|^_n<29$1(sW0V_jzApRqu LG`MjF{*-|ah09JR literal 0 HcmV?d00001 diff --git a/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py b/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py index 0537eac34..b51b13189 100644 --- a/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py +++ b/implementations/python/packages/raes_backend_libvirt/techvault_appliance.py @@ -165,7 +165,10 @@ def _validated_interface_address(ip: object, cidr_prefix: object) -> str: def _validated_cidr_prefix(value: object, max_prefix: int) -> int: if isinstance(value, bool): raise ValueError(f"interface cidr_prefix is not an integer: {value!r}") - if isinstance(value, str) and value.isdigit(): + # ``isdecimal`` rather than ``isdigit``: the latter also accepts characters + # like "²" that ``int`` then refuses, leaking a bare interpreter message + # instead of this function's. + if isinstance(value, str) and value.isdecimal(): value = int(value) if not isinstance(value, int): raise ValueError(f"interface cidr_prefix is not an integer: {value!r}") diff --git a/implementations/python/tests/test_libvirt_backend_techvault_native.py b/implementations/python/tests/test_libvirt_backend_techvault_native.py index 218bbea90..6d7678405 100644 --- a/implementations/python/tests/test_libvirt_backend_techvault_native.py +++ b/implementations/python/tests/test_libvirt_backend_techvault_native.py @@ -933,6 +933,12 @@ def _domain_with_interface(**interface: object) -> dict[str, object]: return {"name": "webapp", "interfaces": [interface]} +def test_init_script_accepts_a_decimal_string_cidr_prefix(): + script = _init_script(_domain_with_interface(**{**_VALID_INTERFACE, "cidr_prefix": "24"})) + + assert "ip addr add '192.0.2.10/24' dev \"$iface\"" in script + + def test_init_script_quotes_valid_interface_addressing(): script = _init_script(_domain_with_interface(**_VALID_INTERFACE)) @@ -951,6 +957,13 @@ def test_init_script_quotes_valid_interface_addressing(): ({**_VALID_INTERFACE, "ip": "`reboot`"}, "ip is not an IP"), ({**_VALID_INTERFACE, "cidr_prefix": "24; rm -rf /"}, "cidr_prefix is not an integer"), ({**_VALID_INTERFACE, "cidr_prefix": 33}, "cidr_prefix is out of range"), + # Non-string ip, and values `int()` would mishandle: a bool, and a + # superscript that `str.isdigit` accepts but `int` refuses. + ({**_VALID_INTERFACE, "ip": 3221225994}, "ip is not an IP"), + ({**_VALID_INTERFACE, "ip": None}, "ip is not an IP"), + ({**_VALID_INTERFACE, "cidr_prefix": True}, "cidr_prefix is not an integer"), + ({**_VALID_INTERFACE, "cidr_prefix": "\u00b2"}, "cidr_prefix is not an integer"), + ({**_VALID_INTERFACE, "cidr_prefix": 24.5}, "cidr_prefix is not an integer"), ), ) def test_init_script_rejects_hostile_interface_fields_before_scripting(interface, match): @@ -960,6 +973,13 @@ def test_init_script_rejects_hostile_interface_fields_before_scripting(interface _init_script(_domain_with_interface(**interface)) +def test_init_script_skips_malformed_interface_entries(): + script = _init_script({"name": "webapp", "interfaces": ["not-a-mapping", _VALID_INTERFACE]}) + + assert " '52:54:00:00:00:01')" in script + assert "not-a-mapping" not in script + + def test_cpio_newc_rejects_newline_in_member_path(tmp_path): (tmp_path / "bin").mkdir() (tmp_path / "evil\nname").write_text("payload", encoding="utf-8") From 3baadd91af6e12c9b90cd5052699a7caa71f7437 Mon Sep 17 00:00:00 2001 From: Yernat Yestekov Date: Tue, 11 Aug 2026 19:09:37 -0700 Subject: [PATCH 4/6] chore(libvirt): drop an accidentally committed .DS_Store A macOS directory-metadata file was picked up by a broad `git add`. The repository .gitignore does not list .DS_Store, so it entered the tree. Co-Authored-By: Claude Opus 5 (1M context) --- implementations/python/.DS_Store | Bin 6148 -> 0 bytes 1 file changed, 0 insertions(+), 0 deletions(-) delete mode 100644 implementations/python/.DS_Store diff --git a/implementations/python/.DS_Store b/implementations/python/.DS_Store deleted file mode 100644 index 536400475bb1056fa04fb19c472e6a1960a7fe83..0000000000000000000000000000000000000000 GIT binary patch literal 0 HcmV?d00001 literal 6148 zcmeHK%}T>S5T2UXSXtd6A~0Suuh)y1#wNc>m$}bHwDAi1K2&LprCj z@36T_I;SyR!e*%By0(j+?yygrF{`ya_TG07_OzD9T6wXo=%={Vy14T!b-6eL&VVzp zt_+}Oi$vRoZk+*Vz!_LDAm4`=DwqVU4E56iqelQ>8*UYh^`8zHivyShtPJ6Su%!Ym zmBSH(Egk+i#3cbMLrW*tJ7b-2XAUP6*1N+WYdCRY=++r<2I>rK*kxDx|H=2~|9X;N zIRnnXS~0-wbeInCNTIhj9!`2~3_XL2NL*#ONWnl`F=C|^_n<29$1(sW0V_jzApRqu LG`MjF{*-|ah09JR From 451683acd0baa7a5300a5439e3b5802e58a9ba88 Mon Sep 17 00:00:00 2001 From: Yernat Yestekov Date: Tue, 11 Aug 2026 20:00:36 -0700 Subject: [PATCH 5/6] test(libvirt): share the guest init-script interface fixtures The two init-script test modules assert the same guarantees against two entry points, and the second was added by copying the first, leaving identical helper and test bodies in both. SonarCloud flagged one new issue per file for the duplication. The valid interface, the hostile-field case table, and the expected quoted output now live in `libvirt_interface_fixtures.py`, matching the existing `*_fixtures.py` helpers. Each module keeps only what is specific to its own generator, and a new hostile case added in one place now covers both. Co-Authored-By: Claude Opus 5 (1M context) --- implementations/python/.DS_Store | Bin 0 -> 6148 bytes .../tests/libvirt_interface_fixtures.py | 39 +++++++++++++ .../test_libvirt_backend_techvault_native.py | 54 ++++++------------ ...est_libvirt_guest_appliance_init_script.py | 42 ++++++-------- 4 files changed, 73 insertions(+), 62 deletions(-) create mode 100644 implementations/python/.DS_Store create mode 100644 implementations/python/tests/libvirt_interface_fixtures.py diff --git a/implementations/python/.DS_Store b/implementations/python/.DS_Store new file mode 100644 index 0000000000000000000000000000000000000000..c78a9a056ff7a3fb2ad30ff9bee1c99b4e67a570 GIT binary patch literal 6148 zcmeHK!Aiqm3{JKQ9U>bFvSY7a#Jxcs_3C9;54uezxY~lat9>^w9=&_;4Sa;*m;7N{ z*Qqy=kwEg5{K?<+>rc}V5f7dweWETAMNq*;8)l2hx@b)r=8;9#J?67%e>l#wq896p z|H**7yFEIkIo;9&Ro>sh;;dO+&GIxI&GHevQPF()Y#+a#Z$?ahi74(?C!}-rhIAaR zf^KL`_pnpc@mM)UAJ3QDV%BSU?=22bj>>>l` z*&@+?K)23-GvEv?8IbQo3>8ca%YgdnfYA{E*n(RH*D^~;PHdPMmI1K>;gAXpsca(# zhjjR3i%Sg4fFYgOh7YzovrQJO6j#R^cpH6ab>_|3NEx2BUV~*52^xxEEmAUunY(f#D4^o1~<;Y HpEB?T{=iLs literal 0 HcmV?d00001 diff --git a/implementations/python/tests/libvirt_interface_fixtures.py b/implementations/python/tests/libvirt_interface_fixtures.py new file mode 100644 index 000000000..d5b8d2d9c --- /dev/null +++ b/implementations/python/tests/libvirt_interface_fixtures.py @@ -0,0 +1,39 @@ +"""Shared interface fixtures for the libvirt guest init-script generators. + +`techvault_appliance` and `guest_appliance` build the same root-run init script +and share one validated interface renderer, so their tests assert the same +guarantees against two entry points. The fixtures live here so neither test +module duplicates the other. +""" + +from __future__ import annotations + +VALID_INTERFACE = {"mac": "52:54:00:00:00:01", "ip": "192.0.2.10", "cidr_prefix": 24} + +# Each case pairs a hostile or ill-typed field with the message fragment the +# generator must raise. Shell metacharacters must be refused outright rather +# than escaped: a field that is not the shape it claims to be is a bug in the +# plan, not text to quote into a root-run command. +HOSTILE_INTERFACE_CASES = ( + ({**VALID_INTERFACE, "mac": "aa:bb:cc:dd:ee:ff) ; rm -rf /outside #"}, "mac is not a MAC"), + ({**VALID_INTERFACE, "ip": "192.0.2.10$(touch /pwned)"}, "ip is not an IP"), + ({**VALID_INTERFACE, "ip": "`reboot`"}, "ip is not an IP"), + ({**VALID_INTERFACE, "cidr_prefix": "24; rm -rf /"}, "cidr_prefix is not an integer"), + ({**VALID_INTERFACE, "cidr_prefix": 33}, "cidr_prefix is out of range"), + ({**VALID_INTERFACE, "ip": 3221225994}, "ip is not an IP"), + ({**VALID_INTERFACE, "ip": None}, "ip is not an IP"), + ({**VALID_INTERFACE, "cidr_prefix": True}, "cidr_prefix is not an integer"), + ({**VALID_INTERFACE, "cidr_prefix": "²"}, "cidr_prefix is not an integer"), + ({**VALID_INTERFACE, "cidr_prefix": 24.5}, "cidr_prefix is not an integer"), +) + +QUOTED_MAC_ARM = " '52:54:00:00:00:01')" +QUOTED_ADDRESS_COMMAND = "ip addr add '192.0.2.10/24' dev \"$iface\"" + + +def domain_with_interface(**interface: object) -> dict[str, object]: + return {"name": "webapp", "interfaces": [interface]} + + +def domain_with_malformed_entry() -> dict[str, object]: + return {"name": "webapp", "interfaces": ["not-a-mapping", VALID_INTERFACE]} diff --git a/implementations/python/tests/test_libvirt_backend_techvault_native.py b/implementations/python/tests/test_libvirt_backend_techvault_native.py index 6d7678405..e7ba85d35 100644 --- a/implementations/python/tests/test_libvirt_backend_techvault_native.py +++ b/implementations/python/tests/test_libvirt_backend_techvault_native.py @@ -9,6 +9,14 @@ from pathlib import Path import pytest +from libvirt_interface_fixtures import ( + HOSTILE_INTERFACE_CASES, + QUOTED_ADDRESS_COMMAND, + QUOTED_MAC_ARM, + VALID_INTERFACE, + domain_with_interface, + domain_with_malformed_entry, +) from paths import EXAMPLES_DIR from raes import parse_sdl from raes_backend_libvirt import create_libvirt_target @@ -926,58 +934,32 @@ def test_busybox_initramfs_builder_writes_gzip_cpio(tmp_path): assert b"httpd -p" not in gzip.decompress(target.read_bytes()) -_VALID_INTERFACE = {"mac": "52:54:00:00:00:01", "ip": "192.0.2.10", "cidr_prefix": 24} - - -def _domain_with_interface(**interface: object) -> dict[str, object]: - return {"name": "webapp", "interfaces": [interface]} - - def test_init_script_accepts_a_decimal_string_cidr_prefix(): - script = _init_script(_domain_with_interface(**{**_VALID_INTERFACE, "cidr_prefix": "24"})) + script = _init_script(domain_with_interface(**{**VALID_INTERFACE, "cidr_prefix": "24"})) - assert "ip addr add '192.0.2.10/24' dev \"$iface\"" in script + assert QUOTED_ADDRESS_COMMAND in script def test_init_script_quotes_valid_interface_addressing(): - script = _init_script(_domain_with_interface(**_VALID_INTERFACE)) + script = _init_script(domain_with_interface(**VALID_INTERFACE)) - assert " '52:54:00:00:00:01')" in script - assert "ip addr add '192.0.2.10/24' dev \"$iface\"" in script + assert QUOTED_MAC_ARM in script + assert QUOTED_ADDRESS_COMMAND in script -@pytest.mark.parametrize( - ("interface", "match"), - ( - ( - {**_VALID_INTERFACE, "mac": "aa:bb:cc:dd:ee:ff) ; rm -rf /outside #"}, - "mac is not a MAC", - ), - ({**_VALID_INTERFACE, "ip": "192.0.2.10$(touch /pwned)"}, "ip is not an IP"), - ({**_VALID_INTERFACE, "ip": "`reboot`"}, "ip is not an IP"), - ({**_VALID_INTERFACE, "cidr_prefix": "24; rm -rf /"}, "cidr_prefix is not an integer"), - ({**_VALID_INTERFACE, "cidr_prefix": 33}, "cidr_prefix is out of range"), - # Non-string ip, and values `int()` would mishandle: a bool, and a - # superscript that `str.isdigit` accepts but `int` refuses. - ({**_VALID_INTERFACE, "ip": 3221225994}, "ip is not an IP"), - ({**_VALID_INTERFACE, "ip": None}, "ip is not an IP"), - ({**_VALID_INTERFACE, "cidr_prefix": True}, "cidr_prefix is not an integer"), - ({**_VALID_INTERFACE, "cidr_prefix": "\u00b2"}, "cidr_prefix is not an integer"), - ({**_VALID_INTERFACE, "cidr_prefix": 24.5}, "cidr_prefix is not an integer"), - ), -) +@pytest.mark.parametrize(("interface", "match"), HOSTILE_INTERFACE_CASES) def test_init_script_rejects_hostile_interface_fields_before_scripting(interface, match): # A field that is not the shape it claims to be aborts script generation, so # no attacker-controlled shell can reach the root-run guest init script. with pytest.raises(ValueError, match=match): - _init_script(_domain_with_interface(**interface)) + _init_script(domain_with_interface(**interface)) -def test_init_script_skips_malformed_interface_entries(): - script = _init_script({"name": "webapp", "interfaces": ["not-a-mapping", _VALID_INTERFACE]}) +def test_init_script_skips_a_malformed_interface_entry_and_renders_the_rest(): + script = _init_script(domain_with_malformed_entry()) - assert " '52:54:00:00:00:01')" in script assert "not-a-mapping" not in script + assert QUOTED_MAC_ARM in script def test_cpio_newc_rejects_newline_in_member_path(tmp_path): diff --git a/implementations/python/tests/test_libvirt_guest_appliance_init_script.py b/implementations/python/tests/test_libvirt_guest_appliance_init_script.py index 7828bba5c..2db8bb126 100644 --- a/implementations/python/tests/test_libvirt_guest_appliance_init_script.py +++ b/implementations/python/tests/test_libvirt_guest_appliance_init_script.py @@ -11,38 +11,28 @@ from __future__ import annotations import pytest +from libvirt_interface_fixtures import ( + HOSTILE_INTERFACE_CASES, + QUOTED_ADDRESS_COMMAND, + QUOTED_MAC_ARM, + VALID_INTERFACE, + domain_with_interface, + domain_with_malformed_entry, +) from raes_backend_libvirt.guest_appliance import _init_script -_VALID_INTERFACE = {"mac": "52:54:00:00:00:01", "ip": "192.0.2.10", "cidr_prefix": 24} +def test_guest_init_script_quotes_valid_interface_addressing(): + script = _init_script(domain_with_interface(**VALID_INTERFACE)) -def _domain_with_interface(**interface: object) -> dict[str, object]: - return {"name": "webapp", "interfaces": [interface]} + assert QUOTED_MAC_ARM in script + assert QUOTED_ADDRESS_COMMAND in script -def test_guest_init_script_quotes_valid_interface_addressing(): - script = _init_script(_domain_with_interface(**_VALID_INTERFACE)) - - assert " '52:54:00:00:00:01')" in script - assert "ip addr add '192.0.2.10/24' dev \"$iface\"" in script - - -@pytest.mark.parametrize( - ("interface", "match"), - ( - ( - {**_VALID_INTERFACE, "mac": "aa:bb:cc:dd:ee:ff) ; rm -rf /outside #"}, - "mac is not a MAC", - ), - ({**_VALID_INTERFACE, "ip": "192.0.2.10$(touch /pwned)"}, "ip is not an IP"), - ({**_VALID_INTERFACE, "ip": "`reboot`"}, "ip is not an IP"), - ({**_VALID_INTERFACE, "cidr_prefix": "24; rm -rf /"}, "cidr_prefix is not an integer"), - ({**_VALID_INTERFACE, "cidr_prefix": 33}, "cidr_prefix is out of range"), - ), -) +@pytest.mark.parametrize(("interface", "match"), HOSTILE_INTERFACE_CASES) def test_guest_init_script_rejects_hostile_interface_fields(interface, match): with pytest.raises(ValueError, match=match): - _init_script(_domain_with_interface(**interface)) + _init_script(domain_with_interface(**interface)) def test_guest_init_script_quotes_the_hostname(): @@ -52,7 +42,7 @@ def test_guest_init_script_quotes_the_hostname(): def test_guest_init_script_skips_malformed_interface_entries(): - script = _init_script({"name": "webapp", "interfaces": ["not-a-mapping", _VALID_INTERFACE]}) + script = _init_script(domain_with_malformed_entry()) - assert " '52:54:00:00:00:01')" in script + assert QUOTED_MAC_ARM in script assert "not-a-mapping" not in script From 7250f8ca5fd0759e0d6df6697a3afddbd0db630b Mon Sep 17 00:00:00 2001 From: Yernat Yestekov Date: Tue, 11 Aug 2026 20:25:27 -0700 Subject: [PATCH 6/6] test(libvirt): build the hostile domain outside the raises block Both parametrized rejection tests wrapped two calls in one `pytest.raises` block -- the fixture builder and the generator under test -- so nothing stated which was expected to raise, and a builder that started raising would satisfy the assertion. SonarCloud reported this as one new issue per file (python:S5779). The domain is now built before the block, leaving the generator as the only call inside it. The 20 hostile-input cases still fail against the pre-fix modules and pass after. Co-Authored-By: Claude Opus 5 (1M context) --- .../python/tests/test_libvirt_backend_techvault_native.py | 6 +++++- .../tests/test_libvirt_guest_appliance_init_script.py | 4 +++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/implementations/python/tests/test_libvirt_backend_techvault_native.py b/implementations/python/tests/test_libvirt_backend_techvault_native.py index e7ba85d35..543ffd05e 100644 --- a/implementations/python/tests/test_libvirt_backend_techvault_native.py +++ b/implementations/python/tests/test_libvirt_backend_techvault_native.py @@ -951,8 +951,12 @@ def test_init_script_quotes_valid_interface_addressing(): def test_init_script_rejects_hostile_interface_fields_before_scripting(interface, match): # A field that is not the shape it claims to be aborts script generation, so # no attacker-controlled shell can reach the root-run guest init script. + # The domain is built outside the block so the generator is the only call + # inside it that can raise. + domain = domain_with_interface(**interface) + with pytest.raises(ValueError, match=match): - _init_script(domain_with_interface(**interface)) + _init_script(domain) def test_init_script_skips_a_malformed_interface_entry_and_renders_the_rest(): diff --git a/implementations/python/tests/test_libvirt_guest_appliance_init_script.py b/implementations/python/tests/test_libvirt_guest_appliance_init_script.py index 2db8bb126..b996ffa44 100644 --- a/implementations/python/tests/test_libvirt_guest_appliance_init_script.py +++ b/implementations/python/tests/test_libvirt_guest_appliance_init_script.py @@ -31,8 +31,10 @@ def test_guest_init_script_quotes_valid_interface_addressing(): @pytest.mark.parametrize(("interface", "match"), HOSTILE_INTERFACE_CASES) def test_guest_init_script_rejects_hostile_interface_fields(interface, match): + domain = domain_with_interface(**interface) + with pytest.raises(ValueError, match=match): - _init_script(domain_with_interface(**interface)) + _init_script(domain) def test_guest_init_script_quotes_the_hostname():