From 4c305531b1e84355bbd9d596c3f35fd7fe786a3e Mon Sep 17 00:00:00 2001 From: nadavy Date: Tue, 14 Jul 2026 12:36:46 +0300 Subject: [PATCH] fix(init): reject empty/whitespace project names _validate_project_name only checked for path separators and '..', so an empty or whitespace-only name (e.g. `apm install ` auto-bootstrapping apm.yml from Path.cwd().name at a container's unset root WORKDIR, or a blank `apm init` prompt) silently wrote `name: ''` into apm.yml. Every later install/lock/compile then failed with "Invalid apm.yml identity: 'name' must be a non-empty string" once #2155 tightened APMPackage.from_apm_yml's identity check. Reject empty/whitespace names in _validate_project_name, and add _resolve_bootstrap_project_name() so the install auto-bootstrap path falls back to "my-project" instead of writing an empty name. `apm init` had the same gap: when no project_name arg is given, the early _validate_project_name check (guarded by `if project_name and ...`) is skipped entirely, so with --yes the cwd-derived name flowed straight into _get_default_config unvalidated -- silently writing 'name: ""' at a filesystem/drive root. Route it through _resolve_bootstrap_project_name too. Co-Authored-By: Claude Sonnet 5 --- src/apm_cli/commands/_helpers.py | 19 +++++++- src/apm_cli/commands/init.py | 16 +++++-- src/apm_cli/commands/install.py | 2 + tests/unit/test_init_command.py | 80 ++++++++++++++++++++++++++++++++ 4 files changed, 112 insertions(+), 5 deletions(-) diff --git a/src/apm_cli/commands/_helpers.py b/src/apm_cli/commands/_helpers.py index 748c2e4bd..8589b1d0b 100644 --- a/src/apm_cli/commands/_helpers.py +++ b/src/apm_cli/commands/_helpers.py @@ -622,10 +622,15 @@ def _validate_project_name(name): Project names are used directly as directory names and must not contain '/' or '\' so the name is not interpreted as a filesystem path, - and must not be '..' to prevent directory traversal. + and must not be '..' to prevent directory traversal. Must also be + non-empty: an empty/whitespace name writes an apm.yml with 'name: ""', + which APMPackage.from_apm_yml's identity check rejects on every later + install/lock/compile run (#2155). Returns True if valid, False otherwise. """ + if not name or not name.strip(): + return False if "/" in name or "\\" in name: return False if name == "..": # noqa: SIM103 @@ -633,6 +638,18 @@ def _validate_project_name(name): return True +def _resolve_bootstrap_project_name(candidate: str) -> str: + """Return a valid apm.yml project name for the install auto-bootstrap path. + + ``Path.cwd().name`` (or ``Path.home().name``) is '' at a filesystem/drive + root -- e.g. a container with ``WORKDIR /``. Writing that candidate + straight into apm.yml would produce ``name: ''``, which + ``APMPackage.from_apm_yml``'s identity check rejects on every later + install/lock/compile run (#2155). Falls back to a generic default instead. + """ + return candidate if _validate_project_name(candidate) else "my-project" + + def _create_plugin_json(config): """Create plugin.json file with package metadata. diff --git a/src/apm_cli/commands/init.py b/src/apm_cli/commands/init.py index 4dea5a219..22d1ae478 100644 --- a/src/apm_cli/commands/init.py +++ b/src/apm_cli/commands/init.py @@ -27,6 +27,7 @@ _create_plugin_json, _get_console, _get_default_config, + _resolve_bootstrap_project_name, _rich_blank_line, _validate_plugin_name, _validate_project_name, @@ -149,7 +150,8 @@ def _perform_init( if project_name and not _validate_project_name(project_name): logger.error( f"Invalid project name '{project_name}': " - "project names must not contain path separators ('/' or '\\\\') or be '..'." + "project names must be non-empty and must not contain " + "path separators ('/' or '\\\\') or be '..'." ) sys.exit(1) @@ -162,7 +164,11 @@ def _perform_init( final_project_name = project_name else: project_dir = Path.cwd() - final_project_name = project_dir.name + # project_dir.name is '' at a filesystem/drive root (e.g. cwd == "/", + # or a container WORKDIR of "/"). With --yes this flows straight to + # _get_default_config with no further validation, silently writing + # apm.yml with 'name: ""'. + final_project_name = _resolve_bootstrap_project_name(project_dir.name) project_root = Path.cwd() # Validate plugin name early @@ -395,7 +401,8 @@ def _interactive_project_setup(default_name, logger): break console.print( f"[error]Invalid project name '{name}': " - "project names must not contain path separators ('/' or '\\\\') or be '..'.[/error]" + "project names must be non-empty and must not contain " + "path separators ('/' or '\\\\') or be '..'.[/error]" ) version = Prompt.ask("Version", default="1.0.0").strip() @@ -412,7 +419,8 @@ def _interactive_project_setup(default_name, logger): break click.echo( f"{ERROR}Invalid project name '{name}': " - f"project names must not contain path separators ('/' or '\\\\') or be '..'.{RESET}" + f"project names must be non-empty and must not contain " + f"path separators ('/' or '\\\\') or be '..'.{RESET}" ) version = click.prompt("Version", default="1.0.0").strip() diff --git a/src/apm_cli/commands/install.py b/src/apm_cli/commands/install.py index 68147fdfe..8ed3a1619 100644 --- a/src/apm_cli/commands/install.py +++ b/src/apm_cli/commands/install.py @@ -121,6 +121,7 @@ from ._helpers import ( _create_minimal_apm_yml, _get_default_config, + _resolve_bootstrap_project_name, ) # CRITICAL: Shadow Python builtins that share names with Click commands @@ -1429,6 +1430,7 @@ def install( # noqa: PLR0913 if not apm_yml_exists and packages: project_name = Path.cwd().name if scope is InstallScope.PROJECT else Path.home().name + project_name = _resolve_bootstrap_project_name(project_name) config = _get_default_config(project_name) if manifest_targets := manifest_targets_from_target_option(target): config["targets"] = manifest_targets diff --git a/tests/unit/test_init_command.py b/tests/unit/test_init_command.py index 7993b463b..54fcc0ec7 100644 --- a/tests/unit/test_init_command.py +++ b/tests/unit/test_init_command.py @@ -453,6 +453,39 @@ def test_dotdot_in_slash_path_caught_by_slash_check(self): assert _validate_project_name("a/../b") is False # slash catches it + def test_invalid_empty_or_whitespace(self): + """Empty/whitespace names must be rejected (#2155 regression). + + An empty ``name`` passes through undetected into apm.yml, then + every later `from_apm_yml` call (install/lock/compile) fails with + "Invalid apm.yml identity: 'name' must be a non-empty string". + """ + from apm_cli.commands._helpers import _validate_project_name + + assert _validate_project_name("") is False + assert _validate_project_name(" ") is False + assert _validate_project_name("\t\n") is False + + +class TestResolveBootstrapProjectName: + """Unit tests for _resolve_bootstrap_project_name (install auto-bootstrap).""" + + def test_passes_through_valid_name(self): + from apm_cli.commands._helpers import _resolve_bootstrap_project_name + + assert _resolve_bootstrap_project_name("my-project") == "my-project" + + def test_falls_back_when_empty(self): + """Path.cwd().name is '' at a filesystem/drive root (#2155 regression).""" + from apm_cli.commands._helpers import _resolve_bootstrap_project_name + + assert _resolve_bootstrap_project_name("") == "my-project" + + def test_falls_back_when_whitespace(self): + from apm_cli.commands._helpers import _resolve_bootstrap_project_name + + assert _resolve_bootstrap_project_name(" ") == "my-project" + class TestInitProjectNameValidation: """Integration tests: apm init rejects project names with path separators or '..'.""" @@ -493,6 +526,32 @@ def test_init_accepts_plain_name(self): assert result.exit_code == 0 assert (Path(tmp_dir) / "my-project" / "apm.yml").exists() + def test_init_no_arg_yes_validates_cwd_derived_name(self): + """apm init --yes (no project_name) must run the cwd-derived name + through _resolve_bootstrap_project_name, not _get_default_config + directly. + + Regression test for the PR #2200 review finding: when no + project_name arg is given, the early `_validate_project_name` + check (guarded by `if project_name and ...`) is skipped entirely, + so with --yes the cwd-derived name flowed straight into + _get_default_config unvalidated. At a filesystem/drive root + (Path.cwd().name == '') this silently wrote apm.yml with + 'name: ""'. + """ + with self.runner.isolated_filesystem(): + with patch( + "apm_cli.commands.init._resolve_bootstrap_project_name", + return_value="resolved-name", + ) as mock_resolve: + result = self.runner.invoke(cli, ["init", "--yes"], catch_exceptions=False) + + assert result.exit_code == 0 + mock_resolve.assert_called_once() + with open("apm.yml", encoding="utf-8") as f: + data = yaml.safe_load(f) + assert data["name"] == "resolved-name" + def test_init_interactive_reprompts_on_invalid_name_click(self): """In interactive mode, an invalid name triggers a re-prompt.""" with self.runner.isolated_filesystem() as tmp_dir: @@ -520,6 +579,27 @@ def test_init_interactive_reprompts_on_dotdot_click(self): assert "Invalid project name" in result.output assert (Path(tmp_dir) / "apm.yml").exists() + def test_init_interactive_reprompts_on_blank_name_click(self): + """A blank/whitespace name must re-prompt instead of writing name: ''. + + Regression test for #2155: an empty name silently accepted here used + to write an apm.yml that every later `apm install`/`lock`/`compile` + rejected with "Invalid apm.yml identity: 'name' must be a non-empty + string", since APMPackage.from_apm_yml enforces a non-empty name. + """ + with self.runner.isolated_filesystem() as tmp_dir: + result = self.runner.invoke( + cli, + ["init"], + input=" \nmy-project\n1.0.0\n\n\ny\ndone\ny\n", + catch_exceptions=False, + ) + assert "Invalid project name" in result.output + apm_yml_path = Path(tmp_dir) / "apm.yml" + assert apm_yml_path.exists() + data = yaml.safe_load(apm_yml_path.read_text(encoding="utf-8")) + assert data["name"] == "my-project" + class TestInitTargetPrompt: """Test cases for the target selection prompt in apm init (S1-S7)."""