From 64ec6f371aa3ff1fd5977699f6eb4dd886c45d55 Mon Sep 17 00:00:00 2001 From: "Aaron K. Clark" Date: Wed, 20 May 2026 16:46:23 -0500 Subject: [PATCH] validation: reject unsafe slugs in `socrates init ` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `Path(base) / args.project` had three foot-guns when args.project was untrusted: 1. Absolute slug: Path("base") / "/etc/passwd" returns "/etc/passwd" in Python pathlib. `socrates init /etc/passwd --base ~/foo` would try to scaffold under /etc/passwd, not under ~/foo. Even though the user owns the CLI invocation, this violates the mental model ("init creates a project named UNDER --base"). 2. Path-separator slug: `socrates init a/b` quietly creates nested structure; `socrates init ../tmp/bad` escapes --base. 3. Empty slug: `socrates init ""` resolves to the base dir itself, risking damage to existing siblings. Added a `_validate_slug()` helper that rejects: - empty / whitespace-only - containing '/' or '\\' (single path component only) - absolute paths - '..' or any '..' segment - '.' or '..' literal - NUL bytes (defensive) Accepts alphanumerics, dash, underscore, dot — covers normal slugs like `quarterly-rebates`, `v0.8.0`, `.hidden`. Tests (new tests/test_init_slug_safety.py, 22 tests): - 11 parametrized rejection cases - 7 parametrized acceptance cases (normal slugs unchanged) - 4 end-to-end CLI integration tests proving: - absolute slug exits 2, /etc/passwd/docs/ never created - traversal slug exits 2, sibling dir never created - nested slug exits 2 with helpful message - empty slug exits 2 with helpful message 169/169 tests pass; ruff + mypy clean. --- src/socrates120x/cli.py | 46 ++++++++++++++ tests/test_init_slug_safety.py | 110 +++++++++++++++++++++++++++++++++ 2 files changed, 156 insertions(+) create mode 100644 tests/test_init_slug_safety.py diff --git a/src/socrates120x/cli.py b/src/socrates120x/cli.py index dadf51d..02ed716 100644 --- a/src/socrates120x/cli.py +++ b/src/socrates120x/cli.py @@ -362,7 +362,53 @@ def main(argv: list[str] | None = None) -> int: # --------------------------------------------------------------------------- +def _validate_slug(slug: str, *, kind: str = "project") -> str | None: + """Return an error message if *slug* would escape its parent directory or + is otherwise unsafe to use as a single path component; ``None`` if OK. + + Reject: + - empty / whitespace-only slugs (would resolve to the base dir) + - slugs containing a path separator (``/`` or ``\\``) — would nest or + traverse outside the intended parent + - slugs equal to ``.`` or ``..`` or that contain a ``..`` segment + - absolute paths (Path("/x") / "/etc" returns "/etc" — the slug wins) + - slugs containing NUL bytes (defensive against API misuse) + + Allowed: alphanumeric, dash, underscore, dot (for slugs like ``v0.8.0`` + or ``.hidden`` if the operator really wants those). + """ + if not slug or not slug.strip(): + return f"{kind} slug cannot be empty." + if "\x00" in slug: + return f"{kind} slug cannot contain NUL bytes." + if "/" in slug or "\\" in slug: + return ( + f"{kind} slug must be a single path component " + f"(no '/' or '\\\\'); got {slug!r}." + ) + # PurePath of an absolute slug yields an absolute path, which would + # discard the base when joined with /. Reject explicitly. + if Path(slug).is_absolute(): + return ( + f"{kind} slug must be relative, not absolute; got {slug!r}. " + f"Use --base to choose a different parent directory." + ) + parts = Path(slug).parts + if parts and (parts[0] == ".." or any(p == ".." for p in parts)): + return ( + f"{kind} slug cannot contain '..' segments; got {slug!r}. " + f"Use --base to choose a different parent directory." + ) + if slug in {".", ".."}: + return f"{kind} slug cannot be {slug!r}." + return None + + def _cmd_init(args: argparse.Namespace) -> int: + slug_err = _validate_slug(args.project, kind="project") + if slug_err: + print(f"error: {slug_err}", file=sys.stderr) + return 2 target: Path = args.base.expanduser().resolve() / args.project if not args.no_scaffold: diff --git a/tests/test_init_slug_safety.py b/tests/test_init_slug_safety.py new file mode 100644 index 0000000..e60fc32 --- /dev/null +++ b/tests/test_init_slug_safety.py @@ -0,0 +1,110 @@ +"""Slug-safety tests for `socrates init `. + +The slug is appended to --base to form the target directory. Without +validation: +- absolute slugs (e.g. /etc/passwd) replace the base entirely (Python + pathlib: Path("base") / "/etc/passwd" -> "/etc/passwd"). +- slugs containing / or \\ nest unexpectedly or escape via .. +- empty slug resolves to the base dir itself, risking damage to siblings. + +These tests pin the validation behavior added to _validate_slug. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from socrates120x.cli import _validate_slug, main + + +@pytest.mark.parametrize( + "slug", + [ + "", + " ", + "/etc/passwd", + "/tmp/abs", + "foo/bar", + "foo\\bar", + "..", + ".", + "../../tmp/bad", + "valid/with/sep", + "with\x00null", + ], +) +def test_validate_slug_rejects_unsafe(slug: str) -> None: + assert _validate_slug(slug) is not None, f"slug {slug!r} should be rejected" + + +@pytest.mark.parametrize( + "slug", + [ + "quarterly-rebates", + "my_project", + "PROJECT123", + "v0.8.0", + ".hidden", + "a-b-c-d", + "x", + ], +) +def test_validate_slug_accepts_safe(slug: str) -> None: + assert _validate_slug(slug) is None, f"slug {slug!r} should be accepted" + + +def test_init_rejects_absolute_slug_before_scaffold( + tmp_path: Path, capsys: pytest.CaptureFixture[str], monkeypatch: pytest.MonkeyPatch +) -> None: + """End-to-end: `socrates init /etc/passwd --base ` must error out + BEFORE touching the filesystem at /etc/passwd.""" + base = tmp_path / "base" + base.mkdir() + argv = ["init", "/etc/passwd", "--base", str(base)] + rc = main(argv) + assert rc == 2 + err = capsys.readouterr().err + # Any rejection message is fine — the load-bearing assertion is that the + # filesystem stayed clean. Validator order may catch this as either + # "absolute" or "contains /"; either is acceptable. + assert "error:" in err + # Most importantly: /etc/passwd/docs etc. must not have been created. + assert not (Path("/etc/passwd/docs")).exists() + + +def test_init_rejects_traversal_slug( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + base = tmp_path / "base" + base.mkdir() + sibling = tmp_path / "escape-target" + rc = main(["init", "../escape-target", "--base", str(base)]) + assert rc == 2 + err = capsys.readouterr().err + assert ".." in err + # The escape target must not have been created either. + assert not sibling.exists() + + +def test_init_rejects_nested_slug( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + base = tmp_path / "base" + base.mkdir() + rc = main(["init", "a/b", "--base", str(base)]) + assert rc == 2 + err = capsys.readouterr().err + assert "single path component" in err + + +def test_init_rejects_empty_slug( + tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + base = tmp_path / "base" + base.mkdir() + rc = main(["init", "", "--base", str(base)]) + assert rc == 2 + err = capsys.readouterr().err + assert "empty" in err