fix(installer): avoid pipefail marker race
This commit is contained in:
+1
-1
@@ -1393,7 +1393,7 @@ A from-zero install can report success while leaving the target host unusable be
|
||||
12. The expected-RED comparator SHALL validate the complete manifest before selecting a case: exact case population, one exit and P0–P9 disposition per case, pinned require/forbid classes, and no malformed, duplicate, or unknown rows.
|
||||
13. The published installer contract SHALL reject failed fetches, HTTP-success empty bodies, and digest mismatch, then execute the exact digest-verified body. The remote CI arm SHALL enumerate every payload-acquisition path and report a bound/found denominator. It SHALL bind both the installer body and the downstream stack framework/source it consumes to the same immutable CI commit, while retaining `--next` as the lane selector. Source resolution/acquisition SHALL fail closed, and the arm SHALL verify the realised source commit and archive digest after installation. Any out-of-scope unpinned sibling SHALL be named and counted rather than silently included in a broader pinning claim. `ASSUMPTION:` the configured repository's authenticated exact-commit endpoint is trusted to map that commit ID to the returned archive bytes; independent signed provenance/authenticity against repository or TLS trust-root compromise remains excluded by canonical greenfield-install PRD v2 §3.
|
||||
14. Phase diagnostics SHALL be redacted before terminal or durable-log output. A seeded positive-control canary SHALL remain absent from observed argv, output, command logs, npm configuration, generated files, and shell history.
|
||||
15. The CI fixture SHALL publish pipeline-level execution coverage for both the unconditional case set derived from the expected-RED manifest and the explicitly declared required pipeline-arm set, including the immutable remote-installer contract. Per-case and per-arm success markers SHALL be run-scoped, stored beneath an existing checkout-archive exclusion, and written only after that arm's verifier passes. A final step SHALL depend on the complete fixture matrix, run after prior success or failure, emit both `cases_defined=N cases_executed=M` and `arms_defined=N arms_executed=M`, and invoke the same testable aggregation helper whose complete case/arm PASS/FAIL truth table is covered. It SHALL fail unless both expected/executed name sets are exactly equal; missing, unexpected, stale, newly added unexecuted, or checkout-contaminating state SHALL fail closed. The archive-purity control SHALL bind to the production checkout-archive selector so deleting the production exclusion makes the control RED. Coverage tooling SHALL execute under both the canonical Alpine/BusyBox CI image and the Debian greenfield image; a green result from either runtime alone is insufficient portability evidence.
|
||||
15. The CI fixture SHALL publish pipeline-level execution coverage for both the unconditional case set derived from the expected-RED manifest and the explicitly declared required pipeline-arm set, including the immutable remote-installer contract. Per-case and per-arm success markers SHALL be run-scoped, stored beneath an existing checkout-archive exclusion, and written only after that arm's verifier passes. A final step SHALL depend on the complete fixture matrix, run after prior success or failure, emit both `cases_defined=N cases_executed=M` and `arms_defined=N arms_executed=M`, and invoke the same testable aggregation helper whose complete case/arm PASS/FAIL truth table is covered. It SHALL fail unless both expected/executed name sets are exactly equal; missing, unexpected, stale, newly added unexecuted, or checkout-contaminating state SHALL fail closed. The archive-purity control SHALL bind to the production checkout-archive selector so deleting the production exclusion makes the control RED. Coverage tooling SHALL execute under both the canonical Alpine/BusyBox CI image and the Debian greenfield image; a green result from either runtime alone is insufficient portability evidence. Expected-set membership SHALL consume a fully materialized producer result so an early-closing consumer cannot turn a valid marker into a `pipefail` rejection.
|
||||
|
||||
### C1 acceptance criteria
|
||||
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
# #1050 C1 fix round — Round 8 pipefail-safe expected-set membership
|
||||
|
||||
BASE HEAD
|
||||
- 58ada98d2b430da28c46cf92eebdb6d45b8b1846
|
||||
- Review 122 (rev-security-02) found expected_names | grep -Fxq could falsely reject valid markers under pipefail when grep closed the pipe after an early match and the producer exited 141.
|
||||
- Independent canonical-Alpine live-manifest stress reproduced intermittent false failures at the current four-arm set size. No small-manifest safe threshold is claimed.
|
||||
|
||||
DETERMINISTIC RED
|
||||
- The focused suite now generates a 20,001-name manifest with a valid target sorted first and invokes the real mark path.
|
||||
- Before remediation, in canonical Alpine ci-base:
|
||||
docker run --rm -u "$(id -u):$(id -g)" -v "$PWD:/workspace" -w /workspace git.mosaicstack.dev/mosaicstack/stack/ci-base:latest bash tools/verify-greenfield-execution-coverage.test.sh
|
||||
=> canonical_sigpipe_tdd_red_rc=1
|
||||
=> [fixture-suite] case is not in the expected set: a-target
|
||||
- This control exercises the production checker invocation rather than an approximated producer and makes the early-close race deterministic by exceeding pipe capacity.
|
||||
|
||||
IMPLEMENTATION
|
||||
- The mark path now fully materializes expected_names into expected_snapshot and fails closed if production fails.
|
||||
- grep -Fxq reads the completed snapshot through a here-string; there is no producer/consumer pipeline to close early.
|
||||
- Missing names still fail with the same attributable message. No `|| true` or other failure suppression was introduced.
|
||||
- Exact-set checks, marker inventory, aggregation, archive binding, fixture dispositions, expected-RED manifest, e2e installer, #869 wiring, and acquisition behavior are unchanged.
|
||||
|
||||
GREEN
|
||||
- Local focused suite: greenfield execution coverage tests passed.
|
||||
- Canonical Alpine focused suite: greenfield execution coverage tests passed.
|
||||
- Canonical Alpine live production-manifest stress: 1,000 valid greenfield-git-absent arm marks, failures=0.
|
||||
- Canonical grep-q pipeline mutant through the real checker path => rc=1 with the attributable false rejection; restored subject => GREEN.
|
||||
- Existing Round-6 controls retained:
|
||||
ignore-arms aggregation helper => rc=1
|
||||
ignore-cases aggregation helper => rc=1
|
||||
always-RED aggregation helper => rc=1
|
||||
delete production .mosaic-test-work archive exclusion => rc=1
|
||||
- Round-8 mutant workspace: /tmp/c1-round8-mutants.aS1xTI
|
||||
|
||||
FENCED FILES
|
||||
- tools/e2e-install-test.sh HEAD/worktree SHA-256: a93113565aa69f2c6f3d792b78251021bb3bbe3f7813d5fed547ab0099fa3b98 (byte-identical)
|
||||
- tools/fixtures/greenfield-expected-red.tsv HEAD/worktree SHA-256: baae06c664d0ab7ffe4c32ec106bfcb6f9dd74b46461d3c4440b73dc582af1a2 (byte-identical)
|
||||
|
||||
LOCAL BASELINES
|
||||
- /home free before and after validation: 11G
|
||||
- Bash syntax focused scripts: PASS
|
||||
- ShellCheck focused scripts: PASS
|
||||
- pnpm test:installer: PASS
|
||||
- pnpm typecheck: PASS (45/45)
|
||||
- pnpm lint: PASS (25/25)
|
||||
- pnpm format:check: PASS
|
||||
- woodpecker-cli lint --strict .woodpecker/greenfield-install.yml: PASS
|
||||
- git diff --check: PASS
|
||||
|
||||
INDEPENDENT REVIEW
|
||||
- Codex code review: APPROVE, confidence 0.96, six files reviewed, zero findings. It confirmed full materialization removes the SIGPIPE race and the deterministic regression control exercises the real early-match mark path.
|
||||
- Codex security review: risk NONE, confidence 0.98, six files reviewed, zero findings. It confirmed fail-closed producer handling, constrained/quoted names, and no new injection, traversal, secret, access-control, cryptographic, dependency, or logging risk.
|
||||
- The code-review sandbox could not execute the suite or install ShellCheck because its filesystem is read-only; local and canonical-container dynamic evidence above supplies those gates.
|
||||
|
||||
PENDING
|
||||
- commit, queue guard, one lease-pinned push, provider attribution read-back
|
||||
- replacement-head canonical CI; no manual trigger or polling
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
## Required artifacts
|
||||
|
||||
- [x] `docs/PRD.md` exists; #1050 C1 requirement 13 binds the complete in-scope remote stack source chain, and requirement 15 binds pipeline-level exact case+arm execution coverage, executable aggregation, production-bound archive purity, and canonical Alpine/BusyBox plus Debian portability.
|
||||
- [x] `docs/PRD.md` exists; #1050 C1 requirement 13 binds the complete in-scope remote stack source chain, and requirement 15 binds exact case+arm execution coverage, executable aggregation, production-bound archive purity, dual-runtime portability, and pipefail-safe materialized membership.
|
||||
- [x] User guide: not applicable; no user-facing production installer behavior changed.
|
||||
- [x] Admin guide: not applicable; no operator procedure or deployment behavior changed.
|
||||
- [x] Developer guide: existing `docs/guides/installer-state-machine.md` defines the fail-closed installer model; the C1-specific remote-arm contract and evidence live in the PRD, scratchpad, and scoped verification report.
|
||||
@@ -24,4 +24,5 @@
|
||||
- [x] Round-5 exact arm coverage and archive-purity controls reviewed: Codex code APPROVE (confidence 0.93, zero findings) and security risk NONE (confidence 0.96, zero findings).
|
||||
- [x] Round-6 executable aggregation and production archive-selector binding reviewed: Codex code APPROVE (confidence 0.94, zero findings) and security risk NONE (confidence 0.97, zero findings).
|
||||
- [x] Round-7 BusyBox-safe marker inventory and dual-runtime evidence reviewed: Codex code APPROVE (confidence 0.96, zero findings) and security risk NONE (confidence 0.97, zero findings).
|
||||
- [x] Round-8 materialized membership and deterministic canonical-Alpine control reviewed: Codex code APPROVE (confidence 0.96, zero findings) and security risk NONE (confidence 0.98, zero findings).
|
||||
- [x] Canonical evidence remains in-repo. No external publishing action was requested or performed.
|
||||
|
||||
@@ -176,3 +176,11 @@ Denominator corrections were sent to and accepted by the lane lead before implem
|
||||
- The focused suite is green both locally and inside `git.mosaicstack.dev/mosaicstack/stack/ci-base:latest`. Canonical pipeline evidence remains pending the replacement frozen-head push; no stale pipeline is inherited.
|
||||
- Final review: Codex code APPROVE confidence 0.96 and security risk NONE confidence 0.97, both with zero findings. Full installer tests, typecheck 45/45, lint 25/25, format, Bash syntax, ShellCheck, Woodpecker strict lint, dual-runtime focused tests, retained Round-6 mutants, and diff check pass with 11G free.
|
||||
- Full evidence: `docs/reports/verification/1050-c1-fix-round/13-round7-busybox-portability.txt`.
|
||||
|
||||
## Round 8 — pipefail-safe expected-set membership
|
||||
|
||||
- Review 122 found `expected_names | grep -Fxq` could falsely reject a valid early-sorted marker: `grep -q` closes after its match, the upstream sorter can exit 141, and `pipefail` selects that producer failure. Independent canonical-Alpine stress measured failures at the live four-arm manifest size, so no safe small-set threshold is claimed.
|
||||
- A deterministic 20,001-entry real-script mark control makes the unfixed canonical-Alpine path RED with `case is not in the expected set: a-target`. The mark path now materializes and validates the complete expected-name snapshot before applying `grep -q` via a here-string, eliminating the producer/consumer pipe while retaining fail-closed producer errors.
|
||||
- The focused suite is green locally and in canonical Alpine after remediation. A separate 1,000-mark canonical-Alpine stress against the live four-arm production manifest recorded zero false failures.
|
||||
- Final review: Codex code APPROVE confidence 0.96 and security risk NONE confidence 0.98, both with zero findings. Full installer tests, typecheck 45/45, lint 25/25, format, Bash syntax, ShellCheck, Woodpecker strict lint, canonical deterministic/stress controls, retained Round-6 mutants, and diff check pass with 11G free.
|
||||
- Full evidence: `docs/reports/verification/1050-c1-fix-round/14-round8-pipefail-membership.txt`.
|
||||
|
||||
@@ -56,7 +56,11 @@ case "$mode" in
|
||||
|| { echo "[fixture-suite] invalid $singular marker name: $name" >&2; exit 2; }
|
||||
[[ -d "$run_dir" ]] \
|
||||
|| { echo "[fixture-suite] $dimension state was not initialized: $run_id" >&2; exit 1; }
|
||||
expected_names | grep -Fxq -- "$name" \
|
||||
if ! expected_snapshot="$(expected_names)"; then
|
||||
echo "[fixture-suite] failed to read expected $dimension" >&2
|
||||
exit 1
|
||||
fi
|
||||
grep -Fxq -- "$name" <<<"$expected_snapshot" \
|
||||
|| { echo "[fixture-suite] $singular is not in the expected set: $name" >&2; exit 1; }
|
||||
: > "$run_dir/$name.ran"
|
||||
;;
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
set -euo pipefail
|
||||
|
||||
ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
SUBJECT="$ROOT/tools/verify-greenfield-execution-coverage.sh"
|
||||
SUBJECT="${GREENFIELD_COVERAGE_SUBJECT:-$ROOT/tools/verify-greenfield-execution-coverage.sh}"
|
||||
GATE="${GREENFIELD_COVERAGE_GATE:-$ROOT/tools/verify-greenfield-execution-coverage-gate.sh}"
|
||||
WORKFLOW="$ROOT/.woodpecker/greenfield-install.yml"
|
||||
INSTALLER="${GREENFIELD_E2E_INSTALLER:-$ROOT/tools/e2e-install-test.sh}"
|
||||
@@ -66,6 +66,15 @@ done
|
||||
complete_output="$(bash "$SUBJECT" check cases "$manifest" "$state_root" "$run_id")"
|
||||
grep -qF '[fixture-suite] cases_defined=3 cases_executed=3' <<<"$complete_output"
|
||||
|
||||
# A successful early match must consume the full producer and remain green under pipefail.
|
||||
large_manifest="$TMP/large-expected.tsv"
|
||||
awk 'BEGIN {
|
||||
print "a-target\texit\t1"
|
||||
for (i = 0; i < 20000; i++) printf "z-case-%05d\texit\t1\n", i
|
||||
}' > "$large_manifest"
|
||||
bash "$SUBJECT" init cases "$large_manifest" "$state_root" large-set
|
||||
bash "$SUBJECT" mark cases "$large_manifest" "$state_root" large-set a-target
|
||||
|
||||
# The production manifest, including comments, initializes and checks.
|
||||
production_manifest="$ROOT/tools/fixtures/greenfield-expected-red.tsv"
|
||||
production_arms="$ROOT/tools/fixtures/greenfield-expected-arms.txt"
|
||||
|
||||
Reference in New Issue
Block a user