nic-6443 commented on code in PR #2813:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2813#discussion_r3643562612


##########
internal/adc/translator/annotations/plugins/csrf.go:
##########
@@ -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.

Review Comment:
   This still fails open at the route level. plugins.Parse logs handler errors 
and continues, then returns a nil error, so this error never reaches 
reconciliation and the route is still programmed without the csrf plugin. 
Please propagate the handler error from plugins.Parse and cover the 
parser/reconcile path so enable-csrf=true cannot reconcile successfully without 
a key.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to