Restrict device plugin hostPath volumes to allowed prefixes#1292
Conversation
✅ Deploy Preview for kubernetes-sigs-kmm ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: TomerNewman The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1292 +/- ##
==========================================
- Coverage 79.09% 73.57% -5.52%
==========================================
Files 51 67 +16
Lines 5109 5022 -87
==========================================
- Hits 4041 3695 -346
- Misses 882 1157 +275
+ Partials 186 170 -16 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Add webhook validation that rejects hostPath volumes in spec.devicePlugin.volumes unless the path resolves under /dev, /sys, /var or /opt. This prevents device plugins from mounting arbitrary host directories such as / or /etc.
121cd02 to
93a10ba
Compare
|
please update the title and the commit message |
Done |
| func isAllowedHostPath(hostPath string) bool { | ||
| p := filepath.Clean(hostPath) | ||
| for _, prefix := range allowedHostPathPrefixes { | ||
| if p == prefix || strings.HasPrefix(p, prefix+"/") { |
There was a problem hiding this comment.
why do we need prefix + "/"? What happens if the volume is "/var"?
There was a problem hiding this comment.
The check has two parts:
if cleanPath == prefix || strings.HasPrefix(cleanPath, prefix+"/") {cleanPath == prefixhandles the exact match — so/varby itself is allowed.strings.HasPrefix(cleanPath, prefix+"/")handles subdirectories like/var/lib/kubelet.
The + "/" is specifically to prevent false positives: without it, strings.HasPrefix("/variable", "/var") would return true, incorrectly allowing a path like /variable. The trailing slash ensures we only match actual subdirectories.
|
/lgtm |
Summary
Add webhook validation that rejects hostPath volumes in
spec.devicePlugin.volumesunless the path resolves under/dev,/sys,/varor/opt. This prevents device plugins from mounting arbitrary host directories such as/or/etc.Details
validateDevicePluginVolumesfunction in the webhook withfilepath.Cleanto prevent path traversal bypasses (e.g./dev/../etc)validateModuleon both create and updateTest plan
make fmtpassesmake lintpassesmake unit-testpasses/cc @ybettan @yevgeny-shnaidman
/assign @ybettan @yevgeny-shnaidman