From 8224e0f7bf690684c880c33ba7e638a2e042ae14 Mon Sep 17 00:00:00 2001
From: riccardom <riccardomanfrin@gmail.com>
Date: Thu, 1 Oct 2026 13:11:45 +0200
Subject: [PATCH] [client] Bound the wait on a certificate proof helper that
 outlives its launcher

The collector that answers certificate posture challenges allows one collection
at a time, and the latch guarding that is released by the goroutine running the
collection, not by the caller that gave up waiting for it. The collection is
therefore assumed to end.

On macOS and Windows it reads the signed-in user's certificates through a helper
process, and there the assumption does not hold. Wait blocks on the output pipe
rather than on the process, and killing the process we launched does not close
the pipe its own children inherited: on macOS we launch launchctl, which launches
sudo, which launches the helper, so the deadline reaps launchctl and leaves the
other two holding the pipe open. A helper stuck on a keychain prompt is enough.

Wait then never returns, the latch stays set, and the peer sends no proof again
for the life of the daemon. It fails every certificate check and loses the
policies that carry one, logging a single line per sync and nothing else.

Set a wait delay so Wait closes the pipes itself once the process is gone. The
run fails rather than reporting proofs, which is what we want: the output was cut
short, so there is nothing trustworthy to report.
---
 client/internal/certproof/helper_run.go      |  9 +++++++
 client/internal/certproof/helper_run_test.go | 25 ++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/client/internal/certproof/helper_run.go b/client/internal/certproof/helper_run.go
index 9345b50f3..cc221669b 100644
--- a/client/internal/certproof/helper_run.go
+++ b/client/internal/certproof/helper_run.go
@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"os/exec"
 	"strings"
+	"time"
 
 	log "github.com/sirupsen/logrus"
 
@@ -16,6 +17,13 @@ import (
 const (
 	maxHelperStdout = 1 << 20
 	maxHelperStderr = 4 << 10
+
+	// helperWaitDelay bounds how long Wait keeps reading the helper's output after the
+	// process we launched is gone. Killing that process does not close the pipe its own
+	// children inherited, and on macOS they are the ones doing the work: we launch
+	// launchctl, which launches sudo, which launches the helper. Without this, a helper
+	// stuck on a keychain prompt leaves Wait blocked with no deadline at all.
+	helperWaitDelay = time.Second
 )
 
 var errHelperOutputTooLarge = errors.New("helper output exceeds the size limit")
@@ -34,6 +42,7 @@ func runHelperCmd(cmd *exec.Cmd, req HelperRequest) ([]certposture.Proof, error)
 	cmd.Stdin = bytes.NewReader(payload)
 	cmd.Stdout = stdout
 	cmd.Stderr = stderr
+	cmd.WaitDelay = helperWaitDelay
 
 	if err := cmd.Run(); err != nil {
 		return nil, fmt.Errorf("%w: %s", err, strings.TrimSpace(stderr.String()))
diff --git a/client/internal/certproof/helper_run_test.go b/client/internal/certproof/helper_run_test.go
index be4de9cbf..8a893f903 100644
--- a/client/internal/certproof/helper_run_test.go
+++ b/client/internal/certproof/helper_run_test.go
@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -64,6 +65,30 @@ func TestRunHelperCmd_CapsStderrInError(t *testing.T) {
 	assert.True(t, strings.Contains(err.Error(), "exit status 3"), "the exit status is kept: %v", err)
 }
 
+func TestRunHelperCmd_ReturnsWhenAGrandchildHoldsTheOutputPipe(t *testing.T) {
+	req := HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}}
+	resp := HelperResponse{Proofs: []certposture.Proof{{Nonce: []byte("asked"), Signature: []byte("sig")}}}
+
+	// The helper answers and exits, but leaves a background process holding the stdout
+	// it inherited. This is what a wedged `netbird posture cert-proof` behind a keychain
+	// prompt looks like from here: killing the process we launched does not close the
+	// pipe, so the copy out of it never sees EOF.
+	script := printJSON(t, resp) + "; sleep 10 &"
+
+	done := make(chan error, 1)
+	go func() {
+		_, err := runHelperCmd(fakeHelper(t, script), req)
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		assert.ErrorIs(t, err, exec.ErrWaitDelay, "the output is incomplete, so the run must fail rather than report proofs")
+	case <-time.After(5 * time.Second):
+		t.Fatal("runHelperCmd never returned while a grandchild held the output pipe, so the collector's busy latch would stay set for the life of the daemon")
+	}
+}
+
 func TestRunHelperCmd_RejectsGarbage(t *testing.T) {
 	req := HelperRequest{Challenges: []HelperChallenge{{Nonce: []byte("asked")}}}