From e4e3e8d6558f82fccb35d62cea2ea14029536e21 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Wed, 7 Oct 2026 16:53:29 +0100 Subject: [PATCH] Keep captured audit findings past early watchdog cleanup The full installer race suite exposed report deletion despite the subshell-counter guard. Bind EXIT cleanup to the original process using the direct child shell parent PID, including before the inherited counter is initialised. Retain strict and unchanged-graph verdicts, exact arguments and parent cleanup. Align the stale channel-copy assertion with the existing non-stable message; executable consent controls remain unchanged. Change-source: pulse-maintainer --- .../subsystems/deployment-installability.md | 16 ++++++++++++++++ .../installtests/build_release_assets_test.go | 12 +++++++----- scripts/installtests/root_install_sh_test.go | 4 ++-- scripts/npm-audit-retry.sh | 8 +++++--- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/docs/release-control/v6/internal/subsystems/deployment-installability.md b/docs/release-control/v6/internal/subsystems/deployment-installability.md index 4dc633f642..15490caa2b 100644 --- a/docs/release-control/v6/internal/subsystems/deployment-installability.md +++ b/docs/release-control/v6/internal/subsystems/deployment-installability.md @@ -15,6 +15,22 @@ ## Purpose +### Audit report cleanup belongs to the original invoking process + +The npm audit wrapper retains a captured advisory until the parent classifies +it. An early-stopped watchdog can inherit the parent's zero subshell counter; +that counter alone cannot authorise report deletion. The EXIT hook instead +asks a direct `/bin/sh` child whether its actual parent PID is the invoking +script's PID. This keeps the report in an inherited trap while retaining parent +cleanup on Linux and native Bash 3.2. Strict findings still fail; unchanged-graph +findings still warn with their captured detail. No advisory policy, endpoint, +retry budget or severity threshold changes. + +`TestNpmAuditReportCleanupRemainsParentOwned` executes the actual hook with +normal and pre-initialisation child counters, rejects the prior counter-only +and unconditional hooks, and checks parent cleanup. The existing native-scope +argument controls continue to require findings, exact argv and a single audit. + ### Community auto-updates require a verified installed edition Before release discovery or installation, the unattended updater requires a diff --git a/scripts/installtests/build_release_assets_test.go b/scripts/installtests/build_release_assets_test.go index cf8d3e52fc..beca6d71d0 100644 --- a/scripts/installtests/build_release_assets_test.go +++ b/scripts/installtests/build_release_assets_test.go @@ -5292,11 +5292,13 @@ func TestNpmAuditReportCleanupRemainsParentOwned(t *testing.T) { t.Fatal("audit report cleanup hook is missing") } for _, tc := range []struct { - name, hook string - status int + name, hook, childSetup string + status int }{ - {"actual_parent_owned_hook", hook, 0}, - {"legacy_child_cleanup_rejected", `trap 'rm -f "${report_file}"' EXIT`, 23}, + {"actual_parent_owned_hook", hook, "", 0}, + {"actual_hook_before_subshell_counter_initialization", hook, "BASH_SUBSHELL=0\n", 0}, + {"legacy_child_cleanup_rejected", `trap 'rm -f "${report_file}"' EXIT`, "", 23}, + {"legacy_counter_guard_rejected_before_initialization", `trap 'if [ "$BASH_SUBSHELL" -eq 0 ]; then rm -f "${report_file}"; fi' EXIT`, "BASH_SUBSHELL=0\n", 23}, } { t.Run(tc.name, func(t *testing.T) { path := filepath.Join(t.TempDir(), "report") @@ -5304,7 +5306,7 @@ func TestNpmAuditReportCleanupRemainsParentOwned(t *testing.T) { // the early-signal window, without timing a background process. // It must retain the report there, and still remove it at parent exit. script := "report_file=\"$REPORT_FIXTURE\"\nprintf retained >\"$report_file\"\n" + tc.hook + - "\n(\n" + tc.hook + "\n)\ntest -f \"$report_file\" || exit 23\n" + "\n(\n" + tc.hook + "\n" + tc.childSetup + ")\ntest -f \"$report_file\" || exit 23\n" cmd := exec.Command("bash", "-c", script) cmd.Env = append(os.Environ(), "REPORT_FIXTURE="+path) output, err := cmd.CombinedOutput() diff --git a/scripts/installtests/root_install_sh_test.go b/scripts/installtests/root_install_sh_test.go index 49ccec9b28..3d190208a4 100644 --- a/scripts/installtests/root_install_sh_test.go +++ b/scripts/installtests/root_install_sh_test.go @@ -817,8 +817,8 @@ func TestPrereleaseUpdateCopyUsesPreviewFraming(t *testing.T) { t.Fatalf("read pulse-auto-update.sh: %v", err) } autoUpdateScript := string(autoUpdate) - if !strings.Contains(autoUpdateScript, `Prerelease channel detected; unattended auto-updates run only on stable`) { - t.Fatalf("pulse-auto-update.sh missing prerelease channel log message") + if !strings.Contains(autoUpdateScript, `Non-stable channel detected; unattended auto-updates run only on stable`) { + t.Fatalf("pulse-auto-update.sh missing non-stable channel log message") } if strings.Contains(autoUpdateScript, `RC channel detected; unattended auto-updates run only on stable`) { t.Fatalf("pulse-auto-update.sh preserved stale release-candidate channel log message") diff --git a/scripts/npm-audit-retry.sh b/scripts/npm-audit-retry.sh index 094af0e333..95f086a6a4 100755 --- a/scripts/npm-audit-retry.sh +++ b/scripts/npm-audit-retry.sh @@ -231,9 +231,11 @@ else: report_file="$(mktemp)" # A watchdog stopped before its subshell initialises can run this inherited -# EXIT trap. Only the invoking shell owns cleanup: the captured verdict must -# remain available after the watchdog is stopped, on Bash 3.2 as well as 5.x. -trap 'if [ "$BASH_SUBSHELL" -eq 0 ]; then rm -f "${report_file}"; fi' EXIT +# EXIT trap with the parent's BASH_SUBSHELL value. $$ is inherited too. Ask a +# direct child shell for its actual parent PID instead of trusting that counter: +# only the original invoking process may remove the captured verdict. PPID in +# /bin/sh also works on native Bash 3.2, where BASHPID is unavailable. +trap 'if /bin/sh -c "[ \"\$PPID\" = \"\$1\" ]" sh "$$"; then rm -f "${report_file}"; fi' EXIT attempt=1 delay="${DELAY}"