pathpolicy: tweak error messages and add test

pkg/blueprint/filesystem_customizations.go

@@ -2,6 +2,7 @@ package blueprint
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 
 	"github.com/osbuild/images/internal/common"
@@ -73,16 +74,15 @@ func (fsc *FilesystemCustomization) UnmarshalJSON(data []byte) error {
 
 // CheckMountpointsPolicy checks if the mountpoints are allowed by the policy
 func CheckMountpointsPolicy(mountpoints []FilesystemCustomization, mountpointAllowList *pathpolicy.PathPolicies) error {
-	invalidMountpoints := []string{}
+	var errs []error
 	for _, m := range mountpoints {
-		err := mountpointAllowList.Check(m.Mountpoint)
-		if err != nil {
-			invalidMountpoints = append(invalidMountpoints, m.Mountpoint)
+		if err := mountpointAllowList.Check(m.Mountpoint); err != nil {
+			errs = append(errs, err)
 		}
 	}
 
-	if len(invalidMountpoints) > 0 {
-		return fmt.Errorf("The following custom mountpoints are not supported %+q", invalidMountpoints)
+	if len(errs) > 0 {
+		return fmt.Errorf("The following errors occurred while setting up custom mountpoints:\n%w", errors.Join(errs...))
 	}
 
 	return nil

pkg/blueprint/filesystem_customizations_test.go

@@ -0,0 +1,26 @@
+package blueprint
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+
+	"github.com/osbuild/images/pkg/pathpolicy"
+)
+
+func TestCheckMountpointsPolicy(t *testing.T) {
+	policy := pathpolicy.NewPathPolicies(map[string]pathpolicy.PathPolicy{
+		"/": {Exact: true},
+	})
+
+	mps := []FilesystemCustomization{
+		{Mountpoint: "/foo"},
+		{Mountpoint: "/boot/"},
+	}
+
+	expectedErr := `The following errors occurred while setting up custom mountpoints:
+path "/foo" is not allowed
+path "/boot/" must be canonical`
+	err := CheckMountpointsPolicy(mps, policy)
+	assert.EqualError(t, err, expectedErr)
+}

pkg/pathpolicy/path_policy.go

@@ -26,15 +26,14 @@ func NewPathPolicies(entries map[string]PathPolicy) *PathPolicies {
 
 // Check a given path against the PathPolicies
 func (pol *PathPolicies) Check(fsPath string) error {
-
 	// Quickly check we have a path and it is absolute
 	if fsPath == "" || fsPath[0] != '/' {
-		return fmt.Errorf("path must be absolute")
+		return fmt.Errorf("path %q must be absolute", fsPath)
 	}
 
 	// ensure that only clean paths are valid
 	if fsPath != path.Clean(fsPath) {
-		return fmt.Errorf("path must be canonical")
+		return fmt.Errorf("path %q must be canonical", fsPath)
 	}
 
 	node, left := pol.Lookup(fsPath)
@@ -46,7 +45,7 @@ func (pol *PathPolicies) Check(fsPath string) error {
 	// 1) path is explicitly not allowed or
 	// 2) a subpath was match but an explicit match is required
 	if policy.Deny || (policy.Exact && len(left) > 0) {
-		return fmt.Errorf("path '%s ' is not allowed", fsPath)
+		return fmt.Errorf("path %q is not allowed", fsPath)
 	}
 
 	// exact match or recursive path allowed

pkg/pathpolicy/path_policy_test.go

@@ -47,3 +47,13 @@ func TestPathPolicyCheck(t *testing.T) {
 		}
 	}
 }
+
+func TestPathPolicyErrors(t *testing.T) {
+	policy := NewPathPolicies(map[string]PathPolicy{
+		"/": {Exact: true},
+	})
+
+	assert.EqualError(t, policy.Check("/foo"), `path "/foo" is not allowed`)
+	assert.EqualError(t, policy.Check("./"), `path "./" must be absolute`)
+	assert.EqualError(t, policy.Check("/boot/"), `path "/boot/" must be canonical`)
+}