fix: replace <PACK> placeholders, harden path traversal checks, strengthen empty-list tests (#52)

* Initial plan

* fix: replace <PACK> placeholders, add path traversal checks, and strengthen empty-list tests

Agent-Logs-Url: https://github.com/Project-Navi/navi-bootstrap/sessions/604f9871-000e-4f48-b728-1c2a85fe6c4c

Co-authored-by: Fieldnote-Echo <202828230+Fieldnote-Echo@users.noreply.github.com>

* style: ruff-format the new-cmd validation if-chain (one-liner -> multi-line)

The single-line if chain in new() (added in this PR) exceeds ruff-format's
preferred wrapping. Auto-applying `uv run ruff format` to clear the
`lint` (ruff format --check) and `quality-gate` failures on PR #52.

No semantic change. 381 tests still pass, mypy strict clean, both
ruff check and ruff format clean.

* fix: address Codex P1 + Copilot findings on PR #52

1. [Codex P1 + Copilot] pack-validator.md: pack-specific pytest step
   was outside the per-pack `for PACK ... done` loop, so $PACK held
   only the LAST iteration's value and earlier packs' tests were
   silently skipped. A regression in earlier-changed packs would slip
   through the validator. Moved the test inside the same loop body
   and renumbered the cross-cutting tests as step 3.

2. [Copilot] cli.py render_cmd error message: previous wording
   ('without dot or dot-dot components') didn't match the actual
   checks (`startswith('.')` rejects '.foo'; `'..' in name`
   substring rejects 'foo..bar'). Reworded to match the rules:
   'must not start with .', 'must not contain .. or path separators'.

Skipped from the same review batch:
  - Copilot's note that PR description claims apply/diff hardening:
    only render and new construct Path-from-name; apply/diff take
    --target as a Click Path. PR description should be updated but
    code is correct.

381 tests still pass, mypy strict clean, ruff + format clean.

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Fieldnote-Echo <202828230+Fieldnote-Echo@users.noreply.github.com>
Co-authored-by: Nelson Spence <nelson@projectnavi.ai>
Co-authored-by: Navi Bot <267427491+project-navi-bot@users.noreply.github.com>

Author: 3 people

.claude/agents/pack-validator.md

@@ -14,25 +14,27 @@ You are a pack-validation specialist for navi-bootstrap. Your job: confirm that
    git diff --name-only origin/main...HEAD | grep '^packs/' | cut -d'/' -f2 | sort -u
    ```
 
-2. For each changed pack, run the full validation chain:
+2. For each changed pack, run the full validation chain **including** its
+   pack-specific tests inside the same loop body — otherwise `pack_snake`
+   takes the value of the last iteration only and earlier packs' tests are
+   silently skipped:
 
    ```bash
-   uv run nboot validate --spec nboot-spec.json
-   scratch=$(mktemp -d -t nboot-scratch-XXXX)
-   uv run nboot new "$scratch"
-   uv run nboot apply --spec nboot-spec.json --pack <PACK> --target "$scratch"
-   uv run nboot diff  --spec nboot-spec.json --pack <PACK> --target "$scratch"
+   for PACK in $(git diff --name-only origin/main...HEAD | grep '^packs/' | cut -d'/' -f2 | sort -u); do
+     uv run nboot validate --spec nboot-spec.json
+     scratch=$(mktemp -d -t nboot-scratch-XXXX)
+     uv run nboot new "$scratch"
+     uv run nboot apply --spec nboot-spec.json --pack "$PACK" --target "$scratch"
+     uv run nboot diff  --spec nboot-spec.json --pack "$PACK" --target "$scratch"
+
+     # Pack-specific test for this iteration's pack
+     pack_snake=$(echo "$PACK" | tr '-' '_')
+     uv run pytest tests/test_${pack_snake}_pack.py -v || \
+       uv run pytest tests/ -k "$pack_snake" -v
+   done
    ```
 
-3. Run the pack-specific test:
-
-   ```bash
-   pack_snake=$(echo <PACK> | tr '-' '_')
-   uv run pytest tests/test_${pack_snake}_pack.py -v 2>/dev/null || \
-     uv run pytest tests/ -k "$pack_snake" -v
-   ```
-
-4. Run cross-cutting tests that commonly break on pack changes:
+3. Run cross-cutting tests that commonly break on pack changes:
 
    ```bash
    uv run pytest tests/test_engine.py tests/test_manifest.py tests/test_integration.py -v

src/navi_bootstrap/cli.py

@@ -119,12 +119,21 @@ def render_cmd(
 
     if out is None:
         name = spec_data["name"]
-        if not name or "/" in name or "\\" in name:
+        stripped_name = name.strip() if isinstance(name, str) else ""
+        if (
+            not stripped_name
+            or "/" in stripped_name
+            or "\\" in stripped_name
+            or ".." in stripped_name
+            or stripped_name.startswith(".")
+        ):
             raise click.ClickException(
                 f"Unsafe spec name {name!r} cannot be used as output directory. "
+                "Name must be a non-empty single path segment, must not start with "
+                "'.', and must not contain '..' or path separators. "
                 "Use --out to specify an explicit output path."
             )
-        output_dir = Path(name)
+        output_dir = Path(stripped_name)
     else:
         output_dir = out
 
@@ -577,12 +586,19 @@ def new(
 ) -> None:
     """Create a new Python project with operational infrastructure."""
     # Validate name before using as path
-    if not name or "/" in name or "\\" in name or name.startswith("."):
+    stripped_name = name.strip() if name else ""
+    if (
+        not stripped_name
+        or "/" in stripped_name
+        or "\\" in stripped_name
+        or ".." in stripped_name
+        or stripped_name.startswith(".")
+    ):
         raise click.ClickException(
             f"Unsafe project name {name!r}. "
-            "Names must not contain path separators or start with a dot."
+            "Names must not contain path separators, '..', or start with a dot."
         )
-    output_dir = Path(name)
+    output_dir = Path(stripped_name)
     if output_dir.exists():
         raise click.ClickException(
             f"Directory {name!r} already exists. nboot new is for greenfield projects only."

tests/test_hooks.py

@@ -29,9 +29,11 @@ def test_reports_failures_without_stopping(self, mock_run: MagicMock, tmp_path:
         assert not results[0].success
         assert results[1].success
 
-    def test_empty_hooks(self, tmp_path: Path) -> None:
+    @patch("navi_bootstrap.hooks.subprocess.run")
+    def test_empty_hooks(self, mock_run: MagicMock, tmp_path: Path) -> None:
         results = run_hooks([], tmp_path)
         assert results == []
+        mock_run.assert_not_called()
 
     @patch("navi_bootstrap.hooks.subprocess.run")
     def test_captures_output(self, mock_run: MagicMock, tmp_path: Path) -> None:

tests/test_init.py

@@ -455,6 +455,7 @@ def test_detects_async_test_functions(self, tmp_path: Path) -> None:
         test_file = tests / "test_example.py"
         test_file.write_text("def test_sync(): pass\nasync def test_async(): pass\n")
         result = detect_test_info(tmp_path)
+        assert result["test_framework"] == "pytest"
         assert result["test_count"] == 2
 
 

tests/test_validate.py

@@ -41,9 +41,11 @@ def test_warnings_accepted(self, mock_run: MagicMock, tmp_path: Path) -> None:
         assert len(results) == 1
         assert results[0].passed
 
-    def test_empty_validations(self, tmp_path: Path) -> None:
+    @patch("navi_bootstrap.validate.subprocess.run")
+    def test_empty_validations(self, mock_run: MagicMock, tmp_path: Path) -> None:
         results = run_validations([], tmp_path)
         assert results == []
+        mock_run.assert_not_called()
 
     @patch("navi_bootstrap.validate.subprocess.run")
     def test_skips_method_based_validations(self, mock_run: MagicMock, tmp_path: Path) -> None: