From 378bc1afe3bc485adb8614897d66c5edccd4a527 Mon Sep 17 00:00:00 2001 From: be-coder-07 Date: Wed, 5 Aug 2026 19:00:22 -0500 Subject: [PATCH] fix(installer): close detector false-pass gaps --- .../1050-successor-remediation/README.md | 66 +++++++++++++++++++ .../1050-install-state-machine-red-fixture.md | 1 + .../quality/scripts/test-upgrade-rollback.sh | 46 +++++++------ tools/e2e-install-test.sh | 15 +++-- tools/fixtures/greenfield-expected-red.tsv | 5 +- tools/install-next-lane.test.sh | 26 ++++++++ tools/install-state-machine.test.sh | 28 ++++++++ tools/install.sh | 40 ++++++++--- tools/verify-greenfield-expected-red.sh | 21 +++++- tools/verify-greenfield-expected-red.test.sh | 18 ++++- 10 files changed, 227 insertions(+), 39 deletions(-) create mode 100644 docs/reports/verification/1050-successor-remediation/README.md diff --git a/docs/reports/verification/1050-successor-remediation/README.md b/docs/reports/verification/1050-successor-remediation/README.md new file mode 100644 index 00000000..9afab24c --- /dev/null +++ b/docs/reports/verification/1050-successor-remediation/README.md @@ -0,0 +1,66 @@ +# #1050 successor remediation verification + +Head under test before remediation: `e5d5c8495a070af2dcd393cace287fe74a8a819e`. + +This change strengthens the expected-RED detector; it does not repair the intentionally failing greenfield rows. The #869 hooks remain unwired. + +## A1 — P0 reason binding + +RED first: + +```text +$ bash tools/verify-greenfield-expected-red.test.sh +[test] FAIL: vacuous P0 PASS satisfied the expected-RED contract without identity/context evidence +exit=1 +``` + +The pinned manifest now has an explicit `phase-reason` binding against the final P0 row: target `mosaic`, uid `1001`, equal `HOME` and passwd HOME, `/bin/bash`, `privilege=user`, `x86_64`, glibc, and version-shaped Node/npm evidence. All three cases structurally require exactly one P0 reason binding. The greenfield fixture's final P0 row now emits and validates the same complete identity/context evidence, so an unrelated earlier P0 line cannot satisfy the binding for a vacuous final row. + +Pipeline 2224 and the successor's pre-change fixture run also exposed a stale next-lane P6 reason left behind by the already-closed B6 remediation: actual behavior is a fail-closed runtime-link action refusal with `#869` hooks left inactive and a persisted required P6 failure, while the manifest still expected dead hooks to be active. The pinned reason now matches the stronger measured refusal (`runtime linking/activation action reported a required failure`); no verdict changed and #869 remains unwired. + +GREEN: + +```text +[test] PASS: P0 PASS must bind target identity, HOME, shell, privilege, architecture, and runtime reason +``` + +## A2 — fail-closed P4 enumeration + +RED first: a `find` control emitted only the safe root, omitted an unsafe mode-`0666` child, and exited `73`. The process-substitution consumer discarded that status: + +```text +[test] FAIL: P4 accepted a partial created-path inventory after find failed +[test] FAIL: P4 did not report failed created-path enumeration +exit=1 +``` + +P4 now captures the NUL-delimited walk into a temporary file, checks `find` to completion, and only then evaluates the complete inventory. A failed walk reports that enumeration failed and returns a P4 finding. + +GREEN: + +```text +[test] PASS: P4 rejects an incomplete created-path inventory +[test] PASS: P4 attributes the failed created-path enumeration +``` + +## B — deterministic TERM no-exit control + +Woodpecker pipeline 2224 at the original head reported `32 passed, 2 failed`: the no-exit fixture did not exit zero or report sync success. The premise was not stale: the same fixture passed `34/34` on another filesystem. + +A controlled reverse-sorted `find -print0` walk reproduced the pipeline result exactly (`32 passed, 2 failed`). Root cause: signal injection was tied to `guides/E2E-DELIVERY.md`; whether required `tools/` content remained after restore depended on filesystem enumeration order. The test was measuring path order as well as trap semantics. + +The generated fixtures now damage a real target path after the snapshot is armed, self-signal immediately before the complete normal sync, and differ only in the explicit handler exit. Therefore a no-exit handler always restores, returns, runs the full sync, mutates the restored target again, and reports completion independent of walk order. + +GREEN on both native and reverse-sorted enumeration: + +```text +RESULT: 36 passed, 0 failed +``` + +Mutation sensitivity: restoring `exit 1` to the nominal no-exit fixture makes the control RED (`32 passed, 4 failed`), including failures of the zero-exit and resumed-success assertions. The control can still fail for its stated reason. + +## Enumeration-class sweep + +The sweep covered production enumeration in `tools/install.sh` and `packages/mosaic/framework/install.sh`, plus process-substitution consumers in the C1 shell-test surfaces. Framework installer file, operator, durable-snapshot, and pruning walks already capture and check their producer status. P4's created-path walk was the reviewed unchecked instance. + +One additional order/completeness dependency was found in source acquisition: `find "$WORK_DIR" ... | head -1` hid `find` failure and selected arbitrarily when an archive produced multiple top-level directories. RED first, the extraction fake produced two roots and the lane test stopped at that new assertion with exit 1 because today's code selected one. Source acquisition now captures and checks the complete NUL-delimited walk and requires exactly one extracted root. The lane suite is green with the multiple-root rejection. No remaining production installer enumeration uses unchecked process substitution or first-row order as authority. diff --git a/docs/scratchpads/1050-install-state-machine-red-fixture.md b/docs/scratchpads/1050-install-state-machine-red-fixture.md index 5fe9d10e..67aa65b5 100644 --- a/docs/scratchpads/1050-install-state-machine-red-fixture.md +++ b/docs/scratchpads/1050-install-state-machine-red-fixture.md @@ -51,6 +51,7 @@ Implement C1 from the canonical greenfield-install PRD v2: a transactional P0– - [x] State-machine implementation complete: private pre-mutation journal/snapshot, P0–P8 `--check`, P2–P8 fault seam, rollback, durable manifest/journal seal, action-status persistence, safe rollback roots, and stale-projection recovery. - [x] Debian/glibc checkout fixture now packages the complete current checkout, verifies its digest in-container, and reaches the expected attributable RED without host inheritance. CI compares its exact final phase map/reasons to `tools/fixtures/greenfield-expected-red.tsv`; the fixture remains red while the detector job is green only on an exact match. - [ ] Reviews complete. Reviews 80 (`rev-security-02`) and 81 (`rev-974`) requested changes at `3934e03f`; their eight non-overlapping detector findings are being remediated red-first. Current remediation adds canonical-image portability, absolute P3 CLI propagation, exact expected-RED schema/cardinality, passwd-HOME binding, created-path owner/mode policy, real-action P2–P8 fault injection, verified non-empty remote installer execution, and seeded secret-canary/redacted diagnostics. Both old verdicts become void when the remediation head moves and require fresh independent review. +- [x] Successor remediation for review 90 is RED-first and recorded in `docs/reports/verification/1050-successor-remediation/`: the manifest now binds the complete supported final P0 reason; P4 rejects an incomplete created-path walk instead of discarding `find` failure; and the TERM no-exit control is independent of filesystem enumeration order while retaining a proven RED mutation. Pipeline 2224's 32/2 result was a path-order-sensitive control, not evidence that the resume bug's premise became stale. The enumeration-class sweep additionally replaced order-dependent `find | head -1` source-root selection with a checked complete inventory requiring exactly one extracted root. ## Risks / blockers diff --git a/packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh b/packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh index 1a8ea800..30ce4e4a 100644 --- a/packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh +++ b/packages/mosaic/framework/tools/quality/scripts/test-upgrade-rollback.sh @@ -180,15 +180,16 @@ chk "[control] without -E the mid-sync corruption survives (no rollback)" \ # ── Part C: an INT/TERM interrupt must terminate, not resume (blocker-A) ────── # A bash signal trap that merely returns lets the script continue past the -# interrupt — restoring the snapshot, then resuming the sync and reporting -# success. The earlier test used a child cp shim to signal its parent, making -# child completion race Bash's interrupted wait. Concurrency is not part of the -# guarded property: sync_framework_keep() runs in the installer's own Bash -# process, and `kill` is a builtin. Generate two installer fixtures that signal -# themselves at the same known mid-sync point. Their TERM handlers emit the same -# observable before diverging, so missing signal delivery fails BOTH arms rather -# than manufacturing a pass. The only semantic difference between fixtures is -# the explicit `exit 1` whose load-bearing behavior this control proves. +# interrupt — restoring the snapshot, then resuming the install and reporting +# success. Generate two installer fixtures that first damage a real target path +# after the snapshot is armed, then signal their own Bash process immediately +# before the normal sync. This fixed injection point is independent of `find` +# enumeration order: after a no-exit handler restores and returns, the complete +# sync still remains to run, so the historical resume bug is deterministic on +# every filesystem. Their TERM handlers emit the same observable before +# diverging, so missing signal delivery fails BOTH arms rather than manufacturing +# a pass. The only semantic difference between fixtures is the explicit `exit 1` +# whose load-bearing behavior this control proves. TERM_MARKER='[test-control] TERM handler entered' HANDLER_WITH_EXIT="trap 'echo \"$TERM_MARKER\" >&2; restore_snapshot; exit 1' TERM # TEST-TERM-HANDLER" HANDLER_WITHOUT_EXIT="trap 'echo \"$TERM_MARKER\" >&2; restore_snapshot' TERM # TEST-TERM-HANDLER" @@ -196,19 +197,17 @@ HANDLER_WITHOUT_EXIT="trap 'echo \"$TERM_MARKER\" >&2; restore_snapshot' TERM # make_signal_installer() { local output="$1" handler="$2" local target_trap="trap 'restore_snapshot; exit 1' ERR INT TERM" - local target_cp=' cp "$abs" "$dst/$rel"' - local inject_open=" if [[ \"\$rel\" == \"$POISON_REL\" ]]; then" - local inject_kill=' kill -TERM "$$" # TEST-TERM-INJECTION' - local inject_close=' fi' + local target_sync='sync_framework' + local inject_damage="printf '%s' '$GARBAGE' > \"\$TARGET_DIR/$POISON_REL\" # TEST-TERM-DAMAGE" + local inject_kill='kill -TERM "$$" # TEST-TERM-INJECTION' if ! awk \ - -v target_trap="$target_trap" -v target_cp="$target_cp" \ - -v handler="$handler" -v inject_open="$inject_open" \ - -v inject_kill="$inject_kill" -v inject_close="$inject_close" ' - $0 == target_cp { - print inject_open + -v target_trap="$target_trap" -v target_sync="$target_sync" \ + -v handler="$handler" -v inject_damage="$inject_damage" \ + -v inject_kill="$inject_kill" ' + $0 == target_sync { + print inject_damage print inject_kill - print inject_close injection_sites++ } { print } @@ -231,7 +230,8 @@ make_signal_installer "$SIGNALED" "$HANDLER_WITH_EXIT" make_signal_installer "$NOEXIT" "$HANDLER_WITHOUT_EXIT" signal_fixture_ready() { local fixture="$1" expected_handler="$2" - [[ "$(grep -cF '# TEST-TERM-INJECTION' "$fixture")" -eq 1 ]] \ + [[ "$(grep -cF '# TEST-TERM-DAMAGE' "$fixture")" -eq 1 ]] \ + && [[ "$(grep -cF '# TEST-TERM-INJECTION' "$fixture")" -eq 1 ]] \ && [[ "$(grep -cF '# TEST-TERM-HANDLER' "$fixture")" -eq 1 ]] \ && grep -Fqx "$expected_handler" "$fixture" } @@ -261,10 +261,12 @@ run_signal_upgrade() { IFS=$'\t' read -r rcC OUTC HC < <(run_signal_upgrade "$SIGNALED") chk "[signal] TERM handler observable fires exactly once" \ "[ \"\$(grep -cF '$TERM_MARKER' '$OUTC')\" -eq 1 ]" -chk "[signal] SIGTERM mid-sync aborts non-zero (trap exits, does not resume)" \ +chk "[signal] SIGTERM after target mutation aborts non-zero (trap exits, does not resume)" \ "[ '$rcC' -ne 0 ]" chk "[signal] restore_snapshot fires on the interrupt" \ "grep -q 'restoring previous state from snapshot' '$OUTC'" +chk "[signal] the deliberately damaged target is restored before termination" \ + "[ \"\$(cat '$HC/$POISON_REL')\" = '$GOOD' ]" chk "[signal] does NOT resume to report sync success after the interrupt" \ "! grep -q 'file phase complete' '$OUTC'" @@ -277,6 +279,8 @@ chk "[control] without 'exit 1' the installer exits zero after resuming" \ "[ '$rcD' -eq 0 ]" chk "[control] without 'exit 1' the trap resumes and reports sync success (the bug)" \ "grep -q 'file phase complete' '$OUTD'" +chk "[control] the resumed full sync mutates the restored target again" \ + "! grep -qxF '$GOOD' '$HD/$POISON_REL' && cmp -s '$FW/$POISON_REL' '$HD/$POISON_REL'" # ── Part D: a failed source/prune `find` scan must abort + roll back (D1) ───── # A `< <(find …)` process substitution discards find's exit status, so an diff --git a/tools/e2e-install-test.sh b/tools/e2e-install-test.sh index 0938b537..878776cb 100755 --- a/tools/e2e-install-test.sh +++ b/tools/e2e-install-test.sh @@ -182,14 +182,19 @@ fi printf '[fixture] installer_exit=%d done_claims=%s\n' \ "$install_status" "$(grep -cF 'Done.' "$install_log" || true)" -# P0 Resolve context -shell="$(getent passwd "$(id -u)" | cut -d: -f7)" -if [[ "$(id -u)" -ne 0 && "$home" == "/home/mosaic" && "$shell" == "/bin/bash" ]] \ +# P0 Resolve context. Keep this final fixture row as discriminating as the +# installer's own P0 row: the expected-RED comparator binds this exact reason. +passwd_row="$(getent passwd "$(id -u)")" +target_user="$(printf '%s' "$passwd_row" | cut -d: -f1)" +passwd_home="$(printf '%s' "$passwd_row" | cut -d: -f6)" +shell="$(printf '%s' "$passwd_row" | cut -d: -f7)" +if [[ "$(id -u)" -eq 1001 && "$target_user" == "mosaic" \ + && "$home" == "/home/mosaic" && "$home" == "$passwd_home" && "$shell" == "/bin/bash" ]] \ && ldd --version 2>&1 | grep -i 'glibc\|gnu libc' >/dev/null \ && [[ "$(node -p 'Number(process.versions.node.split(".")[0])')" -ge 20 ]]; then - phase_pass P0 "target=mosaic uid=$(id -u) HOME=$home shell=$shell libc=glibc node=$(node --version)" + phase_pass P0 "target=$target_user uid=$(id -u) HOME=$home passwd_HOME=$passwd_home shell=$shell privilege=user arch=$(uname -m) libc=glibc node=$(node --version) npm=$(npm --version)" else - phase_fail P0 "context unresolved or unsupported (uid=$(id -u) HOME=$home shell=${shell:-unknown})" + phase_fail P0 "context unresolved or unsupported (target=${target_user:-unknown} uid=$(id -u) HOME=$home passwd_HOME=${passwd_home:-unknown} shell=${shell:-unknown} privilege=user)" fi # P1 Preflight diff --git a/tools/fixtures/greenfield-expected-red.tsv b/tools/fixtures/greenfield-expected-red.tsv index 445b31db..de9353ba 100644 --- a/tools/fixtures/greenfield-expected-red.tsv +++ b/tools/fixtures/greenfield-expected-red.tsv @@ -13,10 +13,11 @@ next-git-present phase P8=FAIL next-git-present phase P9=FAIL next-git-present require ^\[fixture\] resolved lane=next .*version=[0-9]+\.[0-9]+\.[0-9]+-next\. next-git-present require ^\[fixture\] installer_exit=1 done_claims=0$ +next-git-present phase-reason P0=target=mosaic uid=1001 HOME=/home/mosaic passwd_HOME=/home/mosaic shell=/bin/bash privilege=user arch=x86_64 libc=glibc node=v[0-9]+\.[0-9]+\.[0-9]+ npm=[0-9]+\.[0-9]+\.[0-9]+ next-git-present require ^\[SECRET-CONTROL\] PASS: next-git-present require ^\[P3\] PASS: absolute_path=.* version=.* equals resolved lane version$ next-git-present require ^\[P4\] FAIL: NOT-MEASURED / UNDECLARED: -next-git-present require ^\[P6\] FAIL: broker absent but dead enforcement hooks are active +next-git-present require ^\[P6\] FAIL: runtime linking/activation action reported a required failure$ next-git-present forbid Done\.|MOSAIC_C1_CANARY_|CLI not found on PATH main-git-present exit 1 main-git-present phase P0=PASS @@ -31,6 +32,7 @@ main-git-present phase P8=FAIL main-git-present phase P9=FAIL main-git-present require ^\[fixture\] resolved lane=main .*version=[0-9]+\.[0-9]+\.[0-9]+$ main-git-present require ^\[fixture\] installer_exit=1 done_claims=0$ +main-git-present phase-reason P0=target=mosaic uid=1001 HOME=/home/mosaic passwd_HOME=/home/mosaic shell=/bin/bash privilege=user arch=x86_64 libc=glibc node=v[0-9]+\.[0-9]+\.[0-9]+ npm=[0-9]+\.[0-9]+\.[0-9]+ main-git-present require ^\[SECRET-CONTROL\] PASS: main-git-present require ^\[P3\] PASS: absolute_path=.* version=.* equals resolved lane version$ main-git-present require ^\[P4\] FAIL: NOT-MEASURED / UNDECLARED: @@ -48,6 +50,7 @@ next-git-absent phase P7=PASS next-git-absent phase P8=FAIL next-git-absent phase P9=FAIL next-git-absent require ^\[fixture\] installer_exit=1 done_claims=0$ +next-git-absent phase-reason P0=target=mosaic uid=1001 HOME=/home/mosaic passwd_HOME=/home/mosaic shell=/bin/bash privilege=user arch=x86_64 libc=glibc node=v[0-9]+\.[0-9]+\.[0-9]+ npm=[0-9]+\.[0-9]+\.[0-9]+ next-git-absent require ^\[SECRET-CONTROL\] PASS: next-git-absent require ^\[P1\] FAIL: undeclared/missing prerequisite\(s\)=git; next-git-absent require ^\[P3\] FAIL: .*executable=no diff --git a/tools/install-next-lane.test.sh b/tools/install-next-lane.test.sh index b69bdd2a..fed59b1c 100755 --- a/tools/install-next-lane.test.sh +++ b/tools/install-next-lane.test.sh @@ -201,6 +201,9 @@ if [[ -z "$dest" ]]; then exit 1 fi mkdir -p "$dest/stack/packages/mosaic/framework" "$dest/stack/apps/gateway" +if [[ "${MOSAIC_TEST_EXTRA_ARCHIVE_ROOT:-0}" == "1" ]]; then + mkdir -p "$dest/unexpected-second-root" +fi cat > "$dest/stack/packages/mosaic/framework/install.sh" <<'FRAMEWORK' #!/usr/bin/env bash set -euo pipefail @@ -384,6 +387,29 @@ set -e grep -qF 'archive safety/integrity check failed' <<<"$OUTPUT" [[ "$(node -p "require('$ACTIVE').status")" == "rolled-back" ]] +reset_state +echo "[test] source archive with multiple extracted roots fails instead of selecting by find order" +before_prefix="$(prefix_fingerprint)" +set +e +OUTPUT="$( + HOME="$HOME_DIR" \ + MOSAIC_HOME="$MOSAIC_HOME" \ + MOSAIC_PREFIX="$PREFIX" \ + MOSAIC_NO_COLOR=1 \ + MOSAIC_TEST_NPM_LOG="$LOG" \ + MOSAIC_TEST_STATE="$STATE" \ + MOSAIC_TEST_FAIL_NEXT_GATEWAY_INSTALL=1 \ + MOSAIC_TEST_EXTRA_ARCHIVE_ROOT=1 \ + PATH="$FAKE_BIN:$PATH" \ + bash "$ROOT/tools/install.sh" --cli --next --yes --no-auto-launch 2>&1 +)" +FAIL_STATUS=$? +set -e +[[ "$FAIL_STATUS" -ne 0 ]] +[[ "$(prefix_fingerprint)" == "$before_prefix" ]] +grep -qF 'expected exactly one extracted source root' <<<"$OUTPUT" +[[ "$(node -p "require('$ACTIVE').status")" == "rolled-back" ]] + reset_state echo "[test] --dev source install does not require registry version resolution" OUTPUT="$( diff --git a/tools/install-state-machine.test.sh b/tools/install-state-machine.test.sh index 1fb31b16..65645c03 100755 --- a/tools/install-state-machine.test.sh +++ b/tools/install-state-machine.test.sh @@ -304,6 +304,34 @@ grep -q '^\[P4\] FAIL:.*owner/mode policy' "$TMP/p4-tree-mode.log" \ || fail_case 'P4 did not attribute unsafe created-path mode' chmod 0644 "$good_mosaic/AGENTS.md" +printf '[test] case: P4 fails closed when created-path enumeration is incomplete\n' +real_find="$(command -v find)" +cat > "$good_bin/find" <