fix(artifacts): env dual-mint id remap, backslash escapes, legacy ENV fidelity

Three review findings on the deployment-env config-key extraction:

- yaml env-dual-mint ids now use an internal "env." prefix (same id_key
  remap the Dockerfile ARG path already used), so a top-level yaml key
  sharing a name with a recognized compose/k8s env var no longer collides
  on id with the plain yaml mint of that same top-level key.
- _split_ws_respecting_quotes gained backslash-escape awareness: an
  unquoted `\ ` keeps a literal space instead of splitting the token, `\\`
  collapses to one literal backslash -- fixes silent truncation on Docker's
  own documented ENV example (`MY_DOG=Rex\ The\ Dog`).
- The legacy `ENV KEY value` space form no longer runs quote-stripping
  (real Docker keeps quotes verbatim there, unlike the key=value form) and
  now splits on general whitespace instead of a literal space, so a
  tab-separated legacy line parses instead of being silently dropped.

Author: rahlk

codeanalyzer/artifacts/config_keys.py

@@ -26,13 +26,19 @@
 ADDITIONAL namespace-`env` keys keyed on the bare var name, alongside the
 normal namespace-`yaml` dotted-path ones -- dual-minting is intentional
 (so `os.environ`/`os.getenv` reads bind to compose/k8s-declared vars too),
