Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,57 @@ jobs:
working-directory: plugins/disk-hygiene/skills/clean/scripts
run: python -m unittest -v test_hygiene.GuardTests

# #2834: an MSYS path literal (`/d/...`) handed to a Windows-native consumer
# re-anchors to the CURRENT drive's root, so the consumer creates a phantom
# `<drive>:\<letter>\` tree and writes there. Nothing errors — the run just
# measures something other than what it claims. docs/conventions/
# windows-path-emit/ owns the rule; this lane holds its two artifacts to
# their contracts.
#
# What is REQUIRED here is the detectors' own unit contract — deterministic,
# fixture-scoped, and including the assertion that the drive-root scan is a
# reported no-op on a non-Windows host. Pointing the live scan at a runner's
# drive roots is deliberately NOT wired: it would put an unquantified
# false-positive tail on the required aggregate, which docs/adr/0003 rules
# out until a guard has measured precision. Self-test first, so a broken
# detector cannot mask a regression.
windows-path-emit-gate:
runs-on: ubuntu-24.04
timeout-minutes: 15
steps:
- name: Check out
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- name: Test the MSYS-to-Windows emit helper
run: bash scripts/emit-windows-path.test.sh
- name: Test the drive-root litter detector
run: bash scripts/check-drive-root-litter.test.sh
- name: Assert the drive-root scan is a no-op on a non-Windows host
run: scripts/check-drive-root-litter.sh

# The emit helper's whole reason to exist is its NT branch: cygpath's answer
# cannot be faked on Linux, so on a Linux runner those cases report NOT
# EXERCISED and the suite is green having proved nothing about them. That is
# exactly how an inert Windows-only surface shipped green in #2774. This job
# runs the same suites where the branch is real. Keep it SMALL: these two
# suites only, and never the live drive-root scan.
windows-path-emit-windows:
runs-on: windows-2025
timeout-minutes: 20
defaults:
run:
shell: bash
steps:
- name: Check out
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
persist-credentials: false
- name: Test the MSYS-to-Windows emit helper on Windows
run: bash scripts/emit-windows-path.test.sh
- name: Test the drive-root litter detector on Windows
run: bash scripts/check-drive-root-litter.test.sh

hook-utils-sync:
runs-on: ubuntu-24.04
timeout-minutes: 15
Expand Down Expand Up @@ -1267,6 +1318,8 @@ jobs:
- hook-utils-sync
- hook-utils-windows
- disk-hygiene-guard-windows
- windows-path-emit-gate
- windows-path-emit-windows
- parse-concern-value-sync
- resolve-convention-pattern-sync
- standards-contract-sync
Expand Down
1 change: 1 addition & 0 deletions docs/PLUGIN-PHILOSOPHY.md
Original file line number Diff line number Diff line change
Expand Up @@ -498,6 +498,7 @@ doc before a second plugin adopts it. Fleet audits check conformance per row.
| Detector findings (non-fanout producers reaching the apply relay) | [`docs/conventions/detector-findings/`](conventions/detector-findings/README.md) |
| Fresh-eyes declaration pattern contract | `skill-quality` plugin (`skills/check/reference/fresh-eyes-declarations.md`) |
| Upstream-drift verification stamps and recheck triggers | [`docs/conventions/upstream-drift/`](conventions/upstream-drift/README.md) |
| Windows path emission across the Git Bash → native boundary | [`docs/conventions/windows-path-emit/`](conventions/windows-path-emit/README.md) |

## Cross-platform contract

Expand Down
120 changes: 120 additions & 0 deletions docs/conventions/windows-path-emit/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
# Windows path emission — convert before a path crosses out of Git Bash

