From 7f37fbd6af8c5f19447b21c94fa4314a7bbb70f9 Mon Sep 17 00:00:00 2001 From: umut-polat <52835619+umut-polat@users.noreply.github.com> Date: Thu, 12 Mar 2026 18:33:02 +0000 Subject: [PATCH 01/20] quadlet: allow empty Entrypoint to clear image default Setting Entrypoint= (empty value) in a quadlet .container file should produce --entrypoint "" in the generated podman command, which clears the image's default entrypoint. Previously this was silently ignored because lookupAndAddString skips empty values. Move the Entrypoint key out of the generic stringKeys map and handle it separately so that an empty value is passed through. Closes #28213 Signed-off-by: umut-polat <52835619+umut-polat@users.noreply.github.com> (cherry picked from commit 926f562bd59394f9852e343a6bb8041611aa864a) Signed-off-by: Paul Holzinger --- pkg/systemd/quadlet/quadlet.go | 8 +++++++- test/e2e/quadlet/entrypoint-empty.container | 6 ++++++ test/e2e/quadlet/entrypoint.container | 2 +- test/e2e/quadlet_test.go | 1 + 4 files changed, 15 insertions(+), 2 deletions(-) create mode 100644 test/e2e/quadlet/entrypoint-empty.container 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/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_test.go b/test/e2e/quadlet_test.go index a7df96b8730..57c76d779bf 100644 --- a/test/e2e/quadlet_test.go +++ b/test/e2e/quadlet_test.go @@ -912,6 +912,7 @@ 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"), From 28a39dd1ba0edc3c4b538145bbb48451ae48ee44 Mon Sep 17 00:00:00 2001 From: Doug Rabson Date: Wed, 18 Mar 2026 12:41:41 +0000 Subject: [PATCH 02/20] libpod: Don't dereference ctrSpec.Linux if it is nil This prevents a nil pointer crash when running network=host containers on a FreeBSD host using podman-remote. Fixes: #28289 Signed-off-by: Doug Rabson (cherry picked from commit 9a10cefd18a3f19e49aa6ae5683d1bcea37bdc97) Signed-off-by: Paul Holzinger --- libpod/container.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) 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 { From 759df25a888a28d008142dd71af10d1af545a712 Mon Sep 17 00:00:00 2001 From: Tim Zhou Date: Mon, 10 Nov 2025 12:36:50 -0500 Subject: [PATCH 03/20] new image sfx for debian 14 Signed-off-by: Tim Zhou (cherry picked from commit 98c398b6660eb294d0725a6eaabd7da9f924bfa2) Signed-off-by: Paul Holzinger --- .cirrus.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 6ab49135cb7..20f4cc6cbb0 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -28,10 +28,10 @@ env: FEDORA_AARCH64_NAME: "${FEDORA_NAME}-aarch64" PRIOR_FEDORA_NAME: "fedora-41" RAWHIDE_NAME: "rawhide" - DEBIAN_NAME: "debian-13" + DEBIAN_NAME: "debian-14" # Image identifiers - IMAGE_SUFFIX: "c20250910t092246z-f42f41d13" + IMAGE_SUFFIX: "c20251110t154831z-f42f41d14" # EC2 images FEDORA_AMI: "fedora-aws-${IMAGE_SUFFIX}" From 1a9ae9dcba19013ae07e5b323e2f7641280251f2 Mon Sep 17 00:00:00 2001 From: Tim Zhou Date: Fri, 21 Nov 2025 12:47:47 -0500 Subject: [PATCH 04/20] update fedoral base image to 43 and related tests Signed-off-by: Tim Zhou (cherry picked from commit 5d79adbd6a8f1612c51e81ca68ed9b5d32ff7e19) Signed-off-by: Paul Holzinger --- .cirrus.yml | 6 +++--- contrib/cirrus/setup_environment.sh | 2 ++ test/apiv2/python/rest_api/test_v2_0_0_image.py | 7 +++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 20f4cc6cbb0..62cd032ceef 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-14" # Image identifiers - IMAGE_SUFFIX: "c20251110t154831z-f42f41d14" + IMAGE_SUFFIX: "c20251211t152018z-f43f42d14" # EC2 images FEDORA_AMI: "fedora-aws-${IMAGE_SUFFIX}" diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh index 2b19eeaccc7..292a5cd922a 100755 --- a/contrib/cirrus/setup_environment.sh +++ b/contrib/cirrus/setup_environment.sh @@ -73,6 +73,8 @@ echo -e "\n# Begin single-use VM global variables (${BASH_SOURCE[0]})" \ done <<<"$(passthrough_envars)" ) >> "/etc/ci_environment" +# Unset NOTIFY_SOCKET based on: https://github.com/containers/podman/pull/27514#issuecomment-3529125596 +unset NOTIFY_SOCKET # This is a possible manual maintenance gaff, i.e. forgetting to update a # *_NAME variable in .cirrus.yml. check to be sure at least one comparison # matches the actual OS being run. Ignore details, such as debian point-release 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") From d49a9208bdf8a695aa8ac62b6f56b11816218a78 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Fri, 12 Dec 2025 14:09:18 +0100 Subject: [PATCH 05/20] cirrus: ensure NOTIFY_SOCKET is properly unset for all tests Podman uses this env toi do various things, we don't want the external env to affect our tests here. Signed-off-by: Paul Holzinger (cherry picked from commit 7e3caaed84fe1427b942f893975035145a71be15) Signed-off-by: Paul Holzinger --- contrib/cirrus/setup_environment.sh | 2 -- 1 file changed, 2 deletions(-) diff --git a/contrib/cirrus/setup_environment.sh b/contrib/cirrus/setup_environment.sh index 292a5cd922a..2b19eeaccc7 100755 --- a/contrib/cirrus/setup_environment.sh +++ b/contrib/cirrus/setup_environment.sh @@ -73,8 +73,6 @@ echo -e "\n# Begin single-use VM global variables (${BASH_SOURCE[0]})" \ done <<<"$(passthrough_envars)" ) >> "/etc/ci_environment" -# Unset NOTIFY_SOCKET based on: https://github.com/containers/podman/pull/27514#issuecomment-3529125596 -unset NOTIFY_SOCKET # This is a possible manual maintenance gaff, i.e. forgetting to update a # *_NAME variable in .cirrus.yml. check to be sure at least one comparison # matches the actual OS being run. Ignore details, such as debian point-release From 26047f43b562031f91079a1af1995b5918b0902b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 19 Mar 2026 21:16:31 +0100 Subject: [PATCH 06/20] New images 2026-03-19 From https://github.com/containers/automation_images/pull/438 Signed-off-by: Paul Holzinger (cherry picked from commit f2bc5b86de1bc2d1427210659f8bc04c728a2fe5) Signed-off-by: Paul Holzinger --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index 62cd032ceef..1c6e0e22bcc 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -31,7 +31,7 @@ env: DEBIAN_NAME: "debian-14" # Image identifiers - IMAGE_SUFFIX: "c20251211t152018z-f43f42d14" + IMAGE_SUFFIX: "c20260319t182308z-f43f42d14" # EC2 images FEDORA_AMI: "fedora-aws-${IMAGE_SUFFIX}" From c91cd9929129d922e216fcded1c4290b1c1e54da Mon Sep 17 00:00:00 2001 From: Ricardo Branco Date: Sat, 21 Mar 2026 21:43:37 +0100 Subject: [PATCH 07/20] test: relax rootless runc pid namespace assertion runc may or may not issue a warning here. https://github.com/opencontainers/runc/issues/4732 Signed-off-by: Ricardo Branco (cherry picked from commit b77531879b8e16590176f3868a18d76533445487) Signed-off-by: Paul Holzinger --- test/system/195-run-namespaces.bats | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 From e5fe3fdf69bdc8a8693533af05e9a4a5d0361965 Mon Sep 17 00:00:00 2001 From: crawfordxx Date: Tue, 24 Mar 2026 12:12:19 +0800 Subject: [PATCH 08/20] api: fix missing return after error in SystemCheck handler In the SystemCheck HTTP handler, when parsing the unreferenced_layer_max_age query parameter fails, the error response is sent but execution continues to `unreferencedLayerMaximumAge = &duration` where `duration` is the zero value. This causes the system check to run with a zero duration instead of returning the 400 error to the client. Add the missing `return` after the error response. Fixes #28350 Signed-off-by: crawfordxx (cherry picked from commit 29d80a26afc89fa51fb5d173a62c70bb094653c7) Signed-off-by: Paul Holzinger --- pkg/api/handlers/libpod/system.go | 1 + 1 file changed, 1 insertion(+) 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 } From 15a2a7d6059ff11bb27784fd41c6f3b28d5add0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Rod=C3=A1k?= Date: Mon, 23 Mar 2026 17:42:01 +0100 Subject: [PATCH 09/20] Remote build: `nTar` secrets with relative paths and ignore bypass MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `podman-remote` tars the context, extra `podman-build-secret*` paths were either dropped by `.dockerignore` (#25314) or archived as absolute paths so `COPY . .` pulled host-shaped trees into the image (#28334). Use relative names under the primary context for extra sources and do not apply `.dockerignore` to those forced entries. Fixes: https://github.com/containers/podman/issues/25314 Fixes: https://github.com/containers/podman/issues/28334 Signed-off-by: Jan Rodák (cherry picked from commit e18bb9ea8bd66f3c23f24b6bf552e4e8c9372e51) Signed-off-by: Paul Holzinger --- pkg/bindings/images/build.go | 18 +++++++++++++++++- test/e2e/build/remote-secret-copy/Dockerfile | 5 +++++ test/e2e/build/remote-secret-copy/hello | 1 + .../.dockerignore | 1 + .../remote-secret-dockerignore-star/Dockerfile | 5 +++++ .../host-secret.txt | 1 + .../ignored-by-dockerignore.txt | 1 + test/e2e/build_test.go | 16 ++++++++++++++++ 8 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 test/e2e/build/remote-secret-copy/Dockerfile create mode 100644 test/e2e/build/remote-secret-copy/hello create mode 100644 test/e2e/build/remote-secret-dockerignore-star/.dockerignore create mode 100644 test/e2e/build/remote-secret-dockerignore-star/Dockerfile create mode 100644 test/e2e/build/remote-secret-dockerignore-star/host-secret.txt create mode 100644 test/e2e/build/remote-secret-dockerignore-star/ignored-by-dockerignore.txt 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/test/e2e/build/remote-secret-copy/Dockerfile b/test/e2e/build/remote-secret-copy/Dockerfile new file mode 100644 index 00000000000..125faa2a5ab --- /dev/null +++ b/test/e2e/build/remote-secret-copy/Dockerfile @@ -0,0 +1,5 @@ +FROM alpine +COPY . . +RUN test -e hello +RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret +RUN sh -c 'test -z "$(find / -name '\''podman-build-secret*'\'' 2>/dev/null | head -n1)"' 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..e5c036eaffd --- /dev/null +++ b/test/e2e/build/remote-secret-dockerignore-star/Dockerfile @@ -0,0 +1,5 @@ +FROM alpine +COPY . . +RUN test ! -e ignored-by-dockerignore.txt +RUN --mount=type=secret,id=MY_SECRET cat /run/secrets/MY_SECRET +RUN sh -c 'test -z "$(find / -name '\''podman-build-secret*'\'' 2>/dev/null | head -n1)"' 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..05e174b4f22 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -136,6 +136,22 @@ var _ = Describe("Podman build", func() { Expect(session).Should(ExitCleanly()) }) + It("podman remote build file secret with dockerignore that ignores all files", func() { + // Test for: https://github.com/containers/podman/issues/25314 + session := podmanTest.PodmanExitCleanly("build", "-f", "build/remote-secret-dockerignore-star/Dockerfile", "--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")) + }) + + It("podman remote build env secret with COPY does not leak podman-build-secret temp files", func() { + // Test for: https://github.com/containers/podman/issues/28334 + secret := "somesecretvalue" + os.Setenv("MYSECRET", secret) + defer os.Unsetenv("MYSECRET") + + session := podmanTest.PodmanExitCleanly("build", "-f", "build/remote-secret-copy/Dockerfile", "--secret", "id=mysecret,env=MYSECRET", "build/remote-secret-copy") + Expect(session.OutputToString()).To(ContainSubstring(secret)) + }) + It("podman build with not found Containerfile or Dockerfile", func() { targetPath := filepath.Join(podmanTest.TempDir, "notfound") err = os.Mkdir(targetPath, 0o755) From 51b5c593106a86ed715f1025af9c635ef1b45af6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Rod=C3=A1k?= Date: Wed, 25 Mar 2026 15:11:55 +0100 Subject: [PATCH 10/20] Consolidate build secret tests and assert no podman-build-secret leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Jan Rodák (cherry picked from commit 765ffb9a2e04712d4a75fc6d482f17b899f500af) Signed-off-by: Paul Holzinger --- test/e2e/build/remote-secret-copy/Dockerfile | 1 - .../Dockerfile | 1 - .../Containerfile.with-secret-verify-leak | 3 - test/e2e/build_test.go | 85 +++++++++---------- 4 files changed, 40 insertions(+), 50 deletions(-) delete mode 100644 test/e2e/build/secret-verify-leak/Containerfile.with-secret-verify-leak diff --git a/test/e2e/build/remote-secret-copy/Dockerfile b/test/e2e/build/remote-secret-copy/Dockerfile index 125faa2a5ab..816b48cdd3e 100644 --- a/test/e2e/build/remote-secret-copy/Dockerfile +++ b/test/e2e/build/remote-secret-copy/Dockerfile @@ -2,4 +2,3 @@ FROM alpine COPY . . RUN test -e hello RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret -RUN sh -c 'test -z "$(find / -name '\''podman-build-secret*'\'' 2>/dev/null | head -n1)"' diff --git a/test/e2e/build/remote-secret-dockerignore-star/Dockerfile b/test/e2e/build/remote-secret-dockerignore-star/Dockerfile index e5c036eaffd..ccf49a15841 100644 --- a/test/e2e/build/remote-secret-dockerignore-star/Dockerfile +++ b/test/e2e/build/remote-secret-dockerignore-star/Dockerfile @@ -2,4 +2,3 @@ FROM alpine COPY . . RUN test ! -e ignored-by-dockerignore.txt RUN --mount=type=secret,id=MY_SECRET cat /run/secrets/MY_SECRET -RUN sh -c 'test -z "$(find / -name '\''podman-build-secret*'\'' 2>/dev/null | head -n1)"' diff --git a/test/e2e/build/secret-verify-leak/Containerfile.with-secret-verify-leak b/test/e2e/build/secret-verify-leak/Containerfile.with-secret-verify-leak deleted file mode 100644 index 0957ac6a60d..00000000000 --- a/test/e2e/build/secret-verify-leak/Containerfile.with-secret-verify-leak +++ /dev/null @@ -1,3 +0,0 @@ -FROM alpine -COPY * / -RUN --mount=type=secret,id=mysecret cat /run/secrets/mysecret diff --git a/test/e2e/build_test.go b/test/e2e/build_test.go index 05e174b4f22..cb617bb9bfb 100644 --- a/test/e2e/build_test.go +++ b/test/e2e/build_test.go @@ -89,67 +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") - It("podman remote build file secret with dockerignore that ignores all files", func() { - // Test for: https://github.com/containers/podman/issues/25314 - session := podmanTest.PodmanExitCleanly("build", "-f", "build/remote-secret-dockerignore-star/Dockerfile", "--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")) - }) + podmanTest.PodmanExitCleanly("rmi", image) - It("podman remote build env secret with COPY does not leak podman-build-secret temp files", func() { - // Test for: https://github.com/containers/podman/issues/28334 - secret := "somesecretvalue" - os.Setenv("MYSECRET", secret) - defer os.Unsetenv("MYSECRET") + // 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", "--secret", "id=mysecret,env=MYSECRET", "build/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() { From 7250b06e253f43a770da1fe3d99c1220df15b695 Mon Sep 17 00:00:00 2001 From: crawfordxx Date: Sun, 29 Mar 2026 12:03:59 +0800 Subject: [PATCH 11/20] libpod: fix data race on deferredErr in attachExecHTTP ExecContainerHTTP returns attachChan to its caller before attachExecHTTP finishes. The caller's deferred close(holdConnOpen) can therefore fire while attachExecHTTP is still running (including its own deferred cleanups), which races with the goroutine inside attachExecHTTP that reads deferredErr after <-holdConnOpen unblocks. Fix this by introducing an errCh channel: deferred functions write deferredErr into it, and the goroutine reads from it after <-holdConnOpen unblocks, ensuring the read happens-after all writes to the named return value. Fixes: #28277 Signed-off-by: crawfordxx (cherry picked from commit 497185c14ba888e40e8117f11775e4736216ff3a) Signed-off-by: Paul Holzinger --- libpod/oci_conmon_exec_common.go | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) 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) From e9fe245626e793ed24cd3baa39d6f237991a135c Mon Sep 17 00:00:00 2001 From: Joe Doss Date: Mon, 30 Mar 2026 00:51:38 -0500 Subject: [PATCH 12/20] Fix shell driver DriverOpts cross-contamination in secret creation When creating a secret with driver=shell via the API, the file driver's default DriverOpts (including path) were applied because DriverOpts was empty. The shell driver rejects path as an unknown option, making it impossible to create shell-driver secrets via the REST API or podman-remote. Only apply default DriverOpts from config when the requested driver matches the configured default driver. Signed-off-by: Joe Doss (cherry picked from commit 667f55c2ce37fa9ff39cf3da961b80733bfeec1e) Signed-off-by: Paul Holzinger --- pkg/domain/infra/abi/secrets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 { From 75820ddac571f0c386b72ac95be53dd581b5d83a Mon Sep 17 00:00:00 2001 From: Joe Doss Date: Mon, 30 Mar 2026 08:36:00 -0500 Subject: [PATCH 13/20] Add e2e test for shell driver DriverOpts cross-contamination fix Verify that creating a secret with driver=shell and no --driver-opts does not inherit the file driver's default path option. Before the fix, this produced "invalid shell driver option"; after, it correctly fails with "missing config value" for unconfigured shell commands. Signed-off-by: Joe Doss (cherry picked from commit 8b905613b5a388fe4bd5e98f27a36cf620d454d1) Signed-off-by: Paul Holzinger --- test/e2e/secret_test.go | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) 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() From 9c262736e4bcb426c66a42c1d43798203c558854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Rod=C3=A1k?= Date: Mon, 30 Mar 2026 15:20:39 +0200 Subject: [PATCH 14/20] Fix Quadlet `Lookup()` stripping unmatched quotes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: https://github.com/containers/podman/issues/28409 Signed-off-by: Jan Rodák (cherry picked from commit cf6404f387efaa2bdafc9651c55863e3f9ad8053) Signed-off-by: Paul Holzinger --- pkg/systemd/parser/unitfile.go | 10 +++++-- pkg/systemd/parser/unitfile_test.go | 30 +++++++++++++++++++ .../quadlet/health-cmd-shell-quotes.container | 4 +++ test/e2e/quadlet_test.go | 1 + 4 files changed, 43 insertions(+), 2 deletions(-) create mode 100644 test/e2e/quadlet/health-cmd-shell-quotes.container 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/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 57c76d779bf..c027a175ff8 100644 --- a/test/e2e/quadlet_test.go +++ b/test/e2e/quadlet_test.go @@ -917,6 +917,7 @@ BOGUS=foo 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"), From 0fa30434150f31b86871961d99bac17331217473 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Thu, 2 Apr 2026 15:06:37 +0200 Subject: [PATCH 15/20] add missing O_CLOEXEC to open calls The go std os package to will always make sure to use O_CLOEXEC, however in cases where we directly call unix.Open() we need to pass that flag explicitly. I looked at this as there was a report of a leaked fd on the pasta list, though I am not sure this will address it. But anyway doing this should be rather safe and avoid leaks into other processes. Signed-off-by: Paul Holzinger (cherry picked from commit d20933df027fcaf5c51cbb1be9e5c9d334d73243) Signed-off-by: Paul Holzinger --- cmd/rootlessport/main.go | 2 +- libpod/oci_conmon_attach_linux.go | 2 +- pkg/pidhandle/pidhandle_linux.go | 2 +- pkg/specgen/generate/config_linux.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) 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/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/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) } From 92cd24903fbe8dddea0a59868363cc8d3b6bba3f Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 8 Apr 2026 13:15:44 +0200 Subject: [PATCH 16/20] fix symlink handling in checkpoint restore Under normal circumstances this is not a problem as the archive file created podman container checkpoint will no create symlinks. However if a user passes a custom archive they could contain symlinks that point outside our root. To resolve them within the root use securejoin. Note this is not a security problem because the full archive must be trusted by a user to begin with as it contain the full container config. Fixes: #27977 Signed-off-by: Paul Holzinger (cherry picked from commit abb512062484fd2d81bf158d1488cd0186551571) Signed-off-by: Paul Holzinger --- pkg/checkpoint/crutils/checkpoint_restore_utils.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/checkpoint/crutils/checkpoint_restore_utils.go b/pkg/checkpoint/crutils/checkpoint_restore_utils.go index 1709429bad8..033cdc028f8 100644 --- a/pkg/checkpoint/crutils/checkpoint_restore_utils.go +++ b/pkg/checkpoint/crutils/checkpoint_restore_utils.go @@ -11,6 +11,7 @@ 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" ) @@ -87,7 +88,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) } } From a49ad4be81c5afaff6034a94c07c3f27d345f8f1 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 8 Apr 2026 13:22:33 +0200 Subject: [PATCH 17/20] use chrootarchive over plain archive package Just as additional hardening. Note chrootarchive does not work on macos/windows, in that case it still falls back to the regular pkg/archive. Signed-off-by: Paul Holzinger (cherry picked from commit 25aee24cbd7b885a6e45a5a7b2173559b6da7297) Signed-off-by: Paul Holzinger --- libpod/container_internal_common.go | 22 +++++++++---------- libpod/volume.go | 4 ++-- pkg/api/handlers/compat/images_build.go | 2 +- pkg/api/handlers/libpod/kube.go | 5 ++--- pkg/api/handlers/libpod/quadlets.go | 5 ++--- .../crutils/checkpoint_restore_utils.go | 11 +++++----- pkg/domain/infra/abi/play.go | 4 ++-- pkg/domain/infra/tunnel/images.go | 3 ++- 8 files changed, 28 insertions(+), 28 deletions(-) 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/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/checkpoint/crutils/checkpoint_restore_utils.go b/pkg/checkpoint/crutils/checkpoint_restore_utils.go index 033cdc028f8..652bb0b3357 100644 --- a/pkg/checkpoint/crutils/checkpoint_restore_utils.go +++ b/pkg/checkpoint/crutils/checkpoint_restore_utils.go @@ -14,6 +14,7 @@ import ( 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 @@ -35,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) } @@ -65,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) } @@ -114,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) } @@ -157,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/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) { From add385e31c1116e1301c0fa551c56060cfa2f35b Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Wed, 8 Apr 2026 18:35:45 +0200 Subject: [PATCH 18/20] bindings: artifact extract reject invalid names The server already does validate this so this is not strictly needed and the client must trust the server no matter what. But adding an extra check here does not hurt and may help prevent future bugs. Signed-off-by: Paul Holzinger (cherry picked from commit 12bec19426ea4783b1099d0adad7f75f471ab56b) Signed-off-by: Paul Holzinger --- pkg/bindings/artifacts/extract.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) 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 { From d1cf366b0f06896f16adab5362a1ec55bf9bab64 Mon Sep 17 00:00:00 2001 From: Mario Loriedo Date: Tue, 18 Nov 2025 13:18:41 +0100 Subject: [PATCH 19/20] Remove iptables references in upgrade tests Signed-off-by: Mario Loriedo (cherry picked from commit aa5361ac5307d8e16bf2481a971eaaf5fa5fb176) Signed-off-by: Paul Holzinger --- .cirrus.yml | 8 ++------ test/upgrade/README.md | 17 +++++++---------- test/upgrade/test-upgrade.bats | 27 +++++++-------------------- 3 files changed, 16 insertions(+), 36 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 1c6e0e22bcc..eaaeed69ef7 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -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/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" { From f13de01b6d64c8ad375143712919f5352eff7058 Mon Sep 17 00:00:00 2001 From: Paul Holzinger Date: Mon, 16 Mar 2026 15:38:17 +0100 Subject: [PATCH 20/20] cirrus: bump linux machine aarch64 test timeout Bump the timeout to 60 minutes. We already bumped to 40 minutes a while ago in commit 623cb5f but it seems this is not enough. The x86_64 test needs only 25 minutes so I am confused why aarch64 got so much slower, they used to be around the same time. Of course our tests should not take that long so we really need to figure out what is actually causing this slow down. Looking at logs a test that boots and stops a VM takes 30s on x86_64 while it takes 50s on aarch64. However at the same time the aarch64 test on the macs with libkrun and applehv are quite fast (15-17m) so it seems unlikely to me that it is something inside the aarch64 machine image that causes but rather related to the host side. Signed-off-by: Paul Holzinger (cherry picked from commit 938d677ad99edf9fbd51accc22195cc23014c5cd) Signed-off-by: Paul Holzinger --- .cirrus.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cirrus.yml b/.cirrus.yml index eaaeed69ef7..c7cc5e26d50 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -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: ""