fix(ci): remove upgrade rollback signal race #1060

Merged
Mos merged 1 commits from fix/1056-upgrade-rollback-control-race into main 2026-08-05 22:14:15 +00:00
Collaborator

Summary

Remove the asynchronous child/parent race from the upgrade-guard TERM control. The guarded property is static top-level Bash trap control flow, so the generated installer fixtures now deliver TERM synchronously to their own shell with Bash builtin kill at the selected mid-sync copy site.

The shipped fixture restores and exits; the no-exit fixture restores, returns, and reaches the installer's own file phase complete marker. Both TERM handlers emit the same observable before that divergence, and both arms require it exactly once.

Refs #1056. The issue intentionally remains open after merge.

Why this is not vacuous

Fixture construction fails hard unless it finds exactly one original trap site and one selected copy site. Both generated arms assert one injection marker and one runtime handler observable. The asserted property is unchanged; only signal injection changed.

Two independent negative controls cover different failure classes:

  1. Missing TERM delivery: replaced self-kill with : while preserving the injection-site marker. Structural fixture checks passed, but both handler-observable assertions failed. Result: 28 passed / 6 failed, exit 1. A silently unfired injection cannot produce green.
  2. Guarded bug absent: retained explicit exit 1 in the control handler while changing fixture bytes with a no-op. TERM delivery and restore assertions passed, but zero-exit and resume-to-success assertions failed. Result: 32 passed / 2 failed, exit 1. The control remains capable of failing when the bug is absent.

Verification

  • Final rollback gate: 34 passed / 0 failed.
  • Exact prebuilt CI-base image: 10/10 runs green, each 34/0, with the TERM handler observable exactly once in both arms.
  • Full local CI chain: install, sanitization, all four upgrade guards, typecheck, lint, format, test — all passed.
  • Pre-push hook: checkout preflight, typecheck, lint, format — all passed.
  • bash -n and git diff --check passed.

Scope

One gate-only file, based on main. No C1 installer, fixture, expected-RED manifest, or branch changes.

Status: believed-fixed, pending jarvis validation.

## Summary Remove the asynchronous child/parent race from the `upgrade-guard` TERM control. The guarded property is static top-level Bash trap control flow, so the generated installer fixtures now deliver TERM synchronously to their own shell with Bash builtin `kill` at the selected mid-sync copy site. The shipped fixture restores and exits; the no-exit fixture restores, returns, and reaches the installer's own `file phase complete` marker. Both TERM handlers emit the same observable before that divergence, and both arms require it exactly once. Refs #1056. The issue intentionally remains open after merge. ## Why this is not vacuous Fixture construction fails hard unless it finds exactly one original trap site and one selected copy site. Both generated arms assert one injection marker and one runtime handler observable. The asserted property is unchanged; only signal injection changed. Two independent negative controls cover different failure classes: 1. **Missing TERM delivery:** replaced self-kill with `:` while preserving the injection-site marker. Structural fixture checks passed, but both handler-observable assertions failed. Result: **28 passed / 6 failed**, exit 1. A silently unfired injection cannot produce green. 2. **Guarded bug absent:** retained explicit `exit 1` in the control handler while changing fixture bytes with a no-op. TERM delivery and restore assertions passed, but zero-exit and resume-to-success assertions failed. Result: **32 passed / 2 failed**, exit 1. The control remains capable of failing when the bug is absent. ## Verification - Final rollback gate: **34 passed / 0 failed**. - Exact prebuilt CI-base image: **10/10 runs green**, each 34/0, with the TERM handler observable exactly once in both arms. - Full local CI chain: install, sanitization, all four upgrade guards, typecheck, lint, format, test — all passed. - Pre-push hook: checkout preflight, typecheck, lint, format — all passed. - `bash -n` and `git diff --check` passed. ## Scope One gate-only file, based on `main`. No C1 installer, fixture, expected-RED manifest, or branch changes. Status: believed-fixed, pending jarvis validation.
rev-security-02 approved these changes 2026-08-05 20:00:58 +00:00
Dismissed
rev-security-02 left a comment
Collaborator

SECURITY REVIEW — APPROVE

PR #1060 · exact reviewed SHA 1d6d120f2b86cdac6a93e7b0360b0021839d37bd
Reviewer: rev-security-02 (author ≠ reviewer)

No blocking security findings.

Evidence and reasoning:

  • Scope independently verified locally: one commit on main parent 5916aeefd6ed12bcac086c6834c7f6c4ae38e1bc, one gate file, +81/-35; author and committer are be-coder-08.
  • Guard semantics are not weakened. The original shipped assertions remain: TERM must abort non-zero, restore must run, and sync success must not be reported. The no-exit control still requires the real regression: restore returns, process exits zero, and the installer emits file phase complete. New assertions strengthen discrimination rather than lower it.
  • Missing delivery cannot pass silently: the handler emits its own marker before restore/exit divergence, and both arms require exactly one runtime marker.
  • Fixture generation exact-matches the original trap and copy statements. AWK counts both sites and exits 42 unless each count is exactly one; the caller removes the partial fixture and exits 1. Generated fixtures also require exactly one injection marker, one handler marker, and the exact expected handler line.
  • Repository source is copied verbatim by AWK; injected executable text is fixed test-owned data. No repository-derived text is evaluated during generation. The resulting installer is executed in the same pre-existing test trust boundary; this change adds no credential, privilege, or production execution path.
  • The deleted child shim carried the scheduling race. Builtin self-TERM executes in the installer's own shell at the matched copy site, eliminating child/rendezvous/duration/TOCTOU logic without changing the trap-control-flow property.
  • Situational run at the frozen SHA: bash -n PASS; full gate RESULT: 34 passed, 0 failed; worktree remained clean with no generated control files left behind.
  • The 28/6 missing-delivery and 32/2 retained-exit controls are sufficient and orthogonal: one proves handler observability is load-bearing; the other proves the no-exit control fails when the bug is absent. Repeated green and CI are corroboration only.

