From 9f429f40b6910946ef88f6ae44268c816b17f076 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Fri, 16 Oct 2026 23:38:02 +0000 Subject: [PATCH 1/2] setup-project.py: a project name typed at the prompt is checked as `--project` is and an empty line says no name was typed, so `../x`, `a/b`, `..` and `--help` are refused by name, not by an `--id` hint, a help page or a probe write (#729) The name typed at the `project name` prompt was used as typed. The same value as `--project` or as the positional goes through `checked_value` at parse time and is refused there. Typed, it reached step (3), which builds the clone path from it and asks the disk about that path: `..` leaves `--into`, an absolute name replaces it, and #608's write probe is made in the nearest folder above it. Step (4) did not catch it either: the naming validator reads only the last part of what it is handed (`../x` passes as `x`), has no length bound and reads a leading `-` as its own option, so `--help` printed its help and exited 0. The scaffold's plan then refused `../x` as `scaffold-bad-id: --id '../x'`, an id nobody passed, and `-h` with a usage dump. Now `checked_value("project name", typed)` runs right after the `ask`, the check `take_positional` makes, so the refusal is the tool's own wording with `project name` as its subject, exit 2, before anything reads or writes a path. `a b` and `Bad_Name` still go to step (4): `NAME_RE` is permissive on purpose and the policy rules on those. Nothing in step (3) changes, and the validator's basename rule stays as it is. An empty typed line (and Ctrl-D) now says "no project name was typed; nothing was created.", exit 2, with no usage text. It said "no --project given, and no terminal to ask on." after the full usage text, which is the refusal for no terminal, and `can_ask()` still gives that one. tests/test_setup_project_typed_name.py types each name at a pseudo-terminal (POSIX only, as the other terminal tests are). Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 14 ++- tests/test_setup_project_typed_name.py | 136 +++++++++++++++++++++++++ 2 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 tests/test_setup_project_typed_name.py diff --git a/setup-project.py b/setup-project.py index 1b2cab7f..b774914b 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1477,7 +1477,19 @@ def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: say("") say("(3) project") if not opts.project and can_ask(): - opts.project = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + # AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL (#729): + # `can_ask()` above is the gate that refuses for want of one, so this + # refusal says what happened and does not print the usage text. + if not typed: + die("no project name was typed; nothing was created.") + # THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` + # or as the positional (#729), before anything builds a path from it: + # `..` leaves `--into`, an absolute name replaces it, and step (4)'s + # validator reads only the last part of what it is handed. `a b` and + # `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) + # is where the policy rules on those. + opts.project = checked_value("project name", typed) if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py new file mode 100644 index 00000000..f1a3c26c --- /dev/null +++ b/tests/test_setup_project_typed_name.py @@ -0,0 +1,136 @@ +# SPDX-License-Identifier: Apache-2.0 +"""The project name typed at `setup-project.py`'s prompt (#729). + +A name that arrives as `--project` or as the positional is checked at parse +time, before step (1) prints, by `checked_value`. A name typed at the +`project name` prompt reached step (3) unchecked: `..` left `--into`, an +absolute name replaced it, `--help` printed the naming validator's help at +step (4) and exited 0, and the refusals that did come named an `--id` nobody +passed or dumped the usage text. These tests type each of those at a real +terminal and hold the run to the `--project` refusal, worded for what the +person did: the value is a `project name`. + +POSIX only, as every test that gives a run a terminal is (`PTY_SKIP`). +""" + +from __future__ import annotations + +import sys + +import pytest + +from conftest import PTY_SKIP, REPO, run_in_a_terminal +from test_setup_project_py import (SETUP_PROJECT, SETUP_SH, piped, run_entry, + terminal_env) + +PROMPT = "project name (ONE CamelCase token, e.g. Atlas): " + +#: What a terminal in canonical mode takes as "the next character is text": +#: ^V, which is how a control character is typed into a line. +LNEXT = chr(0x16) +BELL = chr(7) + +#: Names the check refuses, by case. `absolute` is a path under `tmp_path`, +#: so it is built by `typed_value`. +REFUSED = { + "up-and-out": "../x", + "slash": "a/b", + "dot-dot": "..", + "dot": ".", + "dash": "-x", + "help": "--help", + "control": "a" + BELL + "b", + "long": "x" * 256, + "blank": " ", + "absolute": None, +} + +#: Names step (4)'s policy rules on, as it did before: `NAME_RE` is +#: permissive on purpose. +LEFT_TO_STEP_FOUR = ("Bad_Name", "a b") + + +def typed_value(tmp_path, case): + value = REFUSED[case] + return str(tmp_path / "elsewhere" / "Name") if value is None else value + + +def at_the_prompt(tmp_path, typed, entry="py"): + """Setup, with no project, asked at a terminal and answered `typed` + (`"EOF"` is Ctrl-D). `entry` is the file run directly, that file with + its stdin a pipe (the terminal is then the controlling one), or + `setup.sh`. Returns the exit status and the screen.""" + flags = ["--org", "demoorg", "--local-remote-dir", + str(tmp_path / "remotes"), "--into", + str(tmp_path / "parent" / "into")] + argv = {"py": [sys.executable, str(SETUP_PROJECT), *flags], + "sh": ["bash", str(SETUP_SH), *flags], + "piped": piped("", *flags)}[entry] + if typed != "EOF": + typed = typed.replace(BELL, LNEXT + BELL) + "\n" + status, screen, answered = run_in_a_terminal( + argv, cwd=REPO, env=terminal_env(), answers=[typed], prompt=PROMPT, + timeout=60) + assert answered == 1, f"it never asked for a project name:\n{screen}" + assert "(3) project" in screen, screen + return status, screen + + +def the_refusal(text): + return text[text.index("REFUSED: "):].strip() + + +def assert_refused_at_the_prompt(tmp_path, value, entry="py"): + status, screen = at_the_prompt(tmp_path, value, entry) + assert status == 2, f"status {status}:\n{screen}" + assert f"project name is {ascii(value)}, which is not a value" in screen + assert "(4) naming policy" not in screen, screen + assert "usage: setup-project.py" not in screen, screen + assert "--id" not in screen, screen + assert list(tmp_path.iterdir()) == [], "the refused name left a trace" + return screen + + +@PTY_SKIP +@pytest.mark.parametrize("case", REFUSED) +def test_a_typed_project_name_is_refused_as_the_flag_refuses_it(tmp_path, + case): + value = typed_value(tmp_path, case) + screen = assert_refused_at_the_prompt(tmp_path, value) + flagged = run_entry("--project", value, "--org", "demoorg", + "--local-remote-dir", str(tmp_path / "remotes"), + "--into", str(tmp_path / "parent" / "into")) + assert flagged.returncode == 2, flagged.stderr + assert the_refusal(screen) == the_refusal(flagged.stderr).replace( + "REFUSED: --project is", "REFUSED: project name is", 1) + + +@PTY_SKIP +@pytest.mark.parametrize("entry", ["piped", "sh"]) +def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): + """The same check on the one-liner's shape (stdin a pipe, the answer + read from the terminal) and through `setup.sh`, the shim.""" + assert_refused_at_the_prompt(tmp_path, "../x", entry) + + +@PTY_SKIP +@pytest.mark.parametrize("typed", ["", "EOF"]) +def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal(tmp_path, + typed): + status, screen = at_the_prompt(tmp_path, typed) + assert status == 2, f"status {status}:\n{screen}" + assert the_refusal(screen) == ( + "REFUSED: no project name was typed; nothing was created.") + assert "no terminal" not in screen, screen + assert "usage: setup-project.py" not in screen, screen + assert "(4) naming policy" not in screen, screen + assert list(tmp_path.iterdir()) == [] + + +@PTY_SKIP +@pytest.mark.parametrize("value", LEFT_TO_STEP_FOUR) +def test_a_name_the_policy_decides_is_left_to_step_four(tmp_path, value): + status, screen = at_the_prompt(tmp_path, value) + assert "(4) naming policy" in screen, screen + assert "project name is" not in screen, screen + assert status == 1, f"status {status}:\n{screen}" From b3c77cc2d22fea4d64fb7f4407a6fff6e74ee551 Mon Sep 17 00:00:00 2001 From: Brett Heap Date: Sat, 17 Oct 2026 00:05:46 +0000 Subject: [PATCH 2/2] setup-project.py: the typed project name's prompt, empty-line refusal and check are one helper, and the tests pin where the check sits, so a check made after step (3) has asked the disk fails them (#729) Review round on 9f429f40 (rv729: approve with nits, two SHOULDs). SHOULD-1. SonarCloud reported python:S3776 on resolve_project_and_elector (cognitive complexity 16 of 15). The prompt, the empty-line refusal and the `checked_value("project name", typed)` call move into `ask_for_the_project_name()`, with their comments; the call site is `opts.project = ask_for_the_project_name()`. No behaviour changes. The function is back to 14 by rv729's estimator, and the helper is 1. SHOULD-2. The tests did not pin where the check sits: a check moved to just before `check_names`, or to after `resolve_family`, passed all 16 of them. Now every refused name is typed with a file standing where the folder above `--into` goes, so a check made after step (3) asks the disk would end in "is a file" and not "project name is"; and `[ok] project` and `[ok] legs` must not print. Only the seeded file may be on the disk afterwards, unchanged. Both moved checks now fail 12 tests each. Nits. The empty line and Ctrl-D are also typed through the one-liner's shape (stdin a pipe) and through `setup.sh`, 6 tests in place of 2. `(4) naming policy` is one constant. The module docstring no longer says `--help` "exited 0" of the run: the naming validator exited 0 at step (4), and the run ended at step (5) with exit 2. Lane: openreposhape-5 (openRepoShape-5) Co-Authored-By: Claude Sonnet 5.5 --- setup-project.py | 34 +++++++++------ tests/test_setup_project_typed_name.py | 59 +++++++++++++++++++------- 2 files changed, 64 insertions(+), 29 deletions(-) diff --git a/setup-project.py b/setup-project.py index b774914b..3ec1a195 100755 --- a/setup-project.py +++ b/setup-project.py @@ -1472,24 +1472,32 @@ def run_at_the_terminal(argv) -> subprocess.CompletedProcess: os.close(descriptor) +def ask_for_the_project_name() -> str: + """The `project name` prompt, and the name typed at it, checked (#729). + + AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL: `can_ask()` + at the call is the gate that refuses for want of one, so this refusal says + what happened and does not print the usage text. + + THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` or + as the positional, before anything builds a path from it: `..` leaves + `--into`, an absolute name replaces it, and step (4)'s validator reads + only the last part of what it is handed. `a b` and `Bad_Name` pass here on + purpose (`NAME_RE` is permissive); step (4) is where the policy rules on + those. + """ + typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") + if not typed: + die("no project name was typed; nothing was created.") + return checked_value("project name", typed) + + def resolve_project_and_elector(opts: Options, shape_root: Path) -> None: """setup.sh:355-386, transcribed.""" say("") say("(3) project") if not opts.project and can_ask(): - typed = ask(" project name (ONE CamelCase token, e.g. Atlas): ") - # AN EMPTY LINE IS A NAME NOBODY TYPED, NOT A MISSING TERMINAL (#729): - # `can_ask()` above is the gate that refuses for want of one, so this - # refusal says what happened and does not print the usage text. - if not typed: - die("no project name was typed; nothing was created.") - # THE TYPED NAME IS HELD TO THE CHECK THE SAME NAME GETS AS `--project` - # or as the positional (#729), before anything builds a path from it: - # `..` leaves `--into`, an absolute name replaces it, and step (4)'s - # validator reads only the last part of what it is handed. `a b` and - # `Bad_Name` pass here on purpose (`NAME_RE` is permissive); step (4) - # is where the policy rules on those. - opts.project = checked_value("project name", typed) + opts.project = ask_for_the_project_name() if not opts.project: usage(sys.stderr) die("no --project given, and no terminal to ask on.") diff --git a/tests/test_setup_project_typed_name.py b/tests/test_setup_project_typed_name.py index f1a3c26c..d27eeb68 100644 --- a/tests/test_setup_project_typed_name.py +++ b/tests/test_setup_project_typed_name.py @@ -4,11 +4,17 @@ A name that arrives as `--project` or as the positional is checked at parse time, before step (1) prints, by `checked_value`. A name typed at the `project name` prompt reached step (3) unchecked: `..` left `--into`, an -absolute name replaced it, `--help` printed the naming validator's help at -step (4) and exited 0, and the refusals that did come named an `--id` nobody -passed or dumped the usage text. These tests type each of those at a real -terminal and hold the run to the `--project` refusal, worded for what the -person did: the value is a `project name`. +absolute name replaced it, `--help` made the naming validator print its help +and exit 0 at step (4) (the run itself ended at step (5), with the scaffold's +usage and exit 2), and the refusals that did come named an `--id` nobody passed +or dumped the usage text. These tests type each of those at a real terminal and +hold the run to the `--project` refusal, worded for what the person did: the +value is a `project name`. + +Two things pin WHERE the check sits, not only that it is made. Nothing of step +(3) may print (`[ok] project`, `[ok] legs`), and a FILE stands where the folder +above `--into` goes, so a check made after step (3) has asked the disk would +say "is a file" and not `project name is`. POSIX only, as every test that gives a run a terminal is (`PTY_SKIP`). """ @@ -25,6 +31,13 @@ PROMPT = "project name (ONE CamelCase token, e.g. Atlas): " +#: Step (4)'s heading, which no refused name may reach. +NAMING_POLICY = "(4) naming policy" + +#: A file where the folder above `--into` goes, so any step that asks the +#: disk about the clone path refuses with "is a file". +FILE_ABOVE_INTO = b"a file where the folder above --into goes\n" + #: What a terminal in canonical mode takes as "the next character is text": #: ^V, which is how a control character is typed into a line. LNEXT = chr(0x16) @@ -55,11 +68,14 @@ def typed_value(tmp_path, case): return str(tmp_path / "elsewhere" / "Name") if value is None else value -def at_the_prompt(tmp_path, typed, entry="py"): +def at_the_prompt(tmp_path, typed, entry="py", seeded=False): """Setup, with no project, asked at a terminal and answered `typed` (`"EOF"` is Ctrl-D). `entry` is the file run directly, that file with its stdin a pipe (the terminal is then the controlling one), or - `setup.sh`. Returns the exit status and the screen.""" + `setup.sh`. `seeded` puts `FILE_ABOVE_INTO` where a later step would + refuse it. Returns the exit status and the screen.""" + if seeded: + (tmp_path / "parent").write_bytes(FILE_ABOVE_INTO) flags = ["--org", "demoorg", "--local-remote-dir", str(tmp_path / "remotes"), "--into", str(tmp_path / "parent" / "into")] @@ -80,14 +96,23 @@ def the_refusal(text): return text[text.index("REFUSED: "):].strip() +def assert_nothing_was_made(tmp_path): + """Only the seeded file is on the disk, and it is as it was.""" + assert sorted(p.name for p in tmp_path.iterdir()) == ["parent"] + assert (tmp_path / "parent").read_bytes() == FILE_ABOVE_INTO + + def assert_refused_at_the_prompt(tmp_path, value, entry="py"): - status, screen = at_the_prompt(tmp_path, value, entry) + status, screen = at_the_prompt(tmp_path, value, entry, seeded=True) assert status == 2, f"status {status}:\n{screen}" assert f"project name is {ascii(value)}, which is not a value" in screen - assert "(4) naming policy" not in screen, screen + assert "is a file" not in screen, screen + assert "[ok] project" not in screen, screen + assert "[ok] legs" not in screen, screen + assert NAMING_POLICY not in screen, screen assert "usage: setup-project.py" not in screen, screen assert "--id" not in screen, screen - assert list(tmp_path.iterdir()) == [], "the refused name left a trace" + assert_nothing_was_made(tmp_path) return screen @@ -114,23 +139,25 @@ def test_a_typed_project_name_is_refused_on_every_way_in(tmp_path, entry): @PTY_SKIP +@pytest.mark.parametrize("entry", ["py", "piped", "sh"]) @pytest.mark.parametrize("typed", ["", "EOF"]) -def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal(tmp_path, - typed): - status, screen = at_the_prompt(tmp_path, typed) +def test_an_empty_answer_is_refused_as_no_name_not_as_no_terminal( + tmp_path, typed, entry): + status, screen = at_the_prompt(tmp_path, typed, entry, seeded=True) assert status == 2, f"status {status}:\n{screen}" assert the_refusal(screen) == ( "REFUSED: no project name was typed; nothing was created.") + assert "is a file" not in screen, screen assert "no terminal" not in screen, screen assert "usage: setup-project.py" not in screen, screen - assert "(4) naming policy" not in screen, screen - assert list(tmp_path.iterdir()) == [] + assert NAMING_POLICY not in screen, screen + assert_nothing_was_made(tmp_path) @PTY_SKIP @pytest.mark.parametrize("value", LEFT_TO_STEP_FOUR) def test_a_name_the_policy_decides_is_left_to_step_four(tmp_path, value): status, screen = at_the_prompt(tmp_path, value) - assert "(4) naming policy" in screen, screen + assert NAMING_POLICY in screen, screen assert "project name is" not in screen, screen assert status == 1, f"status {status}:\n{screen}"