Skip to content

fix(signer): preserve asynchronous request body handoff #29

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jinxiao merged 2 commits into from
Oct 4, 2026
Merged

fix(signer): preserve asynchronous request body handoff #29

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
95b01f1
fix(signer): preserve asynchronous request body handoff
jinxiao Oct 4, 2026
7301028
chore: normalize scoped acceptance report line endings
jinxiao Oct 4, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/CHECKLIST.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,3 +275,4 @@ Race-detector execution is intentionally excluded because CGO must remain disabl
- [x] REL-VISIBILITY: Bound post-GoReleaser read-after-write waiting to 30 seconds, validate every observed Release against frozen notes, and propagate conflicts/API errors immediately without repeating writes. Thirty-three offline release regressions passed, including delayed draft/finalization, timeout preservation, and immediate error propagation (2026-10-04). Hosted run 37206952188 published and consumer-verified root v0.1.0, then stopped before the new commons/regex draft became visible; issue #20 tracks recovery.
- [x] REL-PUBLIC: Publish all 27 maintained public modules as v0.1.0 from merged preparation PR #16 at immutable SHA 34fcc751355a2b29bca393748139ff96dd8bbf2a. The hosted GoReleaser OSS v2.18.2 run finalized every Release only after independent public download, tidy, build, module/checksum verification, and rejection of local replacements, with CGO disabled, GOWORK=off and fresh session caches. Release issue #15 closed automatically after completion. The frozen tracer/xray adapter was excluded. See [RELEASE_ACCEPTANCE.json](RELEASE_ACCEPTANCE.json) (2026-10-04). Hosted preparation controls and native PR/main checks were exercised; browser form rendering and default-token authorization remain separate rollout checks.
- [x] REL-UNIFIED: Replace new component-scoped releases with one version for all maintained public modules, preserving independent Go paths and prefixed tags. Add project release_version, aggregate auto-increment policy, cohort-wide reserved-version avoidance, synchronized internal requirements, schema-2 full-cohort validation, component/repository grouped root notes, and root finalization last with the stable Latest label. Twenty-one isolated automation and 37 release regressions passed, including Logger-only cohort bumps, divergent published baselines/reserved nested tags, partial/mixed-plan rejection, aligned requirements, failure/resume ordering, legacy schema-1 recovery, and real GoReleaser OSS v2.18.2 root/component/prerelease draft/finalization and Latest configuration. Eighteen contribution/form regressions and actionlint v1.7.12 passed (2026-10-05). This records implementation acceptance; no new public version was prepared or published. See issue #23 and [Releasing modules](RELEASING.md).
- [x] SIGNER-BODY-HANDOFF: Preserve asynchronous base-transport body ownership for custom signers returning the input request or a clone sharing its body. Four deterministic success/error regressions reproduced premature closure before the fix; packaged Signer tests, vet, tidy verification and its independent consumer build pass with CGO disabled. Distinct/empty bodies and nil signing results retain cleanup coverage. See issue #25 and [Signer ownership](SIGNER.md) (2026-10-05).
164 changes: 9 additions & 155 deletions docs/MODULE_ACCEPTANCE_SCOPED.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,170 +5,24 @@
"completed": true,
"modules": [
{
"path": "github.com/rambow-cloud/powertools-lambda-go/integration",
"directory": "integration",
"public": false,
"path": "github.com/rambow-cloud/powertools-lambda-go/signer",
"directory": "signer",
"public": true,
"version": "v0.1.0",
"archive_sha256": "76b0e5c94b46ca250ea6b16ff8f92d5d8529a530438845e54633920ffb0849a7",
"archive_sha256": "7e7bc65590a5f255d3a4381394ec15d75848455196dab3a5a6aa3f3884e96726",
"tests": true,
"vet": true,
"tidy": true,
"binary_os": "linux",
"binary_architecture": "amd64",
"tests_runtime": "docker-linux-amd64",
"consumer_build": true,
"external_modules": [
"cel.dev/expr",
"cloud.google.com/go/auth",
"cloud.google.com/go/compute/metadata",
"github.com/GoogleCloudPlatform/opentelemetry-operations-go/detectors/gcp",
"github.com/apapsch/go-jsonmerge/v2",
"github.com/aws/aws-cryptographic-material-providers-library/releases/go/dynamodb",
"github.com/aws/aws-cryptographic-material-providers-library/releases/go/kms",
"github.com/aws/aws-cryptographic-material-providers-library/releases/go/mpl",
"github.com/aws/aws-cryptographic-material-providers-library/releases/go/primitives",
"github.com/aws/aws-cryptographic-material-providers-library/releases/go/smithy-dafny-standard-library",
"github.com/aws/aws-encryption-sdk/releases/go/encryption-sdk",
"github.com/aws/aws-lambda-go",
"github.com/aws/aws-sdk-go-v2",
"github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream",
"github.com/aws/aws-sdk-go-v2/config",
"github.com/aws/aws-sdk-go-v2/credentials",
"github.com/aws/aws-sdk-go-v2/feature/dynamodb/attributevalue",
"github.com/aws/aws-sdk-go-v2/feature/ec2/imds",
"github.com/aws/aws-sdk-go-v2/internal/configsources",
"github.com/aws/aws-sdk-go-v2/internal/endpoints/v2",
"github.com/aws/aws-sdk-go-v2/internal/v4a",
"github.com/aws/aws-sdk-go-v2/service/appconfigdata",
"github.com/aws/aws-sdk-go-v2/service/dynamodb",
"github.com/aws/aws-sdk-go-v2/service/dynamodbstreams",
"github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding",
"github.com/aws/aws-sdk-go-v2/service/internal/checksum",
"github.com/aws/aws-sdk-go-v2/service/internal/endpoint-discovery",
"github.com/aws/aws-sdk-go-v2/service/internal/presigned-url",
"github.com/aws/aws-sdk-go-v2/service/internal/s3shared",
"github.com/aws/aws-sdk-go-v2/service/kms",
"github.com/aws/aws-sdk-go-v2/service/route53",
"github.com/aws/aws-sdk-go-v2/service/s3",
"github.com/aws/aws-sdk-go-v2/service/secretsmanager",
"github.com/aws/aws-sdk-go-v2/service/signin",
"github.com/aws/aws-sdk-go-v2/service/sns",
"github.com/aws/aws-sdk-go-v2/service/sqs",
"github.com/aws/aws-sdk-go-v2/service/ssm",
"github.com/aws/aws-sdk-go-v2/service/sso",
"github.com/aws/aws-sdk-go-v2/service/ssooidc",
"github.com/aws/aws-sdk-go-v2/service/sts",
"github.com/aws/smithy-go",
"github.com/bsm/ginkgo/v2",
"github.com/bsm/gomega",
"github.com/cenkalti/backoff/v5",
"github.com/cespare/xxhash/v2",
"github.com/cncf/xds/go",
"github.com/dafny-lang/DafnyRuntimeGo/v4",
"github.com/davecgh/go-spew",
"github.com/dlclark/regexp2",
"github.com/dlclark/regexp2/v2",
"github.com/envoyproxy/go-control-plane",
"github.com/envoyproxy/go-control-plane/envoy",
"github.com/envoyproxy/go-control-plane/ratelimit",
"github.com/envoyproxy/protoc-gen-validate",
"github.com/ettle/strcase",
"github.com/felixge/httpsnoop",
"github.com/go-jose/go-jose/v4",
"github.com/go-logr/logr",
"github.com/go-logr/stdr",
"github.com/go-openapi/analysis",
"github.com/go-openapi/errors",
"github.com/go-openapi/jsonpointer",
"github.com/go-openapi/jsonreference",
"github.com/go-openapi/loads",
"github.com/go-openapi/runtime",
"github.com/go-openapi/runtime/server-middleware",
"github.com/go-openapi/spec",
"github.com/go-openapi/strfmt",
"github.com/go-openapi/swag",
"github.com/go-openapi/swag/cmdutils",
"github.com/go-openapi/swag/conv",
"github.com/go-openapi/swag/fileutils",
"github.com/go-openapi/swag/jsonutils",
"github.com/go-openapi/swag/loading",
"github.com/go-openapi/swag/mangling",
"github.com/go-openapi/swag/netutils",
"github.com/go-openapi/swag/pools",
"github.com/go-openapi/swag/stringutils",
"github.com/go-openapi/swag/typeutils",
"github.com/go-openapi/swag/yamlutils",
"github.com/go-openapi/validate",
"github.com/go-viper/mapstructure/v2",
"github.com/golang/glog",
"github.com/golang/protobuf",
"github.com/golang/snappy",
"github.com/google/go-cmp",
"github.com/google/gofuzz",
"github.com/google/s2a-go",
"github.com/google/uuid",
"github.com/googleapis/enterprise-certificate-proxy",
"github.com/googleapis/gax-go/v2",
"github.com/grpc-ecosystem/grpc-gateway/v2",
"github.com/hamba/avro/v2",
"github.com/jmespath-community/go-jmespath",
"github.com/json-iterator/go",
"github.com/klauspost/compress",
"github.com/klauspost/cpuid/v2",
"github.com/kr/pretty",
"github.com/modern-go/concurrent",
"github.com/modern-go/reflect2",
"github.com/oapi-codegen/runtime",
"github.com/oklog/ulid/v2",
"github.com/planetscale/vtprotobuf",
"github.com/pmezard/go-difflib",
"github.com/redis/go-redis/v9",
"github.com/rogpeppe/fastuuid",
"github.com/rogpeppe/go-internal",
"github.com/santhosh-tekuri/jsonschema/v6",
"github.com/spiffe/go-spiffe/v2",
"github.com/stretchr/objx",
"github.com/stretchr/testify",
"github.com/zeebo/xxh3",
"go.opentelemetry.io/auto/sdk",
"go.opentelemetry.io/contrib/detectors/gcp",
"go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws",
"go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc",
"go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp",
"go.opentelemetry.io/contrib/propagators/aws",
"go.opentelemetry.io/otel",
"go.opentelemetry.io/otel/exporters/otlp/otlptrace",
"go.opentelemetry.io/otel/exporters/otlp/otlptrace/otlptracehttp",
"go.opentelemetry.io/otel/exporters/stdout/stdouttrace",
"go.opentelemetry.io/otel/metric",
"go.opentelemetry.io/otel/sdk",
"go.opentelemetry.io/otel/sdk/metric",
"go.opentelemetry.io/otel/trace",
"go.opentelemetry.io/proto/otlp",
"go.uber.org/atomic",
"go.uber.org/goleak",
"go.yaml.in/yaml/v3",
"golang.org/x/crypto",
"golang.org/x/exp",
"golang.org/x/mod",
"golang.org/x/net",
"golang.org/x/oauth2",
"golang.org/x/sync",
"golang.org/x/sys",
"golang.org/x/term",
"golang.org/x/text",
"golang.org/x/tools",
"gonum.org/v1/gonum",
"google.golang.org/genproto/googleapis/api",
"google.golang.org/genproto/googleapis/rpc",
"google.golang.org/grpc",
"google.golang.org/protobuf",
"gopkg.in/check.v1",
"gopkg.in/yaml.v3"
]
"github.com/aws/smithy-go"
],
"binary_bytes": 5602304
}
],
"scope": "Selected modules only; this report does not replace full-workspace acceptance",
"selected_directories": [
"integration"
"signer"
]
}
2 changes: 1 addition & 1 deletion docs/SIGNER.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ Executable evidence: [signer/signer_test.go](https://github.com/rambow-cloud/pow

`Sign(*http.Request)` signs without sending. It clones the URL and headers and returns a replayable body with `GetBody`. If the input supplies `GetBody`, its body stream is untouched. Otherwise the finite stream is buffered and restored on the original request, preserving its `Close`. After a read failure the successfully read prefix is restored in front of the unread stream. Standalone callers close both bodies and must not use the input concurrently with signing. Buffering is proportional to payload size; context checks cannot forcibly interrupt an arbitrary blocking application Reader.

`Transport` accepts any `Signer` interface and any `http.RoundTripper`. It owns and closes the incoming body even if signing fails, while the downstream transport owns the signed body. It performs no retry loop. `HTTPClient` copies its supplied client and defaults to stopping at redirects. Supply a deliberate `CheckRedirect` policy to allow trusted destinations; each permitted redirect is signed again, and normal Go replay rules apply to 307/308 responses.
`Transport` accepts any `Signer` interface and any `http.RoundTripper`. It closes the incoming body if signing fails. After handoff, the downstream transport owns the signed body's lifetime; closing it also releases the incoming body. Custom signers may return the input request or a clone sharing its body. The base transport can finish reading and closing that body after `RoundTrip` returns, including error paths, as permitted by [Go's transport contract](https://pkg.go.dev/net/http#RoundTripper). Empty signed bodies retain Go's empty-body framing and release the incoming body immediately. It performs no retry loop. `HTTPClient` copies its supplied client and defaults to stopping at redirects. Supply a deliberate `CheckRedirect` policy to allow trusted destinations; each permitted redirect is signed again, and normal Go replay rules apply to 307/308 responses.

Compose with tracing as `signer.HTTPClient(s, tr.HTTPClient(baseClient))`. Neither utility imports the other. AWS SDK service clients already sign their own requests and must not receive another signing layer.

Expand Down
41 changes: 39 additions & 2 deletions signer/transport.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@ package signer

import (
"fmt"
"io"
"net/http"
"sync"
)

// Signer separates request signing from sending and permits custom algorithms.
Expand All @@ -21,9 +23,14 @@ func (t *Transport) RoundTrip(request *http.Request) (*http.Response, error) {
if request == nil {
return nil, &SigningError{fmt.Errorf("request is required")}
}
// A RoundTripper owns its input body, including error paths. Sign may restore it.
// Signers may retain or wrap the input body. Share idempotent closure across
// those aliases without comparing application ReadClosers, which may not be comparable.
if request.Body != nil && request.Body != http.NoBody {
request.Body = &transportBody{ReadCloser: request.Body}
}
handedOff := false
defer func() {
if request.Body != nil {
if !handedOff && request.Body != nil {
_ = request.Body.Close()
}
}()
Expand All @@ -41,9 +48,39 @@ func (t *Transport) RoundTrip(request *http.Request) (*http.Response, error) {
if base == nil {
base = http.DefaultTransport
}
if signed.Body == nil || signed.Body == http.NoBody {
// Preserve net/http's empty-body framing. There is no upload to wait for.
if request.Body != nil {
_ = request.Body.Close()
}
} else {
// A base transport may finish the upload after RoundTrip returns, including
// error paths. Release both bodies only when that transport closes its body.
signed.Body = &transportBody{ReadCloser: signed.Body, input: request.Body}
}
handedOff = true
return base.RoundTrip(signed)
}

type transportBody struct {
io.ReadCloser
input io.Closer
once sync.Once
err error
}

func (b *transportBody) Close() error {
b.once.Do(func() {
b.err = b.ReadCloser.Close()
if b.input != nil {
if err := b.input.Close(); b.err == nil {
b.err = err
}
}
})
return b.err
}

// HTTPClient copies a client and installs signing without changing its owner.
// Redirects are disabled by default to avoid signing an untrusted destination.
// Supply CheckRedirect explicitly to permit trusted redirects; each is re-signed.
Expand Down
116 changes: 116 additions & 0 deletions signer/transport_body_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package signer

import (
"errors"
"io"
"net/http"
"strings"
"sync/atomic"
"testing"
)

type uploadBody struct {
reader io.Reader
closes atomic.Int32
}

func (b *uploadBody) Read(p []byte) (int, error) {
if b.closes.Load() != 0 {
return 0, errors.New("upload body closed before reading finished")
}
return b.reader.Read(p)
}

func (b *uploadBody) Close() error { b.closes.Add(1); return nil }

func TestTransportBodyHandoff(t *testing.T) {
for _, clone := range []bool{false, true} {
for _, baseError := range []bool{false, true} {
t.Run(strings.Join([]string{map[bool]string{false: "identity", true: "clone"}[clone], map[bool]string{false: "success", true: "error"}[baseError]}, "/"), func(t *testing.T) {
body := &uploadBody{reader: strings.NewReader("payload")}
request, _ := http.NewRequest("POST", "https://example.com", body)
start, done := make(chan struct{}), make(chan struct{})
var uploaded []byte
var uploadErr error
failure := errors.New("base transport error")
transport := &Transport{
Signer: signingFunc(func(request *http.Request) (*http.Request, error) {
if clone {
return request.Clone(request.Context()), nil
}
return request, nil
}),
Base: roundTripFunc(func(request *http.Request) (*http.Response, error) {
go func() {
defer close(done)
<-start
uploaded, uploadErr = io.ReadAll(request.Body)
_ = request.Body.Close()
}()
if baseError {
return nil, failure
}
return &http.Response{StatusCode: 200, Body: http.NoBody, Header: make(http.Header)}, nil
}),
}
_, err := transport.RoundTrip(request)
close(start)
<-done
if (baseError && err != failure) || (!baseError && err != nil) {
t.Fatalf("base error changed: %v", err)
}
if uploadErr != nil || string(uploaded) != "payload" {
t.Fatalf("deferred upload: %q, %v", uploaded, uploadErr)
}
if body.closes.Load() != 1 {
t.Fatalf("body closed %d times", body.closes.Load())
}
})
}
}
}

func TestTransportDistinctBodyOwnership(t *testing.T) {
for _, empty := range []bool{false, true} {
input := &uploadBody{reader: strings.NewReader("input")}
output := &uploadBody{reader: strings.NewReader("signed")}
request, _ := http.NewRequest("POST", "https://example.com", input)
transport := &Transport{
Signer: signingFunc(func(request *http.Request) (*http.Request, error) {
signed := request.Clone(request.Context())
if empty {
signed.Body = http.NoBody
} else {
signed.Body = output
}
return signed, nil
}),
Base: roundTripFunc(func(request *http.Request) (*http.Response, error) {
if empty && request.Body != http.NoBody {
t.Fatal("empty body semantics changed")
}
_, _ = io.ReadAll(request.Body)
_ = request.Body.Close()
return &http.Response{StatusCode: 200, Body: http.NoBody}, nil
}),
}
if _, err := transport.RoundTrip(request); err != nil {
t.Fatal(err)
}
if input.closes.Load() != 1 || (!empty && output.closes.Load() != 1) {
t.Fatalf("input/output closes: %d/%d", input.closes.Load(), output.closes.Load())
}
}
}

func TestTransportNilSignedRequestClosesInput(t *testing.T) {
body := &uploadBody{reader: strings.NewReader("input")}
request, _ := http.NewRequest("POST", "https://example.com", body)
transport := &Transport{Signer: signingFunc(func(*http.Request) (*http.Request, error) { return nil, nil })}
if _, err := transport.RoundTrip(request); err == nil {
t.Fatal("nil signed request accepted")
}
if body.closes.Load() != 1 {
t.Fatalf("input closed %d times", body.closes.Load())
}
}
Loading