Describe the bug
The cross-tenant rejection test at integration/ingester_stream_push_test.go:207 does not reliably exercise the security property V01 (PR #7475) was added to enforce. The assertion is structured as:
if err == nil {
require.NotEqual(t, 200, status)
}
A signature mismatch in PushStream typically surfaces as a non-nil client.Push error (the stream is closed by the ingester), so err != nil and the loop asserts nothing. The test would still pass even if stream signing were completely broken — it has no positive check that the rejection is signature-related.
To Reproduce
- Locally remove the signature-verification call inside
pkg/ingester/ingester.go's PushStream handler (or the streaming signing interceptor in pkg/util/grpcclient/signing_handler.go) so it always accepts.
- Run
go test -tags requires_docker,integration ./integration/... -run TestIngesterStreamPushSecurity.
- Observe the test still passes.
Expected behavior
Make the assertion deterministic:
require.NoError(t, err) (or accept the specific stream-closed transport error).
- Assert a non-200 status code.
- Ideally assert the response carries
signature mismatch (the ErrSignatureMismatch from pkg/util/grpcclient/signing_handler.go).
That way a regression to "stream signing accepts everything" is caught instead of silently passing.
Environment:
Additional Context
Describe the bug
The cross-tenant rejection test at
integration/ingester_stream_push_test.go:207does not reliably exercise the security property V01 (PR #7475) was added to enforce. The assertion is structured as:A signature mismatch in
PushStreamtypically surfaces as a non-nilclient.Pusherror (the stream is closed by the ingester), soerr != niland the loop asserts nothing. The test would still pass even if stream signing were completely broken — it has no positive check that the rejection is signature-related.To Reproduce
pkg/ingester/ingester.go'sPushStreamhandler (or the streaming signing interceptor inpkg/util/grpcclient/signing_handler.go) so it always accepts.go test -tags requires_docker,integration ./integration/... -run TestIngesterStreamPushSecurity.Expected behavior
Make the assertion deterministic:
require.NoError(t, err)(or accept the specific stream-closed transport error).signature mismatch(theErrSignatureMismatchfrompkg/util/grpcclient/signing_handler.go).That way a regression to "stream signing accepts everything" is caught instead of silently passing.
Environment:
release-1.21(V01 backport in PR Backport security audit fixes (V01-V07), snappy gRPC fix, and stream push panic fix to release-1.21 #7574).Additional Context