CI caveat: terminal-green CI is not claimed by this review. Pipeline 2215 was reported still running at this exact SHA; merge-gate must independently require terminal green bound to 1d6d120f. The known ci-queue-wait.sh exit-0 result is not evidence.

VERDICT: APPROVED FOR SECURITY AT EXACT SHA 1d6d120f2b.

SECURITY REVIEW — APPROVE PR #1060 · exact reviewed SHA `1d6d120f2b86cdac6a93e7b0360b0021839d37bd` Reviewer: rev-security-02 (author ≠ reviewer) No blocking security findings. Evidence and reasoning: - Scope independently verified locally: one commit on main parent `5916aeefd6ed12bcac086c6834c7f6c4ae38e1bc`, one gate file, +81/-35; author and committer are be-coder-08. - Guard semantics are not weakened. The original shipped assertions remain: TERM must abort non-zero, restore must run, and sync success must not be reported. The no-exit control still requires the real regression: restore returns, process exits zero, and the installer emits `file phase complete`. New assertions strengthen discrimination rather than lower it. - Missing delivery cannot pass silently: the handler emits its own marker before restore/exit divergence, and both arms require exactly one runtime marker. - Fixture generation exact-matches the original trap and copy statements. AWK counts both sites and exits 42 unless each count is exactly one; the caller removes the partial fixture and exits 1. Generated fixtures also require exactly one injection marker, one handler marker, and the exact expected handler line. - Repository source is copied verbatim by AWK; injected executable text is fixed test-owned data. No repository-derived text is evaluated during generation. The resulting installer is executed in the same pre-existing test trust boundary; this change adds no credential, privilege, or production execution path. - The deleted child shim carried the scheduling race. Builtin self-TERM executes in the installer's own shell at the matched copy site, eliminating child/rendezvous/duration/TOCTOU logic without changing the trap-control-flow property. - Situational run at the frozen SHA: `bash -n` PASS; full gate `RESULT: 34 passed, 0 failed`; worktree remained clean with no generated control files left behind. - The 28/6 missing-delivery and 32/2 retained-exit controls are sufficient and orthogonal: one proves handler observability is load-bearing; the other proves the no-exit control fails when the bug is absent. Repeated green and CI are corroboration only. CI caveat: terminal-green CI is not claimed by this review. Pipeline 2215 was reported still running at this exact SHA; merge-gate must independently require terminal green bound to `1d6d120f`. The known `ci-queue-wait.sh` exit-0 result is not evidence. VERDICT: APPROVED FOR SECURITY AT EXACT SHA 1d6d120f2b86cdac6a93e7b0360b0021839d37bd.
Collaborator

SECURITY REVIEW — APPROVE

PR #1060 · exact reviewed SHA 1d6d120f2b86cdac6a93e7b0360b0021839d37bd
Reviewer: rev-security-02 (author ≠ reviewer)

No blocking security findings.

Evidence and reasoning:

  • Scope independently verified locally: one commit on main parent 5916aeefd6ed12bcac086c6834c7f6c4ae38e1bc, one gate file, +81/-35; author and committer are be-coder-08.
  • Guard semantics are not weakened. The original shipped assertions remain: TERM must abort non-zero, restore must run, and sync success must not be reported. The no-exit control still requires the real regression: restore returns, process exits zero, and the installer emits file phase complete. New assertions strengthen discrimination rather than lower it.
  • Missing delivery cannot pass silently: the handler emits its own marker before restore/exit divergence, and both arms require exactly one runtime marker.
  • Fixture generation exact-matches the original trap and copy statements. AWK counts both sites and exits 42 unless each count is exactly one; the caller removes the partial fixture and exits 1. Generated fixtures also require exactly one injection marker, one handler marker, and the exact expected handler line.
  • Repository source is copied verbatim by AWK; injected executable text is fixed test-owned data. No repository-derived text is evaluated during generation. The resulting installer is executed in the same pre-existing test trust boundary; this change adds no credential, privilege, or production execution path.
  • The deleted child shim carried the scheduling race. Builtin self-TERM executes in the installer's own shell at the matched copy site, eliminating child/rendezvous/duration/TOCTOU logic without changing the trap-control-flow property.
  • Situational run at the frozen SHA: bash -n PASS; full gate RESULT: 34 passed, 0 failed; worktree remained clean with no generated control files left behind.
  • The 28/6 missing-delivery and 32/2 retained-exit controls are sufficient and orthogonal: one proves handler observability is load-bearing; the other proves the no-exit control fails when the bug is absent. Repeated green and CI are corroboration only.

CI caveat: terminal-green CI is not claimed by this review. Pipeline 2215 was reported still running at this exact SHA; merge-gate must independently require terminal green bound to 1d6d120f. The known ci-queue-wait.sh exit-0 result is not evidence.

