Add string validation to server-tokens annotation #2733
Conversation
internal/k8s/validation.go
Outdated
| const ( | ||
| commaDelimiter = "," | ||
| commaDelimiter = "," | ||
| specialCharPattern = "[{}$;]" |
There was a problem hiding this comment.
How did you come up with this list? I think the only invalid character is an unescaped \ and $ if we decide that variables are not allowed.
There was a problem hiding this comment.
Also " needs to be disallowed, because the value is enclosed in " " in the templates -- https://github.com/nginxinc/kubernetes-ingress/blob/main/internal/configs/version1/nginx-plus.ingress.tmpl#L61 and https://github.com/nginxinc/kubernetes-ingress/blob/main/internal/configs/version2/nginx-plus.virtualserver.tmpl#L117
Example of a problem:
'nginx.org/server-tokens': 'hello"world'
->
```server_tokens "hello"world";
this will cause an NGINX reload error
There was a problem hiding this comment.
Thanks for catching that. I've added an extra unit test to confirm this scenario is caught.
Codecov Report
@@ Coverage Diff @@
## main #2733 +/- ##
==========================================
- Coverage 53.54% 53.52% -0.02%
==========================================
Files 52 52
Lines 14764 14768 +4
==========================================
Hits 7905 7905
- Misses 6597 6599 +2
- Partials 262 264 +2
📣 Codecov can now indicate which changes are the most critical in Pull Requests. Learn more |
Proposed changes
Describe the use case and detail of the change. If this PR addresses an issue on GitHub, make sure to include a link to that issue here in this description (not in the title of the PR).
Checklist
Before creating a PR, run through this checklist and mark each as complete.