From 28a480879e70a79cbfaf6257c1afe18f84d0812d Mon Sep 17 00:00:00 2001 From: admin Date: Sun, 25 Oct 2026 13:45:11 -0400 Subject: [PATCH 1/3] test(perfbench): register test scenarios once in init so go test -count>1 does not panic (D-0606) --- tools/perfbench/ab_guard_test.go | 5 ++++- tools/perfbench/run_test.go | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tools/perfbench/ab_guard_test.go b/tools/perfbench/ab_guard_test.go index b0d4a1dbc..cee4aaf59 100644 --- a/tools/perfbench/ab_guard_test.go +++ b/tools/perfbench/ab_guard_test.go @@ -108,8 +108,11 @@ func (emptyProbes) Run(context.Context, scenarios.Config) ([]scenarios.Sample, e } func (emptyProbes) Probes() []scenarios.Probe { return nil } +// Registered in init, not in the test: the registry is global and Register +// panics on a duplicate, so registering per test run broke go test -count=N. +func init() { scenarios.Register(emptyProbes{}) } + func TestABRefusesEmptyMetrics(t *testing.T) { - scenarios.Register(emptyProbes{}) bin := script(t, "nself", okScript) for _, extra := range [][]string{{}, {"-json"}} { var out, errb bytes.Buffer diff --git a/tools/perfbench/run_test.go b/tools/perfbench/run_test.go index 045a6d9e4..7b1b3d7ed 100644 --- a/tools/perfbench/run_test.go +++ b/tools/perfbench/run_test.go @@ -66,8 +66,11 @@ func TestUnknownScenarioAndSubcommand(t *testing.T) { } } +// Registered in init, not in the test, so go test -count=N does not hit the +// registry's duplicate-name panic. +func init() { scenarios.Register(mixedScenario{}) } + func TestMixedUnitsFailTheRun(t *testing.T) { - scenarios.Register(mixedScenario{}) var out, errb bytes.Buffer code := dispatch([]string{"run", "-scenario", "test-mixed", "-bin", "x", "-runs", "2"}, &out, &errb) if code != 1 || !strings.Contains(errb.String(), "mixed units") { From ac803fd485079304cae81f7fb9e9ca716160fb3b Mon Sep 17 00:00:00 2001 From: admin Date: Sun, 25 Oct 2026 13:45:55 -0400 Subject: [PATCH 2/3] fix(licensing): open the entitlement file with O_NOFOLLOW|O_NONBLOCK and check the descriptor, closing the Lstat/Open swap race (D-1112) --- sdk/go/licensing/entitlement.go | 15 ++++++--- sdk/go/licensing/entitlement_fifo_test.go | 37 ++++++++++++++++++++++ sdk/go/licensing/entitlement_open_other.go | 17 ++++++++++ sdk/go/licensing/entitlement_open_unix.go | 16 ++++++++++ 4 files changed, 81 insertions(+), 4 deletions(-) create mode 100644 sdk/go/licensing/entitlement_open_other.go create mode 100644 sdk/go/licensing/entitlement_open_unix.go diff --git a/sdk/go/licensing/entitlement.go b/sdk/go/licensing/entitlement.go index d0d3c1282..d3b00a467 100644 --- a/sdk/go/licensing/entitlement.go +++ b/sdk/go/licensing/entitlement.go @@ -49,18 +49,25 @@ func verifyFile(path, slug string, now time.Time) error { return verifyToken(strings.TrimSpace(string(data)), slug, now) } +// beforeTokenOpen is a test seam run just before the token file is opened. +var beforeTokenOpen func() + // Purpose: bound reads of mounted files. Inputs: path. Outputs: bytes or rejection. // Constraints: oversized files never allocate unbounded memory. SPORT: plugin.entitlement-token. func readTokenFile(path string) ([]byte, error) { - info, err := os.Lstat(path) - if err != nil || !info.Mode().IsRegular() { - return nil, ErrEntitlementExpired + if beforeTokenOpen != nil { + beforeTokenOpen() } - f, err := os.Open(path) + f, err := openTokenFile(path) if err != nil { return nil, ErrEntitlementExpired } defer func() { _ = f.Close() }() + // Check the opened descriptor, not the path: a path check followed by an + // open leaves a window to swap in a FIFO or symlink. + if info, err := f.Stat(); err != nil || !info.Mode().IsRegular() { + return nil, ErrEntitlementExpired + } data, err := io.ReadAll(io.LimitReader(f, maxTokenBytes+1)) if len(data) > maxTokenBytes { return nil, errTokenTooLarge diff --git a/sdk/go/licensing/entitlement_fifo_test.go b/sdk/go/licensing/entitlement_fifo_test.go index 1380ee9c1..fd55c1696 100644 --- a/sdk/go/licensing/entitlement_fifo_test.go +++ b/sdk/go/licensing/entitlement_fifo_test.go @@ -5,6 +5,7 @@ package licensing import ( "context" "errors" + "os" "path/filepath" "syscall" "testing" @@ -26,3 +27,39 @@ func TestEntitlementFIFORejectedWithoutBlocking(t *testing.T) { t.Fatal("FIFO authorized") } } + +// A FIFO or symlink swapped in right before the open must be rejected without +// blocking the reader. +func TestEntitlementSwapRaceRejected(t *testing.T) { + for _, kind := range []string{"fifo", "symlink"} { + t.Run(kind, func(t *testing.T) { + dir := t.TempDir() + path := filepath.Join(dir, "entitlement.jwt") + target := filepath.Join(dir, "target.jwt") + for _, p := range []string{path, target} { + if err := os.WriteFile(p, []byte("x"), 0600); err != nil { + t.Fatal(err) + } + } + beforeTokenOpen = func() { + _ = os.Remove(path) + if kind == "fifo" { + _ = syscall.Mkfifo(path, 0600) + } else { + _ = os.Symlink(target, path) + } + } + defer func() { beforeTokenOpen = nil }() + done := make(chan error, 1) + go func() { _, err := readTokenFile(path); done <- err }() + select { + case err := <-done: + if !errors.Is(err, ErrEntitlementExpired) { + t.Fatalf("swapped %s returned %v", kind, err) + } + case <-time.After(5 * time.Second): + t.Fatalf("swapped %s blocked the reader", kind) + } + }) + } +} diff --git a/sdk/go/licensing/entitlement_open_other.go b/sdk/go/licensing/entitlement_open_other.go new file mode 100644 index 000000000..2c2988cc4 --- /dev/null +++ b/sdk/go/licensing/entitlement_open_other.go @@ -0,0 +1,17 @@ +//go:build !unix + +package licensing + +import "os" + +// Purpose: open a mounted token file; platforms without O_NOFOLLOW/O_NONBLOCK +// reject a symlink up front. Inputs: path. Outputs: open file or error. +// Constraints: best effort only, the caller checks the opened descriptor. +// SPORT: plugin.entitlement-token. +func openTokenFile(path string) (*os.File, error) { + info, err := os.Lstat(path) + if err != nil || !info.Mode().IsRegular() { + return nil, ErrEntitlementExpired + } + return os.Open(path) +} diff --git a/sdk/go/licensing/entitlement_open_unix.go b/sdk/go/licensing/entitlement_open_unix.go new file mode 100644 index 000000000..2a1fd2cfc --- /dev/null +++ b/sdk/go/licensing/entitlement_open_unix.go @@ -0,0 +1,16 @@ +//go:build unix + +package licensing + +import ( + "os" + "syscall" +) + +// Purpose: open a mounted token file without following a final symlink or +// blocking on a FIFO. Inputs: path. Outputs: open file or error. +// Constraints: O_NOFOLLOW and O_NONBLOCK make the swap race harmless; the +// caller still checks the opened descriptor is a regular file. SPORT: plugin.entitlement-token. +func openTokenFile(path string) (*os.File, error) { + return os.OpenFile(path, os.O_RDONLY|syscall.O_NOFOLLOW|syscall.O_NONBLOCK, 0) +} From 0b8cba719fc777016eb68d04dc997dcb8085ecba Mon Sep 17 00:00:00 2001 From: admin Date: Sun, 25 Oct 2026 13:46:27 -0400 Subject: [PATCH 3/3] fix(httpx): error on a ContentLength mismatch, keep trailers, set the inferred length on the clone only (D-0621) --- sdk/go/httpx/client.go | 33 ++++++++---- sdk/go/httpx/client_retry_test.go | 88 +++++++++++++++++++++++++++++++ 2 files changed, 111 insertions(+), 10 deletions(-) diff --git a/sdk/go/httpx/client.go b/sdk/go/httpx/client.go index c458903da..cf6d80205 100644 --- a/sdk/go/httpx/client.go +++ b/sdk/go/httpx/client.go @@ -77,7 +77,7 @@ func (c *Client) Do(ctx context.Context, req *http.Request) (*http.Response, err req.Header.Set("User-Agent", c.opts.UserAgent) } - getBody, buffered, err := bodySource(req, c.opts.MaxRetries > 0) + getBody, buffered, inferred, err := bodySource(req, c.opts.MaxRetries > 0) if err != nil { return nil, err } @@ -112,6 +112,9 @@ func (c *Client) Do(ctx context.Context, req *http.Request) (*http.Response, err } reqClone.Body = body } + if buffered && inferred >= 0 { + reqClone.ContentLength = inferred + } } resp, lastErr = c.HTTP.Do(reqClone) if lastErr != nil { @@ -136,31 +139,41 @@ func (c *Client) Do(ctx context.Context, req *http.Request) (*http.Response, err // single attempt (retries false), or a body without GetBody larger than // maxBufferedBody (req.Body is then rebuilt so the one attempt still sends it // whole). buffered reports that req.Body was read into memory and closed, so -// even the first attempt must use the copy; req.ContentLength is set to the -// buffered length. -func bodySource(req *http.Request, retries bool) (getBody func() (io.ReadCloser, error), buffered bool, err error) { +// even the first attempt must use the copy. inferred is the buffered length +// for Do to set on each attempt's clone, or -1 when the length must not be +// inferred: a request with Trailer set stays chunked so its trailers are +// sent. The caller's req.ContentLength is never changed; an explicit +// ContentLength > 0 that differs from the buffered length is an error rather +// than a silently truncated or padded body. +func bodySource(req *http.Request, retries bool) (getBody func() (io.ReadCloser, error), buffered bool, inferred int64, err error) { if !retries || req.Body == nil || req.Body == http.NoBody { - return nil, false, nil + return nil, false, -1, nil } if req.GetBody != nil { - return req.GetBody, false, nil + return req.GetBody, false, -1, nil } orig := req.Body data, err := io.ReadAll(io.LimitReader(orig, maxBufferedBody+1)) if err != nil { _ = orig.Close() - return nil, false, fmt.Errorf("httpx: read request body: %w", err) + return nil, false, -1, fmt.Errorf("httpx: read request body: %w", err) } if int64(len(data)) > maxBufferedBody { req.Body = struct { io.Reader io.Closer }{io.MultiReader(bytes.NewReader(data), orig), orig} - return nil, false, nil + return nil, false, -1, nil } _ = orig.Close() - req.ContentLength = int64(len(data)) + if req.ContentLength > 0 && req.ContentLength != int64(len(data)) { + return nil, false, -1, fmt.Errorf("httpx: request ContentLength %d does not match body length %d", req.ContentLength, len(data)) + } + inferred = int64(len(data)) + if req.Trailer != nil { + inferred = -1 + } return func() (io.ReadCloser, error) { return io.NopCloser(bytes.NewReader(data)), nil - }, true, nil + }, true, inferred, nil } diff --git a/sdk/go/httpx/client_retry_test.go b/sdk/go/httpx/client_retry_test.go index fa3e47a2a..b3a39c9cd 100644 --- a/sdk/go/httpx/client_retry_test.go +++ b/sdk/go/httpx/client_retry_test.go @@ -8,6 +8,7 @@ import ( "net/http/httptest" "strings" "sync" + "sync/atomic" "testing" "time" ) @@ -238,3 +239,90 @@ func TestDo_RedirectReplaysBufferedBody(t *testing.T) { t.Errorf("body at redirect target = %q, want %q", final, payload) } } + +// TestDo_ContentLengthMismatchErrors verifies that an explicit ContentLength +// that differs from the buffered body is rejected, not sent truncated. +func TestDo_ContentLengthMismatchErrors(t *testing.T) { + var hits int32 + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(&hits, 1) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + c := New(ClientOptions{MaxRetries: 1, RetryDelay: time.Millisecond, Timeout: 5 * time.Second}) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, srv.URL, + onlyReader{strings.NewReader(strings.Repeat("x", 100))}) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + req.ContentLength = 10 + resp, err := c.Do(context.Background(), req) + if err == nil { + resp.Body.Close() + t.Fatal("Do accepted ContentLength 10 for a 100 byte body") + } + if atomic.LoadInt32(&hits) != 0 { + t.Errorf("server was contacted %d times, want 0", hits) + } +} + +// TestDo_DoesNotMutateCallerContentLength verifies the inferred length lands +// on the per-attempt clone only. +func TestDo_DoesNotMutateCallerContentLength(t *testing.T) { + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + c := New(ClientOptions{MaxRetries: 1, RetryDelay: time.Millisecond, Timeout: 5 * time.Second}) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, srv.URL, + onlyReader{strings.NewReader("12345")}) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + resp, err := c.Do(context.Background(), req) + if err != nil { + t.Fatalf("Do: %v", err) + } + resp.Body.Close() + if req.ContentLength != 0 { + t.Errorf("caller ContentLength = %d, want it untouched (0)", req.ContentLength) + } +} + +// TestDo_TrailerKeepsChunkedEncoding verifies that a request with Trailer set +// is not given a Content-Length, so its trailers still go out. +func TestDo_TrailerKeepsChunkedEncoding(t *testing.T) { + var mu sync.Mutex + var gotLen int64 + var gotTrailer string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _, _ = io.Copy(io.Discard, r.Body) + mu.Lock() + gotLen = r.ContentLength + gotTrailer = r.Trailer.Get("X-Sum") + mu.Unlock() + w.WriteHeader(http.StatusOK) + })) + defer srv.Close() + + c := New(ClientOptions{MaxRetries: 1, RetryDelay: time.Millisecond, Timeout: 5 * time.Second}) + req, err := http.NewRequestWithContext(context.Background(), http.MethodPost, srv.URL, + onlyReader{strings.NewReader("12345")}) + if err != nil { + t.Fatalf("NewRequest: %v", err) + } + req.Trailer = http.Header{"X-Sum": {"abc"}} + resp, err := c.Do(context.Background(), req) + if err != nil { + t.Fatalf("Do: %v", err) + } + resp.Body.Close() + mu.Lock() + defer mu.Unlock() + if gotLen != -1 || gotTrailer != "abc" { + t.Errorf("server saw ContentLength %d trailer %q, want -1 and %q", gotLen, gotTrailer, "abc") + } +}