fix: close the structural-key gate

compose2pod/emit.py

@@ -11,6 +11,7 @@
 from compose2pod.graph import depends_on, hostnames, startup_order
 from compose2pod.healthcheck import health_cmd, interval_seconds
 from compose2pod.keys import SERVICE_KEYS, Expand, Token, key_value_pairs
+from compose2pod.parsing import validate
 from compose2pod.pod import pod_create_flags
 from compose2pod.resources import deploy_resource_flags
 from compose2pod.shell import to_shell, variable_names
@@ -53,11 +54,17 @@ def _health_flags(healthcheck: dict[str, Any]) -> list[Token]:
     cmd = health_cmd(healthcheck.get("test"))
     if cmd is not None:
         flags += ["--health-cmd", Expand(value=cmd)]
-        if "timeout" in healthcheck:
+        # An explicit `null` scalar means unset, not "emit the literal string
+        # 'None'" -- the same treatment `environment`/`volumes`/`command`
+        # give a null value elsewhere in this package, and matching `docker
+        # compose config`, which treats an explicitly-null key as absent.
+        # Keyed off value, not presence, so `timeout: null` and an omitted
+        # `timeout` behave identically.
+        if healthcheck.get("timeout") is not None:
             flags += ["--health-timeout", str(healthcheck["timeout"])]
-        if "start_period" in healthcheck:
+        if healthcheck.get("start_period") is not None:
             flags += ["--health-start-period", str(healthcheck["start_period"])]
-        if "retries" in healthcheck:
+        if healthcheck.get("retries") is not None:
             flags += ["--health-retries", str(healthcheck["retries"])]
     return flags
 
@@ -221,14 +228,49 @@ def _emit_target(lines: list[str], tokens: list[Token], options: EmitOptions) ->
     lines.append("esac")
 
 
+def _validate_options(options: EmitOptions) -> None:
+    """Check option values emit destructures or interpolates unquoted.
+
+    CLI-unreachable (argparse enforces `artifact` as `str` and
+    `allow_exit_codes` as `int`) but a library caller can pass `EmitOptions`
+    directly: a non-string `artifact` would crash raw on the ':' membership
+    test below, and a non-int `allow_exit_codes` entry is interpolated
+    unquoted into the generated `case "$rc" in ...)` pattern -- shell
+    injection, not just a crash.
+    """
+    for artifact in options.artifacts:
+        if not isinstance(artifact, str) or ":" not in artifact:
+            msg = f"artifact {artifact!r} must be in SRC:DST form"
+            raise UnsupportedComposeError(msg)
+    for code in options.allow_exit_codes:
+        if isinstance(code, bool) or not isinstance(code, int):
+            msg = f"allow_exit_codes entry {code!r} must be an int"
+            raise UnsupportedComposeError(msg)
+
+
 def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript:
     """Walk the target's dependency closure once, building the script and its variables.
 
     Every token source is visited a single time and feeds both outputs: each
     service's `run_tokens` and the `pod_create_flags` render into lines *and*
     have their `Expand` variables collected, so the script and the variable
     list cannot disagree about what the script expands at run time.
+
+    `emit_script` and `referenced_variables` are both public entry points that
+    project this traversal, so a library caller can reach either one without
+    ever calling `validate()` first. Validating `compose` here -- and nowhere
+    else -- makes both safe by construction: this is the one place a caller
+    cannot route around. `cli.py` also calls `validate()` itself (to surface
+    warnings), so this repeats that pass; `validate()` only reads `compose`
+    and returns warnings, so the repeat is side-effect-free. The warnings from
+    this pass have no channel to reach a caller here -- `cli.py` surfaces its
+    own copy from its own call -- so they are discarded on purpose.
     """
+    _validate_options(options)
+    _ = validate(compose)  # re-validate; warnings already surfaced by the caller, if any
+    if not POD_NAME_PATTERN.fullmatch(options.pod):
+        msg = f"invalid pod name {options.pod!r}"
+        raise UnsupportedComposeError(msg)
     services = compose["services"]
     hosts = hostnames(services)
     order = startup_order(services, options.target)
@@ -275,9 +317,6 @@ def _plan(compose: dict[str, Any], options: EmitOptions) -> PlannedScript:
 
 def emit_script(compose: dict[str, Any], options: EmitOptions) -> str:
     """Render the full pod test script for `target` and its dependency closure."""
-    if not POD_NAME_PATTERN.fullmatch(options.pod):
-        msg = f"invalid pod name {options.pod!r}"
-        raise UnsupportedComposeError(msg)
     return _plan(compose, options).script
 
 

compose2pod/extends.py

@@ -24,7 +24,13 @@ def _extends_target(name: str, ext: Any) -> str:  # noqa: ANN401 - Compose value
         raise UnsupportedComposeError(msg)
     unknown = set(ext) - {"service"}
     if unknown:
-        msg = f"service {name!r}: unsupported 'extends' keys {sorted(unknown)}"
+        # sorted(unknown) would crash raw (TypeError: '<' not supported...)
+        # if unknown holds keys of more than one incomparable type -- e.g. an
+        # int key alongside a str key, which a hostile 'extends' mapping can
+        # freely produce since this runs ahead of validate()'s gate. Sorting
+        # by repr keeps the message deterministic without assuming key types
+        # are mutually comparable.
+        msg = f"service {name!r}: unsupported 'extends' keys {sorted(unknown, key=repr)}"
         raise UnsupportedComposeError(msg)
     service = ext.get("service")
     if not isinstance(service, str):
@@ -40,6 +46,10 @@ def _as_mapping(key: str, name: str, value: Any) -> dict[str, Any]:  # noqa: ANN
         if key == "environment":
             return pairs_to_mapping(name, key, value)
         if key == "depends_on":
+            for dep in value:
+                if not isinstance(dep, str):
+                    msg = f"service {name!r}: depends_on entry {dep!r} must be a string"
+                    raise UnsupportedComposeError(msg)
             return {dep: {} for dep in value}
     msg = f"service {name!r}: cannot merge {key!r} across incompatible forms"
     raise UnsupportedComposeError(msg)

compose2pod/graph.py

@@ -9,6 +9,10 @@ def depends_on(svc: dict[str, Any]) -> dict[str, str]:
     """Normalize dependencies of a service to a name -> condition mapping."""
     deps = svc.get("depends_on") or {}
     if isinstance(deps, list):
+        for dep in deps:
+            if not isinstance(dep, str):
+                msg = f"depends_on entry {dep!r} must be a string"
+                raise UnsupportedComposeError(msg)
         return cast(dict[str, str], dict.fromkeys(deps, "service_started"))
     if not isinstance(deps, dict):
         msg = "'depends_on' must be a list or mapping"
@@ -18,7 +22,20 @@ def depends_on(svc: dict[str, Any]) -> dict[str, str]:
         if not isinstance(spec, dict):
             msg = f"depends_on entry {dep!r} must be a mapping"
             raise UnsupportedComposeError(msg)
-        result[dep] = spec.get("condition", "service_started")
+        condition = spec.get("condition", "service_started")
+        if not isinstance(condition, str):
+            # Callers (parsing._validate_depends_on) test membership in a
+            # `set` of known condition strings -- `x in a_set` hashes `x`,
+            # so an unhashable condition (a dict or list) would otherwise
+            # crash raw with `TypeError: unhashable type` instead of failing
+            # clean. Checked here, not there: this function already owns
+            # every other depends_on shape check (list vs mapping, spec must
+            # be a mapping), so a bad condition type belongs with them, and
+            # every caller of `depends_on` -- not just validate() -- gets the
+            # same protection.
+            msg = f"depends_on entry {dep!r}: condition must be a string"
+            raise UnsupportedComposeError(msg)
+        result[dep] = condition
     return result
 
 
@@ -39,7 +56,14 @@ def _host_names(name: str, svc: dict[str, Any]) -> list[str]:
     if isinstance(networks, dict):
         for network in networks.values():
             if isinstance(network, dict):
-                result.extend(network.get("aliases") or [])
+                aliases = network.get("aliases")
+                if aliases is None:
+                    continue
+                # A string would be iterated character-wise by extend() below.
+                if not isinstance(aliases, list) or not all(isinstance(alias, str) for alias in aliases):
+                    msg = f"service {name!r}: aliases must be a list of strings"
+                    raise UnsupportedComposeError(msg)
+                result.extend(aliases)
     return result
 
 

compose2pod/healthcheck.py

@@ -27,12 +27,12 @@ def health_cmd(test: object) -> str | None:
         raise UnsupportedComposeError(msg)
     kind = test[0]
     if kind == "CMD-SHELL":
