From fbe4bf9f1c08203c80b81a348a54bb5e8eae8ed6 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 12 Jul 2026 09:36:45 +0300 Subject: [PATCH 1/4] docs: design emit_script pod-name validation --- ...2026-07-12.01-validate-pod-name-in-emit.md | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 planning/changes/2026-07-12.01-validate-pod-name-in-emit.md diff --git a/planning/changes/2026-07-12.01-validate-pod-name-in-emit.md b/planning/changes/2026-07-12.01-validate-pod-name-in-emit.md new file mode 100644 index 0000000..067d08b --- /dev/null +++ b/planning/changes/2026-07-12.01-validate-pod-name-in-emit.md @@ -0,0 +1,76 @@ +--- +summary: Validate the pod name inside emit_script (not only in the CLI) so a library caller passing an adversarial pod value is rejected loudly instead of producing a malformed EXIT trap; POD_NAME_PATTERN moves to emit.py as the shared source of truth. Closes the deferred trap-hardening item by validating the identifier rather than shell-escaping it. +--- + +# Design: validate the pod name in emit_script + +## Summary + +Make `emit_script` reject a pod name that is not a valid identifier, closing the +deferred "harden the pod-cleanup trap" item. The pod name is embedded raw into +several shell contexts — most fragile is the single-quoted `EXIT` trap, where a +pod name containing a `'` breaks the outer quoting, and the joined +`podman secret rm ` store names are unquoted. The CLI already guards +this with `POD_NAME_PATTERN`, but `emit_script`/`EmitOptions` are public API, so +a library caller can pass an unvalidated pod. Rather than shell-escape an +arbitrary-metacharacter pod name through nested contexts, validate the +identifier at the emit choke point — the same honest-subset move the codebase +already uses for secret/config names. + +## Motivation + +The deferral's revisit trigger is "a library-API test exercises `emit_script` +with adversarial `pod` values, or a bug report surfaces from a caller using the +library path without CLI validation." The pod name reaches shell in the trap +(`podman pod rm -f `, `podman secret rm - ...`), the store +`create` lines (`podman secret create - ...`), and standalone +`shlex.quote`d sites (run flags, pod-create, `wait_healthy`, target `cp`). The +standalone sites are safe; the trap and create lines are not for a pod name with +quotes/spaces/metacharacters. The `` half of every store name is already +charset-validated (the secrets injection fix), so validating the pod name is the +last piece: a metachar-free pod name makes every embedding site safe at once. + +## Design + +- **Move `POD_NAME_PATTERN`** (`^[A-Za-z0-9][A-Za-z0-9_.-]*$`) from `cli.py` to + `emit.py` as the single source of truth. `cli.py` imports it and keeps its + early argument check (fails fast before reading stdin — a small CLI-UX nicety, + and the message is unchanged). +- **`emit_script` validates `options.pod`** at the top: if it does not match + `POD_NAME_PATTERN`, raise `UnsupportedComposeError(f"invalid pod name + {options.pod!r}")`. Every caller — CLI or library — is now guarded, and the + CLI's existing `except UnsupportedComposeError` already turns it into a clean + exit 2. +- **No trap or escaping changes.** A validated pod name has no quotes, spaces, + or shell metacharacters, so the existing trap, `secret rm`, and `create` lines + are provably safe. The standalone `shlex.quote` calls stay as + belt-and-suspenders. + +`POD_NAME_PATTERN` belongs in `emit.py` because the constraint is about what the +emitter can safely embed, not about CLI argument parsing; the CLI is one +consumer of that rule. + +## Non-goals + +- Shell-escaping an arbitrary-metacharacter pod name through the nested trap + (Option A) — a pod name is an identifier, not free text; refuse rather than + contort the emission. +- Validating other already-safe embedded values (service names, project_dir) — + they are `shlex.quote`d / `to_shell`-rendered at every site, and secret/config + names are already charset-validated. + +## Testing + +`just test-ci` at 100%: `emit_script` with adversarial pod values (`"p'; rm -rf +/; '"`, a space, `"$(touch x)"`, an empty string, a leading `-`) raises +`UnsupportedComposeError` naming the bad pod name; a valid pod name still emits a +correct script; `cli.py` still rejects a bad `--pod-name` with its existing +message and exit 2, importing the shared pattern (no duplicate regex). `just +lint-ci` clean and `just check-planning`. Retire the deferred entry. + +## Risk + +- **Behavior change to a public function** (low x low): `emit_script` now raises + on an input it previously mis-rendered. This is strictly safer (a malformed + trap became a clean error) and only affects a pod name that was never valid to + embed; documented in the change and architecture note. From 1de73d1cf1be7cbab55c68ad2f1ccb3f82fb715f Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 12 Jul 2026 09:53:13 +0300 Subject: [PATCH 2/4] fix: validate the pod name in emit_script, not only the CLI --- compose2pod/cli.py | 8 ++------ compose2pod/emit.py | 6 ++++++ tests/test_emit.py | 27 +++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) diff --git a/compose2pod/cli.py b/compose2pod/cli.py index 73acdc4..5668fc7 100644 --- a/compose2pod/cli.py +++ b/compose2pod/cli.py @@ -3,13 +3,12 @@ import argparse import contextlib import json -import re import sys from pathlib import Path from types import ModuleType from typing import Any -from compose2pod.emit import EmitOptions, emit_script, referenced_variables +from compose2pod.emit import POD_NAME_PATTERN, EmitOptions, emit_script, referenced_variables from compose2pod.exceptions import UnsupportedComposeError from compose2pod.extends import resolve_extends from compose2pod.parsing import validate @@ -20,9 +19,6 @@ import yaml as _yaml -POD_NAME_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]*$") - - def _load_yaml(text: str) -> Any: # noqa: ANN401 - returns arbitrary parsed compose data if _yaml is None: msg = "YAML input requires the 'yaml' extra: pip install compose2pod[yaml] (or pipe JSON via yq)" @@ -72,7 +68,7 @@ def main(argv: list[str] | None = None) -> int: help="target exit code treated as success in addition to 0", ) args = parser.parse_args(argv) - if not POD_NAME_PATTERN.match(args.pod_name): + if not POD_NAME_PATTERN.fullmatch(args.pod_name): sys.stderr.write(f"compose2pod: error: invalid pod name {args.pod_name!r}\n") return 2 if args.file: diff --git a/compose2pod/emit.py b/compose2pod/emit.py index 35bb4d1..9f0d973 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -1,10 +1,12 @@ """Render the podman-pod test script for a target service and its dependencies.""" import dataclasses +import re import shlex from pathlib import Path from typing import Any +from compose2pod.exceptions import UnsupportedComposeError from compose2pod.graph import depends_on, hostnames, startup_order from compose2pod.healthcheck import health_cmd, interval_seconds from compose2pod.keys import SERVICE_KEYS, Token, _Expand, _key_value_pairs @@ -16,6 +18,7 @@ HEALTHY_WAIT_BUDGET_SECONDS = 120 +POD_NAME_PATTERN = re.compile(r"^[A-Za-z0-9][A-Za-z0-9_.-]*$") def image_for(svc: dict[str, Any], ci_image: str) -> Token: @@ -187,6 +190,9 @@ def _emit_target(lines: list[str], run_tokens: list[Token], options: EmitOptions 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) services = compose["services"] hosts = hostnames(services) order = startup_order(services, options.target) diff --git a/tests/test_emit.py b/tests/test_emit.py index 1d25af5..82aa056 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -1,3 +1,5 @@ +import pytest + from compose2pod.emit import ( EmitOptions, _Expand, @@ -8,6 +10,7 @@ referenced_variables, run_flags, ) +from compose2pod.exceptions import UnsupportedComposeError from compose2pod.parsing import validate @@ -722,3 +725,27 @@ def test_target_on_a_profiled_service_still_runs_it(self) -> None: } } assert "test-pod-debug-tools" in self._script(doc, target="debug-tools") + + +class TestPodNameValidation: + def _options(self, pod: str) -> EmitOptions: + return EmitOptions( + target="app", + ci_image="i", + command="", + pod=pod, + project_dir="/b", + artifacts=[], + allow_exit_codes=[], + ) + + def test_emit_script_rejects_invalid_pod_names(self) -> None: + doc = {"services": {"app": {"image": "x"}}} + for bad in ("p'; rm -rf /; '", "bad name", "$(touch x)", "", "-p", "p\n"): + with pytest.raises(UnsupportedComposeError, match="invalid pod name"): + emit_script(compose=doc, options=self._options(bad)) + + def test_emit_script_accepts_a_valid_pod_name(self) -> None: + doc = {"services": {"app": {"image": "x"}}} + script = emit_script(compose=doc, options=self._options("test-pod.1")) + assert "podman pod create --name test-pod.1" in script From f8b6cac52ec0d9dc3003846f615bf0f3fefbec6d Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 12 Jul 2026 09:57:13 +0300 Subject: [PATCH 3/4] docs: retire the pod-name trap-hardening deferred entry --- planning/deferred.md | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/planning/deferred.md b/planning/deferred.md index 7e96277..a77dbc9 100644 --- a/planning/deferred.md +++ b/planning/deferred.md @@ -18,17 +18,3 @@ needs the org owner's sign-off. `.github` PR adds `compose2pod` to the brand `MANIFEST` and regenerates the lockup assets. Then wire the `` block above the badges (dropping the `# compose2pod` H1) to match siblings. - -## Harden the pod-cleanup trap's nested `shlex.quote` for the `emit_script` library path - -`emit.py`'s `EXIT` trap quotes the pod name with a nested `shlex.quote` inside -an already-quoted trap command. The CLI never reaches this edge because -`cli.py` validates pod names against `POD_NAME_PATTERN` before calling -`emit_script`, but a library caller invoking `emit_script`/`EmitOptions` -directly can pass an unvalidated `pod` value that produces a malformed or -unsafe trap command. - -**Revisit trigger:** a library-API test is added that exercises `emit_script` -with adversarial `pod` values (quotes, spaces, shell metacharacters), or a bug -report surfaces from a caller using the library path without CLI validation. -Needs its own change file. From fd5fdb03151269efc5d6d8df6cf026add5f7cabf Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Sun, 12 Jul 2026 10:02:11 +0300 Subject: [PATCH 4/4] docs: record the emit_script pod-name identifier constraint --- architecture/supported-subset.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index 0bd8dc0..aa10acb 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -468,7 +468,13 @@ check. A braced reference whose text after the name is not one of these operators (e.g. `${FOO!bar}`) is malformed and raises `UnsupportedComposeError` rather than silently dropping the trailing text. Tool/CLI-supplied values (`--project-dir`, `--image`, the pod name, -the `--command` override) are literal and never interpolated. The CLI +the `--command` override) are literal and never interpolated. The pod +name is embedded into the pod-create line, the single-quoted `EXIT` +trap, and the `-` store names (some of them unquoted), so it +must be a shell-inert identifier — `emit_script` validates it against +`POD_NAME_PATTERN` (`^[A-Za-z0-9][A-Za-z0-9_.-]*$`) and raises +`UnsupportedComposeError` on any other value, guarding library callers +as well as the CLI's `--pod-name` (`compose2pod/emit.py`). The CLI prints one informational stderr note listing the variable names the generated script actually expands at run time — `referenced_variables()` (`compose2pod/emit.py`) collects these from the same tokens `to_shell()`