[brettheap]

Lane: openreposhape-2 (openRepoShape-2)
Claim: #866 (comment)

Base

Not stacked. The branch starts at origin/main 5cbec222, which holds #841 (ca47b71d). Two commits: d183e769, the change, and a2af74a0, fix round 1 on rv866's review.

Lands beside #867 (#858), lane 2's, in the same file: whichever lands second merges the other. At a2af74a0, git merge-tree --write-tree is clean with main d6475f96, with #867's head 8bb85cbf, and with the two together. At d183e769 and #867's earlier head 2d1ff8a1, the predicted tree 0d66e9e8 gave 645 passed in test_openreposhape_command.py and test_install_sh.py, #867's cases and these together. Nothing #867 adds is touched here: not locale_reading, not the bin-dir check, not misdrawn_characters, and the new tests are one block after the CLONE_URLS test, away from #867's hunks.

Fix round 1, on rv866's review (READY, 2 SHOULDs, 2 NITs)

The review: #872 (comment). One commit, a2af74a0.

Finding	What changed
SHOULD-1: a URL, an scp address or a path in $OPENREPOSHAPE_REPO ending in a newline passed, was printed by --version and sent by a scaffold run	checked_repo looked at url="$(shape_clone_url)", and $(…) drops trailing newlines. It now looks for the misdrawn characters in $REPO as typed, and its case reads "$url$REPO": a control character in either is refused, and -* still reads the URL's first character, so -acme/r (cloned as https://github.com/-acme/r.git) passes as before. rv866's probed fix, two lines; the words are unchanged. New cases: the path, URL and scp forms with a trailing newline through --version, the scaffold run and --install (18), and -acme/r printed by --version (2). The one-line revert (F01) fails the 18; rv866's R07 (url="$REPO") fails the 2
SHOULD-2: under C, every run but --help now refuses a URL or path holding a character past ASCII	No code change: rv866 rules it out of scope. Stated under "Not changed", with the four runs measured again here
NIT-1: the order refuse_bad_values states is not pinned	test_bad_values_are_refused_in_the_order_the_check_states: a broken record and a bad repository give the record's refusal (kills rv866's R03); a bad repository and a bad ref give the repository's (kills R04). One row cannot kill both: the record is read only where $OPENREPOSHAPE_REF is unset, so a broken record and a bad ref never both apply
NIT-2: a bare run with a bad value exits 2 with the refusal, not 1 with the usage	No change; stated under "Not changed"

SHOULD-1, measured (LC_ALL=C.UTF-8, the same stubs; <LF> is a line feed):

d183e769:
$ OPENREPOSHAPE_REPO=/srv/r<LF> openRepoShape --version
  exit 0, stdout: openRepoShape (/srv/r<LF> @ main)
$ OPENREPOSHAPE_REPO=https://example.invalid/r<LF><LF> openRepoShape Atlas --org acme
  exit 2, 2 requests, each carrying both line feeds; the refusal prints them three times
$ OPENREPOSHAPE_REPO=git@example.invalid:r<LF> openRepoShape --version
  exit 0, stdout: openRepoShape (git@example.invalid:r<LF> @ main)
$ OPENREPOSHAPE_REPO=acme/r<LF> openRepoShape --version
  exit 2 (refused already: the owner/name keeps the line feed inside its URL)

a2af74a0: each of the four exits 2 with 0 requests:
  REFUSED: OPENREPOSHAPE_REPO is not a value this tool will put on a `git clone` command line: it starts with '-' or carries a control character. Set it to a URL or a path.
$ OPENREPOSHAPE_REPO=-acme/r openRepoShape --version
  exit 0, stdout: openRepoShape (-acme/r @ main)     (as at d183e769)

Decisions

1. Where the one check runs. --version is an argument case (openRepoShape:1003 at 5cbec222). It answers inside the argument loop with its own refuse_bad_record, before the top-level call (:1112), so the check has to be in both places. One function, refuse_bad_values, now does three things: it refuses a broken record as before, checks the repository, then checks the ref. It replaces refuse_bad_record at both call sites, so the two cannot drift apart.
   • --help (:1002) still answers first. It names the variables and their defaults, never their values, so it answers past a bad value as it answers past a broken record.
   • A bare run now refuses a bad value with exit 2, as it already refuses a broken record. Before, it printed the usage and exited 1.