Owner doc for one rule that has already cost this repo real test validity: **a path that originates
in Git Bash and is handed to PowerShell, `cmd`, or a Windows-native interpreter must be converted to
Windows form first.** The [plugin philosophy](../../PLUGIN-PHILOSOPHY.md) owns the
[cross-platform contract](../../PLUGIN-PHILOSOPHY.md#cross-platform-contract) this rests on — "build
paths from documented anchors with platform path APIs"; this doc owns the *emit shape* that keeps it
true at the one boundary where the failure is silent, and names the helper and the detector that back
it.

Scope is every script this repo's authors write on Windows, tracked or not: plugin scripts, repo
tooling under `scripts/`, and — the case that motivated the doc — throwaway verification harnesses,
which are exactly where the trap gets rediscovered because nothing reviews them.

## The mechanism, and why it is silent

Git Bash spells `D:\dir` as `/d/dir`. A Windows-native consumer does not know that mapping: the
leading `/` anchors to the root of the **current drive**, so the literal resolves to
`<current-drive>:\d\dir`. Nothing errors, nothing warns. A native writer creates the phantom chain
and writes there — Python's `shutil.make_archive`, for one, `os.makedirs`-es the destination's parent
before writing rather than failing on it.

The residue at the drive root is the cheap symptom. The expensive one is that **the run measured
something other than what it claims**. In #2834 a harness deleted a real fixture, wrote its
replacement to an MSYS-form absolute path, and so ran two named test cases against a directory that
had no fixture in it at all — mechanically identical to a third case, with two green rows recorded
for scenarios never exercised. Nothing in the run's own output distinguished that from success. The
same mechanism was recorded once before as a machine-level rule in a sibling repo and still recurred
here, which is why it is a repo convention with a detector rather than a note.

## The rules

Four rules, in the order they should be reached for. The first is the one that would have prevented
the motivating incident (#2834) outright; conversion is what to do when it does not apply.

1. **Prefer a path the native side computes itself.** When the consumer is already `cd`-ed into the
directory it should write to — or can be given a base it owns — pass a *relative* destination and
let the native runtime join it. A relative path has no drive anchor to get wrong, crosses the
boundary unchanged, and is shorter than the correct absolute form. In the motivating case the
harness had already `cd`-ed into the target directory, so `scripts/vendor/bundle` would have been
both correct and simpler than any absolute path.
2. **Convert an absolute path at the boundary, not at the source.** When an absolute path genuinely
must cross, convert it in the argument that crosses — keep POSIX form for Bash's own use of the
same path. Converting early forces every later Bash consumer of the variable to cope with a
Windows spelling, which is how a half-converted path ends up worse than an unconverted one.
3. **Convert with `cygpath`, and prefer mixed form.** `cygpath -m` yields `C:/dir/file`; `cygpath -w`
yields `C:\dir\file`. Both are correct to the Win32 API, which accepts either separator. Mixed
form is the default because backslashes are one escape rule away from becoming something else in
every layer a path typically crosses — a shell string, a Python or JSON literal (`C:\temp\new`
carries a newline), a regex. Reach for `-w` only for a consumer that rejects forward slashes.
[`scripts/emit-windows-path.sh`](../../../scripts/emit-windows-path.sh) is that call, with the
default and the failure posture already decided.
4. **Fail loud when conversion is unavailable.** An emit path must never fall back to the
unconverted literal, because the unconverted literal is precisely what writes to the wrong place —
unobserved. `emit-windows-path.sh` exits non-zero when `cygpath` is missing or fails, and prints
nothing on stdout for that argument.

## Do not reuse the hook-utils path helpers for this

[`lib/hook-utils.sh`](../../../lib/hook-utils.sh) is the precedent for `OSTYPE`-gated path handling
and is where this repo's `cygpath` dependency was first established, but neither of its path helpers
is an emit helper:

- `hook::normalize_path` folds a leading drive prefix for a **comparison**, using no `cygpath` at
all. Its own comment is explicit that "the emitted path is always the caller's original." Emitting
its return value is a misuse of it.
- `hook::expand_8dot3` does call `cygpath -m` / `cygpath -l -m`, but to expand **8.3 short names**,
and only for a path containing `~`.

Both fail **open** — degrading to the caller's original path — which is right for a comparison and
wrong for an emit. A comparison that degrades answers one question slightly worse; an emitted path
that degrades writes real bytes somewhere nobody looks.

The helper therefore lives at [`scripts/emit-windows-path.sh`](../../../scripts/emit-windows-path.sh)
rather than in `lib/`. `lib/` in this repo means "canonical source of a byte-identical copy synced
into carrying plugins" (see
[`scripts/cross-plugin-source-registry.txt`](../../../scripts/cross-plugin-source-registry.txt)); a
helper with no plugin consumers does not belong there, and adding one to `hook-utils.sh` would put
every carrying plugin through a version bump for a function none of them calls.

## The detection net

[`scripts/check-drive-root-litter.sh`](../../../scripts/check-drive-root-litter.sh) fails a host that
carries the defect's on-disk fingerprint: a directory at a drive root whose name is a single letter
that is **itself a mounted drive** on that host. Requiring the letter to name a real drive is what
keeps it precise — a one-character folder at a drive root is unremarkable on its own (`<drive>:\a` is
the workspace root on a GitHub-hosted Windows runner), and only becomes this defect's signature when
the letter is one an author could have spelled into an MSYS path. It also ignores a candidate that
contains the current working directory, so a checkout that genuinely lives under one is not called
residue.

Run it after any Windows verification pass:

```bash
scripts/check-drive-root-litter.sh # exit 0 clean, 1 litter found, 2 usage
```

It is a **no-op on non-Windows**: the host gate is the first thing it evaluates, before any
filesystem probing, and the skip is printed rather than silent.

**Advisory, not a required live gate.** CI runs the detector's self-test and asserts the non-Windows
no-op — those are deterministic and fixture-scoped — but does not point the live scan at a runner's
drive roots.
[ADR 0003](../../adr/0003-verification-guards-earn-default-on-by-measured-precision.md) is the
doctrine: a verification guard earns default-on by *measured* precision, and this detector has no
measurements yet. A false positive on a required aggregate blocks every merge in the repo; a missed
one costs a follow-up. Promote it when there is precision to point at.

This is a different concern from
[`plugins/guardrails/hooks/block-windows-drive-tmp.sh`](../../../plugins/guardrails/hooks/block-windows-drive-tmp.sh),
which blocks a *command* aimed at a drive-root temp path before it runs (#2594). That guard reads a
command string ahead of time; this detector reads the filesystem afterwards, and catches the class
where the offending path was never spelled in a command at all — it was computed inside a native
interpreter.

It is also outside the charter of
[`scripts/check-shell-portability.sh`](../../../scripts/check-shell-portability.sh), whose token list
is deliberately scoped to GNU-vs-BSD userland divergence. An MSYS path literal is valid GNU shell on
every platform; nothing about the *shell* is wrong, so a sibling check is the right shape rather than
another token in that list.
128 changes: 128 additions & 0 deletions scripts/check-drive-root-litter.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,128 @@
#!/usr/bin/env bash
# Detect the on-disk fingerprint of an unconverted MSYS path handed to a
# Windows-native consumer: a directory sitting at a drive root whose name is a
# single letter that is ITSELF a mounted drive.
#
# scripts/check-drive-root-litter.sh scan this host's drive roots
#
# Exit: 0 clean, or a no-op on a non-Windows host; 1 litter found; 2 usage error.
#
# WHAT IT DETECTS AND WHY THAT SHAPE. Git Bash spells `D:\dir` as `/d/dir`.
# Handed to PowerShell, cmd, or a Windows-native interpreter, the leading `/`
# anchors to the root of the CURRENT DRIVE, so the literal resolves to
# `<current-drive>:\d\dir` and the consumer creates that whole phantom chain
# before writing. The residue is therefore always the same shape: `<drive>:\<x>\`
# where `<x>` is the single letter of the drive the author MEANT to write to.
# `docs/conventions/windows-path-emit/README.md` owns the rule that prevents it;
# this script is that rule's detection net, for the runs that ignore it.
#
# Requiring the directory name to match a MOUNTED DRIVE is what keeps the
# detector precise. A single-letter directory is not suspicious on its own -
# `D:\a` is the workspace root on a GitHub-hosted Windows runner, and any number
# of projects use a one-character top-level folder deliberately. It becomes the
# fingerprint of THIS defect only when that letter also names a real drive on the
# same host, because that is the coincidence the mechanism requires: the phantom
# path is built from a drive letter the author spelled out. #2594 asked for a
# drive-root guard against an EXTERNAL writer's `C:\tmp` residue (now
# plugins/guardrails/hooks/block-windows-drive-tmp.sh, a PreToolUse guard on the
# command string); this is the same concern pointed at a producer we own, and it
# looks at the filesystem AFTER a run rather than at a command before it.
#
# ADVISORY BY DEFAULT, not wired into a required lane that scans a live machine.
# docs/adr/0003 is this repo's doctrine for that: a verification guard earns
# default-on by measured precision, and this detector has none yet. CI runs its
# self-test (deterministic, fixture-scoped) and asserts the non-Windows no-op;
# the live scan is an operator/harness-author command. Promote it when there is
# precision to point at.
#
# FIXTURE SEAM. `DRIVE_ROOT_LITTER_MOUNT_ROOT` relocates the MSYS mount root the
# scan reads (default `/`, where Git Bash mounts `C:` at `/c`). It is the one
# seam, it is opt-in, and it is never auto-detected - so a bare invocation on a
# CI runner is provably the host scan and nothing else. Drives are found by
# PROBING `<mount-root>/<letter>` with `-d` rather than by listing the mount
# root, because MSYS maps drives lazily: `ls /` shows no single-letter entries on
# a real Git Bash host even though `/c` and `/d` both resolve.
set -uo pipefail

if (($# > 0)); then
echo "usage: check-drive-root-litter.sh (no arguments)" >&2
exit 2
fi

# Host gate, first and unconditional, so the bare invocation on a Linux or macOS
# runner cannot reach any filesystem probing at all. Non-Windows hosts have no
# drive letters, so this defect cannot occur and there is nothing to scan. The
# skip is REPORTED, never silent.
case "${OSTYPE:-}" in
msys* | cygwin* | win32) ;;
*)
echo "check-drive-root-litter.sh: no-op on a non-Windows host (OSTYPE=${OSTYPE:-unset}); drive-root litter is a Windows-only shape."
exit 0
;;
esac

mount_root="${DRIVE_ROOT_LITTER_MOUNT_ROOT:-/}"
mount_root="${mount_root%/}"

# The set of mounted drive letters. This is both the set of roots to scan AND
# the set of directory names that count as a hit.
drives=()
for letter in {a..z}; do
[[ -d "$mount_root/$letter" ]] && drives+=("$letter")
done

if ((${#drives[@]} == 0)); then
echo "check-drive-root-litter.sh: no drives found under '${mount_root:-/}'; nothing to scan."
exit 0
fi

# A candidate that CONTAINS the current working directory is a real checkout
# location, not litter: someone whose repo lives at `D:\c\work\repo` would
# otherwise be told their own checkout is residue. Both sides are compared in
# PHYSICAL form, because MSYS mount aliases make the lexical spellings differ
# for the same directory (`/tmp/x` and `/c/Users/.../Temp/x` are one place).
here="$(pwd -P 2>/dev/null)" || here="$(pwd)"

hits=()
for drive in "${drives[@]}"; do
for name in "${drives[@]}"; do
candidate="$mount_root/$drive/$name"
[[ -d "$candidate" ]] || continue
cand_real="$(cd "$candidate" 2>/dev/null && pwd -P)"
[[ -n "$cand_real" ]] || cand_real="$candidate"
case "$here/" in
"$cand_real"/*) continue ;;
*) ;; # the cwd is elsewhere: the candidate is a real hit
esac
if [[ "$mount_root" == "" ]]; then
hits+=("$candidate (${drive^}:\\${name}\\)")
else
hits+=("$candidate")
fi
done
done

if ((${#hits[@]} == 0)); then
echo "check-drive-root-litter.sh: ${#drives[@]} drive root(s) scanned under '${mount_root:-/}'; no drive-root litter found."
exit 0
fi

echo "check-drive-root-litter.sh: drive-root litter found (${#hits[@]}):" >&2
for hit in "${hits[@]}"; do
echo " $hit" >&2
done
cat >&2 <<'EOF'

Each path above is a directory at a drive root named for another mounted drive -
the fingerprint of an MSYS path literal (/d/...) handed to a Windows-native
consumer, which resolved it against the CURRENT drive's root instead.

The litter is the cheap part. Whatever wrote it wrote to a path it did not
intend, so any run that produced it may have measured something other than what
it claims. Re-check the run's results before trusting them, then:

* fix the emitter - see docs/conventions/windows-path-emit/README.md, and
scripts/emit-windows-path.sh for the conversion itself;
* remove the phantom tree once you have confirmed it holds nothing else.
EOF
exit 1
Loading