test: coverage for build_args_str() and load_env_file() inline comment stripping

dds/builders/docker.py

@@ -10,7 +10,7 @@
 
 from dds.console import console
 from dds.utils.git import git_info
-from dds.utils.shell import run_cmd
+from dds.utils.shell import build_args_str, run_cmd
 
 
 def build_and_push_local(
@@ -21,7 +21,7 @@ def build_and_push_local(
     verbose: bool = False,
 ) -> None:
     """Build locally with Docker, then push to a registry."""
-    args_str = _build_args_str(build_args)
+    args_str = build_args_str(build_args)
 
     console.print(f"[yellow]🔨 Building locally: {image}[/yellow]")
     result = run_cmd(
@@ -58,8 +58,6 @@ def resolve_image_tag(
     return f"{registry}/{project}-{name}:{tag}"
 
 
-def _build_args_str(build_args: dict[str, str] | None) -> str:
-    """Format build args dict into Docker --build-arg flags."""
-    if not build_args:
-        return ""
-    return " ".join(f"--build-arg {k}={v}" for k, v in build_args.items())
+# _build_args_str has moved to dds.utils.shell.build_args_str.
+# Kept as a private alias for any callers that imported it directly.
+_build_args_str = build_args_str

dds/providers/azure/container.py

@@ -12,7 +12,7 @@
 from dds.context import DeployContext
 from dds.providers.azure.utils import az, az_json
 from dds.providers.base import ContainerProvider
-from dds.utils.shell import run_cmd
+from dds.utils.shell import build_args_str, run_cmd
 
 
 class AzureContainerProvider(ContainerProvider):
@@ -284,7 +284,7 @@ def _build_and_push_local(
         image: str, dockerfile: str, context: str,
         build_args: dict[str, str], verbose: bool,
     ) -> None:
-        args_str = _build_args_str(build_args)
+        args_str = build_args_str(build_args)
         console.print(f"[yellow]🔨 Building locally: {image}[/yellow]")
         result = run_cmd(f"docker build -f {dockerfile} -t {image} {args_str} {context}", verbose=verbose)
         if result.returncode != 0:
@@ -305,7 +305,7 @@ def _build_acr(
         build_args: dict[str, str], verbose: bool, platform: str = "linux/amd64",
     ) -> None:
         registry_name = registry.split(".")[0]
-        args_str = _build_args_str(build_args)
+        args_str = build_args_str(build_args)
         console.print(f"[yellow]🔨 ACR build: {image_tag}[/yellow]")
         console.print(f"  Registry: {registry_name} | Dockerfile: {dockerfile} | Platform: {platform}")
         az(
@@ -348,8 +348,6 @@ def _http_check(url: str, timeout: float = 10.0, verbose: bool = False) -> bool:
             return False
 
 
-def _build_args_str(build_args: dict[str, str] | None) -> str:
-    """Format build args dict into Docker --build-arg flags."""
-    if not build_args:
-        return ""
-    return " ".join(f"--build-arg {k}={v}" for k, v in build_args.items())
+# _build_args_str has moved to dds.utils.shell.build_args_str.
+# Kept as a module-level alias for any callers that imported it directly.
+_build_args_str = build_args_str

dds/secrets.py

@@ -95,8 +95,17 @@ def load_env_file(path: str, verbose: bool = False) -> dict[str, str]:
             continue
         key, _, value = line.partition("=")
         key, value = key.strip(), value.strip()
+        # Strip matching surrounding quotes (double or single).
+        # Quoted values may contain inline comments that are part of the value;
+        # unquoted values have inline comments stripped.
         if len(value) >= 2 and value[0] == value[-1] and value[0] in ('"', "'"):
             value = value[1:-1]
+        else:
+            # Remove inline comment: anything after un-quoted " #" or "\t#"
+            for sep in (" #", "\t#"):
+                if sep in value:
+                    value = value[: value.index(sep)].rstrip()
+                    break
         result[key] = value
 
     if verbose:

dds/utils/shell.py

@@ -3,6 +3,7 @@
 from __future__ import annotations
 
 import os
+import shlex
 import subprocess
 
 from dds.console import console
@@ -33,3 +34,21 @@ def run_cmd(
         console.print(result.stdout.rstrip())
 
     return result
+
+
+def build_args_str(build_args: dict[str, str] | None) -> str:
+    """Format a build-args dict into Docker --build-arg flags.
+
+    Values are shell-quoted so that spaces and metacharacters in values do not
+    break the command string when it is passed to the shell with shell=True.
+
+    Example::
+
+        >>> build_args_str({'NODE_ENV': 'production', 'LABEL': 'hello world'})
+        "--build-arg NODE_ENV=production --build-arg LABEL='hello world'"
+    """
+    if not build_args:
+        return ""
+    return " ".join(
+        f"--build-arg {k}={shlex.quote(str(v))}" for k, v in build_args.items()
+    )

tests/test_secrets.py

@@ -76,3 +76,29 @@ def test_missing_env_var_secret(self, monkeypatch) -> None:
             project_cfg={},
         )
         assert "MISSING" not in result
+
+    def test_inline_comment_stripped_from_unquoted_value(self, tmp_path: Path) -> None:
+        """Inline comments after unquoted values must be stripped (dotenv standard)."""
+        env_file = tmp_path / ".env"
+        env_file.write_text("DATABASE_URL=postgres://localhost/mydb  # local dev only\n")
+        result = load_env_file(str(env_file))
+        assert result["DATABASE_URL"] == "postgres://localhost/mydb"
+
+    def test_inline_comment_stripped_tab_separator(self, tmp_path: Path) -> None:
+        env_file = tmp_path / ".env"
+        env_file.write_text("API_URL=https://api.example.com\t# production endpoint\n")
+        result = load_env_file(str(env_file))
+        assert result["API_URL"] == "https://api.example.com"
+
+    def test_inline_comment_not_stripped_from_quoted_value(self, tmp_path: Path) -> None:
+        """Hash inside quoted strings must be preserved as part of the value."""
+        env_file = tmp_path / ".env"
+        env_file.write_text('PASSWORD="hunter#2"  # good password\n')
+        result = load_env_file(str(env_file))
+        assert result["PASSWORD"] == "hunter#2"
+
+    def test_quoted_value_with_trailing_comment(self, tmp_path: Path) -> None:
+        env_file = tmp_path / ".env"
+        env_file.write_text('GREETING="hello world"  # a greeting\n')
+        result = load_env_file(str(env_file))
+        assert result["GREETING"] == "hello world"

tests/test_shell_utils.py

@@ -0,0 +1,84 @@
+"""Tests for dds.utils.shell — build_args_str() and run_cmd()."""
+
+from __future__ import annotations
+
+import subprocess
+from unittest.mock import patch
+
+import pytest
+
+from dds.utils.shell import build_args_str, run_cmd
+
+
+class TestBuildArgsStr:
+    """Tests for build_args_str()."""
+
+    def test_none_returns_empty(self) -> None:
+        assert build_args_str(None) == ""
+
+    def test_empty_dict_returns_empty(self) -> None:
+        assert build_args_str({}) == ""
+
+    def test_single_simple_arg(self) -> None:
+        result = build_args_str({"NODE_ENV": "production"})
+        assert result == "--build-arg NODE_ENV=production"
+
+    def test_multiple_args(self) -> None:
+        result = build_args_str({"A": "1", "B": "2"})
+        assert "--build-arg A=1" in result
+        assert "--build-arg B=2" in result
+
+    def test_value_with_spaces_is_quoted(self) -> None:
+        result = build_args_str({"LABEL": "hello world"})
+        # shlex.quote wraps in single quotes when the value contains spaces
+        assert "--build-arg LABEL='hello world'" == result
+
+    def test_value_with_dollar_sign_is_quoted(self) -> None:
+        """Dollar signs must be quoted to prevent shell variable expansion."""
+        result = build_args_str({"SECRET": "$MY_SECRET"})
+        assert "--build-arg SECRET=" in result
+        # The dollar sign must be inside quotes so the shell doesn't expand it
+        assert "$MY_SECRET" not in result.replace("'$MY_SECRET'", "")
+
+    def test_value_with_equals_sign(self) -> None:
+        """Values containing = (e.g. base64, connection strings) must survive round-trip."""
+        result = build_args_str({"DB_URL": "postgres://u:p@h/db?ssl=require"})
+        assert "--build-arg DB_URL=" in result
+        assert "postgres://u:p@h/db" in result
+
+    def test_simple_value_not_over_quoted(self) -> None:
+        """Simple alphanumeric values should not be wrapped in extra quotes."""
+        result = build_args_str({"VER": "1.2.3"})
+        # shlex.quote leaves safe strings unquoted
+        assert result == "--build-arg VER=1.2.3"
+
+    def test_integer_value_coerced_to_string(self) -> None:
+        result = build_args_str({"PORT": 3000})  # type: ignore[arg-type]
+        assert "--build-arg PORT=3000" == result
+
+
+class TestRunCmd:
+    """Tests for run_cmd()."""
+
+    def test_returns_completed_process(self) -> None:
+        result = run_cmd("echo hello")
+        assert isinstance(result, subprocess.CompletedProcess)
+        assert result.returncode == 0
+
+    def test_captures_stdout(self) -> None:
+        result = run_cmd("echo captured", capture=True)
+        assert "captured" in result.stdout
+
+    def test_failing_command_returns_nonzero(self) -> None:
+        result = run_cmd("exit 1")
+        assert result.returncode != 0
+
+    def test_cwd_is_passed(self, tmp_path) -> None:
+        result = run_cmd("pwd", capture=True, cwd=str(tmp_path))
+        assert str(tmp_path) in result.stdout
+
+    def test_env_vars_are_merged(self, monkeypatch) -> None:
+        monkeypatch.setenv("EXISTING", "yes")
+        result = run_cmd("echo $EXISTING $INJECTED", capture=True, env={"INJECTED": "injected"})
+        assert "yes" in result.stdout
+        assert "injected" in result.stdout