fix: close named pipe descriptor on error path in pipe-bootstrap

[eran132]

Description

Run in command/connect/envoy/pipe-bootstrap/connect_envoy_pipe-bootstrap.go opens a file descriptor with os.OpenFile and writes the Envoy bootstrap payload into the named pipe via buf.WriteTo(f). When that write fails, the function returned 1 without closing f, leaking the descriptor. The success path and the os.OpenFile error path are both fine — only the WriteTo failure path leaked.

This adds an f.Close() on that error path. I deliberately did not switch to defer f.Close(): the success path already performs an explicit f.Close() with error checking, so a deferred close would result in a double close on the happy path. The targeted fix keeps the existing structure intact.

Testing & Reproduction steps

Added a Linux regression test (connect_envoy_pipe-bootstrap_linux_test.go) that exercises the failure path: it targets /dev/full as the pipe path (it opens for writing but every write fails with ENOSPC), feeds non-empty stdin, and asserts via /proc/self/fd that no descriptor is leaked once Run returns.

Verified in a Linux container:

• On the unfixed code the test fails — file descriptor leaked on write-error path: 10 open before Run, 11 after.
• With the fix the test passes, and the full package suite + go vet pass.

The test is gated to Linux via the _linux_test.go filename suffix because it relies on /dev/full and /proc/self/fd.

Links

• Fixes File descriptor leak on error path in Run (command/connect/envoy/pipe-bootstrap/connect_envoy_pipe-bootstrap.go) #23256

PR Checklist

• updated test coverage
• external facing docs updated
• appropriate backport labels added
• not a security concern

PCI review checklist

• I have documented a clear reason for, and description of, the change I am making.
• If applicable, I've documented a plan to revert these changes if they require more than reverting the pull request.
• If applicable, I've documented the impact of any changes to security controls.

[hashicorp-cla-app]

All committers have signed the CLA.

[hashicorp-cla-app]

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

_{Have you signed the CLA already but the status is still pending? Recheck it.}

[Copilot]

Pull request overview

Fixes a file descriptor leak in Consul’s internal Envoy pipe-bootstrap command by ensuring the named pipe file descriptor is closed on the buf.WriteTo error path, and adds a Linux-only regression test to prevent regressions.

Changes:

• Close the named pipe descriptor when buf.WriteTo(f) fails in Run.
• Add a Linux regression test that triggers the write-error path using /dev/full and checks for descriptor leaks via /proc/self/fd.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
command/connect/envoy/pipe-bootstrap/connect_envoy_pipe-bootstrap.go	Closes the named pipe FD on the WriteTo failure path to prevent leaks.
command/connect/envoy/pipe-bootstrap/connect_envoy_pipe-bootstrap_linux_test.go	Adds a Linux-only test to exercise the write-error path and assert no FD leak occurs.

Comments suppressed due to low confidence (1)

command/connect/envoy/pipe-bootstrap/connect_envoy_pipe-bootstrap_linux_test.go:46

• In cleanup, stdinR.Close() ignores the returned error. Consider checking/logging the Close error (even in tests) to avoid masking unexpected failures when restoring/closing stdin.

	os.Stdin = stdinR
	t.Cleanup(func() {
		os.Stdin = origStdin
		stdinR.Close()
	})

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[eran132]

Thanks for the review! Addressed all three points in 02ff00d:

• Production code (buf.WriteTo error path): switched to _ = f.Close(). The error path intentionally drops the close error because the primary WriteTo failure has already been reported via c.UI.Error and we're about to return non-zero — surfacing a likely-cascading second error wouldn't add diagnostic value, but making the ignore explicit is the right call and matches errcheck conventions.
• Test (stdinW.Close and the stdinR.Close in t.Cleanup): same _ = treatment for symmetry.

Re-verified in a Linux container — go vet and the package tests still pass; the regression test still fails (as expected) when the fix is removed and passes with it. No behavior change.