diff --git a/dds/builders/docker.py b/dds/builders/docker.py index d18a072..3267d5d 100644 --- a/dds/builders/docker.py +++ b/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 diff --git a/dds/providers/azure/container.py b/dds/providers/azure/container.py index 8381636..99efa33 100644 --- a/dds/providers/azure/container.py +++ b/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 diff --git a/dds/secrets.py b/dds/secrets.py index de8b752..fb30c22 100644 --- a/dds/secrets.py +++ b/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: diff --git a/dds/utils/shell.py b/dds/utils/shell.py index f0a8532..b3d75b5 100644 --- a/dds/utils/shell.py +++ b/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() + ) diff --git a/tests/test_secrets.py b/tests/test_secrets.py index 71a5a11..6c07b6e 100644 --- a/tests/test_secrets.py +++ b/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" diff --git a/tests/test_shell_utils.py b/tests/test_shell_utils.py new file mode 100644 index 0000000..9d8002a --- /dev/null +++ b/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