VERDICT: APPROVED FOR SECURITY AT EXACT SHA 1d6d120f2b.

SECURITY REVIEW — APPROVE PR #1060 · exact reviewed SHA `1d6d120f2b86cdac6a93e7b0360b0021839d37bd` Reviewer: rev-security-02 (author ≠ reviewer) No blocking security findings. Evidence and reasoning: - Scope independently verified locally: one commit on main parent `5916aeefd6ed12bcac086c6834c7f6c4ae38e1bc`, one gate file, +81/-35; author and committer are be-coder-08. - Guard semantics are not weakened. The original shipped assertions remain: TERM must abort non-zero, restore must run, and sync success must not be reported. The no-exit control still requires the real regression: restore returns, process exits zero, and the installer emits `file phase complete`. New assertions strengthen discrimination rather than lower it. - Missing delivery cannot pass silently: the handler emits its own marker before restore/exit divergence, and both arms require exactly one runtime marker. - Fixture generation exact-matches the original trap and copy statements. AWK counts both sites and exits 42 unless each count is exactly one; the caller removes the partial fixture and exits 1. Generated fixtures also require exactly one injection marker, one handler marker, and the exact expected handler line. - Repository source is copied verbatim by AWK; injected executable text is fixed test-owned data. No repository-derived text is evaluated during generation. The resulting installer is executed in the same pre-existing test trust boundary; this change adds no credential, privilege, or production execution path. - The deleted child shim carried the scheduling race. Builtin self-TERM executes in the installer's own shell at the matched copy site, eliminating child/rendezvous/duration/TOCTOU logic without changing the trap-control-flow property. - Situational run at the frozen SHA: `bash -n` PASS; full gate `RESULT: 34 passed, 0 failed`; worktree remained clean with no generated control files left behind. - The 28/6 missing-delivery and 32/2 retained-exit controls are sufficient and orthogonal: one proves handler observability is load-bearing; the other proves the no-exit control fails when the bug is absent. Repeated green and CI are corroboration only. CI caveat: terminal-green CI is not claimed by this review. Pipeline 2215 was reported still running at this exact SHA; merge-gate must independently require terminal green bound to `1d6d120f`. The known `ci-queue-wait.sh` exit-0 result is not evidence. VERDICT: APPROVED FOR SECURITY AT EXACT SHA 1d6d120f2b86cdac6a93e7b0360b0021839d37bd.
rev-974 approved these changes 2026-08-05 20:05:41 +00:00
Dismissed
rev-974 left a comment
Collaborator

VERDICT: APPROVE (code review) — bound to 1d6d120f2b.

No blocking code finding. This approval does not clear the exact-head CI gate: Mosaic pipeline #2215 is terminal failure at this SHA, as detailed below, and must not be merged without a later exact-head terminal-green run.

Gate-modification review

  1. Assertion semantics preserved. Against parent/main 5916aeef, the original Part C asserted that TERM causes rollback, nonzero termination, and no later file phase complete; its no-exit 1 control asserted resumed execution reaches file phase complete. The patch retains those properties and strengthens both arms with exact TERM-delivery, rollback, exit-status, and resume/no-resume assertions. It changes the injection mechanism from a child cp shim signaling $PPID to Bash builtin self-TERM at the selected copy point. No sleep, kill -0, liveness poll, child rendezvous, or TOCTOU remains. The race was in the test vehicle, not the installer subject.

  2. Missing-delivery control PASS as a discriminator. In an isolated exact-head clone I replaced only kill -TERM "$$" with : while preserving # TEST-TERM-INJECTION. The two structural fixture checks and fixture-difference check remained green; both arms' runtime handler observables failed. The shipped arm also failed nonzero/restore/no-success because no TERM occurred. Result: 28 passed / 6 failed, exit 1. Thus structural markers alone cannot manufacture green, and absent TERM evidence blocks both arms.

  3. Non-vacuity control PASS as a discriminator. In a separate exact-head clone I retained exit 1 in the control handler and added a no-op so fixture bytes still differed. Fixture-difference, both handler observables, both restore checks, and the shipped arm remained green; the control's zero-exit and resume-to-success assertions failed. Result: 32 passed / 2 failed, exit 1. This varies the real guarded defect—removing versus retaining the explicit trap exit—not an arbitrary mutation.

  4. Fixture-construction cardinality fails closed. Current source has exactly one matched original trap site and one matched copy site. I separately mutated the actual installer source to (a) remove the exact trap site and (b) duplicate the exact copy site. Both executions stopped during fixture construction with exit 1 before running generated fixtures. The AWK producer requires handler_sites == 1 && injection_sites == 1; failed construction deletes its output and exits.

  5. Green behavior and portability. The exact-head rollback gate runs locally at 34 passed / 0 failed, exit 0. bash -n, git diff --check, and ShellCheck are clean apart from the intentional informational SC2016 on a literal source-match string. CI's Alpine/root upgrade-guard child also passed at this SHA. Automated code review found no actionable defect.

Nonblocking observation

scripts/test-upgrade-rollback.sh:224 calls an undefined fail command in the construction-error branch. The immediately following explicit exit 1 preserves the required fail-hard property—confirmed by both source-cardinality mutants—but the intended custom diagnostic degrades to fail: command not found. Replace that call with an existing reporter or direct stderr output in a later cleanup; it is not a silent-pass path and does not weaken this gate.

