lane_setup refuses a dash-prefixed --remote before any git argv, and the docstring beside it stops saying default_branch is the only one

[fdaviddpt]

Closes #381.

What was wrong

scripts/lane_setup.py's resolve_base() handed the --remote value straight into git fetch --quiet <remote> <branch>, where argv position 6 is read as an option if it starts with a dash. Two positions over, default_branch was already refused before any argv was built (#368) -- so a guard and a bypass sat in one command line.

The load-bearing half was the sentence beside it. The docstring claimed default_branch was the one value here that reaches git's argv unprefixed. That was false when it was written, and it is the kind of sentence that stops the next guard sweep from looking.

Severity is worse than the issue body says, and it was measured

The issue describes the argv position. Against git 2.46.2 on darwin, the injected command is executed:

$ git -C dn fetch --quiet --upload-pack=/tmp/pwn.sh master
PWNED: /tmp/pwn.sh ran with args: master
fatal: Could not read from remote repository.

So this is arbitrary command execution from that position, not a confusing flag. Reachability is unchanged and is not claimed to be worse: nothing in this repository passes --remote -- a grep over the whole tree finds the add_argument and nothing else -- so the value arrives only on the maintainer's own command line. The splices rank stands.

The judgment call, and why it went this way

The brief left open whether to guard at all or to correct the docstring and record a decision not to. Guarded, for three reasons: the execution measurement above; ten lines and one call site; and nobody types that is a fact about today, which is the category of claim this repository distrusts by construction.

git fetch --quiet -- <remote> <branch> was measured as an alternative and does work -- git refuses the dash-prefixed repository itself with fatal: strange pathname ... blocked while a well-formed remote still fetches. Declined, and the reasons are in the docstring rather than only here: it makes git the thing that reports the refusal, in a sentence this script would then have to interpret to fill detail; and beside a value already refused above it could never fire, and an unfireable guard is the thing this repository keeps finding instead of a fix.

The fix

remote_problem() is consulted before any argv is built, beside oss_config.default_branch_problem(). The rule is a leading dash and nothing else -- the harm is the option position, and a value whose legitimate forms include a bare name, an ssh URL and a filesystem path has no shape this file has authority to invent.

It lives in lane_setup.py rather than oss_config.py because the value does. remote is --remote argv only and is never config-sourced, so a verdict in the config validator would be a rule for a key no config carries and doctor would have no occasion to print it. #345's one-value-one-rule constraint points the other way for default_branch, which is config-sourced. remote_problem's docstring names the migration if that ever changes.

The docstring now names all four values, not two

Re-derived rather than inherited from the audit. Two values are guarded by a rule (remote, default_branch); two are safe by the shape of their argv and are now named as such: branch_occupancy prefixes refs/heads/ and refs/remotes/, and --repo is -C's argument, which git consumes literally (git --no-pager -C -x log answers fatal: cannot change to '-x'). No third unguarded argv site exists -- that was checked rather than assumed, and read_board interpolates nothing.

Tests

tests/test_lane_setup_381.py asserts on captured argv, never on the exit code: a version that ran the fetch and then failed would pass the weaker form. Every silence is paired with a positive control in the same fixture, including a well-formed non-default remote, so a guard that refused everything would not pass either.

The last test is a sweep rather than a case -- every argument the module hands to _git, for a hostile value at each of the four input sites, must not begin with a dash unless the module wrote it as a literal flag. #381 exists because the previous guard was written for the position somebody enumerated; a sweep cannot be wrong about a site nobody thought of.

Red before the fix: 7 failed, 1 passed -- the pass is the recorder control, which is what makes the seven silences mean anything. Green after: 2637 passed, 2 skipped, coverage 92.71%.

Self-review found a real one

Both review spawns independently flagged the same defect in the first draft of the sweep, and both were right: the repo site measured nothing, twice over. It built its hostile path as tmp_path / -repo, which is absolute and therefore does not start with a dash at all, and it asserted only over the recorded args while the recorder files the repository under a separate repo key. Two ways of measuring nothing in one three-line site, inside the very test whose subject is a guard written for the position somebody enumerated.

Fixed in the second commit: the value is now relative from a chdir so it genuinely is -repo; the site asserts the repository stays out of the arguments and reaches the repo key; and every site now asserts its own premise, that its value is dash-prefixed at all. A new test_the_repo_argument_is_always_preceded_by_dash_C measures the argv _git builds directly, and its failure mode was proven reachable with a scratch mutation of _git.

Platform

Observed on darwin / Python 3.13.15. Reasoned for the other twelve legs: the diff adds a string-prefix check and prose, with no path separator, suffix match, drive letter, platform-specific exception, spawn change or non-ASCII output anywhere in it -- both changed files were checked and are pure printable ASCII. The tests patch module-level names looked up at call time, not ones captured through an object attribute at import, so the injection takes on 3.9-3.12 -- reasoned, not run there.

Verified by the maintainer

The escalation is real, and I reproduced it a different way. The report shows the injected script
printing its own marker. Mine went through git's protocol parser instead, which is the harder evidence:

$ git -C dn fetch --quiet "--upload-pack=$PWD/marker.sh" main
fatal: protocol error: bad line length character: EXEC

EXEC is the first four bytes of the executed script's stdout. git ran it and then choked trying to
read that output as pack protocol — so execution is not inferred from a message the script chose, it
is visible in git's own parse failure. git 2.46.2. The control, a well-formed remote name, executes
nothing and fails at resolution.

That is arbitrary command execution from the position --remote occupied, and it is why guarding
beat documenting. The rank is unchanged and correct: nothing in this repository passes --remote,
so the value still arrives only on the maintainer's own command line, which is the splices row's
stated bound. v0.8.0 was not held for it and did not need to be.

Red re-run: this branch's test file on origin/main at d6f56c4, product code untouched —
6 failed, 3 passed. That is a different split from the report's 7 failed, 1 passed and not a
contradiction: the report's red was watched against this branch's own base before its second commit,
mine is the final file against a main that has since taken #368, #382 and #387. Recorded so the two
numbers are not read later as one of them being wrong.

The branch was not rebased and that was the right call. origin/main moved to d6f56c4 mid-lane;
the author merge-tested in a detached probe (2692 passed, probe removed) and left the rebase
decision to me rather than rewriting a branch I had briefed. The base-moved handling is #389.

The doubly-vacuous repo sweep site both review spawns caught — absolute path so never dash-prefixed,
and asserting over args while the recorder files it under repo — is the finding of this diff. It
was inside the test whose whole subject is that a guard written for the position somebody enumerated
misses the one they did not.