2. The rules and the words.
   • $OPENREPOSHAPE_REPO: shape_checkout's guard, moved whole into checked_repo. install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR, the repository or a ref holding a bidi control, a character of no width, a soft hyphen or U+2028 is refused by its code point before any request and never echoed, so no line shows the value as another #841's misdrawn characters come first, then a leading - or [![:print:]]. The leading - is looked for in the URL that shape_clone_url makes; since fix round 1 the characters are looked for in $REPO as typed as well, because $(shape_clone_url) drops a trailing newline. The words are unchanged.
   • The forms $REPO takes are an owner/name (an API path for the fetch, and https://github.com/<o>/<n>.git for a clone), a URL or scp address, and a local path (/…, .…, C:/…, C:\…). None of them is refused unless it holds a control character or a misdrawn one. A leading - survives into the clone URL only when it is typed on a URL or an scp address, and setup.sh's clone and --doctor's clone refuse that already.
   • $OPENREPOSHAPE_REF: the character checks of ref_fault, not its length or shape check. Those two checks (the misdrawn characters, then [![:print:]]) move whole into ref_character_fault, which ref_fault calls in the same place, so ref_fault's order and words are unchanged.
   • Why not all of checked_ref: test_a_ref_set_without_the_opt_in_is_never_recorded pins that --install with OPENREPOSHAPE_REF=bad..ref and no $OPENREPOSHAPE_RECORD_REF exits 0. record_ref's comment gives the reason: workBenches sets a value "that names no ref at all, to make any fetch fail loudly". A ref of any shape or length prints as it reads, and --version "prints the ref in force" (docs/cli.md:168). The length and the shape are still checked wherever a ref is used: by checked_ref when --install records, by shape_checkout before git checkout, and by setup.sh's --shape-ref checks.
3. --version refuses with exit 2. It does not respell the value.
   • A broken record already makes --version refuse with exit 2. This is the same rule, applied to the value from the environment.
   • Every run that would use the value now refuses, so a respelled answer would describe a run that cannot happen.
   • install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR, the repository or a ref holding a bidi control, a character of no width, a soft hyphen or U+2028 is refused by its code point before any request and never echoed, so no line shows the value as another #841 weighed LC_ALL=C printf '%q' and rejected it.
   • install.sh's parse (install.sh:571, case "$shown" in *"@ $version)") asks the installed command with $OPENREPOSHAPE_REF unset, so the ref it sees is the record, which holds $version. That tag has passed install.sh's checked_version, which is stricter than this check. The repository has passed install.sh's checked_repo: an owner/name of [A-Za-z0-9._/-] starting with a letter or digit, which shape_clone_url turns into https://github.com/….git. This check passes it, and --version prints it byte for byte as before. A new test case holds this with a fork that uses every character install.sh allows.
4. shape_checkout's own check is gone, moved into checked_repo. shape_checkout runs only from run_doctor, which runs only after the up-front check, and $REPO does not change in between. git clone --quiet -- "$url" keeps its --. install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR, the repository or a ref holding a bidi control, a character of no width, a soft hyphen or U+2028 is refused by its code point before any request and never echoed, so no line shows the value as another #841's a_repository_to_clone case still pins the words; it meets them up front now.
5. Every line that prints or sends $REPO or $REF runs after the check. Those lines are in fetch_from_repo, fetch_or_die, collect_commands, record_ref, install_commands, shape_checkout, run_doctor and ref_fault. ref_fault's shape refusal comes only after its character checks pass. The others are the --version case, --shape-ref "$REF" and the OPENREPOSHAPE_REPO export. At a2af74a0, the top-level callers are run_doctor (:1186), install_commands (:1195), fetch_or_die (:1264) and the ARGS and export lines (:1252, :1271), all after refuse_bad_values (:1172). --version (:1062) calls it first. read_recorded_ref (:892) runs before the argument loop and prints nothing.

What changed

Before (5cbec222) and after, measured by a probe. The probe puts a gh stub and a curl stub first on PATH, each logging its call and failing; it sets GH_TOKEN=bogus and an empty GH_CONFIG_DIR. Bytes outside printable ASCII are shown as <U+XXXX>.

Before (5cbec222):

$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REPO=acme/r<U+202E>x openRepoShape --version
  exit 0, 0 requests
  stdout: openRepoShape (acme/r<U+202E>x @ main)
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=v1<U+001B>[31mX openRepoShape --version
  exit 0, 0 requests
  stdout: openRepoShape (opensoft/openRepoShape @ v1<U+001B>[31mX)
$ LC_ALL=C OPENREPOSHAPE_REPO=acme/r<U+009B>31mX openRepoShape Atlas --org acme
  exit 2, 2 requests
  stderr: REFUSED: could not fetch setup.sh from acme/r<U+009B>31mX at main. Both ways failed:
  stderr:     gh api repos/acme/r<U+009B>31mX/contents/setup.sh?ref=main
  stderr:     curl -fsSL https://raw.githubusercontent.com/acme/r<U+009B>31mX/main/setup.sh
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=v1<U+200B>x openRepoShape Atlas --org acme
  exit 2, 2 requests
  stderr: REFUSED: could not fetch setup.sh from opensoft/openRepoShape at v1<U+200B>x. Both ways failed:
  stderr:     gh api repos/opensoft/openRepoShape/contents/setup.sh?ref=v1<U+200B>x
  stderr:     curl -fsSL https://raw.githubusercontent.com/opensoft/openRepoShape/v1<U+200B>x/setup.sh
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REPO=acme/r<U+001B>[31mX openRepoShape --install (stdin)
  exit 2, 2 requests
  stderr: REFUSED: could not fetch openRepoShape from acme/r<U+001B>[31mX at main, so NOTHING was installed
  stderr:     and nothing already installed was replaced. `--install` places every one
  stderr:     of the 1 it collects or none of them: a new command beside a missing
  stderr:     one is a half-install that reads like a whole one. Both ways were tried:
  stderr:     gh api repos/acme/r<U+001B>[31mX/contents/<file>?ref=main
  stderr:     curl -fsSL https://raw.githubusercontent.com/acme/r<U+001B>[31mX/main/<file>
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=v1<U+202E>x openRepoShape --install (stdin)
  exit 2, 2 requests
  stderr: REFUSED: could not fetch openRepoShape from opensoft/openRepoShape at v1<U+202E>x, so NOTHING was installed
  stderr:     and nothing already installed was replaced. `--install` places every one
  stderr:     of the 1 it collects or none of them: a new command beside a missing
  stderr:     one is a half-install that reads like a whole one. Both ways were tried:
  stderr:     gh api repos/opensoft/openRepoShape/contents/<file>?ref=v1<U+202E>x
  stderr:     curl -fsSL https://raw.githubusercontent.com/opensoft/openRepoShape/v1<U+202E>x/<file>
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REPO=-x://acme/r openRepoShape --version
  exit 0, 0 requests
  stdout: openRepoShape (-x://acme/r @ main)
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=bad..ref openRepoShape --version
  exit 0, 0 requests
  stdout: openRepoShape (opensoft/openRepoShape @ bad..ref)
$ LC_ALL=C.UTF-8  openRepoShape --version
  exit 0, 0 requests
  stdout: openRepoShape (opensoft/openRepoShape @ main)

After (d183e769):

$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REPO=acme/r<U+202E>x openRepoShape --version
  exit 2, 0 requests
  stderr: REFUSED: OPENREPOSHAPE_REPO holds a character a terminal draws as something other than itself (U+202E): one that is invisible, is drawn as another character, ends a line or reorders the text around it. This tool will not put it on a `git clone` command line. Set it to a URL or a path.
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=v1<U+001B>[31mX openRepoShape --version
  exit 2, 0 requests
  stderr: REFUSED: $OPENREPOSHAPE_REF carries a control character, which is not a branch, tag or commit this tool will pass to `git checkout`.
$ LC_ALL=C OPENREPOSHAPE_REPO=acme/r<U+009B>31mX openRepoShape Atlas --org acme
  exit 2, 0 requests
  stderr: REFUSED: OPENREPOSHAPE_REPO is not a value this tool will put on a `git clone` command line: it starts with '-' or carries a control character. Set it to a URL or a path.
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=v1<U+200B>x openRepoShape Atlas --org acme
  exit 2, 0 requests
  stderr: REFUSED: $OPENREPOSHAPE_REF holds a character a terminal draws as something other than itself (U+200B): one that is invisible, is drawn as another character, ends a line or reorders the text around it. No branch, tag or commit this tool will pass to `git checkout` holds one.
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REPO=acme/r<U+001B>[31mX openRepoShape --install (stdin)
  exit 2, 0 requests
  stderr: REFUSED: OPENREPOSHAPE_REPO is not a value this tool will put on a `git clone` command line: it starts with '-' or carries a control character. Set it to a URL or a path.
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=v1<U+202E>x openRepoShape --install (stdin)
  exit 2, 0 requests
  stderr: REFUSED: $OPENREPOSHAPE_REF holds a character a terminal draws as something other than itself (U+202E): one that is invisible, is drawn as another character, ends a line or reorders the text around it. No branch, tag or commit this tool will pass to `git checkout` holds one.
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REPO=-x://acme/r openRepoShape --version
  exit 2, 0 requests
  stderr: REFUSED: OPENREPOSHAPE_REPO is not a value this tool will put on a `git clone` command line: it starts with '-' or carries a control character. Set it to a URL or a path.
$ LC_ALL=C.UTF-8 OPENREPOSHAPE_REF=bad..ref openRepoShape --version
  exit 0, 0 requests
  stdout: openRepoShape (opensoft/openRepoShape @ bad..ref)
$ LC_ALL=C.UTF-8  openRepoShape --version
  exit 0, 0 requests
  stdout: openRepoShape (opensoft/openRepoShape @ main)

No refusal's words changed. refuse_bad_values prints the words shape_checkout and ref_fault already had.

--version costs about 6 ms more (8 ms before, 14 ms after, the mean of 20 runs): the two misdrawn_characters calls.

Tests

File	Test	What it pins
tests/test_openreposhape_command.py	test_a_repository_or_ref_no_line_can_print_is_refused_before_any_request (72)	--version, openRepoShape Atlas --org acme (no $OPENREPOSHAPE_SETUP_SH, so it fetches) and --install from stdin; each variable holding an escape (C0), U+009B (C1), U+202E or U+200B before a forged tail, the repository as a URL with a leading -, and (fix round 1) a path, a URL and an scp address ending in a newline; under C and C.UTF-8: exit 2, the whole stderr is that one refusal in its words, the fake gh and curl log no call, nothing placed, stdout empty, no part of the value (no forged, no ESC, DEL, U+009B or character of MISDRAWN) in either stream
tests/test_openreposhape_command.py	test_help_answers_past_a_repository_and_ref_no_line_can_print	--help with both values bad: exit 0, the usage, neither value
tests/test_openreposhape_command.py	test_bad_values_are_refused_in_the_order_the_check_states (2, fix round 1)	a broken record and a bad repository: the record's refusal; a bad repository and a bad ref: the repository's; neither value echoed
tests/test_openreposhape_command.py	test_version_prints_every_value_the_check_passes_as_typed (26)	neither set prints openRepoShape (opensoft/openRepoShape @ main); each of the 7 CLONE_URLS forms (owner/name, URL, scp, POSIX path, three drive spellings); -acme/r, an owner/name whose clone URL starts with https (fix round 1); refs bad..ref, x.lock, -x and 256 characters; under C and C.UTF-8: exit 0, stdout byte for byte
tests/test_openreposhape_command.py	test_version_prints_a_path_a_utf_8_locale_prints (7)	a path holding each of NOT_CONTROLS (an e acute, U+00A0, U+200C, U+200D, U+2026, U+FF21, a subdivision flag) passes under C.UTF-8
tests/test_openreposhape_command.py	test_a_ref_set_without_the_opt_in_is_never_recorded (docstring only)	still: --install with bad..ref and no opt-in exits 0; the docstring now says "no check of the shape"
tests/test_install_sh.py	test_openreposhape_repo_names_where_everything_comes_from[a-fork, every-character]	install.sh's shown= parse still reads @ v9.9.9) from the real launcher's --version for an owner/name holding every character install.sh's checked_repo allows (ac.me-1_x/.open-Repo_Shape.2): exit 0, pinned, recorded, no "does not stay on" line

Runs (Linux, bash 5.2.21, Python 3.12.3; env -u GH_TOKEN -u GITHUB_TOKEN GH_CONFIG_DIR=<empty> /usr/bin/python3 -m pytest -q -p no:cacheprovider, one basetemp; the host's load average was about 68, hence the times):

Set	5cbec222	d183e769	a2af74a0 (fix round 1)
test_openreposhape_command.py, test_install_sh.py, test_openreposhape_package.py, test_install_sh_filter_repo.py, test_repo_hygiene.py, test_cli_reference.py, test_agents_md_codes.py	697 passed, 152.64s	784 passed, 509.81s (87 new)	806 passed, 255.96s (22 more)
test_setup_project_py.py, test_setup_sh.py, test_shape_doctor.py (the rest of what git grep -l -E 'OPENREPOSHAPE_(REPO|REF)|openRepoShape --version' -- tests names)	759 passed, 3 skipped, 1101.41s	759 passed, 3 skipped, 1185.61s	759 passed, 3 skipped, 1033.89s
the two launcher files in #867's predicted merge (0d66e9e8, with 2d1ff8a1)		645 passed, 121.55s

render-cli-reference.py --check: ok (no --help text changed).

Mutants, at a2af74a0. Each is one edit to the launcher in a copy of the head (a git archive, made a scratch repository so the doctor's clone has one), run against the selected cases (-k over both files, 130 cases) after an unmutated control (130 passed). The launcher at 5cbec222 in the same copy fails 73: the 72 refusal cases and the order row that needs a refusal at all.

Mutant	Failed	Killed by
M01 refuse_bad_values without checked_repo	51	the 48 repository cases; #841's a_repository_to_clone (2); the order row with a bad ref
M02 refuse_bad_values without the ref refusal	24	the 24 ref cases
M03 --version calls refuse_bad_record, as before	25	the 24 --version cases; the order row with a bad ref
M04 the top-level call is refuse_bad_record, as before	50	the 48 scaffold and install cases; a_repository_to_clone (2)
M05 checked_repo without the misdrawn check	15	the repository U+202E and U+200B cases (12); a_repository_to_clone (2); the order row with a bad ref
M06 checked_repo without *[![:print:]]*	30	the repository escape, U+009B and trailing-newline cases
M07 checked_repo without -*	6	the leading-dash cases
M08 ref_character_fault without the misdrawn check	16	the ref U+202E and U+200B cases (12); #841's a_ref_to_record, a_recorded_ref (4)
M09 ref_character_fault without the control check	13	the ref escape and U+009B cases (12); a_record_that_is_not_a_ref_is_refused_by_name[two-lines]
M10 ref_character_fault without its return 0 (the control words follow the misdrawn ones)	6	the ref U+202E and U+200B cases under C (the whole-stderr assertion)
M11 ref_fault without its return 0 after a character fault (the shape refusal follows)	3	a_recorded_ref (2), two-lines
M12 ref_fault without its character checks	3	a_recorded_ref (2), two-lines
M13 refuse_bad_values before the argument loop (--help no longer first)	6	test_help_answers_past_…; a_record_that_is_not_a_ref_is_refused_by_name (5, its --help assertion)
F01 fix round 1's case "$url$REPO" reverted to case "$url"	18	the 18 trailing-newline cases
F02 fix round 1's misdrawn_characters "$REPO" reverted to "$url"	0	survives, equivalent: the URL is $REPO or $REPO between ASCII, and $(…) drops only newlines, none of which is in MISDRAWN, so the two name the same characters
R03 (rv866) the repository checked before the record	1	the order row with a broken record
R04 (rv866) the ref checked before the repository	1	the order row with a bad ref
R07 (rv866) checked_repo's url="$REPO"	2	-acme/r printed by --version, both locales

M11 and M12 are killed only through the record: an $OPENREPOSHAPE_REF now meets the up-front check before checked_ref, so the record is the one way into ref_fault's character branch. rv866's R06 (the check reads ${OPENREPOSHAPE_REF:-main} rather than $REF) is equivalent and was not run again: a ref from the record has passed all of ref_fault, and a broken record leaves $REF at main. The runner, its logs and both kill tables (d183e769's and a2af74a0's) are in this PR's attachments, under mutants/; the probes under probes/.

Not changed

• install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR the bytes refuse is read again in the locale's own charset, so a GB18030, GBK or Shift_JIS name that holds no control character and nothing drawn as another is placed, and where nothing can read it the bytes decide as before #867 (install.sh and openRepoShape: the control-character check matches bytes, so a GBK or GB18030 name holding 0xC2 and a byte from 0x80 to 0x9F is refused as a control, and a lone 0x9B, which an 8-bit terminal acts on, is placed #858)'s ground. install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR, the repository or a ref holding a bidi control, a character of no width, a soft hyphen or U+2028 is refused by its code point before any request and never echoed, so no line shows the value as another #841's checks on the repository and the ref match bytes alone (misdrawn_characters under LC_ALL=C), so in a multibyte locale that is not UTF-8 (GB18030, GBK, Shift_JIS) they can refuse a value the locale reads as text. locale_reading is not used here.

• rv866's SHOULD-2, ruled out of scope for this PR: under C, a URL or path holding a character past ASCII is refused by every run but --help. [![:print:]] under C matches every byte past ASCII, so OPENREPOSHAPE_REPO=/srv/café/openRepoShape.git is refused, in the words "carries a control character". That is shape_checkout's and setup.sh's rule (setup.sh:207), which already refused it wherever the value is cloned; it now holds for every run. Measured here (each launcher run from inside its own checkout, the stubs first on PATH):

  Run, LC_ALL=C	5cbec222	a2af74a0
  --version	exit 0, openRepoShape (/srv/café/openRepoShape.git @ main)	exit 2, refused
  --install from a file	exit 0, installed	exit 2, refused, nothing placed
  --doctor <dir> beside a checkout	the doctor ran: NOT A SHAPE ROOT (exit 2)	exit 2, refused before the doctor ran
  --preflight with $OPENREPOSHAPE_SETUP_SH in a checkout	setup-project.py's preflight ran (exit 1 on this machine)	exit 2, refused

  Under C.UTF-8 all four behave at a2af74a0 as at 5cbec222, and a test holds the --version case for each of NOT_CONTROLS. rv866's probe of install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR holding : or a newline is refused by install.sh only after its fetch, and one holding a carriage return or an escape is placed and echoed as typed, so both refuse every control character before they fetch or print anything #760's locale-free rule (*[[:cntrl:]]* or the C1 bytes, under LC_ALL=C) passes café under C and still refuses every test value, at the cost of passing a lone raw 0x9B byte. The place for it is a follow-up issue covering checked_repo, ref_character_fault and setup.sh's two guards (setup.sh:207, :232), so the four move together. Lane 2 files nothing without Brett Heap's word.

• Runs that never read the value refuse it too: --doctor beside a checkout, --install from a file, a scaffold run with $OPENREPOSHAPE_SETUP_SH in a checkout, and a bare run, as each already refuses a broken record.

• rv866's NIT-2, a bare run's exit code. With a bad value, a bare run now exits 2 with the refusal, where it printed the usage and exited 1 (measured: 5cbec222 exit 1, 69 lines of usage; a2af74a0 exit 2, the repository's refusal, nothing on stdout). --help, docs/cli.md:132, README.md:276, docs/handbook.html:858 and AGENTS.md:67 still say a bare run exits 1. A broken record (One-line install: curl -fsSL …/releases/latest/download/install.sh | bash installs the newest release pinned to it, with git-filter-repo, and checks the machine (v1.0.1) #564) is already an exception none of them names, and the next real run would refuse anyway, so no doc changes here.

• The words: the repository's refusal says OPENREPOSHAPE_REPO with no $, the ref's $OPENREPOSHAPE_REF (shape_checkout's and ref_fault's words, setup.sh's and the package's too). Kept as they are.

• The package's openRepoShape --version (src/openreposhape/cli.py:385) prints U+202E in $OPENREPOSHAPE_REPO as typed, exit 0 (measured from a copy outside any checkout at 5cbec222: openRepoShape 0.0.0 (acme/r E2 80 AE x @ main)). Its clone_url and ref_fault (src/openreposhape/standard.py:72, :178, :198) check CONTROL_RE (C0, DEL, C1) and not install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR, the repository or a ref holding a bidi control, a character of no width, a soft hyphen or U+2028 is refused by its code point before any request and never echoed, so no line shows the value as another #841's characters, so an escape is refused there. A candidate issue, not filed.

• install.sh: unchanged; its checked_repo and checked_version already run before it uses either value.

• --help, docs/cli.md, README, the handbook, AGENTS.md: unchanged. "--version prints the ref in force" stays true for every value the check passes.

• Not run: bash 3.2 (macOS), Git Bash on Windows (the new refusal and UTF-8 cases skip there as install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR, the repository or a ref holding a bidi control, a character of no width, a soft hyphen or U+2028 is refused by its code point before any request and never echoed, so no line shows the value as another #841's do), any real terminal emulator. CI is Linux-only since ci: run the test suite on Linux only #597.

Closes #866

🤖 Generated with Claude Code

Summary by Sourcery

Validate repository and ref environment values before any output or operation, refusing terminal-unsafe and clone-invalid input without altering accepted values.

New Features:

• Reject unsafe repository and ref values before they are printed, fetched, cloned, or otherwise used.

Bug Fixes:

• Prevent control characters, terminal-misdrawing characters, leading-dash clone URLs, and trailing-newline repository values from bypassing validation or reaching requests and output.
• Ensure validation occurs consistently for --version and normal command execution while preserving accepted values byte-for-byte.

Enhancements:

• Consolidate record, repository, and ref validation into a single ordered preflight check while keeping --help available for invalid values.

Tests:

• Add comprehensive command tests covering unsafe values, validation order, no-request guarantees, accepted repository and ref forms, locale behavior, and unchanged --version output.
• Extend installer coverage to repository names containing all characters accepted by installer validation.

[sourcery-ai]

Reviewer's Guide

The PR centralizes early safety validation for OPENREPOSHAPE_REPO and OPENREPOSHAPE_REF, rejecting terminal-control or confusable characters before output or network activity while preserving --help, accepted value forms, existing refusal wording, and installer compatibility.

File-Level Changes

Change	Details	Files
Adds a single pre-execution validation path for repository and ref environment values, while preserving help behavior and allowing values that pass the existing character rules to be displayed byte-for-byte.	Replaces both broken-record-only checks with shared repository/ref validation, including the --version argument path and the top-level execution path. Moves repository clone-safety checks into checked_repo and ref character checks into ref_character_fault, preserving existing refusal messages and validation ordering. Ensures invalid values fail with exit 2 before any version output, network request, clone, install, export, or other value-dependent operation; --help remains exempt. Keeps shape/length validation scoped to places that use or record refs, so values such as bad..ref retain existing behavior when not opted into recording. Retains -- on git clone and existing accepted repository forms, including URLs, scp addresses, local paths, and install.sh-compatible owner/name values.	openRepoShape
Expands command coverage to verify early rejection, output safety, accepted-value compatibility, locale behavior, and preservation of existing ref-recording semantics.	Adds parameterized tests for control characters, C1 controls, terminal-confusable characters, and leading-dash repositories across --version, scaffold, and stdin install flows. Asserts invalid values produce only the expected refusal, make no fake GitHub requests, create no install artifacts, and leak no value bytes. Verifies --help bypasses value checks and accepted repository/ref forms remain printed exactly as supplied, including non-ASCII paths under C.UTF-8. Documents that non-opt-in refs remain unvalidated for shape while still receiving character-safety checks.	tests/test_openreposhape_command.py
Verifies installer compatibility for repository names containing every character allowed by install.sh's repository validation.	Parameterizes the installer repository test with a normal fork and an owner/name exercising all allowed characters. Confirms the installed command's version parsing still succeeds and does not report a ref/version mismatch.	tests/test_install_sh.py

Assessment against linked issues

Issue	Objective	Addressed	Explanation
#866	Check both repository and ref values for control characters and terminal-misdrawing characters before they can be printed or used in a fetch.	✅
#866	Apply the checks before --version answers and before any fetch, while allowing --help to answer without exposing the values.	✅
#866	Refuse unsafe values with exit 2, while preserving byte-for-byte --version output for values that pass the checks.	✅

Possibly linked issues

• openRepoShape: --version and two fetch refusals print $OPENREPOSHAPE_REPO and $OPENREPOSHAPE_REF with no check, so an escape or a bidi override in either reaches the terminal as typed, and a scaffold run sends its fetch with them #866: The PR implements the issue’s upfront checks, refuses unsafe values before output or requests, and adds regression tests.

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[brettheap]

(lane openRepoShape-2, 2026-10-18T03:52:54Z) Independent review of #872 (#866) at d183e769, for lane openRepoShape-2: READY

Nothing blocks the landing. Of the two SHOULDs, the first is a two-line fix that a probe has tested. The second is a ruling that a follow-up issue can carry.

SHOULD-1. A trailing newline in $OPENREPOSHAPE_REPO, given as a URL, an scp address or a path, passes the check and is then printed and sent.

• Where: openRepoShape:720-725 (checked_repo).
  • url="$(shape_clone_url)": the $(…) drops trailing newlines.
  • So the misdrawn check (:721) and the -* | *[![:print:]]* case (:724) look at a value that no longer has them.
  • The lines that print and send $REPO still have them.
• Not affected:
  • an owner/name, because .git follows the newline in the URL;
  • the ref, because it is checked as an argument.
• Repro (env -i, LC_ALL=C.UTF-8, a stub gh and curl that log and fail):
  • OPENREPOSHAPE_REPO=$'/srv/r\n' openRepoShape --version exits 0. Stdout is openRepoShape (/srv/r, then a line feed, then @ main).
  • OPENREPOSHAPE_REPO=$'https://example.com/r\n\n' openRepoShape Atlas --org acme sends 2 requests, each carrying both newlines. Its refusal prints them three times.
  • git@example.com:r followed by a newline behaves the same way.
  • The base 5cbec222 is the same: shape_checkout had this blind spot too.
• Why it matters: the title and the squash message say a control character in either value is "refused, never printed or sent". This input is a C0 control that is printed and sent.
• Why it is not a MUST: nothing the person typed can follow the newline, so it cannot forge text. It only breaks a line, in a run that fails anyway.
• Fix (probed): use misdrawn_characters "$REPO" at :721 and case "$url$REPO" in at :724.
  • The -* pattern still reads the start of the URL, so -acme/r passes as it does today.
  • $REPO brings the newline into the check.
• What the probe showed with the fix:
  • it refuses all four trailing-newline values;
  • it still refuses an escape, U+202E and -x://acme/r;
  • it still prints the 7 CLONE_URLS forms and -acme/r;
  • both launcher test files pass with it: 560 passed.
• Test to add: pytest.param("OPENREPOSHAPE_REPO", "/srv/forged\n", REPO_NOT_FOR_A_CLONE, id="repo-a-trailing-newline") in UNPRINTABLE_VALUES. Mutant R07 (url="$REPO") survives the current tests for this reason.

SHOULD-2. Under C or POSIX, every run except --help now refuses a URL or path in $OPENREPOSHAPE_REPO that holds a character past ASCII, and says it "carries a control character".

• Where: openRepoShape:724-725, the locale's [![:print:]] check, which refuse_bad_values now runs (:902).

• Probes, 5cbec222 against d183e769, with LC_ALL=C and OPENREPOSHAPE_REPO=/srv/café/openRepoShape.git:

  Run	At 5cbec222	At d183e769
  --version	exit 0	exit 2
  --install from a file	installed	refused
  --doctor <dir> beside a checkout	the doctor ran and printed its report (exit 2, not a shape root)	refused before the doctor ran
  --preflight with $OPENREPOSHAPE_SETUP_SH in a checkout	setup-project.py's preflight ran	refused

  Under C.UTF-8, nothing changes for any of them.

• Who can meet it: a real person can, but few will.

  • The launcher documents an owner/name ("owner/name to fetch from"), and an owner/name on github.com is ASCII.
  • The URL and path forms are setup.sh's clone, the package's (cli.py:80: "owner/name, URL or path to clone") and the tests'.
  • Under C, the installed --doctor's clone and setup.sh:207 already refused this value.
  • The package's openRepoShape --version prints the same path under C with exit 0, so the two --version commands now disagree.

• A fix is small, but it costs something.

  • checked_repo could take install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR holding : or a newline is refused by install.sh only after its fetch, and one holding a carriage return or an escape is placed and echoed as typed, so both refuse every control character before they fetch or print anything #760's locale-free rule, the one install_commands uses: under LC_ALL=C, *[[:cntrl:]]* or *$'\302'[$'\200'-$'\237']*. That is 3 lines.
  • Probe of it: café passes under C. All 9 test values, DEL, TAB and U+0085 are still refused in both locales.
  • The cost: a lone raw 0x9B byte (not UTF-8) then passes in both locales, where the head refuses it. install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR the bytes refuse is read again in the locale's own charset, so a GB18030, GBK or Shift_JIS name that holds no control character and nothing drawn as another is placed, and where nothing can read it the bytes decide as before #867's comment states the same trade for the bin dir.
  • setup.sh's guard would also stay stricter than the launcher's.

• Ruling: out of scope for openRepoShape: $OPENREPOSHAPE_REPO and $OPENREPOSHAPE_REF are checked once, before --version answers and before any fetch, so a control character or a character a terminal draws as another in either is refused, never printed or sent #872.

  • The issue named shape_checkout's rule.
  • The fix moves the clone guard, and setup.sh:207 and :232 should move with it.
  • Resolve it with a follow-up issue covering checked_repo, ref_character_fault and setup.sh's two guards, not only with the "Not changed" line.

NIT-1. The order refuse_bad_values states is not pinned.

• Mutants R03 (the repository checked before the record) and R04 (the ref checked before the repository) survive.
• In practice the order makes no difference:
  • each refusal is right on its own;
  • a broken record and $OPENREPOSHAPE_REF cannot both apply, because the record is read only when the variable is unset.
• Optional: add one case with a broken record and a bad repository, to pin "the record first", as the comment at :894 says.

NIT-2. A bare run with a bad value now exits 2 with the refusal, not 1 with the usage.

• These still say a bare run "exits 1": --help, docs/cli.md:132, README.md:276, docs/handbook.html:858 and AGENTS.md:67.
• A broken record (One-line install: curl -fsSL …/releases/latest/download/install.sh | bash installs the newest release pinned to it, with git-filter-repo, and checks the machine (v1.0.1) #564) is already an exception they do not mention.
• Accepted: the next real run would refuse anyway. No doc change is needed unless lane 2 wants the qualifier.

Checked and true

• The issue, line by line.
  • --version, a scaffold run and --install from stdin each refuse with exit 2 and make no request. That holds for both variables, under C and C.UTF-8, with ESC, U+009B, U+202E and U+200B.
  • The 54 new refusal cases fail with the 5cbec222 launcher and pass at the head.
  • The 33 control cases pass on both.
  • A clean pair still prints openRepoShape (opensoft/openRepoShape @ main).
• Every line that prints or sends $REPO or $REF runs after the check. The lines are :217, :224, :235-237, :301-306, :398-402, :748, :758-759, :933, :1054, :1244 and :1263.
  • Each is either in --version after refuse_bad_values, or reached only from :1178, :1187 or :1256, all of which come after :1164.
  • record_ref prints $REF only after checked_ref (:633).
  • shape_checkout runs checked_ref before git checkout.
  • ref_fault echoes a ref only after its character checks.
• The departure is sound: the up-front check runs only the character checks, not the length and shape checks.
  • test_a_ref_set_without_the_opt_in_is_never_recorded needs --install with bad..ref and no opt-in to exit 0.
  • Mutant R05 (the whole ref_fault up front) breaks that test and 8 --version cases.
  • A ref of any length or shape prints as it reads, and the length and shape are still checked wherever a ref is used.
• --version refusing with exit 2 breaks no caller.
  • install.sh's shown= parse (install.sh:571) holds: an owner/name that passed install.sh's checked_repo, and a tag that passed checked_version, both pass this check. The every-character case passes.
  • The --version steps in release.yml and tests.yml run the package's command, not this launcher. test_release_workflow.py passes.
  • workBenches' sentinel pinned-by-workBenches-no-fetch and its 40-hex ref pass. Its --version checks unset both variables.
• The words are unchanged.
  • The refusal strings are the same set at base and head.
  • checked_repo's words are shape_checkout's and setup.sh:208's.
  • ref_fault's order is still: empty, length, misdrawn, control, shape. W10 and W11 are killed.
• --help still answers first with both values bad: exit 0, 69 lines.
• render-cli-reference.py --check: ok.
• Sonar: no composite assert, no new function near cognitive complexity 15, and no repeated 4-line blocks.
• Tests:
  • 840 passed in the eight launcher, install, package, release and hygiene files;
  • 759 passed (3 skipped) in test_setup_project_py.py, test_setup_sh.py and test_shape_doctor.py;
  • 621 passed in the two launcher files on the merge with install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR the bytes refuse is read again in the locale's own charset, so a GB18030, GBK or Shift_JIS name that holds no control character and nothing drawn as another is placed, and where nothing can read it the bytes decide as before #867 at 18ae33b1.
• Merges:
  • with main d7befb91: clean;
  • with install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR the bytes refuse is read again in the locale's own charset, so a GB18030, GBK or Shift_JIS name that holds no control character and nothing drawn as another is placed, and where nothing can read it the bytes decide as before #867 at 18ae33b1: clean;
  • all three together: clean.
• Mutants: the writer's seven, re-run, fail as the writer reported. Of eight of my own, five are killed. R03 and R04 survive (NIT-1). R06 survives as an equivalent, and R07 survives as an equivalent apart from SHOULD-1's case.
• The writer's candidate is confirmed. The package's openRepoShape --version (src/openreposhape/cli.py:385) prints U+202E and U+200B in $OPENREPOSHAPE_REPO as typed, with exit 0, under C.UTF-8 and C. It refuses an escape with exit 2. Not filed.

Could not verify

• A GBK or GB18030 locale. None is installed, and the locale sources to build one are absent.
  • Under such a locale, [![:print:]] may read U+009B's bytes as a letter, so --version would print them. install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR holding : or a newline is refused by install.sh only after its fetch, and one holding a carriage return or an escape is placed and echoed as typed, so both refuse every control character before they fetch or print anything #760's byte check would refuse them.
  • That is install.sh and openRepoShape: the control-character check matches bytes, so a GBK or GB18030 name holding 0xC2 and a byte from 0x80 to 0x9F is refused as a control, and a lone 0x9B, which an 8-bit terminal acts on, is placed #858's ground, and shape_checkout behaved the same before this PR.
• bash 3.2, Git Bash and a real terminal emulator.

Lane: openreposhape-2 (openRepoShape-2)

[brettheap]

(lane openRepoShape-2, 2026-10-18T04:44:45Z) Re-verification of #872 (#866)'s fix round 1 at a2af74a0: READY

Fix round 1 closes SHOULD-1 and NIT-1 with code and tests. SHOULD-2 and NIT-2 are now stated in the PR body, and those statements are true. No MUST and no SHOULD remain. One NIT, about the squash message's wording, is optional.

NIT-1. Two phrases in the squash message are slightly narrower than the code. File: lane 2's impl866-launcher-repo-ref/squash-message.md.

• The third paragraph says a value ending in a newline "was sent by a scaffold run".
  • At d183e769, --install from stdin sent it too: 2 requests, each carrying the newline.
  • Suggested wording: "was sent by a scaffold run and by --install from stdin".
• It also says the characters are looked for "in $REPO as typed as well as in the clone URL".
  • The misdrawn check now reads $REPO only (openRepoShape:729). Only the case at :732 reads both.
  • The result is the same, as F02 below shows. Optional rewording: "the misdrawn characters in $REPO as typed, and the control characters in it and in the clone URL".
• Applied: lane 2 took both wordings into the squash message.

Checked and true

• The delta d183e769..a2af74a0 is the two-line fix that rv866 probed, plus a comment. It changes openRepoShape:729 (misdrawn_characters "$REPO") and :732 (case "$url$REPO" in).
  • With comments stripped, it is byte for byte the reviewer's fix2 probe copy.
  • The refusal's words are unchanged.
• Every trailing-newline form is now refused, with no request. Probe: env -i, a stub gh and curl, under both C and C.UTF-8.
  • These are refused at a2af74a0 but passed at d183e769: a path, a URL ending in two newlines, an scp address, ./rel/r, C:/src/r and C:\src\r, each with a newline at the end.
  • A scaffold run and --install from stdin with any of them now make 0 requests. At d183e769 they made 2 requests.
  • These are still refused: an owner/name with a trailing newline, a trailing CR, a newline before more text, ESC, U+009B, U+202E, U+200B, -x://acme/r and -x@h:r.
• The forms that passed before still pass, and print the same bytes.
  • The 7 CLONE_URLS forms, -acme/r and /srv/café/x.git (under C.UTF-8) all exit 0.
  • Their stdout is byte for byte what d183e769 printed.
  • url can never be empty, so -* always tests the URL's first character.
• F02 is equivalent. F02 is the misdrawn check reading the URL instead of $REPO.
  • The URL is either $REPO, or $REPO between ASCII bytes (https://github.com/ and .git). No ASCII byte can start or continue a UTF-8 sequence.
  • $(…) drops only newlines, and no newline is in MISDRAWN_CHARACTERS.
  • So both readings find the same characters, in the same order. The mutant also survives all 130 selected cases.
• The new tests fail where they should.
  • The 18 trailing-newline cases fail with the d183e769 launcher and pass at a2af74a0.
  • The two order rows and -acme/r (2 cases) pass on both. They pin behaviour that already held: the order of the checks, and that -acme/r passes.
• Mutants:
  • R03 is now killed by the row "a broken record and a bad repository".
  • R04 is killed by the row "a bad repository and a bad ref".
  • R07 is killed by -acme/r, in both locales.
  • F01 (reverting to case "$url") is killed by the 18 trailing-newline cases.
  • F03 (case "$REPO$url") is killed by -acme/r.
• The PR body is accurate.
  • The "Not changed" table of the four runs under C matches the first review's probes.
  • The NIT-2 entry is accurate: five docs say "exits 1", and a broken record was already an exception none of them names.
  • The other "Not changed" lines still hold.
  • Decision 5's new line numbers are right: :1062, :1172, :1186, :1195, :1252, :1264, :1271 and :892.
  • The test counts are right: 72, 26 and 2.
• The squash message's new paragraph is true, apart from NIT-1's two phrases. The title's claim, that a control character in either value is never printed or sent, now holds for a trailing newline.
• Merges, over main d6071c7d and install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR the bytes refuse is read again in the locale's own charset, so a GB18030, GBK or Shift_JIS name that holds no control character and nothing drawn as another is placed, and where nothing can read it the bytes decide as before #867 at 8bb85cbf:
  • with main: clean (tree 4251b492);
  • with install.sh and openRepoShape: a $OPENREPOSHAPE_BIN_DIR the bytes refuse is read again in the locale's own charset, so a GB18030, GBK or Shift_JIS name that holds no control character and nothing drawn as another is placed, and where nothing can read it the bytes decide as before #867: clean (tree 84206475);
  • all three: clean (tree 79e80dcc);
  • the two launcher test files pass on the three-way merge: 645 passed.
  • Main has not touched openRepoShape, tests/test_openreposhape_command.py or tests/test_install_sh.py since 5cbec222.
• Test runs at a2af74a0:
  • the 8-file verify set: 862 passed, 22 more than at d183e769;
  • test_setup_project_py.py, test_setup_sh.py and test_shape_doctor.py: 759 passed, 3 skipped.
• Scope note: setup.sh, install.sh and the openRepoShape command: each keeps its own copy of the old ref rule (*..* | *.lock), so a ref git refuses part by part (a.lock/b, a/.b, a//b) clones the standard before it fails #874 (filed by lane 5, now claimed by lane 2) will later change ref_fault's shape case. That case is now the only one left in ref_fault's else branch, next to the ref_character_fault call. It is out of this PR's scope and merges as a later change.

Could not verify

• A GBK or GB18030 locale. None is installed here, as in the first review.
• bash 3.2, Git Bash, and a real terminal emulator.

Lane: openreposhape-2 (openRepoShape-2)

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[sourcery-ai]

Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days.

You can request another review in 2 days and 8 hours by commenting @sourcery-ai review. Upgrade to get a review now.