Skip to content

Commit 2941bf1

Browse files
committed
Reject paths with inner '..' in FileLoader.New to prevent silent misresolution
1 parent 1ad9f9d commit 2941bf1

2 files changed

Lines changed: 81 additions & 0 deletions

File tree

api/internal/loader/fileloader.go

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,26 @@ func newLoaderAtConfirmedDir(
147147
}
148148
}
149149

150+
// hasInnerDotDot reports whether path contains a ".." after a non-".."
151+
// component, which causes filepath.Clean to silently absorb preceding
152+
// segments (e.g. "../../base - ../../shared/prod" becomes "../../shared/prod").
153+
func hasInnerDotDot(path string) bool {
154+
pastPrefix := false
155+
for _, part := range strings.Split(filepath.ToSlash(path), "/") {
156+
if part == "" || part == "." {
157+
continue
158+
}
159+
if part == ".." {
160+
if pastPrefix {
161+
return true
162+
}
163+
continue
164+
}
165+
pastPrefix = true
166+
}
167+
return false
168+
}
169+
150170
// New returns a new Loader, rooted relative to current loader,
151171
// or rooted in a temp directory holding a git repo clone.
152172
func (fl *FileLoader) New(path string) (ifc.Loader, error) {
@@ -167,6 +187,13 @@ func (fl *FileLoader) New(path string) (ifc.Loader, error) {
167187
if filepath.IsAbs(path) {
168188
return nil, fmt.Errorf("new root '%s' cannot be absolute", path)
169189
}
190+
if hasInnerDotDot(path) {
191+
return nil, fmt.Errorf(
192+
"path %q is ambiguous: resolves to %q after normalization, "+
193+
"which is likely not the intended target; check for YAML "+
194+
"indentation errors in your kustomization file",
195+
path, filepath.Clean(path))
196+
}
170197
root, err := filesys.ConfirmDir(fl.fSys, fl.root.Join(path))
171198
if err != nil {
172199
return nil, errors.WrapPrefixf(err, "%s", ErrRtNotDir.Error())

api/internal/loader/fileloader_test.go

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,60 @@ func TestNewEmptyLoader(t *testing.T) {
207207
require.Error(t, err)
208208
}
209209

210+
func TestLoaderRejectsMalformedPath(t *testing.T) {
211+
// A YAML indentation error can collapse two resource entries into one:
212+
// resources:
213+
// - ../../base
214+
// - ../../shared/prod
215+
// becomes the single string: "../../base - ../../shared/prod"
216+
//
217+
// filepath.Clean normalizes this to "../../shared/prod", silently
218+
// dropping the "../../base" reference. The loader must reject paths
219+
// with inner ".." components that cause this silent absorption.
220+
// See https://github.com/kubernetes-sigs/kustomize/issues/5979
221+
fSys := filesys.MakeFsInMemory()
222+
require.NoError(t, fSys.MkdirAll("/base"))
223+
require.NoError(t, fSys.MkdirAll("/shared/prod"))
224+
require.NoError(t, fSys.MkdirAll("/overlays/prod1"))
225+
226+
l1 := NewLoaderOrDie(RestrictionNone, fSys, "/overlays/prod1")
227+
228+
// The exact bug from issue #5979.
229+
_, err := l1.New("../../base - ../../shared/prod")
230+
require.Error(t, err)
231+
require.Contains(t, err.Error(), "ambiguous")
232+
233+
// Same structural problem without the YAML artifact.
234+
_, err = l1.New("a/b/../../other")
235+
require.Error(t, err)
236+
require.Contains(t, err.Error(), "ambiguous")
237+
}
238+
239+
func TestHasInnerDotDot(t *testing.T) {
240+
cases := map[string]bool{
241+
// Safe: leading ".." only
242+
"../base": false,
243+
"../../shared/prod": false,
244+
"..": false,
245+
// Safe: no ".." at all
246+
"foo/bar": false,
247+
"foo/bar/": false,
248+
"foo//bar": false,
249+
"./foo/bar": false,
250+
"https://root": false,
251+
// Dangerous: inner ".." absorbs preceding components
252+
"../../base - ../../shared/prod": true,
253+
"a/b/../../c": true,
254+
"foo/../bar": true,
255+
"a/..": true,
256+
}
257+
for path, want := range cases {
258+
t.Run(path, func(t *testing.T) {
259+
require.Equal(t, want, hasInnerDotDot(path), "hasInnerDotDot(%q)", path)
260+
})
261+
}
262+
}
263+
210264
func TestNewRemoteLoaderDoesNotExist(t *testing.T) {
211265
_, err := makeLoader().New("https://example.com/org/repo")
212266
require.ErrorContains(t, err, "fetch")

0 commit comments

Comments
 (0)