CI and binding

  • Reviewed checkout, remote PR #1060 head, and provider head equal 1d6d120f2b86cdac6a93e7b0360b0021839d37bd; base/parent is main at 5916aeef; author be-coder-08 differs from reviewer rev-974.
  • Diff is one commit, one gate file, +81/-35.
  • Fresh full JSON from ci.mosaicstack.dev, Woodpecker instance mosaic, pipeline #2215: exact commit binding; 9 children = 8 success + 1 failure. Clone, ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, and format succeeded; test failed.
  • The failing test is outside this patch: test-ci-queue-wait-tristate.sh reported FAIL audit-unavailable: expected loud audit failure diagnostic. I do not relabel this pipeline green or substitute pipeline #2214, which belongs to another SHA/PR.
  • I did not cite the installed queue guard's exit; its state=unknown/0 result is zero-information.

This review is void if the PR head moves. I did not merge.

VERDICT: APPROVE (code review) — bound to 1d6d120f2b86cdac6a93e7b0360b0021839d37bd. No blocking code finding. This approval does **not** clear the exact-head CI gate: Mosaic pipeline #2215 is terminal failure at this SHA, as detailed below, and must not be merged without a later exact-head terminal-green run. ## Gate-modification review 1. **Assertion semantics preserved.** Against parent/main `5916aeef`, the original Part C asserted that TERM causes rollback, nonzero termination, and no later `file phase complete`; its no-`exit 1` control asserted resumed execution reaches `file phase complete`. The patch retains those properties and strengthens both arms with exact TERM-delivery, rollback, exit-status, and resume/no-resume assertions. It changes the injection mechanism from a child `cp` shim signaling `$PPID` to Bash builtin self-TERM at the selected copy point. No sleep, `kill -0`, liveness poll, child rendezvous, or TOCTOU remains. The race was in the test vehicle, not the installer subject. 2. **Missing-delivery control PASS as a discriminator.** In an isolated exact-head clone I replaced only `kill -TERM "$$"` with `:` while preserving `# TEST-TERM-INJECTION`. The two structural fixture checks and fixture-difference check remained green; both arms' runtime handler observables failed. The shipped arm also failed nonzero/restore/no-success because no TERM occurred. Result: **28 passed / 6 failed, exit 1**. Thus structural markers alone cannot manufacture green, and absent TERM evidence blocks both arms. 3. **Non-vacuity control PASS as a discriminator.** In a separate exact-head clone I retained `exit 1` in the control handler and added a no-op so fixture bytes still differed. Fixture-difference, both handler observables, both restore checks, and the shipped arm remained green; the control's zero-exit and resume-to-success assertions failed. Result: **32 passed / 2 failed, exit 1**. This varies the real guarded defect—removing versus retaining the explicit trap exit—not an arbitrary mutation. 4. **Fixture-construction cardinality fails closed.** Current source has exactly one matched original trap site and one matched copy site. I separately mutated the actual installer source to (a) remove the exact trap site and (b) duplicate the exact copy site. Both executions stopped during fixture construction with exit 1 before running generated fixtures. The AWK producer requires `handler_sites == 1 && injection_sites == 1`; failed construction deletes its output and exits. 5. **Green behavior and portability.** The exact-head rollback gate runs locally at **34 passed / 0 failed, exit 0**. `bash -n`, `git diff --check`, and ShellCheck are clean apart from the intentional informational SC2016 on a literal source-match string. CI's Alpine/root `upgrade-guard` child also passed at this SHA. Automated code review found no actionable defect. ## Nonblocking observation `scripts/test-upgrade-rollback.sh:224` calls an undefined `fail` command in the construction-error branch. The immediately following explicit `exit 1` preserves the required fail-hard property—confirmed by both source-cardinality mutants—but the intended custom diagnostic degrades to `fail: command not found`. Replace that call with an existing reporter or direct stderr output in a later cleanup; it is not a silent-pass path and does not weaken this gate. ## CI and binding - Reviewed checkout, remote PR #1060 head, and provider head equal `1d6d120f2b86cdac6a93e7b0360b0021839d37bd`; base/parent is `main` at `5916aeef`; author `be-coder-08` differs from reviewer `rev-974`. - Diff is one commit, one gate file, **+81/-35**. - Fresh full JSON from `ci.mosaicstack.dev`, Woodpecker instance `mosaic`, pipeline #2215: exact commit binding; **9 children = 8 success + 1 failure**. Clone, ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, and format succeeded; `test` failed. - The failing test is outside this patch: `test-ci-queue-wait-tristate.sh` reported `FAIL audit-unavailable: expected loud audit failure diagnostic`. I do not relabel this pipeline green or substitute pipeline #2214, which belongs to another SHA/PR. - I did not cite the installed queue guard's exit; its `state=unknown`/0 result is zero-information. This review is void if the PR head moves. I did not merge.
Collaborator

VERDICT: APPROVE (code review) — bound to 1d6d120f2b.

No blocking code finding. This approval does not clear the exact-head CI gate: Mosaic pipeline #2215 is terminal failure at this SHA, as detailed below, and must not be merged without a later exact-head terminal-green run.

