mosaicstack/stack
1
0
Fork 1
Code Issues 229 Pull Requests 6 Actions Packages Projects Releases 15 Wiki Activity

guard: close four fail-open holes found by independent review
ci/woodpecker/pr/ci Pipeline was canceled
Details

Browse Source 
An independent reviewer broke all three new controls before they shipped.
Every finding is reproduced as a fixture or a repro, because the class is
recurring rather than incidental: each hole was a case where the answer was
"allow" because something was ABSENT rather than because it was CHECKED.

1. wrapper-guard read only the spellings it knew. `curl -d@body` (no space),
   `--request=POST` (equals form), and a URL path assembled from shell
   variables each carried a real provider write straight through. Write
   detection now covers every body and method form curl accepts, and the
   endpoint match no longer anchors on a literal host path that a variable
   can dissolve.

2. wrapper-guard blocked only when the wrapper FILE existed. A host with a
   broken or partial install therefore permitted exactly the raw writes the
   guard exists to stop. Blocking is now on the endpoint; a missing wrapper
   changes the remedy text, not the verdict — a broken install is not
   permission to bypass gate 7.

3. mosaic-worktree read a worktree's safety from two questions, and a clean,
   fully-pushed tree holding a gitignored `local.secret` answered both with
   zero. `git worktree remove` then deleted the one copy in existence. A file
   is gitignored precisely so nothing else holds it, so ignored-but-not-
   disposable files are now a third evidence question. Build junk
   (node_modules, .venv, dist, caches, *.pyc) stays disposable, so the common
   case still reads SAFE.

4. check-tools-index counted a documented tool as discoverable at mode 0644.
   Every caller tests `[ -x ]`, so a non-executable tool is a missing tool;
   it now fails the gate with its own message.

