fix: improve path validation in Kptfile functionConfig#4470
fix: improve path validation in Kptfile functionConfig#4470NETIZEN-11 wants to merge 6 commits intokptdev:mainfrom
Conversation
…nctions - Upgrade sigs.k8s.io/kustomize/api from v0.20.1 to v0.21.0 - Upgrade sigs.k8s.io/kustomize/kyaml from v0.20.1 to v0.21.0 - Upgrade k8s.io/api from v0.34.1 to v0.35.0 - Upgrade k8s.io/apimachinery from v0.34.1 to v0.35.0 - Upgrade k8s.io/kubectl from v0.34.1 to v0.35.0 - Upgrade github.com/kptdev/krm-functions-catalog/functions/go/apply-setters from v0.2.2 to v0.2.4 - Update catalog function registry to reference new apply-setters version - Run go mod tidy to resolve transitive dependencies Resolves GitHub Issue kptdev#4406 All catalog functions remain compatible with new APIs - zero breaking changes encountered. Zero compilation errors and zero failing tests across repository. Signed-off-by: NETIZEN-11 <kumarnitesh121411@gmail.com>
…n, and CRLF - Update testdata Kptfiles to use apply-setters:v0.2.4 (was v0.2.0) which is not registered in the functions map, causing TestRender failures - Fix validateFnConfigPathSyntax to use path.IsAbs (forward-slash) instead of filepath.IsAbs so absolute path detection works correctly on all platforms - Normalize CRLF to LF in pkg_context_test.go when reading expected output files to fix TestPkgContextGenerator on Windows - Remove unused absPath helper and os import from executor_test.go Signed-off-by: NETIZEN-11 <kumarnitesh121411@gmail.com>
- Remove duplicate entries for apply-setters (v0.2.2) and krm-functions-sdk (v1.0.2) - Bump k8s.io/apiextensions-apiserver from v0.34.1 to v0.35.0 to align all k8s deps - Remove gogo/protobuf indirect dep (no longer needed) - Run go mod tidy to validate Addresses reviewer feedback on PR kptdev#4432 Signed-off-by: NETIZEN-11 <kumarnitesh121411@gmail.com>
…ility The PR bumps apply-setters to v0.2.4 in go.mod. Adding v0.2.4 to the functions registry is required so Kptfiles referencing the new version work with the built-in runner. v0.2.0 is kept for backward compatibility with existing Kptfiles that already reference that version. Also revert testdata Kptfiles back to v0.2.0 since both versions are now supported by the functions map. Addresses reviewer question on PR kptdev#4432 Signed-off-by: NETIZEN-11 <kumarnitesh121411@gmail.com>
- Replace path.IsAbs with filepath.IsAbs for consistent OS handling - Clean path before validation to normalize separators - Simplify '..' check by cleaning path first - Remove redundant 'path' package import This improves security by ensuring path validation works correctly across different operating systems and prevents directory traversal attacks in functionConfig paths. Signed-off-by: NETIZEN-11 <niteshkumar121411@gmail.com>
NETIZEN-11
requested review from
efiacor,
kispaljr,
liamfallon and
mozesl-nokia
as code owners
✅ Deploy Preview for kptdocs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
dosubot
bot
added
size:M
There was a problem hiding this comment.
Pull request overview
This PR updates Kubernetes and kustomize dependencies to newer versions and adds support for the apply-setters v0.2.4 catalog function while maintaining backward compatibility. It also includes bug fixes for test path handling and cross-platform test improvements. However, the PR description claims to include improvements to path validation in the Kptfile functionConfig, but these changes are not present in the provided diffs.
Changes:
- Fix bugs in executor_test.go where incorrect variable names were used in mkdir operations
- Add cross-platform path handling to tests using an absPath() helper function
- Add backward-compatible support for apply-setters v0.2.4 in the function registry while keeping v0.2.0
- Normalize line endings in pkg_context_test.go for cross-platform test compatibility
- Update Kubernetes dependencies from v0.34.1 to v0.35.0
- Update kustomize/api and kyaml from v0.20.1 to v0.21.0
- Update krm-functions-catalog to v0.2.4 and krm-functions-sdk to v1.0.0
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/util/render/executor_test.go | Fixed bugs using wrong variable names in mkdir calls; added absPath() helper for cross-platform path handling |
| internal/kptops/functions.go | Added support for apply-setters v0.2.4 while keeping v0.2.0 for backward compatibility |
| internal/builtins/pkg_context_test.go | Added line ending normalization for cross-platform test comparison |
| go.mod | Updated Kubernetes and kustomize dependencies to newer versions |
| go.sum | Updated checksums for dependency versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var rootString = "/root" | ||
| var subPkgString = "/root/subpkg" |
There was a problem hiding this comment.
Why are these vars now?
There was a problem hiding this comment.
Are the version changes here beyond apply-setters intentional? Also, definitely don't downgrade the SDK version.
|
What's the relationship with PR #4432 ? |
|
Hi @liamfallon, PR #4470 is a follow-up to #4432. #4432 handles the broader dependency upgrades (kyaml, kustomize API, k8s.io, apply-setters). #4470 addresses the reviewer feedback from that PR (duplicate go.mod entries, k8s.io/apiextensions-apiserver alignment, functions registry question) along with an additional fix to validateFnConfigPathSyntax using filepath.IsAbs for correct cross-platform path validation. Should have referenced this clearly in the PR description — apologies for the confusion. Happy to consolidate if preferred. |
Description
This PR improves the path validation logic in
validateFnConfigPathSyntaxfunction to ensure consistent behavior across different operating systems and enhance security.Changes
path.IsAbswithfilepath.IsAbsfor OS-agnostic path validationfilepath.Cleanto normalize path separatorspathpackage importMotivation
The previous implementation used
path.IsAbswhich is forward-slash based and may not work correctly on Windows. Usingfilepath.IsAbsensures consistent validation across all operating systems.Cleaning the path before validation also helps normalize different path separator styles and makes the ".." check more reliable.
Security Impact
This change strengthens security by:
Testing
Related Issues
Separated from #4432 as suggested by @liamfallon to keep validation changes independent from version-related changes.