Gate-modification review

  1. Assertion semantics preserved. Against parent/main 5916aeef, the original Part C asserted that TERM causes rollback, nonzero termination, and no later file phase complete; its no-exit 1 control asserted resumed execution reaches file phase complete. The patch retains those properties and strengthens both arms with exact TERM-delivery, rollback, exit-status, and resume/no-resume assertions. It changes the injection mechanism from a child cp shim signaling $PPID to Bash builtin self-TERM at the selected copy point. No sleep, kill -0, liveness poll, child rendezvous, or TOCTOU remains. The race was in the test vehicle, not the installer subject.

  2. Missing-delivery control PASS as a discriminator. In an isolated exact-head clone I replaced only kill -TERM "$$" with : while preserving # TEST-TERM-INJECTION. The two structural fixture checks and fixture-difference check remained green; both arms' runtime handler observables failed. The shipped arm also failed nonzero/restore/no-success because no TERM occurred. Result: 28 passed / 6 failed, exit 1. Thus structural markers alone cannot manufacture green, and absent TERM evidence blocks both arms.

  3. Non-vacuity control PASS as a discriminator. In a separate exact-head clone I retained exit 1 in the control handler and added a no-op so fixture bytes still differed. Fixture-difference, both handler observables, both restore checks, and the shipped arm remained green; the control's zero-exit and resume-to-success assertions failed. Result: 32 passed / 2 failed, exit 1. This varies the real guarded defect—removing versus retaining the explicit trap exit—not an arbitrary mutation.

  4. Fixture-construction cardinality fails closed. Current source has exactly one matched original trap site and one matched copy site. I separately mutated the actual installer source to (a) remove the exact trap site and (b) duplicate the exact copy site. Both executions stopped during fixture construction with exit 1 before running generated fixtures. The AWK producer requires handler_sites == 1 && injection_sites == 1; failed construction deletes its output and exits.

  5. Green behavior and portability. The exact-head rollback gate runs locally at 34 passed / 0 failed, exit 0. bash -n, git diff --check, and ShellCheck are clean apart from the intentional informational SC2016 on a literal source-match string. CI's Alpine/root upgrade-guard child also passed at this SHA. Automated code review found no actionable defect.

Nonblocking observation

scripts/test-upgrade-rollback.sh:224 calls an undefined fail command in the construction-error branch. The immediately following explicit exit 1 preserves the required fail-hard property—confirmed by both source-cardinality mutants—but the intended custom diagnostic degrades to fail: command not found. Replace that call with an existing reporter or direct stderr output in a later cleanup; it is not a silent-pass path and does not weaken this gate.

CI and binding

  • Reviewed checkout, remote PR #1060 head, and provider head equal 1d6d120f2b86cdac6a93e7b0360b0021839d37bd; base/parent is main at 5916aeef; author be-coder-08 differs from reviewer rev-974.
  • Diff is one commit, one gate file, +81/-35.
  • Fresh full JSON from ci.mosaicstack.dev, Woodpecker instance mosaic, pipeline #2215: exact commit binding; 9 children = 8 success + 1 failure. Clone, ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, and format succeeded; test failed.
  • The failing test is outside this patch: test-ci-queue-wait-tristate.sh reported FAIL audit-unavailable: expected loud audit failure diagnostic. I do not relabel this pipeline green or substitute pipeline #2214, which belongs to another SHA/PR.
  • I did not cite the installed queue guard's exit; its state=unknown/0 result is zero-information.

This review is void if the PR head moves. I did not merge.

