From a5d92cfacb5be94bc70aa092af9f6976252fd66d Mon Sep 17 00:00:00 2001 From: Abhishek Choudhary Date: Sun, 19 Jul 2026 14:49:19 +0545 Subject: [PATCH] fix: fail loud when enable-csrf is set but csrf-key is missing Previously, when the enable-csrf annotation was set to true but the csrf-key annotation was missing or empty, csrf.Handle returned (nil, nil), which is indistinguishable from the annotation being absent. The route was then programmed without the csrf plugin and the Ingress reconciled cleanly, so an endpoint the operator believed was protected ran unprotected with no error, event, or log. Return an error in this case so the parser surfaces it instead of silently dropping the security-relevant plugin. The error message does not echo the key value. --- internal/adc/translator/annotations/plugins/csrf.go | 7 ++++++- .../adc/translator/annotations/plugins/csrf_test.go | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/internal/adc/translator/annotations/plugins/csrf.go b/internal/adc/translator/annotations/plugins/csrf.go index 1bee407c..df4ab3c9 100644 --- a/internal/adc/translator/annotations/plugins/csrf.go +++ b/internal/adc/translator/annotations/plugins/csrf.go @@ -16,6 +16,8 @@ package plugins import ( + "fmt" + adctypes "github.com/apache/apisix-ingress-controller/api/adc" "github.com/apache/apisix-ingress-controller/internal/adc/translator/annotations" ) @@ -39,7 +41,10 @@ func (c *csrf) Handle(e annotations.Extractor) (any, error) { key := e.GetStringAnnotation(annotations.AnnotationsCsrfKey) if key == "" { - return nil, nil + // csrf requested but the key is missing: fail loud instead of + // silently dropping the plugin and programming the route unprotected. + return nil, fmt.Errorf("annotation %q is enabled but %q is missing or empty", + annotations.AnnotationsEnableCsrf, annotations.AnnotationsCsrfKey) } return &adctypes.CSRFConfig{ diff --git a/internal/adc/translator/annotations/plugins/csrf_test.go b/internal/adc/translator/annotations/plugins/csrf_test.go index 0f681703..b7358714 100644 --- a/internal/adc/translator/annotations/plugins/csrf_test.go +++ b/internal/adc/translator/annotations/plugins/csrf_test.go @@ -43,10 +43,16 @@ func TestCSRFHandler(t *testing.T) { assert.Nil(t, err, "checking given error") assert.Nil(t, out, "checking given output") - // Test with enable-csrf true but no key + // Test with enable-csrf true but no key: must fail loud, not drop silently anno[annotations.AnnotationsEnableCsrf] = "true" delete(anno, annotations.AnnotationsCsrfKey) out, err = p.Handle(annotations.NewExtractor(anno)) - assert.Nil(t, err, "checking given error") + assert.Error(t, err, "expecting an error when csrf-key is missing") assert.Nil(t, out, "checking given output when key is missing") + + // Test with enable-csrf true but empty key: same fail-loud behavior + anno[annotations.AnnotationsCsrfKey] = "" + out, err = p.Handle(annotations.NewExtractor(anno)) + assert.Error(t, err, "expecting an error when csrf-key is empty") + assert.Nil(t, out, "checking given output when key is empty") }