From 86de5475cd90b59fdfc032012e773bc824dd0995 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 9 Jul 2026 07:58:24 +0300 Subject: [PATCH] fix: env null passthrough and reject malformed variable references - environment mapping value of null now emits -e KEY (host passthrough) instead of the literal -e KEY=None, matching Compose and the list form. - a malformed braced reference like ${FOO!bar} now raises UnsupportedComposeError instead of silently dropping the trailing text. - add operator-arg escaping tests for all six operator forms. --- architecture/supported-subset.md | 9 ++++- compose2pod/emit.py | 8 +++- compose2pod/shell.py | 9 ++++- .../2026-07-09.01-env-null-passthrough.md | 34 +++++++++++++++++ ...02-reject-malformed-variable-references.md | 38 +++++++++++++++++++ tests/test_cli.py | 8 ++++ tests/test_emit.py | 6 +++ tests/test_shell.py | 15 ++++++++ 8 files changed, 124 insertions(+), 3 deletions(-) create mode 100644 planning/changes/2026-07-09.01-env-null-passthrough.md create mode 100644 planning/changes/2026-07-09.02-reject-malformed-variable-references.md diff --git a/architecture/supported-subset.md b/architecture/supported-subset.md index c4b9df7..bed0da4 100644 --- a/architecture/supported-subset.md +++ b/architecture/supported-subset.md @@ -25,6 +25,10 @@ warns (ignored, behavior-neutral inside a single pod) or raises accepted but its contents (context, dockerfile, args) are not read — `image_for` (`compose2pod/emit.py`) runs the CI image supplied via `--image` for any service that has one. +- **`environment`:** list form (`- KEY=value`, `- KEY`) or mapping form + (`KEY: value`, `KEY:`). A null mapping value (`KEY:`) means "pass `KEY` + through from the host", emitted as a bare `-e KEY` exactly like the list + form `- KEY`. - **`tmpfs`:** a string or list of strings, each `` or `:` (e.g. `/tmp:mode=1777`), passed through verbatim as `podman run --tmpfs ` — Compose's short syntax maps directly onto @@ -96,7 +100,10 @@ emitted as `${VAR-}` so an unset variable expands to empty under the script's `set -eu` (matching Compose semantics) instead of aborting on `nounset`. `${VAR:?msg}`/`${VAR?msg}` fails the script at run time — with `msg` — if the variable is unset or empty; there is no generation-time -check. Tool/CLI-supplied values (`--project-dir`, `--image`, the pod name, +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 prints one informational stderr note listing the variable names the generated script actually expands at run time — `referenced_variables()` diff --git a/compose2pod/emit.py b/compose2pod/emit.py index 57cf4aa..572df82 100644 --- a/compose2pod/emit.py +++ b/compose2pod/emit.py @@ -59,7 +59,13 @@ def run_flags(name: str, svc: dict[str, Any], pod: str, hosts: list[str], projec for host in hosts: flags += ["--add-host", f"{host}:127.0.0.1"] environment = svc.get("environment") or {} - pairs = environment if isinstance(environment, list) else [f"{k}={v}" for k, v in environment.items()] + # A null mapping value means "pass KEY through from the host" (bare `-e KEY`), + # matching Compose and the list form `- KEY`. + pairs = ( + environment + if isinstance(environment, list) + else [k if v is None else f"{k}={v}" for k, v in environment.items()] + ) for pair in pairs: flags += ["-e", _Expand(str(pair))] env_files = svc.get("env_file") or [] diff --git a/compose2pod/shell.py b/compose2pod/shell.py index 8996feb..17980fa 100644 --- a/compose2pod/shell.py +++ b/compose2pod/shell.py @@ -2,6 +2,8 @@ import re +from compose2pod.exceptions import UnsupportedComposeError + _PATTERN = re.compile( r""" @@ -29,9 +31,14 @@ def _encode_match(match: re.Match[str]) -> str: return "${" + match.group("named") + "-}" # unset -> empty, survives `set -u` name = match.group("braced") op = match.group("op") + arg = match.group("arg") if op is None: + if arg: + # `${NAME}` -- text after the name is not a valid operator. + msg = f"malformed variable reference: ${{{name}{arg}}}" + raise UnsupportedComposeError(msg) return "${" + name + "-}" - return "${" + name + op + _escape_literal(match.group("arg")) + "}" + return "${" + name + op + _escape_literal(arg) + "}" def to_shell(value: str) -> str: diff --git a/planning/changes/2026-07-09.01-env-null-passthrough.md b/planning/changes/2026-07-09.01-env-null-passthrough.md new file mode 100644 index 0000000..3bee210 --- /dev/null +++ b/planning/changes/2026-07-09.01-env-null-passthrough.md @@ -0,0 +1,34 @@ +--- +summary: A null-valued `environment` mapping entry now emits `-e KEY` (host passthrough) instead of the literal `-e KEY=None`, matching Compose and the list-form `- KEY`. +--- + +# Change: Env mapping null value is host passthrough + +## Goal + +`environment: {KEY: }` (a null mapping value) is Compose shorthand for "pass +`KEY` through from the host environment", identical to the list form `- KEY`. +compose2pod rendered it as `-e KEY=None` — the literal Python string `None` +reached the container. Emit `-e KEY` instead. + +## Approach + +In `run_flags`, the mapping branch built every entry as `f"{k}={v}"`. Build a +bare `k` (passthrough) when `v is None`, else `f"{k}={v}"`. The token is still +`_Expand`-wrapped, so `-e KEY` renders as podman host-passthrough at run time — +consistent with the runtime-interpolation model. The list form already handled +`- KEY` correctly. See `architecture/supported-subset.md`. + +## Files + +- `compose2pod/emit.py` — `run_flags` mapping-form env comprehension. +- `architecture/supported-subset.md` — note null-value passthrough. +- `tests/test_emit.py` — null-value passthrough test. + +## Verification + +- [ ] Failing test first: `uv run pytest tests/test_emit.py -k null -v` — asserts `-e KEY` not `-e KEY=None`. +- [ ] Apply the change. +- [ ] Test passes. +- [ ] `just test-ci` — full suite green at 100%. +- [ ] `just lint-ci` — clean. diff --git a/planning/changes/2026-07-09.02-reject-malformed-variable-references.md b/planning/changes/2026-07-09.02-reject-malformed-variable-references.md new file mode 100644 index 0000000..a2e7ff7 --- /dev/null +++ b/planning/changes/2026-07-09.02-reject-malformed-variable-references.md @@ -0,0 +1,38 @@ +--- +summary: A malformed braced reference like `${FOO!bar}` (a name followed by text that is not a valid Compose operator) now raises `UnsupportedComposeError` instead of silently dropping the trailing text. +--- + +# Change: Reject malformed braced variable references + +## Goal + +`${FOO!bar}` — a braced reference whose text after the name is not one of the +recognized operators (`:-`, `-`, `:?`, `?`, `:+`, `+`) — was silently encoded +as `${FOO-}`, discarding `!bar`. Silent-dropping is exactly what this project +refuses; raise `UnsupportedComposeError` instead, naming the offending +reference. Also add the previously-missing operator-arg escaping tests. + +## Approach + +In `shell._encode_match`, the braced branch matches `name`, an optional `op`, +and `arg = [^}]*`. When `op` is absent but `arg` is non-empty, the text after +the name is unrecognized — raise `UnsupportedComposeError(f"malformed variable +reference: ${{{name}{arg}}}")`. `${FOO}` (empty arg) is unaffected. The raise +propagates through `to_shell` -> `emit._render` -> `emit_script`, which the CLI +already catches and reports as a usage error (exit 2). `variable_names` does not +encode, so it is unaffected. See `architecture/supported-subset.md`. + +## Files + +- `compose2pod/shell.py` — raise in `_encode_match`; import `UnsupportedComposeError`. +- `architecture/supported-subset.md` — note the malformed-reference rejection. +- `tests/test_shell.py` — malformed-reference raise + operator-arg escaping for all six forms. +- `tests/test_cli.py` — malformed reference returns exit 2 with a helpful message. + +## Verification + +- [ ] Failing test first: `uv run pytest tests/test_shell.py -k malformed -v` — expects `UnsupportedComposeError`. +- [ ] Apply the change. +- [ ] Test passes. +- [ ] `just test-ci` — full suite green at 100%. +- [ ] `just lint-ci` — clean. diff --git a/tests/test_cli.py b/tests/test_cli.py index 1c4347c..d453870 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -147,6 +147,14 @@ def test_environment_reference_is_emitted_live_not_resolved( assert "${LLM_MODEL-}" in out.out # left live for the runtime shell assert "note: script references variables at run time: LLM_MODEL" in out.err + def test_malformed_variable_reference_returns_2( + self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch + ) -> None: + yaml_text = "services:\n app:\n image: x\n environment:\n K: ${FOO!bad}\n" + rc = run_main(yaml_text, ["--target", "app", "--image", "i", "--format", "yaml"], monkeypatch) + assert rc == EXIT_USAGE_ERROR + assert "malformed variable reference" in capsys.readouterr().err + def test_required_reference_survives_generation( self, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch ) -> None: diff --git a/tests/test_emit.py b/tests/test_emit.py index 0fa6ad2..f8baf8f 100644 --- a/tests/test_emit.py +++ b/tests/test_emit.py @@ -35,6 +35,12 @@ def test_env_map_form(self) -> None: flags = run_flags("app", svc, "p", [], "/builds/x") assert flags[4:8] == ["-e", _Expand("A=1"), "-e", _Expand("B=two words")] + def test_env_map_null_value_is_host_passthrough(self) -> None: + # A null mapping value means "pass KEY through from the host", like `- KEY`. + svc = {"image": "x", "environment": {"PASSTHRU": None, "SET": "v"}} + flags = run_flags("app", svc, "p", [], "/builds/x") + assert flags[4:8] == ["-e", _Expand("PASSTHRU"), "-e", _Expand("SET=v")] + def test_env_file_and_volume_resolved_against_project_dir(self) -> None: svc = {"image": "x", "env_file": "tests.env", "volumes": [".:/srv/www/"]} flags = run_flags("app", svc, "p", [], "/builds/chats") diff --git a/tests/test_shell.py b/tests/test_shell.py index 2207e57..822bd3f 100644 --- a/tests/test_shell.py +++ b/tests/test_shell.py @@ -1,6 +1,9 @@ import os import subprocess +import pytest + +from compose2pod.exceptions import UnsupportedComposeError from compose2pod.shell import to_shell, variable_names @@ -46,6 +49,18 @@ def test_bare_dollar_not_a_ref_is_escaped(self) -> None: def test_default_text_is_escaped(self) -> None: assert to_shell("${FOO:-a`b}") == '"${FOO:-a\\`b}"' + def test_all_operator_forms_escape_arg_text(self) -> None: + assert to_shell("${A:-x`y}") == '"${A:-x\\`y}"' + assert to_shell("${A-x`y}") == '"${A-x\\`y}"' + assert to_shell("${A:?x`y}") == '"${A:?x\\`y}"' + assert to_shell("${A?x`y}") == '"${A?x\\`y}"' + assert to_shell("${A:+x`y}") == '"${A:+x\\`y}"' + assert to_shell("${A+x`y}") == '"${A+x\\`y}"' + + def test_malformed_braced_reference_raises(self) -> None: + with pytest.raises(UnsupportedComposeError, match=r"malformed variable reference: \$\{FOO!bar\}"): + to_shell("${FOO!bar}") + def test_multiple_refs_and_literals(self) -> None: assert to_shell("$A-${B}") == '"${A-}-${B-}"'