diff --git a/.cirrus.yml b/.cirrus.yml index 6ab49135cb7..c7cc5e26d50 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -24,14 +24,14 @@ env: #### #### Cache-image names to test with (double-quotes around names are critical) #### - FEDORA_NAME: "fedora-42" + FEDORA_NAME: "fedora-43" FEDORA_AARCH64_NAME: "${FEDORA_NAME}-aarch64" - PRIOR_FEDORA_NAME: "fedora-41" + PRIOR_FEDORA_NAME: "fedora-42" RAWHIDE_NAME: "rawhide" - DEBIAN_NAME: "debian-13" + DEBIAN_NAME: "debian-14" # Image identifiers - IMAGE_SUFFIX: "c20250910t092246z-f42f41d13" + IMAGE_SUFFIX: "c20260319t182308z-f43f42d14" # EC2 images FEDORA_AMI: "fedora-aws-${IMAGE_SUFFIX}" @@ -810,7 +810,7 @@ podman_machine_aarch64_task: depends_on: *build ec2_instance: <<: *standard_build_ec2_aarch64 - timeout_in: 40m + timeout_in: 60m env: TEST_FLAVOR: "machine-linux" TEST_BUILD_TAGS: "" @@ -1099,13 +1099,9 @@ upgrade_test_task: depends_on: *build matrix: - env: - # 2024-02: as long as possible/reasonable, try to keep - # one version < 4.8 so we can test boltdb. v4.3.1 is - # the lowest we can go right now, builds before that - # have netavark <1.4 which hangs on f39 kernel (#21863). - PODMAN_UPGRADE_FROM: v4.3.1 + PODMAN_UPGRADE_FROM: v5.3.1 - env: - PODMAN_UPGRADE_FROM: v4.8.0 + PODMAN_UPGRADE_FROM: v5.6.2 gce_instance: *standardvm env: TEST_FLAVOR: upgrade_test diff --git a/cmd/rootlessport/main.go b/cmd/rootlessport/main.go index 107f5403e74..0662526ddf1 100644 --- a/cmd/rootlessport/main.go +++ b/cmd/rootlessport/main.go @@ -201,7 +201,7 @@ outer: _ = os.Remove(socketfile) // workaround to bypass the 108 char socket path limit // open the fd and use the path to the fd as bind argument - fd, err := unix.Open(socketDir, unix.O_PATH, 0) + fd, err := unix.Open(socketDir, unix.O_PATH|unix.O_CLOEXEC, 0) if err != nil { return err } diff --git a/libpod/container.go b/libpod/container.go index 12b6bf46d92..1af841fdff6 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -1442,18 +1442,20 @@ func (c *Container) NetworkMode() string { // If there is none, it's host networking. // If there is one and it has a path, it's "ns:". foundNetNS := false - for _, ns := range ctrSpec.Linux.Namespaces { - if ns.Type == spec.NetworkNamespace { - foundNetNS = true - if ns.Path != "" { - networkMode = fmt.Sprintf("ns:%s", ns.Path) - } else { - // We're making a network ns, but not - // configuring with Slirp or CNI. That - // means it's --net=none - networkMode = "none" + if ctrSpec.Linux != nil { + for _, ns := range ctrSpec.Linux.Namespaces { + if ns.Type == spec.NetworkNamespace { + foundNetNS = true + if ns.Path != "" { + networkMode = fmt.Sprintf("ns:%s", ns.Path) + } else { + // We're making a network ns, but not + // configuring with Slirp or CNI. That + // means it's --net=none + networkMode = "none" + } + break } - break } } if !foundNetNS { diff --git a/libpod/container_internal_common.go b/libpod/container_internal_common.go index b7b888abf5e..395490d2a30 100644 --- a/libpod/container_internal_common.go +++ b/libpod/container_internal_common.go @@ -54,6 +54,7 @@ import ( "go.podman.io/common/pkg/umask" is "go.podman.io/image/v5/storage" "go.podman.io/storage/pkg/archive" + "go.podman.io/storage/pkg/chrootarchive" "go.podman.io/storage/pkg/fileutils" "go.podman.io/storage/pkg/idtools" "go.podman.io/storage/pkg/lockfile" @@ -1207,11 +1208,10 @@ func (c *Container) exportCheckpoint(options ContainerCheckpointOptions) error { if mp == "" { return fmt.Errorf("volume %s is not mounted, cannot export: %w", volume.Name(), define.ErrInternal) } - - input, err := archive.TarWithOptions(mp, &archive.TarOptions{ + input, err := chrootarchive.Tar(mp, &archive.TarOptions{ Compression: archive.Uncompressed, IncludeSourceDir: true, - }) + }, mp) if err != nil { return fmt.Errorf("reading volume directory %q: %w", v.Dest, err) } @@ -1226,12 +1226,12 @@ func (c *Container) exportCheckpoint(options ContainerCheckpointOptions) error { } } - input, err := archive.TarWithOptions(c.bundlePath(), &archive.TarOptions{ + bundle := c.bundlePath() + input, err := chrootarchive.Tar(bundle, &archive.TarOptions{ Compression: options.Compression, IncludeSourceDir: true, IncludeFiles: includeFiles, - }) - + }, bundle) if err != nil { return fmt.Errorf("reading checkpoint directory %q: %w", c.ID(), err) } @@ -1312,10 +1312,10 @@ func (c *Container) checkpoint(ctx context.Context, options ContainerCheckpointO } defer shmDirTarFile.Close() - input, err := archive.TarWithOptions(c.config.ShmDir, &archive.TarOptions{ + input, err := chrootarchive.Tar(c.config.ShmDir, &archive.TarOptions{ Compression: archive.Uncompressed, IncludeSourceDir: true, - }) + }, c.config.ShmDir) if err != nil { return nil, 0, err } @@ -1488,7 +1488,7 @@ func (c *Container) importPreCheckpoint(input string) error { defer archiveFile.Close() - err = archive.Untar(archiveFile, c.bundlePath(), nil) + err = chrootarchive.Untar(archiveFile, c.bundlePath(), nil) if err != nil { return fmt.Errorf("unpacking of pre-checkpoint archive %s failed: %w", input, err) } @@ -1751,7 +1751,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti } defer shmDirTarFile.Close() - if err := archive.UntarUncompressed(shmDirTarFile, c.config.ShmDir, nil); err != nil { + if err := chrootarchive.UntarUncompressed(shmDirTarFile, c.config.ShmDir, nil); err != nil { return nil, 0, err } } @@ -1791,7 +1791,7 @@ func (c *Container) restore(ctx context.Context, options ContainerCheckpointOpti if mountPoint == "" { return nil, 0, fmt.Errorf("unable to import volume %s as it is not mounted: %w", volume.Name(), err) } - if err := archive.UntarUncompressed(volumeFile, mountPoint, nil); err != nil { + if err := chrootarchive.UntarUncompressed(volumeFile, mountPoint, nil); err != nil { return nil, 0, fmt.Errorf("failed to extract volume %s to %s: %w", volumeFilePath, mountPoint, err) } } diff --git a/libpod/oci_conmon_attach_linux.go b/libpod/oci_conmon_attach_linux.go index 10435ee5e04..fb5a3d39359 100644 --- a/libpod/oci_conmon_attach_linux.go +++ b/libpod/oci_conmon_attach_linux.go @@ -10,7 +10,7 @@ import ( ) func openUnixSocket(path string) (*net.UnixConn, error) { - fd, err := unix.Open(path, unix.O_PATH, 0) + fd, err := unix.Open(path, unix.O_PATH|unix.O_CLOEXEC, 0) if err != nil { return nil, err } diff --git a/libpod/oci_conmon_exec_common.go b/libpod/oci_conmon_exec_common.go index 45350f93827..644e561af0d 100644 --- a/libpod/oci_conmon_exec_common.go +++ b/libpod/oci_conmon_exec_common.go @@ -510,6 +510,13 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp return err } + // errCh receives deferredErr after all deferred cleanup in this function + // has completed. The goroutine below reads from errCh so that it never + // races with the deferred functions that may still be writing deferredErr + // when holdConnOpen is closed by the caller. + errCh := make(chan error, 1) + defer func() { errCh <- deferredErr }() + defer func() { if !pipes.startClosed { errorhandling.CloseQuiet(pipes.startPipe) @@ -608,7 +615,11 @@ func attachExecHTTP(c *Container, sessionID string, r *http.Request, w http.Resp // Can't be a defer, because this would block the function from // returning. <-holdConnOpen - hijackWriteErrorAndClose(deferredErr, c.ID(), isTerminal, httpCon, httpBuf) + // Block until all deferred cleanups in attachExecHTTP have run and + // the final deferredErr value has been sent to errCh. This avoids + // the data race that would occur if we read deferredErr directly + // while deferred functions in this function may still be writing it. + hijackWriteErrorAndClose(<-errCh, c.ID(), isTerminal, httpCon, httpBuf) }() stdoutChan := make(chan error) diff --git a/libpod/volume.go b/libpod/volume.go index 0997f6eaf6a..a552f18aa78 100644 --- a/libpod/volume.go +++ b/libpod/volume.go @@ -13,7 +13,7 @@ import ( "github.com/containers/podman/v5/libpod/plugin" "github.com/containers/podman/v5/utils" "github.com/sirupsen/logrus" - "go.podman.io/storage/pkg/archive" + "go.podman.io/storage/pkg/chrootarchive" "go.podman.io/storage/pkg/directory" ) @@ -342,7 +342,7 @@ func (v *Volume) Import(r io.Reader) error { } }() - if err := archive.Untar(r, mountPoint, nil); err != nil { + if err := chrootarchive.Untar(r, mountPoint, nil); err != nil { return fmt.Errorf("extracting into volume %s: %w", v.Name(), err) } diff --git a/pkg/api/handlers/compat/images_build.go b/pkg/api/handlers/compat/images_build.go index 5cf0ae01711..c874de1e96f 100644 --- a/pkg/api/handlers/compat/images_build.go +++ b/pkg/api/handlers/compat/images_build.go @@ -1237,6 +1237,6 @@ func extractTarFile(anchorDir string, r io.ReadCloser) (string, error) { return "", err } - err = archive.Untar(r, buildDir, nil) + err = chrootarchive.Untar(r, buildDir, nil) return buildDir, err } diff --git a/pkg/api/handlers/libpod/kube.go b/pkg/api/handlers/libpod/kube.go index 6b6a47e2d0e..fd5e9eee55b 100644 --- a/pkg/api/handlers/libpod/kube.go +++ b/pkg/api/handlers/libpod/kube.go @@ -12,8 +12,6 @@ import ( "os" "path/filepath" - "go.podman.io/storage/pkg/archive" - "github.com/containers/podman/v5/libpod" "github.com/containers/podman/v5/pkg/api/handlers/utils" api "github.com/containers/podman/v5/pkg/api/types" @@ -23,6 +21,7 @@ import ( "github.com/gorilla/schema" "github.com/sirupsen/logrus" "go.podman.io/image/v5/types" + "go.podman.io/storage/pkg/chrootarchive" ) // ExtractPlayReader provide an io.Reader given a http.Request object @@ -52,7 +51,7 @@ func extractPlayReader(anchorDir string, r *http.Request) (io.Reader, error) { reader = r.Body case "application/x-tar": // un-tar the content - err := archive.Untar(r.Body, anchorDir, nil) + err := chrootarchive.Untar(r.Body, anchorDir, nil) if err != nil { return nil, err } diff --git a/pkg/api/handlers/libpod/quadlets.go b/pkg/api/handlers/libpod/quadlets.go index dbca037d804..65ebc81d24c 100644 --- a/pkg/api/handlers/libpod/quadlets.go +++ b/pkg/api/handlers/libpod/quadlets.go @@ -11,8 +11,6 @@ import ( "path/filepath" "strings" - "go.podman.io/storage/pkg/archive" - "github.com/containers/podman/v5/libpod" "github.com/containers/podman/v5/libpod/define" "github.com/containers/podman/v5/pkg/api/handlers/utils" @@ -23,6 +21,7 @@ import ( "github.com/containers/podman/v5/pkg/util" "github.com/gorilla/schema" "github.com/sirupsen/logrus" + "go.podman.io/storage/pkg/chrootarchive" ) func ListQuadlets(w http.ResponseWriter, r *http.Request) { @@ -94,7 +93,7 @@ func extractQuadletFiles(tempDir string, r io.ReadCloser) ([]string, error) { return nil, err } - err = archive.Untar(r, quadletDir, nil) + err = chrootarchive.Untar(r, quadletDir, nil) if err != nil { return nil, err } diff --git a/pkg/api/handlers/libpod/system.go b/pkg/api/handlers/libpod/system.go index b4ff77245c4..a6f4cff1cd5 100644 --- a/pkg/api/handlers/libpod/system.go +++ b/pkg/api/handlers/libpod/system.go @@ -96,6 +96,7 @@ func SystemCheck(w http.ResponseWriter, r *http.Request) { if err != nil { utils.Error(w, http.StatusBadRequest, fmt.Errorf("failed to parse unreferenced_layer_max_age parameter %q for %s: %w", query.UnreferencedLayerMaximumAge, r.URL.String(), err)) + return } unreferencedLayerMaximumAge = &duration } diff --git a/pkg/bindings/artifacts/extract.go b/pkg/bindings/artifacts/extract.go index 6e9e21a94cb..6e8279133d4 100644 --- a/pkg/bindings/artifacts/extract.go +++ b/pkg/bindings/artifacts/extract.go @@ -73,7 +73,14 @@ func Extract(ctx context.Context, artifactName string, target string, options *E // If destination isn't a file, extract to target/filename fileTarget := target if targetIsDirectory { - fileTarget = filepath.Join(target, header.Name) + filename := header.Name + // This matches the logic from generateArtifactBlobName(). + for i := range len(filename) { + if os.IsPathSeparator(filename[i]) { + return fmt.Errorf("invalid filename: %q cannot contain %c", filename, filename[i]) + } + } + fileTarget = filepath.Join(target, filename) } if header.Typeflag == tar.TypeReg { diff --git a/pkg/bindings/images/build.go b/pkg/bindings/images/build.go index 92745e0590d..8ad8e0518bc 100644 --- a/pkg/bindings/images/build.go +++ b/pkg/bindings/images/build.go @@ -1084,6 +1084,10 @@ func build(ctx context.Context, containerFiles []string, options types.BuildOpti return processBuildResponse(response, stdout, saveFormat) } +// nTar builds a gzip-compressed tar of sources and returns it as a ReadCloser. +// sources[0] is the build context directory (walked recursively). Remaining entries +// are extra regular files (e.g. secrets) that are always archived; exclude patterns +// apply only to the first source. func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { pm, err := fileutils.NewPatternMatcher(excludes) if err != nil { @@ -1104,6 +1108,7 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { defer gw.Close() defer tw.Close() seen := make(map[devino]string) + firstSourceAbs := "" for i, src := range sources { source, err := filepath.Abs(src) if err != nil { @@ -1111,6 +1116,9 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { merr = multierror.Append(merr, err) return } + if i == 0 { + firstSourceAbs = source + } err = filepath.WalkDir(source, func(path string, dentry fs.DirEntry, err error) error { if err != nil { return err @@ -1144,7 +1152,15 @@ func nTar(excludes []string, sources ...string) (io.ReadCloser, error) { if !dentry.Type().IsRegular() { return fmt.Errorf("path %s must be a regular file", path) } - name = filepath.ToSlash(path) + // Additional sources are absolute host paths (Containerfiles passed by path, or + // temp files such as secrets placed under the context directory). Strip the + // resolved context directory so tar member names are relative to that root, + // like entries from the primary walk (i == 0). Using the raw absolute path would + // produce members that unpack as a spurious host path tree (e.g. Users/...) + // instead of files beside the Dockerfile. + // https://github.com/containers/podman/issues/28334 + after, _ := strings.CutPrefix(path, firstSourceAbs) + name = filepath.ToSlash(after) } // If name is absolute path, then it has to be containerfile outside of build context. // If not, we should check it for being excluded via pattern matcher. diff --git a/pkg/checkpoint/crutils/checkpoint_restore_utils.go b/pkg/checkpoint/crutils/checkpoint_restore_utils.go index 1709429bad8..652bb0b3357 100644 --- a/pkg/checkpoint/crutils/checkpoint_restore_utils.go +++ b/pkg/checkpoint/crutils/checkpoint_restore_utils.go @@ -11,8 +11,10 @@ import ( metadata "github.com/checkpoint-restore/checkpointctl/lib" "github.com/checkpoint-restore/go-criu/v7/stats" + securejoin "github.com/cyphar/filepath-securejoin" "github.com/opencontainers/selinux/go-selinux/label" "go.podman.io/storage/pkg/archive" + "go.podman.io/storage/pkg/chrootarchive" ) // This file mainly exists to make the checkpoint/restore functions @@ -34,7 +36,7 @@ func CRImportCheckpointWithoutConfig(destination, input string) error { metadata.SpecDumpFile, }, } - if err = archive.Untar(archiveFile, destination, options); err != nil { + if err = chrootarchive.Untar(archiveFile, destination, options); err != nil { return fmt.Errorf("unpacking of checkpoint archive %s failed: %w", input, err) } @@ -64,7 +66,7 @@ func CRImportCheckpointConfigOnly(destination, input string) error { metadata.CheckpointVolumesDirectory, }, } - if err = archive.Untar(archiveFile, destination, options); err != nil { + if err = chrootarchive.Untar(archiveFile, destination, options); err != nil { return fmt.Errorf("unpacking of checkpoint archive %s failed: %w", input, err) } @@ -87,7 +89,11 @@ func CRRemoveDeletedFiles(id, baseDirectory, containerRootDirectory string) erro for _, deleteFile := range deletedFiles { // Using RemoveAll as deletedFiles, which is generated from 'podman diff' // lists completely deleted directories as a single entry: 'D /root'. - if err := os.RemoveAll(filepath.Join(containerRootDirectory, deleteFile)); err != nil { + path, err := securejoin.SecureJoin(containerRootDirectory, deleteFile) + if err != nil { + return fmt.Errorf("failed to resolve path %q in container %s: %w", deleteFile, id, err) + } + if err := os.RemoveAll(path); err != nil { return fmt.Errorf("failed to delete files from container %s during restore: %w", id, err) } } @@ -109,7 +115,7 @@ func CRApplyRootFsDiffTar(baseDirectory, containerRootDirectory string) error { } defer rootfsDiffFile.Close() - if err := archive.Untar(rootfsDiffFile, containerRootDirectory, nil); err != nil { + if err := chrootarchive.Untar(rootfsDiffFile, containerRootDirectory, nil); err != nil { return fmt.Errorf("failed to apply root file-system diff file %s: %w", rootfsDiffPath, err) } @@ -152,11 +158,11 @@ func CRCreateRootFsDiffTar(changes *[]archive.Change, mountPoint, destination st } if len(rootfsIncludeFiles) > 0 { - rootfsTar, err := archive.TarWithOptions(mountPoint, &archive.TarOptions{ + rootfsTar, err := chrootarchive.Tar(mountPoint, &archive.TarOptions{ Compression: archive.Uncompressed, IncludeSourceDir: true, IncludeFiles: rootfsIncludeFiles, - }) + }, mountPoint) if err != nil { return includeFiles, fmt.Errorf("exporting root file-system diff to %q: %w", rootfsDiffPath, err) } diff --git a/pkg/domain/infra/abi/play.go b/pkg/domain/infra/abi/play.go index bf41e73a440..770368193aa 100644 --- a/pkg/domain/infra/abi/play.go +++ b/pkg/domain/infra/abi/play.go @@ -43,7 +43,7 @@ import ( "go.podman.io/common/pkg/secrets" "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/types" - "go.podman.io/storage/pkg/archive" + "go.podman.io/storage/pkg/chrootarchive" "go.podman.io/storage/pkg/fileutils" yamlv3 "gopkg.in/yaml.v3" "sigs.k8s.io/yaml" @@ -1503,7 +1503,7 @@ func (ic *ContainerEngine) importVolume(ctx context.Context, vol *libpod.Volume, } // dont care if volume is mounted or not we are gonna import everything to mountPoint - return archive.Untar(tarFile, mountPoint, nil) + return chrootarchive.Untar(tarFile, mountPoint, nil) } // readConfigMapFromFile returns a kubernetes configMap obtained from --configmap flag diff --git a/pkg/domain/infra/abi/secrets.go b/pkg/domain/infra/abi/secrets.go index 2bb2ae2b9c5..37bcdf8c285 100644 --- a/pkg/domain/infra/abi/secrets.go +++ b/pkg/domain/infra/abi/secrets.go @@ -33,7 +33,7 @@ func (ic *ContainerEngine) SecretCreate(_ context.Context, name string, reader i if options.Driver == "" { options.Driver = cfg.Secrets.Driver } - if len(options.DriverOpts) == 0 { + if len(options.DriverOpts) == 0 && options.Driver == cfg.Secrets.Driver { options.DriverOpts = cfg.Secrets.Opts } if options.DriverOpts == nil { diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go index 0a86dba08d1..316eedfc4bc 100644 --- a/pkg/domain/infra/tunnel/images.go +++ b/pkg/domain/infra/tunnel/images.go @@ -24,6 +24,7 @@ import ( "go.podman.io/image/v5/docker/reference" "go.podman.io/image/v5/types" "go.podman.io/storage/pkg/archive" + "go.podman.io/storage/pkg/chrootarchive" ) func (ir *ImageEngine) Exists(_ context.Context, nameOrID string) (*entities.BoolReport, error) { @@ -371,7 +372,7 @@ func (ir *ImageEngine) Save(_ context.Context, nameOrID string, tags []string, o return err } - return archive.Untar(f, opts.Output, &archive.TarOptions{NoLchown: true}) + return chrootarchive.Untar(f, opts.Output, &archive.TarOptions{NoLchown: true}) } func (ir *ImageEngine) Search(_ context.Context, term string, opts entities.ImageSearchOptions) ([]entities.ImageSearchReport, error) { diff --git a/pkg/pidhandle/pidhandle_linux.go b/pkg/pidhandle/pidhandle_linux.go index 2420ed86c41..6e0140069f3 100644 --- a/pkg/pidhandle/pidhandle_linux.go +++ b/pkg/pidhandle/pidhandle_linux.go @@ -118,7 +118,7 @@ func NewPIDHandleFromString(pid int, pidData string) (PIDHandle, error) { return nil, err } defer unix.Close(fd) - pidfd, err := openByHandleAt(fd, fh, 0) + pidfd, err := openByHandleAt(fd, fh, unix.O_CLOEXEC) if err != nil { if err == unix.ESTALE { h.normalHandle.pidData = noSuchProcessID diff --git a/pkg/specgen/generate/config_linux.go b/pkg/specgen/generate/config_linux.go index 0b7c9233d8d..c730e6dfb52 100644 --- a/pkg/specgen/generate/config_linux.go +++ b/pkg/specgen/generate/config_linux.go @@ -153,7 +153,7 @@ func addDevice(g *generate.Generator, device string) error { } else if src == "/dev/fuse" { // if the user is asking for fuse inside the container // make sure the module is loaded. - f, err := unix.Open(src, unix.O_RDONLY|unix.O_NONBLOCK, 0) + f, err := unix.Open(src, unix.O_RDONLY|unix.O_NONBLOCK|unix.O_CLOEXEC, 0) if err == nil { unix.Close(f) } diff --git a/pkg/systemd/parser/unitfile.go b/pkg/systemd/parser/unitfile.go index 19556e0cd72..30bfd6195ae 100644 --- a/pkg/systemd/parser/unitfile.go +++ b/pkg/systemd/parser/unitfile.go @@ -609,14 +609,20 @@ func (f *UnitFile) LookupLast(groupName string, key string) (string, bool) { } // Look up the last instance of the named key in the group (if any) -// The result have no trailing whitespace and line continuations are applied +// The result have no trailing whitespace and line continuations are applied. +// Surrounding matched double-quote pairs are stripped, but unmatched quotes +// are preserved to avoid mangling embedded shell syntax. func (f *UnitFile) Lookup(groupName string, key string) (string, bool) { v, ok := f.LookupLast(groupName, key) if !ok { return "", false } - return strings.Trim(strings.TrimRightFunc(v, unicode.IsSpace), "\""), true + v = strings.TrimRightFunc(v, unicode.IsSpace) + if len(v) >= 2 && v[0] == '"' && v[len(v)-1] == '"' { + v = v[1 : len(v)-1] + } + return v, true } // Lookup the last instance of a key and convert the value to a bool diff --git a/pkg/systemd/parser/unitfile_test.go b/pkg/systemd/parser/unitfile_test.go index 6d1cdc8d3f7..fff4f4445df 100644 --- a/pkg/systemd/parser/unitfile_test.go +++ b/pkg/systemd/parser/unitfile_test.go @@ -1,6 +1,7 @@ package parser import ( + "fmt" "reflect" "strings" "testing" @@ -341,6 +342,35 @@ Name=my-container assert.Equal(t, "; another comment", comments[1]) } +func TestLookupQuoteStripping(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {`"hello"`, "hello"}, + {`hello`, "hello"}, + {`"hello`, `"hello`}, + {`/bin/sh -c "echo "hello""`, `/bin/sh -c "echo "hello""`}, + {`"/bin/sh -c "echo "hello"""`, `/bin/sh -c "echo "hello""`}, + {`""`, ""}, + {`hello"`, `hello"`}, + {`/bin/sh -c "echo 'hello world!'"`, `/bin/sh -c "echo 'hello world!'"`}, + } + + for _, tt := range tests { + t.Run(fmt.Sprintf("Lookup(%q)", tt.input), func(t *testing.T) { + unitData := "[Container]\nKey=" + tt.input + "\n" + f := NewUnitFile() + err := f.Parse(unitData) + assert.NoError(t, err) + + val, ok := f.Lookup("Container", "Key") + assert.True(t, ok) + assert.Equal(t, tt.expected, val) + }) + } +} + func FuzzParser(f *testing.F) { for _, sample := range samples { f.Add([]byte(sample)) diff --git a/pkg/systemd/quadlet/quadlet.go b/pkg/systemd/quadlet/quadlet.go index ea12ac62d41..c4d95d5f4f4 100644 --- a/pkg/systemd/quadlet/quadlet.go +++ b/pkg/systemd/quadlet/quadlet.go @@ -675,11 +675,17 @@ func ConvertContainer(container *parser.UnitFile, unitsInfoMap map[string]*UnitI podman.add("--cgroups=split") } + // Entrypoint needs special handling: an empty value is valid and means + // "clear the image entrypoint" (podman run --entrypoint ""), so it + // cannot go through lookupAndAddString which skips empty values. + if val, ok := container.Lookup(ContainerGroup, KeyEntrypoint); ok { + podman.addf("--entrypoint=%s", val) + } + stringKeys := map[string]string{ KeyTimezone: "--tz", KeyPidsLimit: "--pids-limit", KeyShmSize: "--shm-size", - KeyEntrypoint: "--entrypoint", KeyWorkingDir: "--workdir", KeyIP: "--ip", KeyIP6: "--ip6", diff --git a/test/apiv2/python/rest_api/test_v2_0_0_image.py b/test/apiv2/python/rest_api/test_v2_0_0_image.py index e62b210ebc9..bd37d7a61fd 100644 --- a/test/apiv2/python/rest_api/test_v2_0_0_image.py +++ b/test/apiv2/python/rest_api/test_v2_0_0_image.py @@ -1,6 +1,6 @@ import json import unittest -from multiprocessing import Process +import multiprocessing as mp import requests from dateutil.parser import parse @@ -168,9 +168,12 @@ def do_search5(): self.assertEqual(r.status_code, 400, f"#5: {r.text}") i = 1 + # Need to explicitly set start method + # # https://docs.python.org/dev/library/multiprocessing.html#contexts-and-start-methods + mp.set_start_method('fork') for fn in [do_search1, do_search2, do_search3, do_search4, do_search5]: with self.subTest(i=i): - search = Process(target=fn) + search = mp.Process(target=fn) search.start() search.join(timeout=10) self.assertFalse(search.is_alive(), f"#{i} /images/search took too long") diff --git a/test/e2e/build/secret-verify-leak/Containerfile.with-secret-verify-leak b/test/e2e/build/remote-secret-copy/Dockerfile similarity index 73% rename from test/e2e/build/secret-verify-leak/Containerfile.with-secret-verify-leak rename to test/e2e/build/remote-secret-copy/Dockerfile index 0957ac6a60d..816b48cdd3e 100644 --- a/test/e2e/build/secret-verify-leak/Containerfile.with-secret-verify-leak +++ b/test/e2e/build/remote-secret-copy/Dockerfile @@ -1,3 +1,4 @@ FROM alpine -COPY * / +COPY . . +RUN test -e hello RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret diff --git a/test/e2e/build/remote-secret-copy/hello b/test/e2e/build/remote-secret-copy/hello new file mode 100644 index 00000000000..45b983be36b --- /dev/null +++ b/test/e2e/build/remote-secret-copy/hello @@ -0,0 +1 @@ +hi diff --git a/test/e2e/build/remote-secret-dockerignore-star/.dockerignore b/test/e2e/build/remote-secret-dockerignore-star/.dockerignore new file mode 100644 index 00000000000..72e8ffc0db8 --- /dev/null +++ b/test/e2e/build/remote-secret-dockerignore-star/.dockerignore @@ -0,0 +1 @@ +* diff --git a/test/e2e/build/remote-secret-dockerignore-star/Dockerfile b/test/e2e/build/remote-secret-dockerignore-star/Dockerfile new file mode 100644 index 00000000000..ccf49a15841 --- /dev/null +++ b/test/e2e/build/remote-secret-dockerignore-star/Dockerfile @@ -0,0 +1,4 @@ +FROM alpine +COPY . . +RUN test ! -e ignored-by-dockerignore.txt +RUN --mount=type=secret,id=MY_SECRET cat /run/secrets/MY_SECRET diff --git a/test/e2e/build/remote-secret-dockerignore-star/host-secret.txt b/test/e2e/build/remote-secret-dockerignore-star/host-secret.txt new file mode 100644 index 00000000000..7031a3ed69a --- /dev/null +++ b/test/e2e/build/remote-secret-dockerignore-star/host-secret.txt @@ -0,0 +1 @@ +Super Secret diff --git a/test/e2e/build/remote-secret-dockerignore-star/ignored-by-dockerignore.txt b/test/e2e/build/remote-secret-dockerignore-star/ignored-by-dockerignore.txt new file mode 100644 index 00000000000..2e1a27620db --- /dev/null +++ b/test/e2e/build/remote-secret-dockerignore-star/ignored-by-dockerignore.txt @@ -0,0 +1 @@ +this file must not appear in the image diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index 36a8585c43a..cb617bb9bfb 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -89,51 +89,62 @@ var _ = Describe("Podman build", func() { }) It("podman build with a secret from file", func() { - session := podmanTest.Podman([]string{"build", "-f", "build/Containerfile.with-secret", "-t", "secret-test", "--secret", "id=mysecret,src=build/secret.txt", "build/"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) + image := "secret-test" + + session := podmanTest.PodmanExitCleanly("build", "-f", "build/Containerfile.with-secret", "-t", image, "--secret", "id=mysecret,src=build/secret.txt", "build/") Expect(session.OutputToString()).To(ContainSubstring("somesecret")) - session = podmanTest.Podman([]string{"rmi", "secret-test"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) - }) + session = podmanTest.PodmanExitCleanly("run", "--rm", image, "sh", "-c", "find / -xdev -name 'podman-build-secret*' -print") + Expect(session.OutputToString()).To(BeEmpty(), "podman-build-secret path leaked into image") - It("podman build with a secret from env", func() { - os.Setenv("MYSECRET", "somesecret") - defer os.Unsetenv("MYSECRET") - session := podmanTest.PodmanExitCleanly("build", "-f", "build/Containerfile.with-secret", "-t", "secret-test", "--secret", "id=mysecret,env=MYSECRET", "build/") - Expect(session.OutputToString()).To(ContainSubstring("somesecret")) + podmanTest.PodmanExitCleanly("rmi", image) - podmanTest.PodmanExitCleanly("rmi", "secret-test") - }) + // Test for: https://github.com/containers/podman/issues/25314 - file secrets must reach the server when .dockerignore is '*'. + image = "e2e-remote-secret-dignore" - It("podman build with multiple secrets from files", func() { - session := podmanTest.Podman([]string{"build", "-f", "build/Containerfile.with-multiple-secret", "-t", "multiple-secret-test", "--secret", "id=mysecret,src=build/secret.txt", "--secret", "id=mysecret2,src=build/anothersecret.txt", "build/"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) + session = podmanTest.PodmanExitCleanly("build", "-f", "build/remote-secret-dockerignore-star/Dockerfile", "-t", image, "--secret", "id=MY_SECRET,type=file,src=build/remote-secret-dockerignore-star/host-secret.txt", "build/remote-secret-dockerignore-star") + Expect(session.OutputToString()).To(ContainSubstring("Super Secret")) + + session = podmanTest.PodmanExitCleanly("run", "--rm", image, "sh", "-c", "find / -xdev -name 'podman-build-secret*' -print") + Expect(session.OutputToString()).To(BeEmpty(), "podman-build-secret path leaked into image") + + podmanTest.PodmanExitCleanly("rmi", image) + + // build with multiple secrets from files + image = "multiple-secret-test" + session = podmanTest.PodmanExitCleanly("build", "-f", "build/Containerfile.with-multiple-secret", "-t", image, "--secret", "id=mysecret,src=build/secret.txt", "--secret", "id=mysecret2,src=build/anothersecret.txt", "build/") Expect(session.OutputToString()).To(ContainSubstring("somesecret")) Expect(session.OutputToString()).To(ContainSubstring("anothersecret")) - session = podmanTest.Podman([]string{"rmi", "multiple-secret-test"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) + session = podmanTest.PodmanExitCleanly("run", "--rm", image, "sh", "-c", "find / -xdev -name 'podman-build-secret*' -print") + Expect(session.OutputToString()).To(BeEmpty(), "podman-build-secret path leaked into image") + + podmanTest.PodmanExitCleanly("rmi", image) }) - It("podman build with a secret from file and verify if secret file is not leaked into image", func() { - session := podmanTest.Podman([]string{"build", "-f", "build/secret-verify-leak/Containerfile.with-secret-verify-leak", "-t", "secret-test-leak", "--secret", "id=mysecret,src=build/secret.txt", "build/secret-verify-leak"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) - Expect(session.OutputToString()).To(ContainSubstring("somesecret")) + It("podman build with a secret from env", func() { + secret := "somesecretvalue" + GinkgoT().Setenv("MYSECRET", secret) + image := "secret-test" - session = podmanTest.Podman([]string{"run", "--rm", "secret-test-leak", "ls"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) - Expect(session.OutputToString()).To(Not(ContainSubstring("podman-build-secret"))) + session := podmanTest.PodmanExitCleanly("build", "-f", "build/Containerfile.with-secret", "-t", image, "--secret", "id=mysecret,env=MYSECRET", "build/") + Expect(session.OutputToString()).To(ContainSubstring(secret)) - session = podmanTest.Podman([]string{"rmi", "secret-test-leak"}) - session.WaitWithDefaultTimeout() - Expect(session).Should(ExitCleanly()) + session = podmanTest.PodmanExitCleanly("run", "--rm", image, "sh", "-c", "find / -xdev -name 'podman-build-secret*' -print") + Expect(session.OutputToString()).To(BeEmpty(), "podman-build-secret path leaked into image") + + podmanTest.PodmanExitCleanly("rmi", image) + + // Test for: https://github.com/containers/podman/issues/28334 - env secrets + COPY must not add host-shaped podman-build-secret paths to the image. + image = "e2e-remote-secret-copy" + + session = podmanTest.PodmanExitCleanly("build", "-f", "build/remote-secret-copy/Dockerfile", "-t", image, "--secret", "id=mysecret,env=MYSECRET", "build/remote-secret-copy") + Expect(session.OutputToString()).To(ContainSubstring(secret)) + + session = podmanTest.PodmanExitCleanly("run", "--rm", image, "sh", "-c", "find / -xdev -name 'podman-build-secret*' -print") + Expect(session.OutputToString()).To(BeEmpty(), "podman-build-secret path leaked into image") + + podmanTest.PodmanExitCleanly("rmi", image) }) It("podman build with not found Containerfile or Dockerfile", func() { diff --git a/test/e2e/quadlet/entrypoint-empty.container b/test/e2e/quadlet/entrypoint-empty.container new file mode 100644 index 00000000000..743a091e500 --- /dev/null +++ b/test/e2e/quadlet/entrypoint-empty.container @@ -0,0 +1,6 @@ +## assert-podman-final-args localhost/imagename +## assert-podman-args "--entrypoint=" + +[Container] +Image=localhost/imagename +Entrypoint= diff --git a/test/e2e/quadlet/entrypoint.container b/test/e2e/quadlet/entrypoint.container index d72b1cd4c53..a1d747d7549 100644 --- a/test/e2e/quadlet/entrypoint.container +++ b/test/e2e/quadlet/entrypoint.container @@ -1,5 +1,5 @@ ## assert-podman-final-args localhost/imagename -## assert-podman-args "--entrypoint" "top -b" +## assert-podman-args "--entrypoint=top -b" [Container] Image=localhost/imagename diff --git a/test/e2e/quadlet/health-cmd-shell-quotes.container b/test/e2e/quadlet/health-cmd-shell-quotes.container new file mode 100644 index 00000000000..4cbd4dfd4e1 --- /dev/null +++ b/test/e2e/quadlet/health-cmd-shell-quotes.container @@ -0,0 +1,4 @@ +[Container] +Image=localhost/imagename +## assert-podman-args "--health-cmd" "/bin/sh -c \"echo 'hello world!'\"" +HealthCmd=/bin/sh -c "echo 'hello world!'" diff --git a/test/e2e/quadlet_test.go b/test/e2e/quadlet_test.go index a7df96b8730..c027a175ff8 100644 --- a/test/e2e/quadlet_test.go +++ b/test/e2e/quadlet_test.go @@ -912,10 +912,12 @@ BOGUS=foo Entry("env-host.container", "env-host.container"), Entry("env.container", "env.container"), Entry("entrypoint.container", "entrypoint.container"), + Entry("entrypoint-empty.container", "entrypoint-empty.container"), Entry("escapes.container", "escapes.container"), Entry("exec.container", "exec.container"), Entry("group-add.container", "group-add.container"), Entry("health.container", "health.container"), + Entry("health-cmd-shell-quotes.container", "health-cmd-shell-quotes.container"), Entry("host.container", "host.container"), Entry("httpproxy-false.container", "httpproxy-false.container"), Entry("httpproxy-true.container", "httpproxy-true.container"), diff --git a/test/e2e/secret_test.go b/test/e2e/secret_test.go index cbf02faf04a..2b8fca97b09 100644 --- a/test/e2e/secret_test.go +++ b/test/e2e/secret_test.go @@ -508,6 +508,25 @@ var _ = Describe("Podman secret", func() { Expect(exists).Should(ExitWithError(1, "")) }) + It("podman secret create with non-default driver does not inherit default driver opts", func() { + secretFilePath := filepath.Join(podmanTest.TempDir, "secret") + err := os.WriteFile(secretFilePath, []byte("mysecret"), 0o755) + Expect(err).ToNot(HaveOccurred()) + + // Create a secret with driver=shell but no --driver-opts. + // Before the fix, the file driver's default "path" opt bled + // into the shell driver, causing "invalid shell driver option". + // After the fix, the shell driver correctly receives no + // foreign opts and fails only because its required commands + // (store, lookup, list, delete) are not configured. + session := podmanTest.Podman([]string{ + "secret", "create", "-d", "shell", + "shell-secret", secretFilePath, + }) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError(125, "missing config value")) + }) + It("podman secret create from stdin", func() { secretData := "mysecretdata" secretName := "stdin-secret-" + stringid.GenerateRandomID() diff --git a/test/system/195-run-namespaces.bats b/test/system/195-run-namespaces.bats index 230669d15be..fadcbb60dd8 100644 --- a/test/system/195-run-namespaces.bats +++ b/test/system/195-run-namespaces.bats @@ -41,7 +41,7 @@ uts | uts if [[ "$option" = "pid" ]] && is_rootless && ! is_remote && [[ "$(podman_runtime)" = "runc" ]]; then # Replace "pid:[1234567]" with "pid:\[1234567\]" con1_ns_esc="${con1_ns//[\[\]]/\\&}" - assert "$con2_ns" =~ "${con1_ns_esc}.*warning .*" "($name) namespace matches (type: $type)" + assert "$con2_ns" =~ "${con1_ns_esc}" "($name) namespace matches (type: $type)" else assert "$con1_ns" == "$con2_ns" "($name) namespace matches (type: $type)" fi diff --git a/test/upgrade/README.md b/test/upgrade/README.md index bd70a5f0be2..c03667f8117 100644 --- a/test/upgrade/README.md +++ b/test/upgrade/README.md @@ -21,22 +21,19 @@ container image from quay.io/podman, uses it to create and run a number of containers, then uses new-podman to interact with those containers. -As of 2024-02-05 the available old-podman versions are: +Testing updates from versions earlier than v5.3.1 fails. Testing updates from +tags that do not respect semantic versioning fails too (e.g. v5.6.0-immutable or +v5.6). As of 2025-11-18 the available old-podman versions to test against are: ```console -$ bin/podman search --list-tags --limit=400 quay.io/podman/stable | awk '$2 ~ /^v/ { print $2}' | sort | column -c 75 -v1.4.2 v1.9.1 v3.2.0 v3.4.0 v4.1.0 v4.3.1 v4.5.1 v4.8 -v1.4.4 v2.0.2 v3.2.1 v3.4.1 v4.1.1 v4.4 v4.6 v4.8.0 -v1.5.0 v2.0.6 v3.2.2 v3.4.2 v4.2 v4.4.1 v4.6.1 v4.8.1 -v1.5.1 v2.1.1 v3.2.3 v3.4.4 v4.2.0 v4.4.2 v4.6.2 v4.8.2 -v1.6 v2.2.1 v3.3.0 v3.4.7 v4.2.1 v4.4.4 v4.7 v4.8.3 -v1.6.2 v3 v3.3.1 v4 v4.3 v4.5 v4.7.0 v4.9 -v1.9.0 v3.1.2 v3.4 v4.1 v4.3.0 v4.5.0 v4.7.2 v4.9.0 +$ bin/podman search --list-tags --limit=400 quay.io/podman/stable | awk '$2 ~ /^v[0-9]+\.[0-9]+\.[0-9]+$/ { print $2}' | sort | awk '/v5.3.1/,0' | column -c 75 +v5.3.1 v5.4.0 v5.4.2 v5.5.1 v5.6.0 v5.6.2 +v5.3.2 v5.4.1 v5.5.0 v5.5.2 v5.6.1 ``` Test invocation is: ```console -$ sudo env PODMAN=bin/podman PODMAN_UPGRADE_FROM=v4.1.0 PODMAN_UPGRADE_TEST_DEBUG= bats test/upgrade +$ sudo env PODMAN=bin/podman PODMAN_UPGRADE_FROM=v5.3.1 PODMAN_UPGRADE_TEST_DEBUG= bats test/upgrade ``` (Path assumes you're cd'ed to top-level podman repo). `PODMAN_UPGRADE_FROM` can be any of the versions above. `PODMAN_UPGRADE_TEST_DEBUG` is empty diff --git a/test/upgrade/test-upgrade.bats b/test/upgrade/test-upgrade.bats index 4760aac2165..df10f02619c 100644 --- a/test/upgrade/test-upgrade.bats +++ b/test/upgrade/test-upgrade.bats @@ -49,9 +49,9 @@ setup() { # the default c/storage behavior is to make the mount propagation private. export _PODMAN_TEST_OPTS="--storage-opt=skip_mount_home=true --cgroup-manager=cgroupfs --root=$PODMAN_UPGRADE_WORKDIR/root --runroot=$PODMAN_UPGRADE_WORKDIR/runroot --tmpdir=$PODMAN_UPGRADE_WORKDIR/tmp" - # Old netavark used iptables but newer versions might uses nftables. - # Networking can only work correctly if both use the same firewall driver so force iptables. - printf "[network]\nfirewall_driver=\"iptables\"\n" > $PODMAN_UPGRADE_WORKDIR/containers.conf + + # Starting with v6.0.0 we only test upgrade from versions that support nftables. + printf "[network]\nfirewall_driver=\"nftables\"\n" > $PODMAN_UPGRADE_WORKDIR/containers.conf export CONTAINERS_CONF_OVERRIDE=$PODMAN_UPGRADE_WORKDIR/containers.conf } @@ -64,21 +64,6 @@ setup() { OLD_PODMAN=quay.io/podman/stable:$PODMAN_UPGRADE_FROM $PODMAN pull $OLD_PODMAN - # Can't mix-and-match iptables. - # This can only fail when we bring in new CI VMs. If/when it does fail, - # we'll need to figure out how to solve it. Until then, punt. - iptables_old_version=$($PODMAN run --rm $OLD_PODMAN iptables -V) - run -0 expr "$iptables_old_version" : ".*(\(.*\))" - iptables_old_which="$output" - - iptables_new_version=$(iptables -V) - run -0 expr "$iptables_new_version" : ".*(\(.*\))" - iptables_new_which="$output" - - if [[ "$iptables_new_which" != "$iptables_old_which" ]]; then - die "Cannot mix iptables; $PODMAN_UPGRADE_FROM container uses $iptables_old_which, host uses $iptables_new_which" - fi - # Shortcut name, because we're referencing it a lot pmroot=$PODMAN_UPGRADE_WORKDIR @@ -227,8 +212,10 @@ EOF @test "images" { run_podman images -a --format '{{.Names}}' - assert "${lines[0]}" =~ "\[localhost/podman-pause:${PODMAN_UPGRADE_FROM##v}-.*\]" "podman images, line 0" - assert "${lines[1]}" = "[$IMAGE]" "podman images, line 1" + # Filter out the podman-pause image which isn't present for + # versions >= 5.5.0 + run -0 grep -v "localhost/podman-pause" <<< "$output" + assert "${lines[0]}" = "[$IMAGE]" "podman images, line 0" } @test "ps : one container running" {