From 2c0ba6b1e43e292f349d9b8293e1b235fa5b9f3a Mon Sep 17 00:00:00 2001 From: "Thor (Guardrail)" Date: Wed, 22 Apr 2026 14:02:36 -0700 Subject: [PATCH] refactor: consolidate _build_args_str to utils.shell, fix env file inline comment parsing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two improvements: 1. _build_args_str() was duplicated identically in both dds/providers/azure/container.py and dds/builders/docker.py. Moved the canonical implementation to dds/utils/shell.build_args_str() (with shell-quoting via shlex.quote — see PR #1 for the safety fix). Both original sites now import and delegate to the shared version. Module-level _build_args_str aliases kept for backward compatibility. 2. load_env_file() in dds/secrets.py did not strip inline comments from unquoted values, so a line like: DATABASE_URL=postgres://localhost/mydb # local dev only would produce 'postgres://localhost/mydb # local dev only' as the value. This is incorrect — standard .env file parsers (dotenv et al.) strip inline comments from unquoted values. Quoted values are left untouched (the comment is considered part of the value when quoted). Added stripping for ' #' and '\t#' comment separators on unquoted values. --- dds/builders/docker.py | 12 +++++------- dds/providers/azure/container.py | 14 ++++++-------- dds/secrets.py | 9 +++++++++ dds/utils/shell.py | 19 +++++++++++++++++++ 4 files changed, 39 insertions(+), 15 deletions(-) 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() + )