-        if len(test) < _CMD_MIN_LENGTH:
+        if len(test) < _CMD_MIN_LENGTH or not isinstance(test[1], str):
             msg = f"unsupported healthcheck test: {test!r}"
             raise UnsupportedComposeError(msg)
-        return test[1]  # ty: ignore
+        return test[1]
     if kind == "CMD":
-        if len(test) < _CMD_MIN_LENGTH:
+        if len(test) < _CMD_MIN_LENGTH or not all(isinstance(item, str) for item in test[1:]):
             msg = f"unsupported healthcheck test: {test!r}"
             raise UnsupportedComposeError(msg)
         return json.dumps(test[1:])

compose2pod/keys.py

@@ -17,14 +17,44 @@
 
 @dataclasses.dataclass(frozen=True, slots=True, kw_only=True)
 class Expand:
-    """A token whose Compose variable references expand at script-run time."""
+    """A token whose Compose variable references expand at script-run time.
+
+    Every emitted `Expand` eventually reaches `shell.to_shell`/`variable_names`,
+    both of which assume `value` is a `str` (they run a compiled regex over
+    it) and crash raw (a `TypeError`, not `UnsupportedComposeError`) otherwise.
+    Rather than trust every call site across keys.py/emit.py/pod.py/
+    resources.py to have cast or validated first, this is the one chokepoint:
+    a non-str value is rejected right here, so any per-key validator gap
+    becomes a clean error instead of a raw crash downstream, no matter which
+    key leaked it. This is defense-in-depth, not a substitute for validating
+    shape at the gate -- it only turns an otherwise-unhandled crash into a
+    clean one; it can't catch a non-str value that a caller has already
+    coerced to str() (see the silent str()/repr() corruption class instead).
+    """
 
     value: str
 
+    def __post_init__(self) -> None:
+        if not isinstance(self.value, str):
+            msg = f"internal: Expand.value must be a string, got {type(self.value).__name__}: {self.value!r}"
+            raise UnsupportedComposeError(msg)
+
 
 Token = str | Expand
 
 
+def _render_scalar(value: Any) -> str:  # noqa: ANN401 - Compose values are untyped YAML/JSON
+    """Render a map scalar value the way `docker compose config` does.
+
+    A bool renders lowercase ('true'/'false'), matching Docker's own
+    normalization -- Python's str(True) == 'True' would otherwise leak into
+    the emitted flag value verbatim.
+    """
+    if isinstance(value, bool):
+        return "true" if value else "false"
+    return str(value)
+
+
 def key_value_pairs(value: list[Any] | dict[str, Any]) -> list[Any]:
     """Compose list/map key-value section as 'KEY=value' / 'KEY' entries.
 
@@ -33,7 +63,7 @@ def key_value_pairs(value: list[Any] | dict[str, Any]) -> list[Any]:
     """
     if isinstance(value, list):
         return value
-    return [key if val is None else f"{key}={val}" for key, val in value.items()]
+    return [key if val is None else f"{key}={_render_scalar(val)}" for key, val in value.items()]
 
 
 @dataclasses.dataclass(frozen=True, slots=True, kw_only=True)
@@ -61,22 +91,60 @@ def is_number(value: Any) -> bool:  # noqa: ANN401 - Compose values are untyped
     return not isinstance(value, bool) and isinstance(value, int | float | str)
 
 
+def require_string_keys(where: str, mapping: dict[Any, Any]) -> None:
+    """Check every key of a raw YAML/JSON mapping is a string.
+
+    PyYAML routinely produces non-string keys (a bare `3:` is an int; under
+    YAML 1.1, a bare `on:`/`off:` is a bool). Every mapping-key consumer
+    downstream (`sorted()`, `str.startswith`, a compiled regex) assumes
+    `str` and crashes raw otherwise. This is the one shared check the gate
+    runs before it reads any of `mapping`'s keys, so a non-string key fails
+    clean regardless of which mapping it turned up in.
+    """
+    for key in mapping:
+        if not isinstance(key, str):
+            msg = f"{where}: key {key!r} must be a string"
+            raise UnsupportedComposeError(msg)
+
+
 def _validate_number(name: str, key: str, value: Any) -> None:  # noqa: ANN401 - Compose values are untyped YAML/JSON
     if not is_number(value):
         msg = f"service {name!r}: '{key}' must be a number or string"
         raise UnsupportedComposeError(msg)
 
 
