Skip to content

msb --clone: origin-URL carry has three gaps (multi-valued URL, credential file mode, silent failure) #467

Closed

Description

@wz-gsa

Follow-up to #453 / PR #455. All three are in _acq_msb_clone_copy_config
(acq.backends/msb.sh:2463-2471) or its caller.

B1. Multi-valued remote.origin.url picks the WRONG value

git config --get returns the LAST value; git fetch uses the FIRST.
Measured, git 2.50.1:

$ git config --get-all remote.origin.url
https://github.com/example/a.git      <- git fetch uses this
https://gitlab.com/example/a.git      <- config --get returns this (rc=0, no stderr)
$ git ls-remote origin
fatal: repository 'https://github.com/example/a.git/' not found

A push-to-two-forges setup is legal (git remote set-url --add origin ...).
The scratch's origin is silently pointed at a DIFFERENT forge than the host
checkout, with no warning. Same for remote.origin.pushurl.

Fix: use --get-all and write each with --add, or count values and warn/refuse
when >1. No test covers this today (the only set-url in 116-clone-option.bats:297
is --push).

B2. The scratch .git/config can hold a credential and gets no mode hardening

mkdir -p "$dir" (msb.sh:2420) runs under the ambient umask, no chmod. If the
user's origin URL embeds a token, PR #455 copies it into a SECOND file on host
disk. Reproduced with umask 022:

drwxr-xr-x  <state>/clones/sb/src/.git
-rw-r--r--  <state>/clones/sb/src/.git/config
url = https://user:[email protected]/example/a.git

The comment's "exactly as in a direct mount" equivalence does not hold: a direct
mount has ONE config with the checkout's own mode; this is a new copy at a path
the user does not know exists, persisting until acq rm.

The same file already hardens two sibling state dirs and says why:

  • msb.sh:2225 chmod 700 — "Private staging dir (0700) under the acq state tree
    — not a world-readable /tmp"
  • msb.sh:4606 chmod 0700 — ssh key dir
    There is no umask anywhere in msb.sh. Fix: ( umask 077; mkdir -p "$dir" ) +
    chmod 700. (ACQ_MSB_PORTS_DIR at :4668/:4781 is likewise unhardened.)

B3. Origin-copy failure is warn-only and restores the silent #453 state

The helper is best-effort/always-0 (msb.sh:2468-2469). For user.* that is fine —
a failed identity copy fails loudly at the first commit ("Author identity unknown").
For remote.origin.url a failed copy silently reproduces #453: git push origin
APPEARS to succeed (it pushes into the scratch) and the developer believes the work
reached the forge. One stderr line is not proportionate to silent data loss.

Fix: fail the create on an origin-copy failure, or emit a distinct loud warning
naming the consequence. Per the repo's degraded-path rule, "could not set it" must
not be indistinguishable from success. No test exercises this path.

Attribution

Found during an AI-assisted (OpenCode) adversarial post-merge review of PR #455.
Advisory — every item wants a human to confirm before it drives a change.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions