shreemaan-abhishek commented on PR #2813:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2813#issuecomment-5088115622
@AlinsRan agreed, and I have moved this to the Ingress webhook.
`enable-csrf: true` with an empty/missing `csrf-key` is now a hard error in
`ValidateCreate` and `ValidateUpdate`, rejected at `kubectl apply`. I reverted
the translation-failure change entirely.
Both your points held up when I traced them:
- **Blast radius.** Failing the whole translation on any annotation error
coupled a non-security typo to the entire Ingress. Since an Ingress syncs
`{Service, SSL}` together, a bad `upstream-scheme` would have taken TLS down
with it. Enforcing at admission with error-vs-warning severity is the right
split; the translation layer is all-or-nothing per Ingress and structurally
can't do that.
- **Route drop != fail closed.** Correct that a dropped route falls through
to a broader wildcard route. The webhook avoids this by blocking the bad intent
before it is ever stored, so the route never changes state.
One thing I did not carry over yet: turning
`upstream-scheme`/retry/timeout/regex errors into admission **warnings**. That
is a good improvement but it is a separate concern from this CSRF finding, so I
left it out to keep the PR focused. Happy to do it as a follow-up.
Note for anyone reading: the Ingress webhook is deployed with
`failurePolicy: Ignore`, so this is a best-effort gate rather than a hard
boundary. Hardening that bypass is out of scope here.
--
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]
