diff --git a/.woodpecker/ci.yml b/.woodpecker/ci.yml index ae488a73..26ea0848 100644 --- a/.woodpecker/ci.yml +++ b/.woodpecker/ci.yml @@ -61,6 +61,13 @@ steps: # endpoints and ordinary commands through. Both directions are asserted — # a guard that over-blocks gets routed around, which fails just as hard. - bash packages/mosaic/framework/tools/git/test-wrapper-guard.sh + # Hermetic regression for mosaic-worktree.sh at fleet scale: stubs git onto + # PATH so `list` faces ~450 KB of porcelain. The defect it pins is invisible + # at small size — `git … | awk '…exit'` gives the producer SIGPIPE, which + # under `set -euo pipefail` aborts the caller silently with rc=141 and no + # output. A repo only reaches that once it has enough worktrees, so the + # stub supplies the scale instead of the host's own checkout. + - bash packages/mosaic/framework/tools/git/test-mosaic-worktree-large-repo.sh # Blocking gate (#791): a framework upgrade must never write or delete an # operator-owned path. The HARD GATE proves an unanticipated operator sentinel diff --git a/packages/mosaic/framework/tools/git/test-mosaic-worktree-large-repo.sh b/packages/mosaic/framework/tools/git/test-mosaic-worktree-large-repo.sh old mode 100644 new mode 100755 diff --git a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh index aaccc77f..309c64d4 100755 --- a/packages/mosaic/framework/tools/git/test-wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/test-wrapper-guard.sh @@ -267,12 +267,34 @@ FIXTURES="$TMP/fixtures.tsv" # and the call went through. An unreadable request is not a cleared one. printf '2\t{"tool_input":{"command":"curl --config /tmp/req https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews"}}\tthe request in a config file is unreadable, so it is refused\t--config/-K\n' printf '2\t{"tool_input":{"command":"curl -K /tmp/req https://git.example.invalid/api/v1/repos/a/b/issues"}}\tthe short spelling, same answer\t--config/-K\n' - # Cost, stated: the scope gate admits any https URL, so this refuses a - # --config read against a host that has nothing to do with a forge. The - # alternative is to require a forge marker in a command whose URL may itself - # be in the file, which is the hole again. + # Cost, stated: this refuses a --config read against a host that has nothing + # to do with a forge. The alternative is to require a forge marker in a + # command whose URL may itself be in the file, which is the hole again. printf '2\t{"tool_input":{"command":"curl --config /tmp/req https://example.invalid/anything"}}\tan unrelated https URL with --config is refused too, by decision\t--config/-K\n' - printf '0\t{"tool_input":{"command":"eslint --config .eslintrc.json src/"}}\t--config outside an API-shaped command is nobody'"'"'s business\n' + printf '0\t{"tool_input":{"command":"eslint --config .eslintrc.json src/"}}\t--config on a command that is not curl is nobody'"'"'s business\n' + + # ROUND TEN. The --config check above was first written INSIDE the API-shape + # gate, so it was guarded by a condition that the capability it guards against + # removes. A config file can carry the URL; delete the URL from the command and + # nothing is API-shaped, the branch is never entered, and the guard reports + # clean on precisely the call it exists to refuse. It is now asked of any curl. + printf '2\t{"tool_input":{"command":"curl --config /tmp/provider-write.cfg"}}\ta config file can own the URL, so there is nothing API-shaped left to gate on\t--config/-K\n' + printf '2\t{"tool_input":{"command":"curl -K/tmp/provider-write.cfg"}}\tcurl accepts the value attached to the short flag\t--config/-K\n' + printf '2\t{"tool_input":{"command":"curl -sK /tmp/provider-write.cfg"}}\tand inside a bundle, which a space-separated test does not see\t--config/-K\n' + printf '0\t{"tool_input":{"command":"tar -K /tmp/archive.tar"}}\t-K on a command that is not curl is not this hook'"'"'s business\n' + + # Percent-encoded endpoints. Not hypothetical: /issues/1174 and /iss%%75es/1174 + # both returned HTTP 200 with the same object from the live forge, so the + # encoded spelling IS the wrapped endpoint and the literal comparison below it + # sees a segment matching nothing. Refused rather than decoded — a decoder has + # to be exactly right about depth and normalization, which is the parser + # mistake this file declines everywhere else. + printf '2\t{"tool_input":{"command":"gh api -X POST repos/a/b/iss%%75es/1/comments -f body=x"}}\tan encoded path segment reaches the wrapped endpoint\tpercent-escape\n' + printf '2\t{"tool_input":{"command":"curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/revi%%65ws"}}\tsame for the review endpoint, which is the one that matters most\tpercent-escape\n' + printf '2\t{"tool_input":{"command":"gh api -X POST repos/a/b/iss%%2575es/1/comments -f body=x"}}\tdouble-encoded too, which is why this refuses instead of decoding\tpercent-escape\n' + # Scoped to writes, deliberately. Reads are never blocked by this guard and a + # query string carrying %%20 is an ordinary URL, not a hazard. + printf '0\t{"tool_input":{"command":"curl -s https://git.example.invalid/api/v1/repos/a/b/issues?q=a%%20b"}}\ta percent-escape in a READ is not this hook'"'"'s business\n' } > "$FIXTURES" fail=0 n=0 @@ -358,6 +380,62 @@ home_case 'HOME unset does not block an ordinary command' \ home_case 'HOME unset does not stop the API arms doing their job' \ 2 '@unset' 'curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews' 'pr-review.sh' +# ---- the guard standing on its own ----------------------------------------- +# Every case above runs the guard from the directory holding its siblings, so +# `[ -x "$W/pr-review.sh" ]` succeeds and the $HOME fallback beside it never +# evaluates. That is a property of the HARNESS, not of the guard, and it hid a +# live fail-open: with the guard copied somewhere alone AND no HOME, the +# fallback expanded an unset variable under `set -u` and the script died at +# rc=1 — on EVERY arm, before any adjudication. A PreToolUse hook exiting +# nonzero-but-not-2 is a non-blocking error, so that seat ran with no guard and +# nothing reported it. +# +# The first remediation moved that expansion four lines earlier and called it +# closed. It was not closed, because the test could not reach it. So the guard +# is copied ALONE here — no siblings, no installed mosaic home — which is the +# deployment this file already claims to support ("still works from a repo +# checkout with no installed mosaic home"). +LONE="$TMP/lone"; mkdir -p "$LONE" +cp "$GUARD" "$LONE/wrapper-guard.sh"; chmod +x "$LONE/wrapper-guard.sh" + +lone_case() { + local why="$1" want="$2" homeval="$3" cmd="$4" needle="${5:-}" + local out got + n=$((n + 1)) + if [ "$homeval" = "@unset" ]; then + out="$(printf '%s' "{\"tool_input\":{\"command\":\"$cmd\"}}" | env -u HOME "$LONE/wrapper-guard.sh" 2>&1)" + else + out="$(printf '%s' "{\"tool_input\":{\"command\":\"$cmd\"}}" | env HOME="$homeval" "$LONE/wrapper-guard.sh" 2>&1)" + fi + got=$? + if [ "$got" != "$want" ]; then + printf 'FAIL %s [standalone] (want exit %s, got %s)\n' "$why" "$want" "$got" + fail=1 + return + fi + if [ -n "$needle" ] && ! printf '%s' "$out" | grep -Fq -- "$needle"; then + printf 'FAIL %s [standalone] (exit %s, but the message does not say %s)\n' "$why" "$got" "$needle" + fail=1 + return + fi + printf 'ok %s [standalone]\n' "$why" +} + +lone_case 'no siblings and no HOME: an ordinary command still passes, not rc=1' \ + 0 '@unset' 'ls -la /src' +lone_case 'no siblings and no HOME: a wrapped write is still refused' \ + 2 '@unset' 'curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews' 'pr-review.sh' +lone_case 'no siblings and no HOME: a checkout is refused, not adjudicated' \ + 2 '@unset' 'git clone https://example.invalid/x /src/wt' 'unset or unusable' +# Spelled without quotes on purpose. The payload is interpolated into a JSON +# string by the helper, so a fixture carrying bare double quotes produces +# malformed JSON, jq returns empty, and the guard exits 0 on an empty command — +# a PASS that measures nothing. That is what the first version of this case did. +lone_case 'no siblings and no HOME: the APPROVE trap still fires' \ + 2 '@unset' 'gh api -X POST repos/a/b/pulls/1/reviews -f event=APPROVE' +lone_case 'no siblings, usable HOME: ordinary commands unaffected' \ + 0 '/home/tester' 'ls -la /src' + printf '\n' if [ "$fail" -eq 0 ]; then printf 'wrapper-guard: %d/%d fixtures behaved as specified.\n' "$n" "$n" diff --git a/packages/mosaic/framework/tools/git/wrapper-guard.sh b/packages/mosaic/framework/tools/git/wrapper-guard.sh index 423ece4b..abc8e6e2 100755 --- a/packages/mosaic/framework/tools/git/wrapper-guard.sh +++ b/packages/mosaic/framework/tools/git/wrapper-guard.sh @@ -100,8 +100,26 @@ override_prefixed && exit 0 # still works from a repo checkout with no installed mosaic home (which is how it # is exercised in CI). $HOME remains the fallback for a guard invoked by an # absolute path from somewhere unusual. +# $HOME is resolved HERE, once, before anything expands it. The previous attempt +# fixed the checkout arm's use of $HOME and left this one, four lines earlier, +# reading it raw — so under `set -u` a seat with no HOME still died before +# reaching the adjudication that was supposed to handle exactly that. Moving a +# fail-open earlier in the file is not closing it. There is now exactly one +# expansion of HOME in this script and it is guarded; every later use reads +# HOME_DIR / home_known instead, so a new use cannot reintroduce the abort +# without going through this block. +# +# Empty and unset are different values and neither one is a home directory. '/' +# is rejected for the same reason as '': every path is under it, so comparing +# against it stops discriminating at all. +HOME_DIR="" +home_known=0 +case "${HOME-}" in + /?*) HOME_DIR="$HOME"; home_known=1 ;; +esac + W="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" -[ -x "$W/pr-review.sh" ] || W="$HOME/.config/mosaic/tools/git" +[ -x "$W/pr-review.sh" ] || W="$HOME_DIR/.config/mosaic/tools/git" # ---- 1. checkout into $HOME ------------------------------------------------ # $HOME has to be RESOLVED before anything can be compared against it, and the @@ -125,11 +143,9 @@ W="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd)" # was unable to ask. The blast radius of that fail-closed arm is exactly one # command shape (clone / worktree add), not the session. home_re='~|\$HOME' -home_known=0 -case "${HOME-}" in - /?*) home_re="$home_re|$(printf '%s' "$HOME" | sed 's/[][\.*^$+?(){}|]/\\&/g')" - home_known=1 ;; -esac +if [ "$home_known" -eq 1 ]; then + home_re="$home_re|$(printf '%s' "$HOME_DIR" | sed 's/[][\.*^$+?(){}|]/\\&/g')" +fi if printf '%s' "$CMD" | grep -Eq 'git[^|;&]*(clone|worktree[[:space:]]+add)'; then if [ "$home_known" -eq 0 ]; then @@ -254,30 +270,35 @@ fi # `gh pr merge`) is NOT covered — catching that means modelling every CLI's verb # grammar, which is the parser mistake again in a new costume. Porcelain is a # gate-7 gap for prose and review to hold, not this hook. -API_SHAPED='https?://|/api/v[0-9]|/repos/' -API_SHAPED="$API_SHAPED"'|(^|[[:space:]|;&(])(gh|tea|glab|hub)[[:space:]]+api([[:space:]]|$)' -if printf '%s' "$CMD" | grep -Eq "$API_SHAPED"; then - - # curl takes its options from a file with -K/--config, and that file may carry - # the method, the body, the headers and the URL itself. Every write test below - # reads the command TEXT, and none of those things are in it: - # - # curl --config /tmp/req .../repos/a/b/pulls/1/reviews - # - # carries no -X, no -d, no -f, so is_write stayed 0 and the call went straight - # through. This is not a spelling the write detection was missing — it is the - # write question being unaskable, which is the same situation as an endpoint - # assembled from expansions, and it gets the same answer. Refusing a READ that - # happens to use --config is the acceptable side of that trade, and the message - # says how to proceed. - if printf '%s' "$CMD" | grep -Eq -- '(^|[[:space:]])(-K|--config)([[:space:]=]|$)'; then - cat < %69 -> i) and about which characters the provider + # normalizes, and being exactly right about someone else's parser is the + # mistake this file declines everywhere else. Refusing is correct at every + # depth at once. + # + # Scoped to WRITES. Reads are never blocked by this guard, and a query + # string carrying %20 is not a hazard — it is an ordinary URL. Putting this + # test on the whole API-shaped branch would have refused those too, which is + # how a guard earns being routed around. + if printf '%s' "$CMD" | grep -Eq '%[0-9A-Fa-f][0-9A-Fa-f]'; then + cat <