Skip to content

Reject symlinked Containerfile and ignore files escaping build context #6886

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Honny1 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Reject symlinked Containerfile and ignore files escaping build context #6886

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 51 additions & 22 deletions pkg/parse/parse.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1422,37 +1422,66 @@ func ContainerIgnoreFile(contextDir, path string, containerFiles []string) ([]st
excludes, err := imagebuilder.ParseIgnore(path)
return excludes, path, err
}
// If path was not supplied give priority to `<containerfile>.containerignore` first.
// If path was not supplied, look for `<containerfile>.dockerignore` and
// `<containerfile>.containerignore`. When both exist, `.containerignore` wins.
// securejoin confines lookups with RESOLVE_IN_ROOT semantics, matching
// Docker BuildKit behavior. When the containerfile is inside contextDir
// we resolve relative to contextDir; when it is outside (e.g.
// overlay-mounted context or -f pointing elsewhere) we resolve relative
// to the containerfile's parent directory.
for _, containerfile := range containerFiles {
if !filepath.IsAbs(containerfile) {
containerfile = filepath.Join(contextDir, containerfile)
}
containerfileIgnore := ""
if err := fileutils.Exists(containerfile + ".containerignore"); err == nil {
containerfileIgnore = containerfile + ".containerignore"
cleanPath := filepath.Clean(containerfile)
relPath, relErr := filepath.Rel(contextDir, cleanPath)
insideContext := relErr == nil && relPath != ".." && !strings.HasPrefix(relPath, ".."+string(filepath.Separator))
var rootDir, baseName string
if insideContext {
rootDir = contextDir
baseName = relPath
} else {
rootDir = filepath.Dir(cleanPath)
baseName = filepath.Base(cleanPath)
}
if err := fileutils.Exists(containerfile + ".dockerignore"); err == nil {
containerfileIgnore = containerfile + ".dockerignore"
excludes, resolved, err := findIgnoreFile(rootDir, baseName+".dockerignore", baseName+".containerignore")
if err != nil {
return nil, "", err
}
if containerfileIgnore != "" {
excludes, err := imagebuilder.ParseIgnore(containerfileIgnore)
return excludes, containerfileIgnore, err
if resolved != "" {
return excludes, resolved, nil
}
}
path, symlinkErr := securejoin.SecureJoin(contextDir, ".containerignore")
if symlinkErr != nil {
return nil, "", symlinkErr
excludes, resolved, err := findIgnoreFile(contextDir, ".dockerignore", ".containerignore")
if err != nil {
return nil, "", err
}
excludes, err := imagebuilder.ParseIgnore(path)
if errors.Is(err, os.ErrNotExist) {
path, symlinkErr = securejoin.SecureJoin(contextDir, ".dockerignore")
if symlinkErr != nil {
return nil, "", symlinkErr
}
excludes, err = imagebuilder.ParseIgnore(path)
if resolved != "" {
return excludes, resolved, nil
}
if errors.Is(err, os.ErrNotExist) {
return excludes, "", nil
return nil, "", nil
}

// findIgnoreFile tries each candidate name resolved under rootDir using
// securejoin (RESOLVE_IN_ROOT semantics). When both exist the last one wins.
func findIgnoreFile(rootDir string, candidates ...string) ([]string, string, error) {
var excludes []string
var matched string
for _, name := range candidates {
resolved, err := securejoin.SecureJoin(rootDir, name)
if err != nil {
continue
}
f, err := os.Open(resolved)
if err != nil {
continue
}
excludes, err = imagebuilder.ParseIgnoreReader(f)
f.Close()
if err != nil {
return nil, "", err
}
matched = resolved
}
return excludes, path, err
return excludes, matched, nil
}
84 changes: 60 additions & 24 deletions pkg/util/util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package util //nolint:revive,nolintlint

import (
"fmt"
"io/fs"
"os"
"path/filepath"
"strings"

securejoin "github.com/cyphar/filepath-securejoin"
"go.podman.io/buildah/pkg/parse"
)

Expand Down Expand Up @@ -41,42 +43,76 @@ func MirrorToTempFileIfPathIsDescriptor(file string) (string, bool) {
}

// DiscoverContainerfile tries to find a Containerfile or a Dockerfile within the provided `path`.
// The path may be a directory (in which case Containerfile/Dockerfile is searched inside it)
// or a direct path to a container file.
//
// Symlinked Containerfile/Dockerfile entries are only used when their real
// target stays inside the build context directory. Symlinks that resolve
// outside the context or are dangling are skipped.
func DiscoverContainerfile(path string) (foundCtrFile string, err error) {
// Test for existence of the file
target, err := os.Stat(path)
path, err = filepath.Abs(path)
if err != nil {
return "", fmt.Errorf("discovering Containerfile: %w", err)
}

switch mode := target.Mode(); {
case mode.IsDir():
// If the path is a real directory, we assume a Containerfile or a Dockerfile within it
ctrfile := filepath.Join(path, "Containerfile")
target, err := os.Lstat(path)

@simonbrauner simonbrauner Jul 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If lstat is ran there, it seems to cause trouble when the symlink is to a directory.

$ ls -l
lrwxrwxrwx. 1 sbrauner sbrauner   16 Jul 14 14:36 test-repository5 -> test-repository3

$ cd test-repository5

$ ls Dockerfile 
Dockerfile

$ docker build -t demo .
[+] Building 1.3s (6/6) FINISHED                                     docker:default

$ buildah build -t demo .
Error: assumed Containerfile "test-repository5" is not a file

I would propose either a special case for directories, or delaying the lstat call after we know that it is not a directory. Because building in a directory which is a symlink is a valid use case, right?

@Honny1 Honny1 Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that seems right. I will take a look.

@simonbrauner simonbrauner Jul 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good now

if err != nil {
return "", fmt.Errorf("discovering Containerfile: %w", err)
}

// Test for existence of the Containerfile file
file, err := os.Stat(ctrfile)
if err != nil {
// See if we have a Dockerfile within it
ctrfile = filepath.Join(path, "Dockerfile")
// If path is a symlink to a directory (e.g. the build context itself is
// a symlink), follow it so the IsDir() branch handles it.
if target.Mode()&os.ModeSymlink != 0 {
if realInfo, err := os.Stat(path); err == nil && realInfo.IsDir() {
target = realInfo
}
}

// Test for existence of the Dockerfile file
file, err = os.Stat(ctrfile)
if err != nil {
return "", fmt.Errorf("cannot find Containerfile or Dockerfile in context directory: %w", err)
switch {
case target.IsDir():
for _, name := range []string{"Containerfile", "Dockerfile"} {
ctrfile := filepath.Join(path, name)
if resolved, ok := isRegularFileInContext(path, ctrfile); ok {
return resolved, nil
}
}
return "", fmt.Errorf("cannot find Containerfile or Dockerfile in context directory: %w", fs.ErrNotExist)

case target.Mode().IsRegular():
return path, nil

// The file exists, now verify the correct mode
if mode := file.Mode(); mode.IsRegular() {
foundCtrFile = ctrfile
} else {
return "", fmt.Errorf("assumed Containerfile %q is not a file", ctrfile)
case target.Mode()&os.ModeSymlink != 0:
if resolved, ok := isRegularFileInContext(filepath.Dir(path), path); ok {
return resolved, nil
}

@nalind nalind Jul 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what we gain by ensuring that the last component in a pathname is not a symbolic link to somewhere outside of its parent directory when that directory can already be outside of any known build context directory.
I would be fine with having this function reject outright context locations that are neither directories nor symlinks to directories.

@simonbrauner simonbrauner Jul 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would be fine with having this function reject outright context locations that are neither directories nor symlinks to directories.

But that would be a breaking change, no? Currently we do support file as an argument in the place of [context].

buildah build -t demo test-repository6/Dockerfile seems to do the same thing as buildah build -t demo test-repository6.

But it does not seem to work together with -f:

$ buildah build -t demo -f test-repository6/Dockerfile test-repository6/Dockerfile
Error: mounting an overlay over build context directory: creating overlay scaffolding for build context directory: mount overlay:/var/tmp/buildah-context-231279313/overlay/388852530/merge, data: lowerdir=/home/sbrauner/Desktop/cve-work/buildah-build/test-repository6/Dockerfile,upperdir=/var/tmp/buildah-context-231279313/overlay/388852530/upper,workdir=/var/tmp/buildah-context-231279313/overlay/388852530/work,context="system_u:object_r:container_file_t:s0:c231,c966",userxattr: invalid argument

return "", fmt.Errorf("assumed Containerfile %q is not a file", path)

case mode.IsRegular():
// If the context dir is a file, we assume this as Containerfile
foundCtrFile = path
default:
return "", fmt.Errorf("assumed Containerfile %q is not a file", path)
}
}

return foundCtrFile, nil
// isRegularFileInContext checks whether path resolves to a regular file
// inside contextDir using RESOLVE_IN_ROOT semantics (securejoin.SecureJoin):
// ".." components are clamped to the root and absolute symlink targets are
// re-rooted under contextDir. This matches Docker BuildKit's behavior.
//
// On success it returns the resolved host path.
func isRegularFileInContext(contextDir, path string) (string, bool) {
name, err := filepath.Rel(contextDir, path)
if err != nil {
return "", false
}
resolved, err := securejoin.SecureJoin(contextDir, name)
if err != nil {
return "", false
}
fi, err := os.Stat(resolved)
if err != nil {
return "", false
}
if !fi.Mode().IsRegular() {
return "", false
}
return resolved, true
}
122 changes: 118 additions & 4 deletions pkg/util/util_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,22 @@
package util //nolint:revive,nolintlint

import (
"io/fs"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func absPath(t *testing.T, rel string) string {
t.Helper()
p, err := filepath.Abs(rel)
require.NoError(t, err)
return p
}

func TestDiscoverContainerfile(t *testing.T) {
t.Parallel()
_, err := DiscoverContainerfile("./bogus")
Expand All @@ -16,17 +27,120 @@ func TestDiscoverContainerfile(t *testing.T) {

name, err := DiscoverContainerfile("test/test1/Dockerfile")
assert.Nil(t, err)
assert.Equal(t, name, "test/test1/Dockerfile")
assert.Equal(t, absPath(t, "test/test1/Dockerfile"), name)

name, err = DiscoverContainerfile("test/test1/Containerfile")
assert.Nil(t, err)
assert.Equal(t, name, "test/test1/Containerfile")
assert.Equal(t, absPath(t, "test/test1/Containerfile"), name)

name, err = DiscoverContainerfile("test/test1")
assert.Nil(t, err)
assert.Equal(t, name, "test/test1/Containerfile")
assert.Equal(t, absPath(t, "test/test1/Containerfile"), name)

name, err = DiscoverContainerfile("test/test2")
assert.Nil(t, err)
assert.Equal(t, name, "test/test2/Dockerfile")
assert.Equal(t, absPath(t, "test/test2/Dockerfile"), name)
}

func TestDiscoverContainerfileRejectsSymlinkOutsideContext(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()

secretFile := filepath.Join(tmpDir, "secret-Containerfile")
require.NoError(t, os.WriteFile(secretFile, []byte("FROM scratch\n"), 0o644))

contextDir := filepath.Join(tmpDir, "context")
require.NoError(t, os.Mkdir(contextDir, 0o755))
require.NoError(t, os.Symlink(secretFile, filepath.Join(contextDir, "Containerfile")))

_, err := DiscoverContainerfile(contextDir)
assert.Error(t, err)
assert.ErrorIs(t, err, fs.ErrNotExist)
}

func TestDiscoverContainerfileAcceptsSymlinkInsideContext(t *testing.T) {
t.Parallel()
contextDir := t.TempDir()

subdir := filepath.Join(contextDir, "subdir")
require.NoError(t, os.Mkdir(subdir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(subdir, "Containerfile.real"), []byte("FROM scratch\n"), 0o644))
require.NoError(t, os.Symlink(filepath.Join("subdir", "Containerfile.real"), filepath.Join(contextDir, "Containerfile")))

name, err := DiscoverContainerfile(contextDir)
require.NoError(t, err)
assert.Equal(t, filepath.Join(contextDir, "subdir", "Containerfile.real"), name)
}

func TestDiscoverContainerfileAcceptsEscapeClampedToRoot(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()

contextDir := filepath.Join(tmpDir, "context")
require.NoError(t, os.Mkdir(contextDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(contextDir, "file"), []byte("FROM scratch\n"), 0o644))
require.NoError(t, os.Symlink("../file", filepath.Join(contextDir, "Containerfile")))

name, err := DiscoverContainerfile(contextDir)
require.NoError(t, err)
assert.Equal(t, filepath.Join(contextDir, "file"), name)
}
Comment thread
nalind marked this conversation as resolved.

func TestDiscoverContainerfileAcceptsAbsoluteSymlinkRerooted(t *testing.T) {
t.Parallel()
contextDir := t.TempDir()

subdir := filepath.Join(contextDir, "subdirectory")
require.NoError(t, os.Mkdir(subdir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(subdir, "real.file"), []byte("FROM scratch\n"), 0o644))
require.NoError(t, os.Symlink("/subdirectory/real.file", filepath.Join(contextDir, "Containerfile")))

name, err := DiscoverContainerfile(contextDir)
require.NoError(t, err)
assert.Equal(t, filepath.Join(contextDir, "subdirectory", "real.file"), name)
}

func TestDiscoverContainerfileAcceptsMultiLevelEscapeClampedToRoot(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()

contextDir := filepath.Join(tmpDir, "context")
require.NoError(t, os.Mkdir(contextDir, 0o755))
subdir := filepath.Join(contextDir, "subdir")
require.NoError(t, os.Mkdir(subdir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(subdir, "file"), []byte("FROM scratch\n"), 0o644))
require.NoError(t, os.Symlink("../../subdir/file", filepath.Join(contextDir, "Containerfile")))

name, err := DiscoverContainerfile(contextDir)
require.NoError(t, err)
assert.Equal(t, filepath.Join(contextDir, "subdir", "file"), name)
}

func TestDiscoverContainerfileAcceptsSymlinkedDirectory(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()

realDir := filepath.Join(tmpDir, "real-context")
require.NoError(t, os.Mkdir(realDir, 0o755))
require.NoError(t, os.WriteFile(filepath.Join(realDir, "Containerfile"), []byte("FROM scratch\n"), 0o644))

symlinkedDir := filepath.Join(tmpDir, "symlinked-context")
require.NoError(t, os.Symlink(realDir, symlinkedDir))

name, err := DiscoverContainerfile(symlinkedDir)
require.NoError(t, err)
assert.Equal(t, filepath.Join(symlinkedDir, "Containerfile"), name)
}

func TestDiscoverContainerfileRejectsNonExistentClampedTarget(t *testing.T) {
t.Parallel()
tmpDir := t.TempDir()

contextDir := filepath.Join(tmpDir, "context")
require.NoError(t, os.Mkdir(contextDir, 0o755))
require.NoError(t, os.Symlink("../nonexistent", filepath.Join(contextDir, "Containerfile")))

_, err := DiscoverContainerfile(contextDir)
assert.Error(t, err)
assert.ErrorIs(t, err, fs.ErrNotExist)
}
Loading