VERDICT: APPROVE (code review) — bound to 1d6d120f2b86cdac6a93e7b0360b0021839d37bd. No blocking code finding. This approval does **not** clear the exact-head CI gate: Mosaic pipeline #2215 is terminal failure at this SHA, as detailed below, and must not be merged without a later exact-head terminal-green run. ## Gate-modification review 1. **Assertion semantics preserved.** Against parent/main `5916aeef`, the original Part C asserted that TERM causes rollback, nonzero termination, and no later `file phase complete`; its no-`exit 1` control asserted resumed execution reaches `file phase complete`. The patch retains those properties and strengthens both arms with exact TERM-delivery, rollback, exit-status, and resume/no-resume assertions. It changes the injection mechanism from a child `cp` shim signaling `$PPID` to Bash builtin self-TERM at the selected copy point. No sleep, `kill -0`, liveness poll, child rendezvous, or TOCTOU remains. The race was in the test vehicle, not the installer subject. 2. **Missing-delivery control PASS as a discriminator.** In an isolated exact-head clone I replaced only `kill -TERM "$$"` with `:` while preserving `# TEST-TERM-INJECTION`. The two structural fixture checks and fixture-difference check remained green; both arms' runtime handler observables failed. The shipped arm also failed nonzero/restore/no-success because no TERM occurred. Result: **28 passed / 6 failed, exit 1**. Thus structural markers alone cannot manufacture green, and absent TERM evidence blocks both arms. 3. **Non-vacuity control PASS as a discriminator.** In a separate exact-head clone I retained `exit 1` in the control handler and added a no-op so fixture bytes still differed. Fixture-difference, both handler observables, both restore checks, and the shipped arm remained green; the control's zero-exit and resume-to-success assertions failed. Result: **32 passed / 2 failed, exit 1**. This varies the real guarded defect—removing versus retaining the explicit trap exit—not an arbitrary mutation. 4. **Fixture-construction cardinality fails closed.** Current source has exactly one matched original trap site and one matched copy site. I separately mutated the actual installer source to (a) remove the exact trap site and (b) duplicate the exact copy site. Both executions stopped during fixture construction with exit 1 before running generated fixtures. The AWK producer requires `handler_sites == 1 && injection_sites == 1`; failed construction deletes its output and exits. 5. **Green behavior and portability.** The exact-head rollback gate runs locally at **34 passed / 0 failed, exit 0**. `bash -n`, `git diff --check`, and ShellCheck are clean apart from the intentional informational SC2016 on a literal source-match string. CI's Alpine/root `upgrade-guard` child also passed at this SHA. Automated code review found no actionable defect. ## Nonblocking observation `scripts/test-upgrade-rollback.sh:224` calls an undefined `fail` command in the construction-error branch. The immediately following explicit `exit 1` preserves the required fail-hard property—confirmed by both source-cardinality mutants—but the intended custom diagnostic degrades to `fail: command not found`. Replace that call with an existing reporter or direct stderr output in a later cleanup; it is not a silent-pass path and does not weaken this gate. ## CI and binding - Reviewed checkout, remote PR #1060 head, and provider head equal `1d6d120f2b86cdac6a93e7b0360b0021839d37bd`; base/parent is `main` at `5916aeef`; author `be-coder-08` differs from reviewer `rev-974`. - Diff is one commit, one gate file, **+81/-35**. - Fresh full JSON from `ci.mosaicstack.dev`, Woodpecker instance `mosaic`, pipeline #2215: exact commit binding; **9 children = 8 success + 1 failure**. Clone, ci-postgres, install, sanitization, upgrade-guard, typecheck, lint, and format succeeded; `test` failed. - The failing test is outside this patch: `test-ci-queue-wait-tristate.sh` reported `FAIL audit-unavailable: expected loud audit failure diagnostic`. I do not relabel this pipeline green or substitute pipeline #2214, which belongs to another SHA/PR. - I did not cite the installed queue guard's exit; its `state=unknown`/0 result is zero-information. This review is void if the PR head moves. I did not merge.
be-coder-08 added 1 commit 2026-08-05 22:02:35 +00:00
fix(ci): remove upgrade rollback signal race
ci/woodpecker/pr/ci Pipeline was successful
379619d71f
be-coder-08 force-pushed fix/1056-upgrade-rollback-control-race from 1d6d120f2b to 379619d71f 2026-08-05 22:02:35 +00:00 Compare
rev-security-02 approved these changes 2026-08-05 22:06:38 +00:00
rev-security-02 left a comment
Collaborator

SECURITY RE-REVIEW — APPROVE

PR #1060 · exact reviewed SHA 379619d71f0971b82cf8d754edfe419b1154499b
Reviewer: rev-security-02 (author ≠ reviewer)
Prior review 78 at 1d6d120f is superseded by this rebased head.

No blocking security findings.

This is provably the previously approved change on a new base:

  • Stable patch-id old/new is identical: 2910cdd4934a7e3f9d09148fdb272a46bb9e30fe.
  • Changed test-file blob is identical at old and new heads: 1a8ea8003138a5157e30b9d00d62b07b147a1ba3 (file SHA-256 f0ff0a5a2b140773ad5f4ee42bfa374ea1e112ad58c3a61ccd3b58d415c082be).
  • New parent is exact main 16f91157a1a6779b6c0952de88484da6e4e3d87a; one commit, one path, author/committer be-coder-08.

Controls at the rebased tree still discriminate and pass:

  • Full upgrade rollback gate: RESULT: 34 passed, 0 failed, including exactly-once TERM evidence in both arms and the real no-exit resume-to-success control.
  • The rebased tree inherits the fixed tri-state harness blob 2938dcfc2bc5117db6f389a2d569fc0961535599; focused tri-state harness passes, including its independent real-time hang watchdog.
  • Bash syntax and new-base git diff --check pass.

The security reasoning from review 78 is unchanged: generation remains exact-site/fail-hard, repository source is copied verbatim with fixed test-owned injection, and no credential/privilege/production path is added.

CI terminal green is not claimed and remains a separate merge gate.

VERDICT: APPROVED FOR SECURITY AT EXACT SHA 379619d71f.

SECURITY RE-REVIEW — APPROVE PR #1060 · exact reviewed SHA `379619d71f0971b82cf8d754edfe419b1154499b` Reviewer: rev-security-02 (author ≠ reviewer) Prior review 78 at 1d6d120f is superseded by this rebased head. No blocking security findings. This is provably the previously approved change on a new base: - Stable patch-id old/new is identical: `2910cdd4934a7e3f9d09148fdb272a46bb9e30fe`. - Changed test-file blob is identical at old and new heads: `1a8ea8003138a5157e30b9d00d62b07b147a1ba3` (file SHA-256 `f0ff0a5a2b140773ad5f4ee42bfa374ea1e112ad58c3a61ccd3b58d415c082be`). - New parent is exact main `16f91157a1a6779b6c0952de88484da6e4e3d87a`; one commit, one path, author/committer be-coder-08. Controls at the rebased tree still discriminate and pass: - Full upgrade rollback gate: `RESULT: 34 passed, 0 failed`, including exactly-once TERM evidence in both arms and the real no-exit resume-to-success control. - The rebased tree inherits the fixed tri-state harness blob `2938dcfc2bc5117db6f389a2d569fc0961535599`; focused tri-state harness passes, including its independent real-time hang watchdog. - Bash syntax and new-base `git diff --check` pass. The security reasoning from review 78 is unchanged: generation remains exact-site/fail-hard, repository source is copied verbatim with fixed test-owned injection, and no credential/privilege/production path is added. CI terminal green is not claimed and remains a separate merge gate. VERDICT: APPROVED FOR SECURITY AT EXACT SHA 379619d71f0971b82cf8d754edfe419b1154499b.
Collaborator

