Require absolute path for local API

Signed-off-by: Jan Rodák <[email protected]>

Author: Honny1

internal/localapi/types.go

@@ -1,7 +1,11 @@
 package localapi
 
+import "errors"
+
 // LocalAPIMap is a map of local paths to their target paths in the VM
 type LocalAPIMap struct {
 	ClientPath string `json:"ClientPath,omitempty"`
 	RemotePath string `json:"RemotePath,omitempty"`
 }
+
+var ErrPathNotAbsolute = errors.New("path is not absolute")

internal/localapi/utils.go

@@ -259,7 +259,7 @@ func IsHyperVProvider(ctx context.Context) (bool, error) {
 // It returns an error if the path is not absolute or does not exist on the filesystem.
 func ValidatePathForLocalAPI(path string) error {
 	if !filepath.IsAbs(path) {
-		return fmt.Errorf("path %q is not absolute", path)
+		return fmt.Errorf("%w: %q", ErrPathNotAbsolute, path)
 	}
 
 	if err := fileutils.Exists(path); err != nil {

pkg/api/handlers/libpod/artifacts.go

@@ -270,6 +270,9 @@ func AddLocalArtifact(w http.ResponseWriter, r *http.Request) {
 	switch err := localapi.ValidatePathForLocalAPI(cleanPath); {
 	case err == nil:
 		// no error -> continue
+	case errors.Is(err, localapi.ErrPathNotAbsolute):
+		utils.Error(w, http.StatusBadRequest, err)
+		return
 	case errors.Is(err, fs.ErrNotExist):
 		utils.Error(w, http.StatusNotFound, fmt.Errorf("file does not exist: %q", cleanPath))
 		return

pkg/api/handlers/libpod/images.go

@@ -16,6 +16,7 @@ import (
 	"strings"
 
 	"github.com/containers/buildah"
+	"github.com/containers/podman/v6/internal/localapi"
 	"github.com/containers/podman/v6/libpod"
 	"github.com/containers/podman/v6/libpod/define"
 	"github.com/containers/podman/v6/pkg/api/handlers"
@@ -41,7 +42,6 @@ import (
 	"go.podman.io/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"
 )
 
@@ -396,10 +396,13 @@ func ImagesLocalLoad(w http.ResponseWriter, r *http.Request) {
 
 	cleanPath := filepath.Clean(query.Path)
 	// Check if the path exists on server side.
-	// Note: fileutils.Exists returns nil if the file exists, not an error.
-	switch err := fileutils.Exists(cleanPath); {
+	// Note: localapi.ValidatePathForLocalAPI returns nil if the file exists and path is absolute, not an error.
+	switch err := localapi.ValidatePathForLocalAPI(cleanPath); {
 	case err == nil:
 		// no error -> continue
+	case errors.Is(err, localapi.ErrPathNotAbsolute):
+		utils.Error(w, http.StatusBadRequest, err)
+		return
 	case errors.Is(err, fs.ErrNotExist):
 		utils.Error(w, http.StatusNotFound, fmt.Errorf("file does not exist: %q", cleanPath))
 		return

pkg/api/server/register_images.go

@@ -952,7 +952,7 @@ func (s *APIServer) registerImagesHandlers(r *mux.Router) error {
 	//     name: path
 	//     type: string
 	//     required: true
-	//     description: Path to the image archive file on the server filesystem
+	//     description: Absolute path to the image archive file on the server filesystem
 	// produces:
 	// - application/json
 	// responses:

test/apiv2/10-images.at

@@ -502,7 +502,7 @@ t GET libpod/images/quay.io/libpod/busybox:latest/exists  204
 
 # Test with directory instead of file
 mkdir -p ${TMPD}/testdir
-t POST libpod/local/images/load?path="${TMPD}/testdir" 500
+t POST libpod/local/images/load?path="${TMPD}/testdir" 400
 
 cleanLoad
 
@@ -512,6 +512,6 @@ t POST libpod/local/images/load?invalid=arg 400
 
 t POST libpod/local/images/load?path="" 400
 
-t POST libpod/local/images/load?path="../../../etc/passwd" 404
+t POST libpod/local/images/load?path="../../../etc/passwd" 400
 
 # vim: filetype=sh

test/apiv2/python/rest_api/test_v2_0_0_artifact.py

@@ -390,12 +390,12 @@ def test_add_local_with_not_absolute_path(self):
         rjson = r.json()
 
         # Assert correct response code
-        self.assertEqual(r.status_code, 500, r.text)
+        self.assertEqual(r.status_code, 400, r.text)
 
         # Assert return error response is json and contains correct message
         self.assertEqual(
             rjson["cause"],
-            'path "../../etc/passwd" is not absolute',
+            'path is not absolute',
         )
 
     def test_inspect(self):