From 7a95bac64fea1209d355d41c8354cb57713e6bc2 Mon Sep 17 00:00:00 2001 From: Varun Chawla <34209028+veeceey@users.noreply.github.com> Date: Tue, 3 Mar 2026 07:12:05 -0800 Subject: [PATCH] Fix HasSecureHeadersDefined returning false when stsSeconds is 0 --- pkg/config/dynamic/middleware_test.go | 63 +++++++++++++++++++-- pkg/config/dynamic/middlewares.go | 4 +- pkg/config/dynamic/zz_generated.deepcopy.go | 5 ++ pkg/config/label/label_test.go | 4 +- pkg/middlewares/headers/headers_test.go | 9 +++ pkg/middlewares/headers/secure.go | 3 +- pkg/middlewares/headers/secure_test.go | 5 +- pkg/provider/kv/kv_test.go | 2 +- pkg/redactor/redactor_config_test.go | 2 +- 9 files changed, 83 insertions(+), 14 deletions(-) diff --git a/pkg/config/dynamic/middleware_test.go b/pkg/config/dynamic/middleware_test.go index 87a0b34df4..2fb575d255 100644 --- a/pkg/config/dynamic/middleware_test.go +++ b/pkg/config/dynamic/middleware_test.go @@ -5,6 +5,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "k8s.io/utils/ptr" ) func Test_GetStrategy_ipv6Subnet(t *testing.T) { @@ -19,16 +20,16 @@ func Test_GetStrategy_ipv6Subnet(t *testing.T) { { desc: "Zero subnet", expectError: true, - ipv6Subnet: intPtr(0), + ipv6Subnet: ptr.To(0), }, { desc: "Subnet greater that 128", expectError: true, - ipv6Subnet: intPtr(129), + ipv6Subnet: ptr.To(129), }, { desc: "Valid subnet", - ipv6Subnet: intPtr(128), + ipv6Subnet: ptr.To(128), }, } @@ -52,6 +53,58 @@ func Test_GetStrategy_ipv6Subnet(t *testing.T) { } } -func intPtr(value int) *int { - return &value +func TestHasSecureHeadersDefined(t *testing.T) { + testCases := []struct { + desc string + headers *Headers + expected bool + }{ + { + desc: "Nil headers", + headers: nil, + expected: false, + }, + { + desc: "Empty headers", + headers: &Headers{}, + expected: false, + }, + { + desc: "STSSeconds set to non-zero", + headers: &Headers{ + STSSeconds: ptr.To(int64(42)), + }, + expected: true, + }, + { + desc: "STSSeconds set to zero", + headers: &Headers{ + STSSeconds: ptr.To(int64(0)), + }, + expected: true, + }, + { + desc: "STSSeconds nil (not set)", + headers: &Headers{ + FrameDeny: true, + }, + expected: true, + }, + { + desc: "Only ForceSTSHeader", + headers: &Headers{ + ForceSTSHeader: true, + }, + expected: true, + }, + } + + for _, test := range testCases { + t.Run(test.desc, func(t *testing.T) { + t.Parallel() + + result := test.headers.HasSecureHeadersDefined() + assert.Equal(t, test.expected, result) + }) + } } diff --git a/pkg/config/dynamic/middlewares.go b/pkg/config/dynamic/middlewares.go index 8032f61551..a04f99713a 100644 --- a/pkg/config/dynamic/middlewares.go +++ b/pkg/config/dynamic/middlewares.go @@ -331,7 +331,7 @@ type Headers struct { // STSSeconds defines the max-age of the Strict-Transport-Security header. // If set to 0, the header is not set. // +kubebuilder:validation:Minimum=0 - STSSeconds int64 `json:"stsSeconds,omitempty" toml:"stsSeconds,omitempty" yaml:"stsSeconds,omitempty" export:"true"` + STSSeconds *int64 `json:"stsSeconds,omitempty" toml:"stsSeconds,omitempty" yaml:"stsSeconds,omitempty" export:"true"` // STSIncludeSubdomains defines whether the includeSubDomains directive is appended to the Strict-Transport-Security header. STSIncludeSubdomains bool `json:"stsIncludeSubdomains,omitempty" toml:"stsIncludeSubdomains,omitempty" yaml:"stsIncludeSubdomains,omitempty" export:"true"` // STSPreload defines whether the preload flag is appended to the Strict-Transport-Security header. @@ -407,7 +407,7 @@ func (h *Headers) HasSecureHeadersDefined() bool { (h.SSLForceHost != nil && *h.SSLForceHost) || (h.SSLHost != nil && *h.SSLHost != "") || len(h.SSLProxyHeaders) != 0 || - h.STSSeconds != 0 || + h.STSSeconds != nil || h.STSIncludeSubdomains || h.STSPreload || h.ForceSTSHeader || diff --git a/pkg/config/dynamic/zz_generated.deepcopy.go b/pkg/config/dynamic/zz_generated.deepcopy.go index 7e774bc922..88a956e18e 100644 --- a/pkg/config/dynamic/zz_generated.deepcopy.go +++ b/pkg/config/dynamic/zz_generated.deepcopy.go @@ -660,6 +660,11 @@ func (in *Headers) DeepCopyInto(out *Headers) { (*out)[key] = val } } + if in.STSSeconds != nil { + in, out := &in.STSSeconds, &out.STSSeconds + *out = new(int64) + **out = **in + } if in.FeaturePolicy != nil { in, out := &in.FeaturePolicy, &out.FeaturePolicy *out = new(string) diff --git a/pkg/config/label/label_test.go b/pkg/config/label/label_test.go index 390b2ebb6f..903aa1a097 100644 --- a/pkg/config/label/label_test.go +++ b/pkg/config/label/label_test.go @@ -640,7 +640,7 @@ func TestDecodeConfiguration(t *testing.T) { "name1": "foobar", }, SSLForceHost: pointer(true), - STSSeconds: 42, + STSSeconds: pointer(int64(42)), STSIncludeSubdomains: true, STSPreload: true, ForceSTSHeader: true, @@ -1195,7 +1195,7 @@ func TestEncodeConfiguration(t *testing.T) { "name1": "foobar", }, SSLForceHost: pointer(true), - STSSeconds: 42, + STSSeconds: pointer(int64(42)), STSIncludeSubdomains: true, STSPreload: true, ForceSTSHeader: true, diff --git a/pkg/middlewares/headers/headers_test.go b/pkg/middlewares/headers/headers_test.go index df71e68964..b968b90797 100644 --- a/pkg/middlewares/headers/headers_test.go +++ b/pkg/middlewares/headers/headers_test.go @@ -13,6 +13,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/traefik/traefik/v3/pkg/config/dynamic" + "k8s.io/utils/ptr" ) func TestNew_withoutOptions(t *testing.T) { @@ -24,6 +25,14 @@ func TestNew_withoutOptions(t *testing.T) { assert.Nil(t, mid) } +func TestNew_withSTSSecondsZero(t *testing.T) { + next := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(http.StatusOK) }) + + mid, err := New(t.Context(), next, dynamic.Headers{STSSeconds: ptr.To(int64(0))}, "testing") + require.NoError(t, err) + assert.NotNil(t, mid) +} + func TestNew_allowedHosts(t *testing.T) { testCases := []struct { desc string diff --git a/pkg/middlewares/headers/secure.go b/pkg/middlewares/headers/secure.go index 9769627d1c..b2858b4cbe 100644 --- a/pkg/middlewares/headers/secure.go +++ b/pkg/middlewares/headers/secure.go @@ -6,6 +6,7 @@ import ( "github.com/traefik/traefik/v3/pkg/config/dynamic" "github.com/traefik/traefik/v3/pkg/middlewares" "github.com/unrolled/secure" + "k8s.io/utils/ptr" ) type secureHeader struct { @@ -33,7 +34,7 @@ func newSecure(next http.Handler, cfg dynamic.Headers, contextKey string) *secur AllowedHosts: cfg.AllowedHosts, HostsProxyHeaders: cfg.HostsProxyHeaders, SSLProxyHeaders: cfg.SSLProxyHeaders, - STSSeconds: cfg.STSSeconds, + STSSeconds: ptr.Deref(cfg.STSSeconds, 0), PermissionsPolicy: cfg.PermissionsPolicy, SecureContextKey: contextKey, } diff --git a/pkg/middlewares/headers/secure_test.go b/pkg/middlewares/headers/secure_test.go index d16c8c273a..5b81efe312 100644 --- a/pkg/middlewares/headers/secure_test.go +++ b/pkg/middlewares/headers/secure_test.go @@ -7,6 +7,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/traefik/traefik/v3/pkg/config/dynamic" + "k8s.io/utils/ptr" ) // Middleware tests based on https://github.com/unrolled/secure @@ -27,7 +28,7 @@ func Test_newSecure_modifyResponse(t *testing.T) { { desc: "STSSeconds", cfg: dynamic.Headers{ - STSSeconds: 1, + STSSeconds: ptr.To(int64(1)), ForceSTSHeader: true, }, expected: http.Header{"Strict-Transport-Security": []string{"max-age=1"}}, @@ -35,7 +36,7 @@ func Test_newSecure_modifyResponse(t *testing.T) { { desc: "STSSeconds and STSPreload", cfg: dynamic.Headers{ - STSSeconds: 1, + STSSeconds: ptr.To(int64(1)), ForceSTSHeader: true, STSPreload: true, }, diff --git a/pkg/provider/kv/kv_test.go b/pkg/provider/kv/kv_test.go index 0a4ed49dd1..1dd995ab84 100644 --- a/pkg/provider/kv/kv_test.go +++ b/pkg/provider/kv/kv_test.go @@ -621,7 +621,7 @@ func Test_buildConfiguration(t *testing.T) { "name0": "foobar", }, SSLForceHost: pointer(true), - STSSeconds: 42, + STSSeconds: pointer(int64(42)), STSIncludeSubdomains: true, STSPreload: true, ForceSTSHeader: true, diff --git a/pkg/redactor/redactor_config_test.go b/pkg/redactor/redactor_config_test.go index a461f474c9..3fc4f344a7 100644 --- a/pkg/redactor/redactor_config_test.go +++ b/pkg/redactor/redactor_config_test.go @@ -209,7 +209,7 @@ func init() { AllowedHosts: []string{"foo"}, HostsProxyHeaders: []string{"foo"}, SSLProxyHeaders: map[string]string{"foo": "bar"}, - STSSeconds: 42, + STSSeconds: pointer(int64(42)), STSIncludeSubdomains: true, STSPreload: true, ForceSTSHeader: true,