feat: whitelist for allowed env and paths for lua#9220
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: db0ebb7b72
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
rudrakhp
force-pushed
the
configurable_critical_lua
branch
from
db0ebb7 to
54c5aca
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9220 +/- ##
==========================================
- Coverage 74.89% 74.88% -0.02%
==========================================
Files 252 252
Lines 40799 40830 +31
==========================================
+ Hits 30558 30575 +17
- Misses 8158 8169 +11
- Partials 2083 2086 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rudrakhp
force-pushed
the
configurable_critical_lua
branch
2 times, most recently
from
b71ccdb to
47fdd01
Compare
Signed-off-by: Rudrakh Panigrahi <rudrakh97@gmail.com>
rudrakhp
force-pushed
the
configurable_critical_lua
branch
from
47fdd01 to
65635d5
Compare
| // +kubebuilder:validation:items:MaxLength=4096 | ||
| // +kubebuilder:validation:XValidation:rule="self.all(p, p.trim() != '')",message="allowedPaths entries must not be blank or whitespace-only" | ||
| // +optional | ||
| AllowedPaths []string `json:"allowedPaths,omitempty"` |
There was a problem hiding this comment.
the configuration looks a little redundant.
luaValidationAllowlist:
allowedPaths:what about using following?
luaValidation:
allowedPaths:There was a problem hiding this comment.
Problem is there is an existing luaValidation API used for configuring the mode/level. It is a string enum type so can't have child params. How about:
luaStrictValidationAllowlist:
paths:
envVars:
There was a problem hiding this comment.
Problem is there is an existing
luaValidationAPI used for configuring the mode/level. It is a string enum type so can't have child params. How about:luaStrictValidationAllowlist: paths: envVars:
I'm mostly concerts with the Allow in the naming, which might be a blocker if we want to add blacklist for the validation instead of whitelist.
There was a problem hiding this comment.
Got it how about:
luaStrictValidation:
allowedPaths:
allowedEnvVars:
2 participants
What type of PR is this?
feat: whitelist for allowed env and paths for lua
What this PR does / why we need it:
To allow gateway admins to configure env vars and paths to be explicitly allowed for access by Lua configured in the gateway.
Which issue(s) this PR fixes:
Fixes #7955
Release Notes: Yes