Local gates green: sanitization, resident budget, test enumeration,
tools-index (4/4 self-test, 100% on the enforced git suite), and
wrapper-guard 20/20.
This commit is contained in:
Hermes Agent
2026-08-12 17:20:11 -05:00
parent 96609bdade
commit 8b7ac5b51e
4 changed files with 146 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/mosaic/framework/tools/git/mosaic-worktree.sh
+46 -15
View File
@@ -31,9 +31,11 @@
# mosaic-worktree.sh rm <branch> [--force] remove; refuses to lose work
# mosaic-worktree.sh gc [--apply] report/remove clean+pushed worktrees
#
# `rm` and `gc` refuse to delete a worktree with uncommitted changes or with
# commits absent from every remote. That check is by EVIDENCE, never by size or
# age. --force overrides it and is yours to type deliberately.
# `rm` and `gc` refuse to delete a worktree with uncommitted changes, with
# commits absent from every remote, or holding ignored files that are not of the
# well-known regenerable kind (a `.env` is ignored so it is never committed,
# which is also why nothing else holds a copy). That check is by EVIDENCE, never
# by size or age. --force overrides it and is yours to type deliberately.
#
# Run from anywhere inside the repo, or pass --repo <path>.
@@ -98,16 +100,41 @@ configuration, credentials, state and caches — not checkouts." ;;
# Two independent questions, both answered from git, neither from size or age:
# dirty — anything uncommitted in the tree
# unpushed — commits reachable from HEAD that no remote ref contains
# precious — IGNORED files git will not mention and will not miss
#
# The third question is not obvious and was missed on the first pass. An
# independent reviewer demonstrated it in four commands: a pushed, clean
# worktree whose .gitignore covers `*.secret`, holding one `local.secret`.
# `git status --porcelain` is empty, `rev-list --count HEAD --not --remotes` is
# 0 — the evidence reads SAFE — and `git worktree remove` deletes the file. The
# same shape covers `.env`, credentials, scratch notes, downloaded fixtures:
# precisely the files that are ignored BECAUSE they must not be committed, which
# is also why nothing else is holding a copy.
#
# So ignored files count as work unless they are the well-known regenerable
# kind. Getting that set wrong is asymmetric: an over-broad list preserves a
# worktree that could have been reclaimed (cheap, visible, fixable by --force),
# an over-narrow one deletes the only copy of a secret (silent, permanent).
# The list stays short and conservative for that reason.
DISPOSABLE_RE='(^|/)(node_modules|\.venv|venv|__pycache__|\.mypy_cache|\.pytest_cache|\.ruff_cache|\.turbo|\.cache|\.parcel-cache|\.gradle|dist|build|out|target|coverage|\.next|\.nuxt|\.svelte-kit)(/|$)|\.(pyc|pyo|o|class)$'
wt_dirty() { git -C "$1" status --porcelain 2>/dev/null | head -200 | wc -l; }
wt_unpushed() { git -C "$1" rev-list --count HEAD --not --remotes 2>/dev/null || echo "?"; }
# Default --ignored (not =matching) so a 40k-file node_modules collapses to one
# directory entry instead of being enumerated and then discarded.
wt_precious() {
git -C "$1" status --porcelain --ignored 2>/dev/null \
| awk '/^!! /{print substr($0,4)}' \
| grep -Ev "$DISPOSABLE_RE" | head -200 | wc -l
}
wt_state() {
local wt="$1" d u
d="$(wt_dirty "$wt")"; u="$(wt_unpushed "$wt")"
if [ "$d" -eq 0 ] && [ "$u" = "0" ]; then
printf 'SAFE\tclean; 0 unpushed'
local wt="$1" d u p
d="$(wt_dirty "$wt")"; u="$(wt_unpushed "$wt")"; p="$(wt_precious "$wt")"
if [ "$d" -eq 0 ] && [ "$u" = "0" ] && [ "$p" -eq 0 ]; then
printf 'SAFE\tclean; 0 unpushed; no ignored files worth keeping'
else
printf 'PRESERVE\t%s uncommitted; %s unpushed' "$d" "$u"
printf 'PRESERVE\t%s uncommitted; %s unpushed; %s ignored-but-not-disposable' "$d" "$u" "$p"
fi
}
@@ -180,14 +207,18 @@ cmd_rm() {
local path; path="$(derive_path "$branch")"
[ -d "$path" ] || die "no worktree at $path"
local d u
d="$(wt_dirty "$path")"; u="$(wt_unpushed "$path")"
if [ "$force" -eq 0 ] && { [ "$d" -ne 0 ] || [ "$u" != "0" ]; }; then
local d u p
d="$(wt_dirty "$path")"; u="$(wt_unpushed "$path")"; p="$(wt_precious "$path")"
if [ "$force" -eq 0 ] && { [ "$d" -ne 0 ] || [ "$u" != "0" ] || [ "$p" -ne 0 ]; }; then
die "refusing to remove $path
uncommitted files: $d
unpushed commits: $u
Commit and push first — that is the contract. If this work is genuinely
disposable, re-run with --force."
uncommitted files: $d
unpushed commits: $u
ignored, not disposable: $p
Commit and push first — that is the contract. Ignored files are counted because
git will neither report them nor miss them: a .env or a *.secret is ignored
precisely so it is never committed, which is also why nothing else holds a copy.
List them with: git -C $path status --porcelain --ignored | grep '^!!'
If this work is genuinely disposable, re-run with --force."
fi
# NB: ${force:+--force} would expand for force=0 too ("0" is non-empty).
packages/mosaic/framework/tools/git/test-wrapper-guard.sh
+13
View File
@@ -39,6 +39,19 @@ FIXTURES="$TMP/fixtures.tsv"
printf '0\t{"tool_input":{"command":"ls -la /src"}}\tordinary commands are untouched\n'
printf '0\t{"tool_input":{"command":"MOSAIC_WRAPPER_OVERRIDE=1 curl -X POST -d @b https://git.example.invalid/api/v1/repos/a/b/pulls"}}\tbreak-glass works\n'
printf '0\t{"tool_input":{}}\tan empty payload does not block the session\n'
# --- bypasses an independent reviewer demonstrated against the first version.
# Each of these returned 0 (allowed) and each is a real write. They are pinned
# as fixtures rather than fixed-and-forgotten because the class is recurring:
# the guard reads text, so every spelling it does not know is a hole.
printf '2\t{"tool_input":{"command":"curl -d@b https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews"}}\t-d@body with no space is still a body\n'
printf '2\t{"tool_input":{"command":"curl --request=POST -d@b https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews"}}\t--request=POST equals-form is still a method\n'
printf '2\t{"tool_input":{"command":"p=/api/v1/repo; q=s/a/b/pulls/1/reviews; curl -d@b https://git.example.invalid${p}${q}"}}\ta path split across variables is still that path\n'
printf '2\t{"tool_input":{"command":"curl --data-binary @b https://git.example.invalid/api/v1/repos/a/b/issues/1/comments"}}\t--data-binary is a body\n'
printf '2\t{"tool_input":{"command":"curl -F f=@b https://git.example.invalid/api/v1/repos/a/b/issues"}}\t-F multipart is a body\n'
# Reads must survive every one of those broadenings, or the guard gets disabled.
printf '0\t{"tool_input":{"command":"curl -s https://git.example.invalid/api/v1/repos/a/b/pulls/1/reviews"}}\tno body and no verb is a read\n'
printf '0\t{"tool_input":{"command":"grep -rn /pulls/ src/ | head -20"}}\ta path fragment in a grep is not an API call\n'
printf '0\t{"tool_input":{"command":"curl -X POST -d @b https://registry.example.invalid/v2/x/manifests/latest"}}\tan unwrapped API is not this guard'"'"'s business\n'
} > "$FIXTURES"
fail=0 n=0
packages/mosaic/framework/tools/git/wrapper-guard.sh
+51 -20
View File
@@ -74,35 +74,68 @@ EOF
fi
# ---- 2/3. provider API writes ---------------------------------------------
# Only consider calls that are (a) to a provider API path and (b) mutating.
is_api=0
printf '%s' "$CMD" | grep -Eq '/api/v1/repos/|api\.github\.com/repos/' && is_api=1
if [ "$is_api" -eq 1 ]; then
# A raw provider write is four things at once: an HTTP client, a URL, a mutating
# verb or a request body, and a path fragment naming an endpoint a wrapper
# already owns. All four are required, which is what keeps reads and unwrapped
# endpoints flowing.
#
# Deliberately NOT gated on the literal "/api/v1/repos/". An independent reviewer
# broke that version in one line: build the path in shell variables
# p=/api/v1/repo; q=s/a/b/pulls/1/reviews; curl -d@body "https://host${p}${q}"
# and the host-anchored literal never appears, so the check read clean while the
# write went through. The endpoint fragments below survive it, because the
# fragment has to appear somewhere for the URL to be constructible at all.
if printf '%s' "$CMD" | grep -Eq 'curl|wget|http(ie)?[[:space:]]' \
&& printf '%s' "$CMD" | grep -Eq 'https?://'; then
# Write detection. Every spelling curl accepts, because the guard is defeated
# by the one spelling it does not know: `-d@body` (no space) and
# `--request=POST` (equals form) both slipped past the first version.
is_write=0
printf '%s' "$CMD" | grep -Eq -- '-X[[:space:]]*(POST|PATCH|PUT|DELETE)|--request[[:space:]]*(POST|PATCH|PUT|DELETE)' && is_write=1
# curl sends POST implicitly when given a body.
printf '%s' "$CMD" | grep -Eq -- '--data|-d[[:space:]]' && is_write=1
printf '%s' "$CMD" | grep -Eq -- \
'-X[[:space:]]*(POST|PATCH|PUT|DELETE)|--request[[:space:]=]*(POST|PATCH|PUT|DELETE)' && is_write=1
# curl sends POST implicitly when handed a body, in any of these forms.
printf '%s' "$CMD" | grep -Eq -- \
'(^|[[:space:]])(-d|-F|-T)|--data([-a-z]*)?[[:space:]=]|--json[[:space:]=]|--form|--upload-file' && is_write=1
if [ "$is_write" -eq 1 ]; then
endpoint=""; wrapper=""
case "$CMD" in
*"/pulls/"*"/reviews"*|*"/pulls/"*"/requested_reviewers"*)
endpoint="pull-request review"; wrapper="$W/pr-review.sh" ;;
*"/pulls/"*"/merge"*) endpoint="pull-request merge"; wrapper="$W/pr-merge.sh" ;;
*"/issues/"*"/comments"*) endpoint="issue comment"; wrapper="$W/issue-comment.sh" ;;
*"/pulls"*) endpoint="pull request"; wrapper="$W/pr-create.sh" ;;
*"/issues"*) endpoint="issue"; wrapper="$W/issue-create.sh" ;;
*"/milestones"*) endpoint="milestone"; wrapper="$W/milestone-create.sh" ;;
endpoint="pull-request review"; wrapper="pr-review.sh" ;;
*"/pulls/"*"/merge"*) endpoint="pull-request merge"; wrapper="pr-merge.sh" ;;
*"/issues/"*"/comments"*) endpoint="issue comment"; wrapper="issue-comment.sh" ;;
*"/pulls"*) endpoint="pull request"; wrapper="pr-create.sh" ;;
*"/issues"*) endpoint="issue"; wrapper="issue-create.sh" ;;
*"/milestones"*) endpoint="milestone"; wrapper="milestone-create.sh" ;;
esac
if [ -n "$wrapper" ] && [ -x "$wrapper" ]; then
# Block on the ENDPOINT, never on whether the wrapper file happens to exist.
# The previous version required `[ -x "$W/$wrapper" ]`, which meant a host
# with a broken or absent install allowed exactly the raw writes the guard
# exists to stop — an absence-driven allow, and the second one found in this
# file. A missing wrapper is a broken install; it is not a licence to bypass
# gate 7. Say so, and say which is which.
if [ -n "$endpoint" ]; then
if [ -x "$W/$wrapper" ]; then
remedy="Use the wrapper the Constitution (gate 7) requires:
$W/$wrapper
Run \`$wrapper --help\` for the flags."
else
remedy="The wrapper that covers this endpoint is \`$wrapper\`, and it is NOT
present or not executable at:
$W/$wrapper
That is a broken or incomplete install, not permission to send the call raw.
Repair the install (\`mosaic doctor\`) and use the wrapper."
fi
cat <<EOF
BLOCKED: raw provider API write to the $endpoint endpoint.
A Mosaic wrapper already covers this and the Constitution (gate 7) requires it
before any raw provider call:
$wrapper
$remedy
The wrappers are not a formality. They carry provider-dialect differences that
raw curl silently gets wrong — Gitea's review event is APPROVED, GitHub's is
@@ -110,8 +143,6 @@ APPROVE, and Gitea accepts the wrong one with HTTP 200 while filing the review
as PENDING. They also resolve identity explicitly, which matters on a host
where the default login is an admin account.
Run \`$(basename "$wrapper") --help\` for the flags.
If no wrapper flag can express this call, that is a wrapper gap: extend the
wrapper. To proceed anyway for a genuine gap, prefix MOSAIC_WRAPPER_OVERRIDE=1.
EOF
packages/mosaic/framework/tools/quality/scripts/check-tools-index.sh
+36 -9
View File
@@ -151,7 +151,7 @@ run_check() {
case "$suite" in _*) continue ;; esac
local total=0 found=0
local -a suite_missing=()
local -a suite_missing=() suite_noexec=()
for tool in "$dir"*.sh; do
[ -e "$tool" ] || continue
base="$(basename -- "$tool")"
@@ -159,6 +159,13 @@ run_check() {
total=$((total + 1))
if documented "$base"; then
found=$((found + 1))
# Documented AND present is not enough. The index presents these as
# commands to run, and every caller — the wrapper guard included —
# decides "is this tool here?" with `[ -x ]`. A 0644 wrapper is
# documented, present, and dead: it reads as absent to every check that
# matters while scoring 100% here. That is a false green, which is worse
# than a red, so it fails rather than warns.
[ -x "$tool" ] || suite_noexec+=("$base")
else
suite_missing+=("$base")
fi
@@ -166,11 +173,16 @@ run_check() {
[ "$total" -eq 0 ] && continue
local pct=$(( found * 100 / total ))
if [ "$enforced" -eq 1 ] && [ ${#suite_noexec[@]} -gt 0 ]; then
printf 'FAIL %-12s %3d%% (%d/%d) documented but not executable: %s\n' \
"$suite" "$pct" "$found" "$total" "${suite_noexec[*]}"
rc=1
fi
if [ "$enforced" -eq 1 ] && [ ${#suite_missing[@]} -gt 0 ]; then
printf 'FAIL %-12s %3d%% (%d/%d) undocumented: %s\n' \
"$suite" "$pct" "$found" "$total" "${suite_missing[*]}"
rc=1
elif [ "$enforced" -eq 1 ]; then
elif [ "$enforced" -eq 1 ] && [ ${#suite_noexec[@]} -eq 0 ]; then
printf 'ok %-12s %3d%% (%d/%d) [enforced]\n' "$suite" "$pct" "$found" "$total"
else
printf 'info %-12s %3d%% (%d/%d) not yet enforced\n' "$suite" "$pct" "$found" "$total"
@@ -222,6 +234,7 @@ self_test() {
mkdir -p "$tmp/tools/git"
printf '#!/bin/sh\n' > "$tmp/tools/git/documented-tool.sh"
printf '#!/bin/sh\n' > "$tmp/tools/git/test-ignored.sh"
chmod +x "$tmp/tools/git/documented-tool.sh" "$tmp/tools/git/test-ignored.sh"
# run_check reads the TOOLS_DIR / DOCS globals; an array cannot ride in a
# command-prefix assignment, so point the globals at the fixture directly.
@@ -231,18 +244,18 @@ self_test() {
# Case 1: fully documented -> pass.
printf 'see tools/git/documented-tool.sh for details\n' > "$tmp/doc.md"
if run_check >/dev/null; then
printf 'self-test 1/3 ok (complete index passes)\n'
printf 'self-test 1/4 ok (complete index passes)\n'
else
printf 'self-test 1/3 FAIL (complete index should pass)\n'; return 1
printf 'self-test 1/4 FAIL (complete index should pass)\n'; return 1
fi
# Case 2: an undocumented tool -> fail.
printf '#!/bin/sh\n' > "$tmp/tools/git/undocumented-tool.sh"
rc=0; run_check >/dev/null || rc=$?
if [ "$rc" -eq 1 ]; then
printf 'self-test 2/3 ok (undocumented tool fails the gate)\n'
printf 'self-test 2/4 ok (undocumented tool fails the gate)\n'
else
printf 'self-test 2/3 FAIL (undocumented tool should fail, got rc=%s)\n' "$rc"; return 1
printf 'self-test 2/4 FAIL (undocumented tool should fail, got rc=%s)\n' "$rc"; return 1
fi
# Case 3: a stale index reference -> fail.
@@ -250,12 +263,26 @@ self_test() {
printf 'also tools/git/deleted-tool.sh\n' >> "$tmp/doc.md"
rc=0; run_check >/dev/null || rc=$?
if [ "$rc" -eq 1 ]; then
printf 'self-test 3/3 ok (stale index reference fails the gate)\n'
printf 'self-test 3/4 ok (stale index reference fails the gate)\n'
else
printf 'self-test 3/3 FAIL (stale reference should fail, got rc=%s)\n' "$rc"; return 1
printf 'self-test 3/4 FAIL (stale reference should fail, got rc=%s)\n' "$rc"; return 1
fi
printf '\nself-test passed: the gate demonstrably reds on both drift directions.\n'
# Case 4: documented, present, and NOT executable -> fail. Found by an
# independent reviewer: a 0644 wrapper scored 100% here while reading as
# absent to every `[ -x ]` in the fleet, including the wrapper guard's.
sed -i '/deleted-tool/d' "$tmp/doc.md"
printf '#!/bin/sh\n' > "$tmp/tools/git/noexec-tool.sh"
chmod 0644 "$tmp/tools/git/noexec-tool.sh"
printf 'and tools/git/noexec-tool.sh\n' >> "$tmp/doc.md"
rc=0; run_check >/dev/null || rc=$?
if [ "$rc" -eq 1 ]; then
printf 'self-test 4/4 ok (documented but non-executable tool fails the gate)\n'
else
printf 'self-test 4/4 FAIL (non-executable tool should fail, got rc=%s)\n' "$rc"; return 1
fi
printf '\nself-test passed: the gate demonstrably reds on every drift direction.\n'
}
if [ "$SELF_TEST" -eq 1 ]; then