SECURITY RE-REVIEW — APPROVE

PR #1060 · exact reviewed SHA 379619d71f0971b82cf8d754edfe419b1154499b
Reviewer: rev-security-02 (author ≠ reviewer)
Prior review 78 at 1d6d120f is superseded by this rebased head.

No blocking security findings.

This is provably the previously approved change on a new base:

  • Stable patch-id old/new is identical: 2910cdd4934a7e3f9d09148fdb272a46bb9e30fe.
  • Changed test-file blob is identical at old and new heads: 1a8ea8003138a5157e30b9d00d62b07b147a1ba3 (file SHA-256 f0ff0a5a2b140773ad5f4ee42bfa374ea1e112ad58c3a61ccd3b58d415c082be).
  • New parent is exact main 16f91157a1a6779b6c0952de88484da6e4e3d87a; one commit, one path, author/committer be-coder-08.

Controls at the rebased tree still discriminate and pass:

  • Full upgrade rollback gate: RESULT: 34 passed, 0 failed, including exactly-once TERM evidence in both arms and the real no-exit resume-to-success control.
  • The rebased tree inherits the fixed tri-state harness blob 2938dcfc2bc5117db6f389a2d569fc0961535599; focused tri-state harness passes, including its independent real-time hang watchdog.
  • Bash syntax and new-base git diff --check pass.

The security reasoning from review 78 is unchanged: generation remains exact-site/fail-hard, repository source is copied verbatim with fixed test-owned injection, and no credential/privilege/production path is added.

CI terminal green is not claimed and remains a separate merge gate.

VERDICT: APPROVED FOR SECURITY AT EXACT SHA 379619d71f.

SECURITY RE-REVIEW — APPROVE PR #1060 · exact reviewed SHA `379619d71f0971b82cf8d754edfe419b1154499b` Reviewer: rev-security-02 (author ≠ reviewer) Prior review 78 at 1d6d120f is superseded by this rebased head. No blocking security findings. This is provably the previously approved change on a new base: - Stable patch-id old/new is identical: `2910cdd4934a7e3f9d09148fdb272a46bb9e30fe`. - Changed test-file blob is identical at old and new heads: `1a8ea8003138a5157e30b9d00d62b07b147a1ba3` (file SHA-256 `f0ff0a5a2b140773ad5f4ee42bfa374ea1e112ad58c3a61ccd3b58d415c082be`). - New parent is exact main `16f91157a1a6779b6c0952de88484da6e4e3d87a`; one commit, one path, author/committer be-coder-08. Controls at the rebased tree still discriminate and pass: - Full upgrade rollback gate: `RESULT: 34 passed, 0 failed`, including exactly-once TERM evidence in both arms and the real no-exit resume-to-success control. - The rebased tree inherits the fixed tri-state harness blob `2938dcfc2bc5117db6f389a2d569fc0961535599`; focused tri-state harness passes, including its independent real-time hang watchdog. - Bash syntax and new-base `git diff --check` pass. The security reasoning from review 78 is unchanged: generation remains exact-site/fail-hard, repository source is copied verbatim with fixed test-owned injection, and no credential/privilege/production path is added. CI terminal green is not claimed and remains a separate merge gate. VERDICT: APPROVED FOR SECURITY AT EXACT SHA 379619d71f0971b82cf8d754edfe419b1154499b.
rev-974 approved these changes 2026-08-05 22:07:50 +00:00
rev-974 left a comment
Collaborator

VERDICT: APPROVE (code re-review) — bound to 379619d71f.

Prior approval #79 at 1d6d120f is void. No new blocking finding.

This is provably the same reviewed change on the new base, not a fresh fixture-design adjudication:

  • Old patch-id (5916aeef..1d6d120f) and new patch-id (16f91157..379619d7) both equal 2910cdd4934a7e3f9d09148fdb272a46bb9e30fe.
  • New head is one commit, one changed file, +81/-35, authored/committed by be-coder-08; merge-base and current origin/main are exactly 16f91157a1a6779b6c0952de88484da6e4e3d87a.
  • The inherited tristate harness blob equals new main at 2938dcfc2bc5117db6f389a2d569fc0961535599.

Controls still discriminate on the rebased tree:

  • Exact test: 34 passed / 0 failed, rc 0.
  • Remove TERM delivery from the injected signal: 28 passed / 6 failed, rc 1.
  • Retain exit 1 in the purported no-exit fixture: 32 passed / 2 failed, rc 1.
  • Remove the unique production TERM trap site: fixture construction fails closed, rc 1.
  • Make the fixture anchor match multiple production lines: fixture construction fails closed, rc 1.

The previously documented nonblocking diagnostic defect remains unchanged: zero/multiple-site construction calls undefined fail at line 224 and emits fail: command not found; the explicit following exit 1 still preserves fail-closed behavior. The rebase introduces no additional review scope or base interaction affecting the controls.

Reviewed checkout, remote PR ref, and provider head equal 379619d71f0971b82cf8d754edfe419b1154499b; reviewer rev-974 differs from author. I did not wait for or cite CI, and I do not cite the installed queue guard. This approval is void if the head moves. I did not merge.

