Script 'mail_helper' called by obssrc Hello community, here is the log from the commit of package apko for openSUSE:Factory checked in at 2026-04-23 17:07:34 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/apko (Old) and /work/SRC/openSUSE:Factory/.apko.new.11940 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "apko" Thu Apr 23 17:07:34 2026 rev:108 rq:1348857 version:1.2.6 Changes: -------- --- /work/SRC/openSUSE:Factory/apko/apko.changes 2026-04-21 12:46:55.211649584 +0200 +++ /work/SRC/openSUSE:Factory/.apko.new.11940/apko.changes 2026-04-23 17:11:29.961710306 +0200 @@ -1,0 +2,12 @@ +Thu Apr 23 05:28:06 UTC 2026 - Johannes Kastl <[email protected]> + +- Update to version 1.2.6: + * fs: strip special mode bits in OpenFile/WriteFile (#2188) +- Update to version 1.2.5: + * fs: Scope all DirFS operations through os.Root (#2187) + * build(deps): bump step-security/harden-runner from 2.18.0 to + 2.19.0 (#2184) + * build(deps): bump chainguard-dev/actions from 1.6.14 to 1.6.15 + (#2182) + +------------------------------------------------------------------- Old: ---- apko-1.2.4.obscpio New: ---- apko-1.2.6.obscpio ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ apko.spec ++++++ --- /var/tmp/diff_new_pack.UNDvB9/_old 2026-04-23 17:11:31.677779736 +0200 +++ /var/tmp/diff_new_pack.UNDvB9/_new 2026-04-23 17:11:31.677779736 +0200 @@ -17,7 +17,7 @@ Name: apko -Version: 1.2.4 +Version: 1.2.6 Release: 0 Summary: Build OCI images from APK packages directly without Dockerfile License: Apache-2.0 ++++++ _service ++++++ --- /var/tmp/diff_new_pack.UNDvB9/_old 2026-04-23 17:11:31.713781192 +0200 +++ /var/tmp/diff_new_pack.UNDvB9/_new 2026-04-23 17:11:31.721781516 +0200 @@ -3,7 +3,7 @@ <param name="url">https://github.com/chainguard-dev/apko.git</param> <param name="scm">git</param> <param name="exclude">.git</param> - <param name="revision">refs/tags/v1.2.4</param> + <param name="revision">refs/tags/v1.2.6</param> <param name="versionformat">@PARENT_TAG@</param> <param name="versionrewrite-pattern">v(.*)</param> <param name="changesgenerate">enable</param> ++++++ _servicedata ++++++ --- /var/tmp/diff_new_pack.UNDvB9/_old 2026-04-23 17:11:31.745782486 +0200 +++ /var/tmp/diff_new_pack.UNDvB9/_new 2026-04-23 17:11:31.749782649 +0200 @@ -3,6 +3,6 @@ <param name="url">https://github.com/chainguard-dev/apko</param> <param name="changesrevision">861f83f69e6fa9114405a2f7bb5cf6585ad00421</param></service><service name="tar_scm"> <param name="url">https://github.com/chainguard-dev/apko.git</param> - <param name="changesrevision">4556aed64043278fb8fa429e620fdbbe2a9e3254</param></service></servicedata> + <param name="changesrevision">09b82d635baa11223ba5b28b421069cadcddb5d9</param></service></servicedata> (No newline at EOF) ++++++ apko-1.2.4.obscpio -> apko-1.2.6.obscpio ++++++ diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/apko-1.2.4/pkg/apk/apk/path_traversal_test.go new/apko-1.2.6/pkg/apk/apk/path_traversal_test.go --- old/apko-1.2.4/pkg/apk/apk/path_traversal_test.go 2026-04-18 17:33:49.000000000 +0200 +++ new/apko-1.2.6/pkg/apk/apk/path_traversal_test.go 2026-04-22 17:22:42.000000000 +0200 @@ -6,6 +6,7 @@ "context" "os" "path/filepath" + "strings" "testing" apkfs "chainguard.dev/apko/pkg/apk/fs" @@ -88,6 +89,342 @@ } } +// TestSymlinkEscape_FileThroughAbsoluteSymlink covers the classic symlink-escape +// shape: a malicious APK plants a symlink inside the image whose target is an +// absolute path pointing outside the rootfs, then a regular file whose tar +// header name traverses that symlink. The install must fail and the outside +// path must remain untouched. +func TestSymlinkEscape_FileThroughAbsoluteSymlink(t *testing.T) { + ctx := t.Context() + + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outsideDir := filepath.Join(sandbox, "outside") + outsideFile := filepath.Join(outsideDir, "pwned") + if err := os.MkdirAll(outsideDir, 0o755); err != nil { + t.Fatal(err) + } + + fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir()) + if fsys == nil { + t.Fatalf("failed to create dirfs for base %s", base) + } + + a, err := New(ctx, WithFS(fsys)) + if err != nil { + t.Fatalf("apk.New: %v", err) + } + + r, err := makeSymlinkThenFileTar("evil", outsideDir, "evil/pwned", []byte("malicious")) + if err != nil { + t.Fatalf("makeSymlinkThenFileTar: %v", err) + } + + if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil { + t.Fatalf("expected installAPKFiles to fail, but it succeeded") + } + + if _, statErr := os.Stat(outsideFile); statErr == nil { + t.Fatalf("expected %s to not exist after fix", outsideFile) + } +} + +// TestSymlinkEscape_FileThroughRelativeSymlink is the ../outside variant. +func TestSymlinkEscape_FileThroughRelativeSymlink(t *testing.T) { + ctx := t.Context() + + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outsideDir := filepath.Join(sandbox, "outside") + outsideFile := filepath.Join(outsideDir, "pwned") + if err := os.MkdirAll(outsideDir, 0o755); err != nil { + t.Fatal(err) + } + + fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir()) + if fsys == nil { + t.Fatalf("failed to create dirfs for base %s", base) + } + + a, err := New(ctx, WithFS(fsys)) + if err != nil { + t.Fatalf("apk.New: %v", err) + } + + r, err := makeSymlinkThenFileTar("evil", "../outside", "evil/pwned", []byte("malicious")) + if err != nil { + t.Fatalf("makeSymlinkThenFileTar: %v", err) + } + + if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil { + t.Fatalf("expected installAPKFiles to fail, but it succeeded") + } + + if _, statErr := os.Stat(outsideFile); statErr == nil { + t.Fatalf("expected %s to not exist after fix", outsideFile) + } +} + +// TestSymlinkEscape_MkdirAllThroughSymlink plants a symlink whose target is an +// outside directory, then a TypeDir entry traversing the symlink. MkdirAll +// must not merge new dirs into the outside location. +func TestSymlinkEscape_MkdirAllThroughSymlink(t *testing.T) { + ctx := t.Context() + + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outsideDir := filepath.Join(sandbox, "outside") + outsideSub := filepath.Join(outsideDir, "sub") + if err := os.MkdirAll(outsideDir, 0o755); err != nil { + t.Fatal(err) + } + + fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir()) + if fsys == nil { + t.Fatalf("failed to create dirfs for base %s", base) + } + + a, err := New(ctx, WithFS(fsys)) + if err != nil { + t.Fatalf("apk.New: %v", err) + } + + r, err := makeSymlinkThenDirTar("evil", outsideDir, "evil/sub") + if err != nil { + t.Fatalf("makeSymlinkThenDirTar: %v", err) + } + + if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil { + t.Fatalf("expected installAPKFiles to fail, but it succeeded") + } + + if _, statErr := os.Stat(outsideSub); statErr == nil { + t.Fatalf("expected %s to not exist after fix", outsideSub) + } +} + +// TestSymlinkEscape_HardlinkThroughSymlink validates the hardlink path: the +// prior GHSA guarded target-side escapes, but the newname side could still be +// redirected through an attacker-planted symlink. +func TestSymlinkEscape_HardlinkThroughSymlink(t *testing.T) { + ctx := t.Context() + + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outsideDir := filepath.Join(sandbox, "outside") + outsideLinked := filepath.Join(outsideDir, "linked") + if err := os.MkdirAll(outsideDir, 0o755); err != nil { + t.Fatal(err) + } + + fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir()) + if fsys == nil { + t.Fatalf("failed to create dirfs for base %s", base) + } + + a, err := New(ctx, WithFS(fsys)) + if err != nil { + t.Fatalf("apk.New: %v", err) + } + + r, err := makeHardlinkThroughSymlinkTar("legit", "evil", outsideDir, "evil/linked") + if err != nil { + t.Fatalf("makeHardlinkThroughSymlinkTar: %v", err) + } + + if _, err := a.installAPKFiles(ctx, r, &Package{}); err == nil { + t.Fatalf("expected installAPKFiles to fail, but it succeeded") + } + + if _, statErr := os.Lstat(outsideLinked); statErr == nil { + t.Fatalf("expected %s to not exist after fix", outsideLinked) + } +} + +// TestInstallSetuidBinary exercises the end-to-end install of a setuid regular +// file — the mount-binary shape that triggered "unsupported file mode" on +// melange qemu runners. *os.Root rejects non-permission bits in OpenFile, so +// dirFS.OpenFile silently strips them; the full mode survives via the memFS +// overlay and is what downstream layer/tar/cpio emitters consume through +// dirFS.Stat().Mode() and DirEntry.Info().Mode(). +func TestInstallSetuidBinary(t *testing.T) { + ctx := t.Context() + + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + fsys := apkfs.DirFS(ctx, base, apkfs.WithCreateDir()) + if fsys == nil { + t.Fatalf("failed to create dirfs for base %s", base) + } + + a, err := New(ctx, WithFS(fsys)) + if err != nil { + t.Fatalf("apk.New: %v", err) + } + + content := []byte("setuid binary payload") + // 0o4755 = S_ISUID | rwxr-xr-x — the shape in the mount APK's tar header. + r, err := makeTestTarWithRegFile("usr/bin/mount", content, 0o4755) + if err != nil { + t.Fatalf("makeTestTarWithRegFile: %v", err) + } + + if _, err := a.installAPKFiles(ctx, r, &Package{}); err != nil { + t.Fatalf("installAPKFiles failed: %v", err) + } + + // fsys.Stat is the authoritative mode for downstream emission. + fi, err := fsys.Stat("usr/bin/mount") + if err != nil { + t.Fatalf("fsys.Stat: %v", err) + } + if fi.Mode()&os.ModeSetuid == 0 { + t.Fatalf("expected setuid bit via fsys.Stat, got %s", fi.Mode()) + } + if fi.Mode().Perm() != 0o755 { + t.Fatalf("expected perm 0o755, got %o", fi.Mode().Perm()) + } + + got, err := os.ReadFile(filepath.Join(base, "usr/bin/mount")) + if err != nil { + t.Fatalf("reading installed file: %v", err) + } + if string(got) != string(content) { + t.Fatalf("content mismatch") + } +} + +func makeTestTarWithRegFile(name string, content []byte, mode int64) (*bytes.Reader, error) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + // Emit a TypeDir entry for each parent so installAPKFiles can MkdirAll + // before the regular-file entry lands. + parts := strings.Split(filepath.ToSlash(name), "/") + for i := 1; i < len(parts); i++ { + if err := tw.WriteHeader(&tar.Header{ + Name: strings.Join(parts[:i], "/") + "/", + Typeflag: tar.TypeDir, + Mode: 0o755, + }); err != nil { + return nil, err + } + } + if err := tw.WriteHeader(&tar.Header{ + Name: name, + Typeflag: tar.TypeReg, + Mode: mode, + Size: int64(len(content)), + }); err != nil { + return nil, err + } + if _, err := tw.Write(content); err != nil { + return nil, err + } + if err := tw.Close(); err != nil { + return nil, err + } + return bytes.NewReader(buf.Bytes()), nil +} + +func makeSymlinkThenFileTar(symlinkName, symlinkTarget, fileName string, content []byte) (*bytes.Reader, error) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + + if err := tw.WriteHeader(&tar.Header{ + Name: symlinkName, + Linkname: symlinkTarget, + Typeflag: tar.TypeSymlink, + Mode: 0o777, + }); err != nil { + return nil, err + } + + if err := tw.WriteHeader(&tar.Header{ + Name: fileName, + Typeflag: tar.TypeReg, + Mode: 0o644, + Size: int64(len(content)), + }); err != nil { + return nil, err + } + if _, err := tw.Write(content); err != nil { + return nil, err + } + + if err := tw.Close(); err != nil { + return nil, err + } + return bytes.NewReader(buf.Bytes()), nil +} + +func makeSymlinkThenDirTar(symlinkName, symlinkTarget, dirName string) (*bytes.Reader, error) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + + if err := tw.WriteHeader(&tar.Header{ + Name: symlinkName, + Linkname: symlinkTarget, + Typeflag: tar.TypeSymlink, + Mode: 0o777, + }); err != nil { + return nil, err + } + + if err := tw.WriteHeader(&tar.Header{ + Name: dirName, + Typeflag: tar.TypeDir, + Mode: 0o755, + }); err != nil { + return nil, err + } + + if err := tw.Close(); err != nil { + return nil, err + } + return bytes.NewReader(buf.Bytes()), nil +} + +func makeHardlinkThroughSymlinkTar(regularName, symlinkName, symlinkTarget, hardlinkName string) (*bytes.Reader, error) { + var buf bytes.Buffer + tw := tar.NewWriter(&buf) + + content := []byte("legitimate") + if err := tw.WriteHeader(&tar.Header{ + Name: regularName, + Typeflag: tar.TypeReg, + Mode: 0o644, + Size: int64(len(content)), + }); err != nil { + return nil, err + } + if _, err := tw.Write(content); err != nil { + return nil, err + } + + if err := tw.WriteHeader(&tar.Header{ + Name: symlinkName, + Linkname: symlinkTarget, + Typeflag: tar.TypeSymlink, + Mode: 0o777, + }); err != nil { + return nil, err + } + + if err := tw.WriteHeader(&tar.Header{ + Name: hardlinkName, + Linkname: regularName, + Typeflag: tar.TypeLink, + Mode: 0o644, + }); err != nil { + return nil, err + } + + if err := tw.Close(); err != nil { + return nil, err + } + return bytes.NewReader(buf.Bytes()), nil +} + func makeTestTar(dirName, symlinkName, symlinkTarget string) (*bytes.Reader, error) { var buf bytes.Buffer tw := tar.NewWriter(&buf) diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/apko-1.2.4/pkg/apk/fs/rwosfs.go new/apko-1.2.6/pkg/apk/fs/rwosfs.go --- old/apko-1.2.4/pkg/apk/fs/rwosfs.go 2026-04-18 17:33:49.000000000 +0200 +++ new/apko-1.2.6/pkg/apk/fs/rwosfs.go 2026-04-22 17:22:42.000000000 +0200 @@ -16,10 +16,12 @@ import ( "context" + "errors" "fmt" "io/fs" "os" "path/filepath" + "runtime" "sort" "strings" "sync" @@ -93,29 +95,33 @@ return nil } + root, err := os.OpenRoot(dir) + if err != nil { + log.Warn("error opening root", "error", err) + return nil + } + var caseSensitive bool if options.caseSensitiveSet { caseSensitive = options.caseSensitive } else { - // check if the underlying filesystem is case-sensitive - // we cannot just use it in TempDir() because these might be different filesystems - // find a file that does not exist + // Probe the underlying filesystem through the root so the probe is + // subject to the same sandboxing as every other write. We cannot reuse + // the caller's TempDir because it might be on a different filesystem. for i := 0; ; i++ { filename := fmt.Sprintf("test-dirfs-%d", i) - // filepath.Join below here is considered safe since we control filename - if _, err := os.Stat(filepath.Join(dir, filename)); err == nil { + if _, err := root.Stat(filename); err == nil { continue } - if err := os.WriteFile(filepath.Join(dir, filename), []byte("test"), 0o600); err != nil { + if err := root.WriteFile(filename, []byte("test"), 0o600); err != nil { caseSensitive = false // If this fails, let's just assume it's not case sensitive. break } - // see if it exists - if _, err := os.Stat(filepath.Join(dir, strings.ToUpper(filename))); err != nil { + if _, err := root.Stat(strings.ToUpper(filename)); err != nil { caseSensitive = true } // clean up our own messes - _ = os.Remove(filepath.Join(dir, filename)) + _ = root.Remove(filename) break } } @@ -126,13 +132,19 @@ } f := &dirFS{ base: dir, + root: root, overrides: m, caseMap: caseMap, } - // need to populate the overrides with appropriate info - root := os.DirFS(dir) - - _ = fs.WalkDir(root, ".", func(path string, d fs.DirEntry, err error) error { + // Safety net for the library-consumer case where the dirFS is stashed + // inside another type (e.g. apk.New's fallback) and never reachable for + // explicit Close. Stopped by Close when it runs deterministically. + f.cleanup = runtime.AddCleanup(f, func(r *os.Root) { _ = r.Close() }, root) + // Seed overrides by walking the backing tree through the root. This + // refuses to follow any pre-existing symlink whose target escapes base, + // and root.Readlink gives us consistent sandbox semantics with the rest + // of the dirFS API surface. + _ = fs.WalkDir(root.FS(), ".", func(path string, d fs.DirEntry, err error) error { if err != nil { return err } @@ -151,11 +163,7 @@ err = f.overrides.Mkdir(path, fullPerm) case fs.ModeSymlink: var target string - target, err = f.sanitizePath(path) - if err != nil { - return err - } - target, err = os.Readlink(target) + target, err = root.Readlink(path) if err == nil { err = f.overrides.Symlink(target, path) } @@ -201,6 +209,19 @@ // else in memory. type dirFS struct { base string + // root is a capability-style handle scoped to base. All disk operations on + // caller-supplied paths go through it so path resolution can never escape + // base via symlinks, hard links, absolute path components, or "..". + root *os.Root + // cleanup releases the root FD on garbage collection as a safety net for + // callers that can't reach Close() (notably the apk.New fallback which + // stashes the FullFS inside *APK). Deterministic release via Close() is + // still preferred. + // + // TODO: revisit and replace with `Close() error` on the FullFS interface + // once we're willing to take the breaking change across in-tree + // implementers and library consumers. + cleanup runtime.Cleanup // overrides is a map of overrides for things that could not be kept on disk because of permission, // filesystem or operating system limitations. // It will include all directories, but no file contents. @@ -212,6 +233,19 @@ caseMapMutex sync.Mutex } +// Close releases the underlying *os.Root file descriptor. A GC cleanup is also +// registered as a safety net for consumers who cannot reach this method. +// Calling Close more than once is safe. +func (f *dirFS) Close() error { + if f.root == nil { + return nil + } + f.cleanup.Stop() + err := f.root.Close() + f.root = nil + return err +} + func (f *dirFS) Readlink(name string) (string, error) { // The underlying filesystem might not support symlinks, and it might be case-insensitive, so just // use the one in memory. @@ -232,42 +266,41 @@ } func (f *dirFS) open(name string) (*fileImpl, error) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return nil, err - } + rel := f.relPath(name) baseName := filepath.Base(name) if f.caseSensitiveOnDisk(name) { - file, err := os.Open(fullpath) + file, err := f.root.Open(rel) if err == nil { return &fileImpl{ - file: file, - name: baseName, - fullpath: fullpath, + file: file, + name: baseName, + root: f.root, + rel: rel, }, nil } if !os.IsPermission(err) { return nil, err } // get the original permissions - fi, err := os.Stat(fullpath) + fi, err := f.root.Stat(rel) if err != nil { - return nil, fmt.Errorf("unable to stat file %s: %w", fullpath, err) + return nil, fmt.Errorf("unable to stat file %s: %w", name, err) } // Try to change permissions and open again. - if err := os.Chmod(fullpath, 0o600); err != nil { + if err := f.root.Chmod(rel, 0o600); err != nil { return nil, fmt.Errorf("unable to read file or change permissions: %s", name) } - file, err = os.Open(fullpath) + file, err = f.root.Open(rel) if err != nil { return nil, fmt.Errorf("unable to read file even after change permissions: %s", name) } perms := fi.Mode() return &fileImpl{ - file: file, - name: baseName, - fullpath: fullpath, - perms: &perms, + file: file, + name: baseName, + root: f.root, + rel: rel, + perms: &perms, }, nil } @@ -278,11 +311,13 @@ return &fileImpl{file: file, name: baseName}, nil } -// Open open a file for reading. Returns fs.File. -// If the file has the wrong permissions for reading, it tries to -// change them, and then change them back when closing. -// This only works if the user reading the file actually has -// permissions to change the file permissions. +// OpenFile opens a file, returning a File. +// +// Non-permission bits in perm (setuid/setgid/sticky) are silently stripped — +// *os.Root refuses them in OpenFile, and applying them eagerly would be +// cleared again by the kernel's file_remove_privs() on the caller's next +// write (on Linux, for non-privileged processes). Callers that need those +// bits on disk must Chmod after the final close. func (f *dirFS) OpenFile(name string, flag int, perm fs.FileMode) (File, error) { var ( file File @@ -296,22 +331,14 @@ // do we create it on disk? if f.createOnDisk(name) { _ = file.Close() - fullpath, err := f.sanitizePath(name) - if err != nil { - return nil, err - } - file, err = os.OpenFile(fullpath, flag, perm) + file, err = f.root.OpenFile(f.relPath(name), flag, perm.Perm()) if err != nil { return nil, err } } } else { if f.caseSensitiveOnDisk(name) { - fullpath, sanErr := f.sanitizePath(name) - if sanErr != nil { - return nil, sanErr - } - file, err = os.OpenFile(fullpath, flag, perm) + file, err = f.root.OpenFile(f.relPath(name), flag, perm.Perm()) } else { file, err = f.overrides.OpenFile(name, flag, perm) } @@ -336,11 +363,7 @@ return nil, err } if f.caseSensitiveOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return nil, err - } - fi, err = os.Stat(fullpath) + fi, err = f.root.Stat(f.relPath(name)) if err != nil { return nil, err } @@ -371,11 +394,7 @@ if f.createOnDisk(name) { // close the memory one _ = file.Close() - fullpath, err := f.sanitizePath(name) - if err != nil { - return nil, err - } - file, err = os.Create(fullpath) + file, err = f.root.Create(f.relPath(name)) if err != nil { return nil, err } @@ -389,11 +408,7 @@ return err } if f.removeOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return err - } - return os.Remove(fullpath) + return f.root.Remove(f.relPath(name)) } return nil } @@ -405,11 +420,14 @@ err error ) if f.caseSensitiveOnDisk(name) { - fullpath, err := f.sanitizePath(name) + // *os.Root does not expose ReadDir; open the directory through it and + // read entries from the returned *os.File. + dir, err := f.root.Open(f.relPath(name)) if err != nil { return nil, err } - onDisk, err = os.ReadDir(fullpath) + onDisk, err = dir.ReadDir(-1) + _ = dir.Close() if err != nil { return nil, err } @@ -447,21 +465,17 @@ } func (f *dirFS) ReadFile(name string) ([]byte, error) { if f.caseSensitiveOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return nil, err - } - return os.ReadFile(fullpath) + return f.root.ReadFile(f.relPath(name)) } return f.overrides.ReadFile(name) } + +// WriteFile writes data to the named file. Non-permission bits in mode +// (setuid/setgid/sticky) are silently stripped for the disk write — *os.Root +// refuses them. Callers that need those bits on disk must Chmod after. func (f *dirFS) WriteFile(name string, b []byte, mode fs.FileMode) error { if f.createOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return err - } - if err := os.WriteFile(fullpath, b, mode); err != nil { + if err := f.root.WriteFile(f.relPath(name), b, mode.Perm()); err != nil { return err } } @@ -473,12 +487,7 @@ func (f *dirFS) Readnod(name string) (dev int, err error) { if f.caseSensitiveOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return 0, err - } - _, err = os.Stat(fullpath) - if err != nil { + if _, err := f.root.Stat(f.relPath(name)); err != nil { return 0, err } } @@ -486,22 +495,10 @@ } func (f *dirFS) Link(oldname, newname string) error { - fullpath, err := f.sanitizePath(oldname) - if err != nil { - return err - } - // for hardlink, we cannot take target as is, as it might be outside of the base. - // So we must sanitize it. It should point to a file that is within the filesystem. - target := filepath.Clean(fullpath) - if !strings.HasPrefix(target, f.base) { - return fmt.Errorf("hardlink target %s is outside of the filesystem", target) - } if f.createOnDisk(newname) { - fullpath, err := f.sanitizePath(newname) - if err != nil { - return err - } - if err := os.Link(target, fullpath); err != nil { + // *os.Root enforces that both endpoints resolve within base, including + // refusing to traverse any attacker-planted symlink in either path. + if err := f.root.Link(f.relPath(oldname), f.relPath(newname)); err != nil { return err } } @@ -509,15 +506,11 @@ } func (f *dirFS) Symlink(oldname, newname string) error { - // For symlink, take target as is. - // If it is outside of the base, it will be resolved by Readlink. - // This enables proper symlink behaviour. + // The target (oldname) is stored verbatim, which preserves legitimate APK + // semantics (e.g., absolute paths within the image). *os.Root refuses to + // traverse the symlink at use time if it would escape the root. if f.createOnDisk(newname) { - fullpath, err := f.sanitizePath(newname) - if err != nil { - return err - } - if err := os.Symlink(oldname, fullpath); err != nil { + if err := f.root.Symlink(oldname, f.relPath(newname)); err != nil { return err } } @@ -528,11 +521,8 @@ // just in case, because some underlying systems miss this fullPerm := os.ModeDir | perm if f.createOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return err - } - if err := os.MkdirAll(fullpath, fullPerm); err != nil { + // *os.Root rejects type bits in the mode; strip to permission bits. + if err := f.root.MkdirAll(f.relPath(name), fullPerm.Perm()); err != nil { return err } } @@ -543,72 +533,82 @@ // just in case, because some underlying systems miss this fullPerm := os.ModeDir | perm if f.createOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { - return err - } - if err := os.Mkdir(fullpath, fullPerm); err != nil { + // *os.Root rejects type bits in the mode; strip to permission bits. + if err := f.root.Mkdir(f.relPath(name), fullPerm.Perm()); err != nil { return err } } return f.overrides.Mkdir(name, fullPerm) } +// isUnsupportedByFS reports whether an error from a best-effort on-disk +// metadata call is one we expect to ignore: +// - ENOTSUP/EOPNOTSUPP/ENOSYS: the filesystem or platform can't perform the +// operation (e.g. a FUSE mount that ignores mode bits). +// - EPERM: the process lacks privilege (e.g. chown as non-root without +// CAP_CHOWN, chmod of a not-owned or immutable file). This is the +// dominant failure mode for unprivileged builds. +// +// In-memory overrides remain the authoritative source for mode/ownership, so +// the build still produces a correctly-attributed tar. Real errors (path +// escapes, I/O failures, missing files) still surface. +func isUnsupportedByFS(err error) bool { + return errors.Is(err, syscall.ENOTSUP) || + errors.Is(err, syscall.EOPNOTSUPP) || + errors.Is(err, syscall.ENOSYS) || + errors.Is(err, syscall.EPERM) +} + func (f *dirFS) Chmod(path string, perm fs.FileMode) error { if f.caseSensitiveOnDisk(path) { - fullpath, err := f.sanitizePath(path) - if err != nil { + if err := f.root.Chmod(f.relPath(path), perm); err != nil && !isUnsupportedByFS(err) { return err } - // ignore error, as we track it in memory anyways, and disk filesystem might not support it - _ = os.Chmod(fullpath, perm) } return f.overrides.Chmod(path, perm) } func (f *dirFS) Chown(path string, uid, gid int) error { if f.caseSensitiveOnDisk(path) { - fullpath, err := f.sanitizePath(path) - if err != nil { + if err := f.root.Chown(f.relPath(path), uid, gid); err != nil && !isUnsupportedByFS(err) { return err } - // ignore error, as we track it in memory anyways, and disk filesystem might not support it - _ = os.Chown(fullpath, uid, gid) } return f.overrides.Chown(path, uid, gid) } func (f *dirFS) Chtimes(path string, atime time.Time, mtime time.Time) error { - fullpath, err := f.sanitizePath(path) - if err != nil { - return err - } - if err := os.Chtimes(fullpath, atime, mtime); err != nil { + if err := f.root.Chtimes(f.relPath(path), atime, mtime); err != nil { return fmt.Errorf("unable to change times: %w", err) } return f.overrides.Chtimes(path, atime, mtime) } +// Mknod stores device metadata in the memFS overrides (the authoritative +// layer) and best-effort materializes the node on disk so other operations +// that consult disk first still find an entry. +// +// os.Root has no Mknod of its own; the disk side is platform-specific and +// implemented in rwosfs_mknod_{linux,other}.go via mknodOnDisk. func (f *dirFS) Mknod(name string, mode uint32, dev int) error { if f.caseSensitiveOnDisk(name) { - fullpath, err := f.sanitizePath(name) - if err != nil { + if err := f.mknodOnDisk(f.relPath(name), mode, dev); err != nil { return err } - // what if we could not create it? Just create a regular file there, and memory will override - if err := unix.Mknod(fullpath, mode, dev); err != nil { - fullpath, err = f.sanitizePath(name) - if err != nil { - return err - } - if err := os.WriteFile(fullpath, nil, 0); err != nil { - return err - } - } } return f.overrides.Mknod(name, mode, dev) } +// placeholderOnDisk creates an empty file through the root so that later +// Stat/Open calls that consult disk first find something. Device metadata is +// tracked in the in-memory overrides; this entry is a visibility stub only. +func (f *dirFS) placeholderOnDisk(rel string) error { + if err := f.root.WriteFile(rel, nil, 0); err != nil { + return fmt.Errorf("unable to create placeholder on disk: %w", err) + } + return nil +} + func (f *dirFS) SetXattr(path string, attr string, data []byte) error { // the underlying filesystem might or might not support xattrs // but we have info on every file in memory, so might as well store it there. @@ -629,33 +629,24 @@ const pathSeparator = string(os.PathSeparator) -// sanitizePath joins base and p safely, preventing path traversal outside base. -// It allows ".." components in the path as long as the resolved path stays within base. -func (f *dirFS) sanitizePath(p string) (string, error) { - if f.base == "" { - return "", fmt.Errorf("empty base") - } - +// relPath normalizes a caller-supplied path into a clean, root-relative form +// suitable for use with *os.Root operations. +// +// It handles two things that *os.Root does not handle gracefully on its own: +// - empty / "/" / absolute-looking inputs, which *os.Root rejects as "path +// escapes from parent" — here we strip to a root-relative form. +// - intermediate ".." components, which *os.Root walks against the real +// filesystem (so "a/c/../b" fails if "c" doesn't exist). filepath.Clean +// collapses them lexically before we hand the path to the root. +// +// Null-byte paths and ".." components that escape the root are left to +// *os.Root to reject; both yield clear errors from the stdlib. +func (f *dirFS) relPath(p string) string { clean := strings.TrimSuffix(strings.TrimPrefix(p, pathSeparator), pathSeparator) if clean == "" { - return f.base, nil - } - - if strings.Contains(clean, "\x00") { - return "", fmt.Errorf("path contains null byte") - } - - // Resolve the path and check if it escapes base using filepath.Rel - resolved := filepath.Clean(filepath.Join(f.base, clean)) - rel, err := filepath.Rel(filepath.Clean(f.base), resolved) - if err != nil || strings.HasPrefix(rel, ".."+pathSeparator) || rel == ".." { - return "", fmt.Errorf("%s: %s", "content filepath is tainted", p) - } - - if os.IsPathSeparator(f.base[len(f.base)-1]) { - return f.base + clean, nil + return "." } - return f.base + pathSeparator + clean, nil + return filepath.Clean(clean) } func (f *dirFS) caseSensitiveOnDisk(p string) bool { @@ -711,18 +702,20 @@ type file File type fileImpl struct { file - name string - fullpath string - perms *os.FileMode + name string + // root and rel are populated when the underlying file lives on disk so + // permission restoration in Close() goes through the sandboxed root. + root *os.Root + rel string + perms *os.FileMode } func (f fileImpl) Close() error { if err := f.file.Close(); err != nil { return err } - if f.perms != nil { - // f.name is the basename of the path, use the f.file.name here - return os.Chmod(f.fullpath, *f.perms) + if f.perms != nil && f.root != nil { + return f.root.Chmod(f.rel, *f.perms) } return nil } diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/apko-1.2.4/pkg/apk/fs/rwosfs_mknod_linux.go new/apko-1.2.6/pkg/apk/fs/rwosfs_mknod_linux.go --- old/apko-1.2.4/pkg/apk/fs/rwosfs_mknod_linux.go 1970-01-01 01:00:00.000000000 +0100 +++ new/apko-1.2.6/pkg/apk/fs/rwosfs_mknod_linux.go 2026-04-22 17:22:42.000000000 +0200 @@ -0,0 +1,40 @@ +// Copyright 2026 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build linux + +package fs + +import ( + "path" + + "golang.org/x/sys/unix" +) + +// mknodOnDisk issues mknodat against a root-constrained parent directory FD +// so the syscall cannot be tricked into traversing a symlink out of the +// sandbox. If the underlying filesystem refuses the mode (e.g. tmpfs +// rejecting certain device types), a zero-byte placeholder is written +// through the root instead. +func (f *dirFS) mknodOnDisk(rel string, mode uint32, dev int) error { + parent, err := f.root.Open(path.Dir(rel)) + if err != nil { + return err + } + defer parent.Close() + if err := unix.Mknodat(int(parent.Fd()), path.Base(rel), mode, dev); err == nil { + return nil + } + return f.placeholderOnDisk(rel) +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/apko-1.2.4/pkg/apk/fs/rwosfs_mknod_other.go new/apko-1.2.6/pkg/apk/fs/rwosfs_mknod_other.go --- old/apko-1.2.4/pkg/apk/fs/rwosfs_mknod_other.go 1970-01-01 01:00:00.000000000 +0100 +++ new/apko-1.2.6/pkg/apk/fs/rwosfs_mknod_other.go 2026-04-22 17:22:42.000000000 +0200 @@ -0,0 +1,24 @@ +// Copyright 2026 Chainguard, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//go:build !linux + +package fs + +// mknodOnDisk has no sandboxed implementation off Linux (no Root.Mknod, no +// unix.Mknodat). The device metadata lives in the memFS overrides; this +// writes a placeholder file through the root so Stat/Open resolve. +func (f *dirFS) mknodOnDisk(rel string, _ uint32, _ int) error { + return f.placeholderOnDisk(rel) +} diff -urN '--exclude=CVS' '--exclude=.cvsignore' '--exclude=.svn' '--exclude=.svnignore' old/apko-1.2.4/pkg/apk/fs/rwosfs_test.go new/apko-1.2.6/pkg/apk/fs/rwosfs_test.go --- old/apko-1.2.4/pkg/apk/fs/rwosfs_test.go 2026-04-18 17:33:49.000000000 +0200 +++ new/apko-1.2.6/pkg/apk/fs/rwosfs_test.go 2026-04-22 17:22:42.000000000 +0200 @@ -18,6 +18,7 @@ "io/fs" "os" "path/filepath" + "syscall" "testing" "github.com/stretchr/testify/require" @@ -455,100 +456,373 @@ require.Equal(t, combinedData, finalData, "ReadFile should return combined data, not zeros") } -func TestSanitizePath(t *testing.T) { +// TestSymlinkEscape_WriteFile_AbsoluteTarget verifies that a symlink planted +// inside the dirFS whose target is an absolute path outside the base cannot be +// used to land a write outside the base. +func TestSymlinkEscape_WriteFile_AbsoluteTarget(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(outside, "evil")) + + err := fsys.WriteFile("evil/pwned", []byte("bad"), 0o644) + require.Error(t, err, "write through escape symlink must fail") + + _, statErr := os.Stat(filepath.Join(outside, "pwned")) + require.True(t, os.IsNotExist(statErr), "outside file must not exist") +} + +// TestSymlinkEscape_WriteFile_RelativeTarget covers the "../outside" target +// variant separately: the lexical path inside dirFS is innocuous but the +// symlink resolves to a location outside the root. +func TestSymlinkEscape_WriteFile_RelativeTarget(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink("../outside", "evil")) + + err := fsys.WriteFile("evil/pwned", []byte("bad"), 0o644) + require.Error(t, err, "write through relative-escape symlink must fail") + + _, statErr := os.Stat(filepath.Join(outside, "pwned")) + require.True(t, os.IsNotExist(statErr), "outside file must not exist") +} + +// TestSymlinkEscape_OpenFileCreate mirrors the WriteFile case for the +// OpenFile+O_CREATE code path, which takes a different branch in dirFS. +func TestSymlinkEscape_OpenFileCreate(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(outside, "evil")) + + fh, err := fsys.OpenFile("evil/pwned", os.O_CREATE|os.O_WRONLY, 0o644) + if err == nil { + _ = fh.Close() + } + require.Error(t, err, "OpenFile+O_CREATE through escape symlink must fail") + + _, statErr := os.Stat(filepath.Join(outside, "pwned")) + require.True(t, os.IsNotExist(statErr), "outside file must not exist") +} + +// TestSymlinkEscape_MkdirAll ensures MkdirAll cannot merge into an +// attacker-planted symlinked directory that points outside the root. +func TestSymlinkEscape_MkdirAll(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(outside, "evil")) + + err := fsys.MkdirAll("evil/sub", 0o755) + require.Error(t, err, "MkdirAll through escape symlink must fail") + + _, statErr := os.Stat(filepath.Join(outside, "sub")) + require.True(t, os.IsNotExist(statErr), "outside dir must not exist") +} + +// TestSymlinkEscape_Link rejects hardlink newnames whose path resolution goes +// through an attacker-planted symlink pointing out of the root. +func TestSymlinkEscape_Link(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.WriteFile("legit", []byte("content"), 0o644)) + require.NoError(t, fsys.Symlink(outside, "evil")) + + err := fsys.Link("legit", "evil/linked") + require.Error(t, err, "Link through escape symlink must fail") + + _, statErr := os.Lstat(filepath.Join(outside, "linked")) + require.True(t, os.IsNotExist(statErr), "outside hardlink must not exist") +} + +// TestSymlinkEscape_ReadFile ensures a symlink planted inside dirFS cannot be +// used to read a file outside the root (read-side exfiltration). +func TestSymlinkEscape_ReadFile(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + secret := filepath.Join(outside, "secret") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.WriteFile(secret, []byte("top-secret"), 0o600)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(secret, "evil")) + + _, err := fsys.ReadFile("evil") + require.Error(t, err, "ReadFile through escape symlink must fail") +} + +// TestSymlinkEscape_Stat ensures Stat refuses to resolve an attacker-planted +// symlink that points outside the root. +func TestSymlinkEscape_Stat(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + secret := filepath.Join(outside, "secret") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.WriteFile(secret, []byte("s"), 0o600)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(secret, "evil")) + + _, err := fsys.Stat("evil") + require.Error(t, err, "Stat through escape symlink must fail") +} + +// TestSymlinkEscape_Chmod verifies Chmod cannot change permissions on files +// outside the root via an attacker-planted symlink. The escape returns an +// error (not a filesystem-compat errno) so it surfaces to the caller. +func TestSymlinkEscape_Chmod(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + target := filepath.Join(outside, "target") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.WriteFile(target, []byte("x"), 0o600)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(target, "evil")) + + require.Error(t, fsys.Chmod("evil", 0o777), "Chmod through escape symlink must fail") + + fi, err := os.Stat(target) + require.NoError(t, err) + require.Equal(t, os.FileMode(0o600), fi.Mode().Perm(), "outside file perms must be unchanged") +} + +// TestSymlinkEscape_Chown verifies Chown cannot change ownership on files +// outside the root via an attacker-planted symlink. Mirrors the Chmod case: +// path-escape errors surface, compat errnos (e.g. EPERM as non-root) don't. +func TestSymlinkEscape_Chown(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + target := filepath.Join(outside, "target") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.WriteFile(target, []byte("x"), 0o600)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(target, "evil")) + + require.Error(t, fsys.Chown("evil", os.Getuid(), os.Getgid()), "Chown through escape symlink must fail") +} + +// TestSymlinkEscape_PreExistingOnDisk plants an escape symlink on disk before +// DirFS is constructed. The constructor walker must not create a usable +// escape; writes through the link must still be refused. +func TestSymlinkEscape_PreExistingOnDisk(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + require.NoError(t, os.Symlink(outside, filepath.Join(base, "evil"))) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + err := fsys.WriteFile("evil/pwned", []byte("bad"), 0o644) + require.Error(t, err, "write through pre-existing escape symlink must fail") + + _, statErr := os.Stat(filepath.Join(outside, "pwned")) + require.True(t, os.IsNotExist(statErr), "outside file must not exist") +} + +// TestSymlinkEscape_Mknod_ParentPath exercises the parent-FD leg of +// mknodOnDisk: a symlink component in the path to the node must be refused at +// root.Open time, before mknodat is ever issued. +func TestSymlinkEscape_Mknod_ParentPath(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(outside, "evil")) + + err := fsys.Mknod("evil/foo", syscall.S_IFCHR|0o644, 0) + require.Error(t, err, "Mknod through escape symlink parent must fail") + + _, statErr := os.Lstat(filepath.Join(outside, "foo")) + require.True(t, os.IsNotExist(statErr), "outside node must not exist") +} + +// TestSymlinkEscape_Mknod_Basename exercises the placeholder fallback: when +// mknodat refuses (EEXIST on an existing symlink, or EPERM without CAP_MKNOD), +// mknodOnDisk falls back to root.OpenFile, which must refuse to follow a +// basename symlink that escapes the root. +func TestSymlinkEscape_Mknod_Basename(t *testing.T) { + sandbox := t.TempDir() + base := filepath.Join(sandbox, "base") + outside := filepath.Join(sandbox, "outside") + target := filepath.Join(outside, "pwned") + require.NoError(t, os.MkdirAll(outside, 0o755)) + require.NoError(t, os.MkdirAll(base, 0o755)) + + fsys := DirFS(t.Context(), base) + require.NotNil(t, fsys) + + require.NoError(t, fsys.Symlink(target, "evil")) + + err := fsys.Mknod("evil", syscall.S_IFCHR|0o644, 0) + require.Error(t, err, "Mknod on escape-symlink basename must fail") + + _, statErr := os.Lstat(target) + require.True(t, os.IsNotExist(statErr), "outside node must not be created via placeholder") +} + +// TestSpecialModeBits pins dirFS's contract around setuid/setgid/sticky: +// OpenFile and WriteFile silently strip them (*os.Root refuses them in these +// calls). Callers that need those bits on disk use Chmod after the write — +// dirFS.Chmod forwards to root.Chmod which does accept them. +func TestSpecialModeBits(t *testing.T) { + specials := fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky for _, tt := range []struct { - name string - base string - path string - want string - wantErr bool + name string + mode os.FileMode }{ - { - name: "empty base", - base: "", - wantErr: true, - }, - { - name: "empty path returns base", - base: "/tmp/1", - want: "/tmp/1", - }, - { - name: "root path returns base", - base: "/tmp/1", - path: "/", - want: "/tmp/1", - }, - { - name: "dot path returns base (plus dot)", - base: "/tmp/1", - path: ".", - want: "/tmp/1/.", - }, - { - name: "base has valid traversal", - base: "../", - path: ".", - want: "../.", - }, - { - name: "base has valid traversal (no trailing slash)", - base: "..", - path: ".", - want: "../.", - }, - { - name: "invalid path with traversal", - base: "/tmp/1", - path: "../", - wantErr: true, - }, - { - name: "path with traversal even within base is valid", - base: "/tmp/1", - path: "a/b/c/../../b", - want: "/tmp/1/a/b/c/../../b", - }, - { - name: "path with trailing dotdot stays within base", - base: "/tmp/1", - path: "a/b/c/..", - want: "/tmp/1/a/b/c/..", - }, - { - name: "path starting with dotdot but resolving within base", - base: "/tmp/1", - path: "../1/a", - want: "/tmp/1/../1/a", - }, - { - name: "path with multiple dotdot stays within base", - base: "/tmp/1", - path: "a/b/../x/../z", - want: "/tmp/1/a/b/../x/../z", - }, - { - name: "relative base with path that escapes", - base: "../", - path: "../..", - wantErr: true, - }, - { - name: "path with null byte", - base: "/tmp/1", - path: "test\x00file", - wantErr: true, - }, + {"setuid", fs.ModeSetuid | 0o755}, + {"setgid", fs.ModeSetgid | 0o755}, + {"sticky", fs.ModeSticky | 0o755}, + {"setuid_setgid_sticky", fs.ModeSetuid | fs.ModeSetgid | fs.ModeSticky | 0o755}, } { - t.Run(tt.name, func(t *testing.T) { - f := &dirFS{base: tt.base} - got, err := f.sanitizePath(tt.path) - if tt.wantErr { - require.Error(t, err) - return - } + t.Run(tt.name+"/OpenFile_strips", func(t *testing.T) { + dir := t.TempDir() + fsys := DirFS(t.Context(), dir) + require.NotNil(t, fsys) + + path := "binary" + file, err := fsys.OpenFile(path, os.O_CREATE|os.O_WRONLY, tt.mode) + require.NoError(t, err, "OpenFile must not error on non-perm mode bits") + _, err = file.Write([]byte("payload")) require.NoError(t, err) - require.Equal(t, tt.want, got) + require.NoError(t, file.Close()) + + fi, err := os.Stat(filepath.Join(dir, path)) + require.NoError(t, err) + require.Equal(t, tt.mode.Perm(), fi.Mode().Perm(), + "on-disk perm bits should match .Perm() of input") + require.Zero(t, fi.Mode()&specials, + "special bits must be stripped by OpenFile") + }) + t.Run(tt.name+"/WriteFile_strips", func(t *testing.T) { + dir := t.TempDir() + fsys := DirFS(t.Context(), dir) + require.NotNil(t, fsys) + + path := "binary" + require.NoError(t, fsys.WriteFile(path, []byte("x"), tt.mode)) + + fi, err := os.Stat(filepath.Join(dir, path)) + require.NoError(t, err) + require.Equal(t, tt.mode.Perm(), fi.Mode().Perm()) + require.Zero(t, fi.Mode()&specials, + "special bits must be stripped by WriteFile") + }) + t.Run(tt.name+"/Chmod_preserves", func(t *testing.T) { + dir := t.TempDir() + fsys := DirFS(t.Context(), dir) + require.NotNil(t, fsys) + + path := "binary" + require.NoError(t, fsys.WriteFile(path, []byte("x"), 0o755)) + require.NoError(t, fsys.Chmod(path, tt.mode)) + + fi, err := os.Stat(filepath.Join(dir, path)) + require.NoError(t, err) + require.Equal(t, tt.mode, fi.Mode(), + "Chmod after write should leave full mode (incl. specials) on disk") + }) + } +} + +// TestDirFSClose ensures the type-assertable Close() releases the root FD. +func TestDirFSClose(t *testing.T) { + dir := t.TempDir() + fsys := DirFS(t.Context(), dir) + require.NotNil(t, fsys) + + closer, ok := fsys.(interface{ Close() error }) + require.True(t, ok, "dirFS should expose Close()") + require.NoError(t, closer.Close()) + // Idempotent. + require.NoError(t, closer.Close()) +} + +func TestRelPath(t *testing.T) { + // relPath is a normalizer; it does not police escapes — *os.Root is the + // authority on that. The cases that *used* to error in relPath now pass + // through normalized, and the rejection happens at the root.* call site. + for _, tt := range []struct { + name string + path string + want string + }{ + {name: "empty path", path: "", want: "."}, + {name: "root path", path: "/", want: "."}, + {name: "dot path", path: ".", want: "."}, + {name: "simple path", path: "a/b", want: "a/b"}, + {name: "absolute path becomes relative", path: "/a/b", want: "a/b"}, + {name: "dotdot middle resolves in place", path: "a/b/c/../../b", want: "a/b"}, + {name: "trailing dotdot resolves in place", path: "a/b/c/..", want: "a/b"}, + {name: "multiple dotdot staying within root", path: "a/b/../x/../z", want: "a/z"}, + + // These used to error; now they normalize and os.Root rejects at use time. + {name: "leading dotdot normalized", path: "../", want: ".."}, + {name: "nested escape normalized", path: "a/../..", want: ".."}, + {name: "leading dotdot with path kept", path: "../1/a", want: "../1/a"}, + {name: "null byte preserved for os.Root to reject", path: "test\x00file", want: "test\x00file"}, + } { + t.Run(tt.name, func(t *testing.T) { + f := &dirFS{} + require.Equal(t, tt.want, f.relPath(tt.path)) }) } } ++++++ apko.obsinfo ++++++ --- /var/tmp/diff_new_pack.UNDvB9/_old 2026-04-23 17:11:32.749823109 +0200 +++ /var/tmp/diff_new_pack.UNDvB9/_new 2026-04-23 17:11:32.753823270 +0200 @@ -1,5 +1,5 @@ name: apko -version: 1.2.4 -mtime: 1776526429 -commit: 4556aed64043278fb8fa429e620fdbbe2a9e3254 +version: 1.2.6 +mtime: 1776871362 +commit: 09b82d635baa11223ba5b28b421069cadcddb5d9 ++++++ vendor.tar.gz ++++++ /work/SRC/openSUSE:Factory/apko/vendor.tar.gz /work/SRC/openSUSE:Factory/.apko.new.11940/vendor.tar.gz differ: char 133, line 2