+def _validate_list_elements(name: str, key: str, value: list[Any]) -> None:
+    """Check every list element is a string, so emit can't str() a non-string into the script."""
+    for item in value:
+        if not isinstance(item, str):
+            msg = f"service {name!r}: '{key}' entries must be strings"
+            raise UnsupportedComposeError(msg)
+
+
+def _validate_map_values(name: str, key: str, value: dict[str, Any]) -> None:
+    """Check every map value is a scalar or null, so emit can't repr() a dict/list into the script."""
+    for val in value.values():
+        if val is not None and not is_number(val) and not isinstance(val, bool):
+            msg = f"service {name!r}: '{key}' values must be a string, number, boolean, or null"
+            raise UnsupportedComposeError(msg)
+
+
 def _validate_list(name: str, key: str, value: Any) -> None:  # noqa: ANN401 - Compose values are untyped YAML/JSON
     if not isinstance(value, list):
         msg = f"service {name!r}: '{key}' must be a list"
         raise UnsupportedComposeError(msg)
+    _validate_list_elements(name, key, value)
 
 
 def validate_map(name: str, key: str, value: Any) -> None:  # noqa: ANN401 - Compose values are untyped YAML/JSON
-    if not isinstance(value, list | dict):
-        msg = f"service {name!r}: '{key}' must be a list or mapping"
-        raise UnsupportedComposeError(msg)
+    if isinstance(value, list):
+        _validate_list_elements(name, key, value)
+        return
+    if isinstance(value, dict):
+        _validate_map_values(name, key, value)
+        return
+    msg = f"service {name!r}: '{key}' must be a list or mapping"
+    raise UnsupportedComposeError(msg)
 
 
 def _as_list(name: str, key: str, value: Any) -> list[Any]:  # noqa: ANN401 - Compose values are untyped YAML/JSON
@@ -101,7 +169,10 @@ def pairs_to_mapping(name: str, key: str, value: Any) -> dict[str, Any]:  # noqa
     if isinstance(value, list):
         result: dict[str, Any] = {}
         for item in value:
-            pair_key, sep, pair_value = str(item).partition("=")
+            if not isinstance(item, str):
+                msg = f"service {name!r}: '{key}' entries must be strings"
+                raise UnsupportedComposeError(msg)
+            pair_key, sep, pair_value = item.partition("=")
             result[pair_key] = pair_value if sep else None
         return result
     msg = f"service {name!r}: cannot merge {key!r} across incompatible forms"
@@ -158,7 +229,7 @@ def extra_host_pairs(value: list[Any] | dict[str, Any]) -> list[Any]:
     """Compose extra_hosts as 'host:ip' entries; map values keep their colons (IPv6-safe)."""
     if isinstance(value, list):
         return value
-    return [f"{host}:{ip}" for host, ip in value.items()]
+    return [f"{host}:{_render_scalar(ip)}" for host, ip in value.items()]
 
 
 def _validate_pull_policy(name: str, key: str, value: Any) -> None:  # noqa: ANN401 - Compose values are untyped
@@ -183,10 +254,17 @@ def _validate_ulimits(name: str, key: str, value: Any) -> None:  # noqa: ANN401
             if set(spec) != {"soft", "hard"}:
                 msg = f"service {name!r}: ulimit {limit!r} mapping must have exactly 'soft' and 'hard'"
                 raise UnsupportedComposeError(msg)
-            if not isinstance(spec["soft"], int | str) or not isinstance(spec["hard"], int | str):
+            # bool IS an int in Python, so a plain `isinstance(..., int | str)`
+            # would silently let a boolean soft/hard value through.
+            if any(
+                isinstance(spec[bound], bool) or not isinstance(spec[bound], int | str) for bound in ("soft", "hard")
+            ):
                 msg = f"service {name!r}: ulimit {limit!r} 'soft' and 'hard' must be int or str"
                 raise UnsupportedComposeError(msg)
-        elif not isinstance(spec, int | str):
+        elif isinstance(spec, bool) or not isinstance(spec, int | str):
+            # A boolean ulimit is meaningless -- unlike environment's bool
+            # (which Docker normalizes to a string), there is no sensible
+            # normalization here, so it is rejected rather than coerced.
             msg = f"service {name!r}: ulimit {limit!r} must be an int or a soft/hard mapping"
             raise UnsupportedComposeError(msg)