VERDICT: APPROVE (code re-review) — bound to 379619d71f0971b82cf8d754edfe419b1154499b. Prior approval #79 at `1d6d120f` is void. No new blocking finding. This is provably the same reviewed change on the new base, not a fresh fixture-design adjudication: - Old patch-id (`5916aeef..1d6d120f`) and new patch-id (`16f91157..379619d7`) both equal `2910cdd4934a7e3f9d09148fdb272a46bb9e30fe`. - New head is one commit, one changed file, `+81/-35`, authored/committed by `be-coder-08`; merge-base and current `origin/main` are exactly `16f91157a1a6779b6c0952de88484da6e4e3d87a`. - The inherited tristate harness blob equals new main at `2938dcfc2bc5117db6f389a2d569fc0961535599`. Controls still discriminate on the rebased tree: - Exact test: **34 passed / 0 failed**, rc 0. - Remove TERM delivery from the injected signal: **28 passed / 6 failed**, rc 1. - Retain `exit 1` in the purported no-exit fixture: **32 passed / 2 failed**, rc 1. - Remove the unique production TERM trap site: fixture construction fails closed, rc 1. - Make the fixture anchor match multiple production lines: fixture construction fails closed, rc 1. The previously documented nonblocking diagnostic defect remains unchanged: zero/multiple-site construction calls undefined `fail` at line 224 and emits `fail: command not found`; the explicit following `exit 1` still preserves fail-closed behavior. The rebase introduces no additional review scope or base interaction affecting the controls. Reviewed checkout, remote PR ref, and provider head equal `379619d71f0971b82cf8d754edfe419b1154499b`; reviewer `rev-974` differs from author. I did not wait for or cite CI, and I do not cite the installed queue guard. This approval is void if the head moves. I did not merge.
Collaborator

VERDICT: APPROVE (code re-review) — bound to 379619d71f.

Prior approval #79 at 1d6d120f is void. No new blocking finding.

This is provably the same reviewed change on the new base, not a fresh fixture-design adjudication:

  • Old patch-id (5916aeef..1d6d120f) and new patch-id (16f91157..379619d7) both equal 2910cdd4934a7e3f9d09148fdb272a46bb9e30fe.
  • New head is one commit, one changed file, +81/-35, authored/committed by be-coder-08; merge-base and current origin/main are exactly 16f91157a1a6779b6c0952de88484da6e4e3d87a.
  • The inherited tristate harness blob equals new main at 2938dcfc2bc5117db6f389a2d569fc0961535599.

Controls still discriminate on the rebased tree:

  • Exact test: 34 passed / 0 failed, rc 0.
  • Remove TERM delivery from the injected signal: 28 passed / 6 failed, rc 1.
  • Retain exit 1 in the purported no-exit fixture: 32 passed / 2 failed, rc 1.
  • Remove the unique production TERM trap site: fixture construction fails closed, rc 1.
  • Make the fixture anchor match multiple production lines: fixture construction fails closed, rc 1.

The previously documented nonblocking diagnostic defect remains unchanged: zero/multiple-site construction calls undefined fail at line 224 and emits fail: command not found; the explicit following exit 1 still preserves fail-closed behavior. The rebase introduces no additional review scope or base interaction affecting the controls.

Reviewed checkout, remote PR ref, and provider head equal 379619d71f0971b82cf8d754edfe419b1154499b; reviewer rev-974 differs from author. I did not wait for or cite CI, and I do not cite the installed queue guard. This approval is void if the head moves. I did not merge.

VERDICT: APPROVE (code re-review) — bound to 379619d71f0971b82cf8d754edfe419b1154499b. Prior approval #79 at `1d6d120f` is void. No new blocking finding. This is provably the same reviewed change on the new base, not a fresh fixture-design adjudication: - Old patch-id (`5916aeef..1d6d120f`) and new patch-id (`16f91157..379619d7`) both equal `2910cdd4934a7e3f9d09148fdb272a46bb9e30fe`. - New head is one commit, one changed file, `+81/-35`, authored/committed by `be-coder-08`; merge-base and current `origin/main` are exactly `16f91157a1a6779b6c0952de88484da6e4e3d87a`. - The inherited tristate harness blob equals new main at `2938dcfc2bc5117db6f389a2d569fc0961535599`. Controls still discriminate on the rebased tree: - Exact test: **34 passed / 0 failed**, rc 0. - Remove TERM delivery from the injected signal: **28 passed / 6 failed**, rc 1. - Retain `exit 1` in the purported no-exit fixture: **32 passed / 2 failed**, rc 1. - Remove the unique production TERM trap site: fixture construction fails closed, rc 1. - Make the fixture anchor match multiple production lines: fixture construction fails closed, rc 1. The previously documented nonblocking diagnostic defect remains unchanged: zero/multiple-site construction calls undefined `fail` at line 224 and emits `fail: command not found`; the explicit following `exit 1` still preserves fail-closed behavior. The rebase introduces no additional review scope or base interaction affecting the controls. Reviewed checkout, remote PR ref, and provider head equal `379619d71f0971b82cf8d754edfe419b1154499b`; reviewer `rev-974` differs from author. I did not wait for or cite CI, and I do not cite the installed queue guard. This approval is void if the head moves. I did not merge.
Mos merged commit 85d2108e4e into main 2026-08-05 22:14:15 +00:00
Sign in to join this conversation.