Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion architecture/supported-subset.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<pod>-<name>` 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()`
Expand Down
8 changes: 2 additions & 6 deletions compose2pod/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)"
Expand Down Expand Up @@ -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:
Expand Down
6 changes: 6 additions & 0 deletions compose2pod/emit.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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:
Expand Down Expand Up @@ -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)
Expand Down
76 changes: 76 additions & 0 deletions planning/changes/2026-07-12.01-validate-pod-name-in-emit.md
Original file line number Diff line number Diff line change
@@ -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 <stores>` 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 <pod>`, `podman secret rm <pod>-<name> ...`), the store
`create` lines (`podman secret create <pod>-<name> ...`), 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 `<name>` 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.
14 changes: 0 additions & 14 deletions planning/deferred.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<picture>` 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.
27 changes: 27 additions & 0 deletions tests/test_emit.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import pytest

from compose2pod.emit import (
EmitOptions,
_Expand,
Expand All @@ -8,6 +10,7 @@
referenced_variables,
run_flags,
)
from compose2pod.exceptions import UnsupportedComposeError
from compose2pod.parsing import validate


Expand Down Expand Up @@ -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