-never deduped away. Ids never collide with the plain yaml mint: the env
-mint's key is always the bare var name while the yaml mint's key always
-carries the full dotted path, so the two differ by construction for every
-shape this pass recognizes. This pass is shape-based, not filename/role
-gated -- any yaml artifact whose content happens to match mints the extra
-keys, matching this module's general overlay posture (permissive, never a
-schema validator).
+never deduped away. The `key` FIELD is always the bare var name (matching
+`env` namespace's exact-match resolution semantics), but the `id` cannot
+reuse that bare name unqualified: a TOP-LEVEL yaml key sharing the same
+name as a recognized env var (e.g. a document with both a bare
+`COMPOSE_ONLY_KEY:` entry and a `services.web.environment.COMPOSE_ONLY_KEY`
+one) would otherwise collide with the plain yaml mint's own bare-key id --
+the yaml mint's key is USUALLY a longer dotted path that can't collide, but
+not always (a top-level leaf's dotted path IS just its bare name). Same fix
+as the dockerfile ARG case: the env-dual-mint's id is disambiguated with an
+internal `env.` prefix (`_build_keys`'s `id_key`), the `key` field itself
+unaffected. This pass is shape-based, not filename/role gated -- any yaml
+artifact whose content happens to match mints the extra keys, matching this
+module's general overlay posture (permissive, never a schema validator).
 
 Span precision differs by shape: env/properties/ini/dockerfile are
 line-oriented, so the parse itself knows the exact defining line. yaml/
@@ -231,18 +237,30 @@ def _join_continuations(lines: List[str], start_i: int) -> Tuple[str, int]:
 
 
 def _split_ws_respecting_quotes(s: str) -> List[str]:
-    """Whitespace-split `s`, except inside a matching `'`/`"` span (a quoted
-    value may contain spaces) -- quote characters stay IN the returned
-    tokens, stripped afterward by `_env_value` so there is one quote-
-    stripping implementation, not two."""
+    r"""Whitespace-split `s`, except inside a matching `'`/`"` span (a quoted
+    value may contain spaces) or right after an unquoted `\` -- a backslash
+    escapes the next character (`\ ` keeps a literal space in the token
+    instead of splitting there, `\\` collapses to one literal backslash),
+    mirroring Docker's own shell-style ENV splitting (moby's `Rex\ The\
+    Dog` example). A dangling trailing `\` with nothing to escape is kept
+    literally rather than raising -- a real trailing continuation backslash
+    is already stripped upstream by `_join_continuations`, so this is only
+    a defensive fallback. Quote characters stay IN the returned tokens,
+    stripped afterward by `_env_value` so there is one quote-stripping
+    implementation, not two."""
     tokens: List[str] = []
     buf: List[str] = []
     quote: Optional[str] = None
-    for ch in s:
+    i, n = 0, len(s)
+    while i < n:
+        ch = s[i]
         if quote:
             buf.append(ch)
             if ch == quote:
                 quote = None
+        elif ch == "\\":
+            i += 1
+            buf.append(s[i] if i < n else ch)
         elif ch in "'\"":
             quote = ch
             buf.append(ch)
@@ -252,6 +270,7 @@ def _split_ws_respecting_quotes(s: str) -> List[str]:
                 buf = []
         else:
             buf.append(ch)
+        i += 1
     if buf:
         tokens.append("".join(buf))
     return tokens
@@ -262,8 +281,12 @@ def _dockerfile_env_entries(text: str, lines: List[str]) -> List[_Entry]:
     `ENV a=1 b=2`, and the legacy single-key `ENV K v` space form (Docker's
     own disambiguation rule: the token right after `ENV` decides the form --
     a `=` in it means one-or-more `key=value` pairs; no `=` means the
-    legacy form, where the key is the first word and the REST of the line,
-    verbatim, is the value)."""
+    legacy form, where the key is the first word and the REST of the line
+    is the value). The legacy form's value is taken VERBATIM -- unlike the
+    `key=value` form, real Docker does no quote processing there at all
+    (moby's `parseNameVal`), so `ENV NAME "John Doe"` keeps its quotes; the
+    key/value separator is general whitespace (a tab is as legal as a
+    space), not a literal `" "`."""
     out: List[_Entry] = []
     i, n = 0, len(lines)
     while i < n:
@@ -282,9 +305,9 @@ def _dockerfile_env_entries(text: str, lines: List[str]) -> List[_Entry]:
                 if sep and _ENV_KEY_NAME.match(key):
                     out.append((key, _env_value(raw_val), span))
         else:
-            key, sep, raw_val = rest.partition(" ")
-            if sep and _ENV_KEY_NAME.match(key):
-                out.append((key, _env_value(raw_val.strip()), span))
+            parts = rest.split(None, 1)
+            if len(parts) == 2 and _ENV_KEY_NAME.match(parts[0]):
+                out.append((parts[0], parts[1], span))
         i += 1
     return out
 
@@ -454,10 +477,14 @@ def _build_keys(
     bool/None that needs that coercion.
 
     `id_key` remaps `dotted_key` for ID CONSTRUCTION only -- the `.key` FIELD
-    always stays the bare `dotted_key`. Used solely so a Dockerfile ARG's id
-    can't collide with an ENV of the same name minting the same bare-name id
-    in the "env" namespace (`ARG X` then `ENV X=$X` is a common promotion
-    idiom); every other namespace omits it, preserving today's id shape."""
+    always stays the bare `dotted_key`. Two call sites need it, both to keep
+    a bare-name mint from colliding with another mint that happens to use
+    the same bare name for its OWN id: a Dockerfile ARG's id (`ARG X` then
+    `ENV X=$X` is a common promotion idiom -- both would otherwise mint id
+    `.../@key/X`), and a yaml artifact's compose/k8s env-dual-mint id (a
+    top-level yaml key sharing a name with a recognized env var would
+    otherwise collide with the plain yaml mint's own bare-key id). Every
+    other namespace omits it, preserving today's id shape."""
     coalesced: Dict[str, Tuple[object, Optional[Span]]] = {}
     for dotted_key, value, span in entries:
         coalesced[dotted_key] = (value, span)
@@ -545,7 +572,10 @@ def extract_config_keys(
             ]
             keys = (
                 _build_keys(artifact.id, "yaml", _parse_yaml(full_text, lines), capture_value)
-                + _build_keys(artifact.id, "env", env_entries, capture_value)
+                + _build_keys(
+                    artifact.id, "env", env_entries, capture_value,
+                    id_key=lambda k: f"env.{k}",
+                )
             )
         else:
             parser = _NAMESPACE_PARSERS.get(artifact.format)

test/test_config_key_extraction.py

@@ -242,6 +242,22 @@ def test_dockerfile_env_legacy_space_form():
     assert _by_key(keys)["MY_NAME"].value == "John Doe"
 
 
+def test_dockerfile_env_legacy_space_form_keeps_quotes_verbatim():
+    # Real Docker does NO quote processing in the legacy form (moby's
+    # parseNameVal) -- unlike the key=value form, quotes stay in the value.
+    art = _artifact("Dockerfile", "dockerfile")
+    keys, ok = extract_config_keys(art, 'ENV MY_NAME "John Doe"\n', True)
+    assert ok is True
+    assert _by_key(keys)["MY_NAME"].value == '"John Doe"'
+
+
+def test_dockerfile_env_legacy_space_form_tab_separated():
+    art = _artifact("Dockerfile", "dockerfile")
+    keys, ok = extract_config_keys(art, "ENV MY_NAME\tJohn Doe\n", True)
+    assert ok is True
+    assert _by_key(keys)["MY_NAME"].value == "John Doe"
+
+
 def test_dockerfile_env_quoted_values_in_multi_key_form():
     art = _artifact("Dockerfile", "dockerfile")
     keys, ok = extract_config_keys(art, 'ENV GREETING="hello world" OTHER=2\n', True)
@@ -260,6 +276,29 @@ def test_dockerfile_env_backslash_continuation():
     assert by["A"].value == "1" and by["B"].value == "2" and by["C"].value == "3"
 
 
+def test_dockerfile_env_docker_docs_example_escaped_spaces_and_continuation():
+    # Docker's own ENV docs example: a quoted value, a backslash-escaped-
+    # space value, and a third key on a continuation line -- all three exact.
+    text = (
+        'ENV MY_NAME="John Doe" MY_DOG=Rex\\ The\\ Dog \\\n'
+        '    MY_CAT=fluffy\n'
+    )
+    art = _artifact("Dockerfile", "dockerfile")
+    keys, ok = extract_config_keys(art, text, True)
+    assert ok is True
+    by = _by_key(keys)
+    assert by["MY_NAME"].value == "John Doe"
+    assert by["MY_DOG"].value == "Rex The Dog"
+    assert by["MY_CAT"].value == "fluffy"
+
+
+def test_dockerfile_env_double_backslash_collapses_to_one():
+    art = _artifact("Dockerfile", "dockerfile")
+    keys, ok = extract_config_keys(art, "ENV PATTERN=a\\\\b\n", True)
+    assert ok is True
+    assert _by_key(keys)["PATTERN"].value == "a\\b"
+
+
 def test_dockerfile_arg_with_and_without_default():
     text = "ARG BUILD_REV\nARG VERSION=1.0\n"
     art = _artifact("Dockerfile", "dockerfile")
@@ -404,6 +443,30 @@ def test_env_recognition_scoped_to_yaml_only_not_json_or_toml():
     assert all(k.namespace == "json" for k in keys)
 
 
+def test_yaml_top_level_key_and_env_dual_mint_same_name_no_id_collision():
+    # Reviewer-reported: a top-level yaml key sharing a name with a
+    # compose/k8s-recognized env var, in the SAME file, must not collide on
+    # id -- the "differs by construction" claim only holds for the RECOGNIZED
+    # shapes' own dotted paths, not for an unrelated top-level leaf.
+    text = textwrap.dedent("""\
+        COMPOSE_ONLY_KEY: top
+        services:
+          web:
+            environment:
+              COMPOSE_ONLY_KEY: nested
+        """)
+    art = _artifact("docker-compose.yml", "yaml")
+    keys, ok = extract_config_keys(art, text, True)
+    assert ok is True
+    matches = [k for k in keys if k.key == "COMPOSE_ONLY_KEY"]
+    assert len(matches) == 2
+    assert len({k.id for k in matches}) == 2
+    assert {k.namespace for k in matches} == {"yaml", "env"}
+    by_ns = {k.namespace: k for k in matches}
+    assert by_ns["yaml"].value == "top"
+    assert by_ns["env"].value == "nested"
+
+
 def test_compose_k8s_env_recognition_is_deterministic_and_sorted():
     text = textwrap.dedent("""\
         services: