Skip to content

Commit af44ee9

Browse files
committed
mounts: prohibit relative paths in YAML
Relative paths are still allowed in CLI: `limactl create --mount=DIR` Fix issue 3948 Signed-off-by: Akihiro Suda <akihiro.suda.cz@hco.ntt.co.jp>
1 parent e963c32 commit af44ee9

File tree

5 files changed

+75
-3
lines changed

5 files changed

+75
-3
lines changed

cmd/limactl/editflags/editflags.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import (
1616
"github.com/spf13/cobra"
1717
flag "github.com/spf13/pflag"
1818

19+
"github.com/lima-vm/lima/v2/pkg/localpathutil"
1920
"github.com/lima-vm/lima/v2/pkg/registry"
2021
)
2122

@@ -174,6 +175,10 @@ func buildMountListExpression(ss []string) (string, error) {
174175
for i, s := range ss {
175176
writable := strings.HasSuffix(s, ":w")
176177
loc := strings.TrimSuffix(s, ":w")
178+
loc, err := localpathutil.Expand(loc)
179+
if err != nil {
180+
return "", err
181+
}
177182
expr += fmt.Sprintf(`{"location": %q, "writable": %v}`, loc, writable)
178183
if i < len(ss)-1 {
179184
expr += ","

cmd/limactl/editflags/editflags_test.go

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,13 @@
44
package editflags
55

66
import (
7+
"strings"
78
"testing"
89

910
"github.com/spf13/cobra"
1011
"gotest.tools/v3/assert"
12+
13+
"github.com/lima-vm/lima/v2/pkg/localpathutil"
1114
)
1215

1316
func TestCompleteCPUs(t *testing.T) {
@@ -160,6 +163,13 @@ func TestParsePortForward(t *testing.T) {
160163
}
161164

162165
func TestYQExpressions(t *testing.T) {
166+
expand := func(s string) string {
167+
s, err := localpathutil.Expand(s)
168+
assert.NilError(t, err)
169+
// `D:\foo` -> `D:\\foo` (appears in YAML)
170+
s = strings.ReplaceAll(s, "\\", "\\\\")
171+
return s
172+
}
163173
tests := []struct {
164174
name string
165175
args []string
@@ -169,15 +179,15 @@ func TestYQExpressions(t *testing.T) {
169179
}{
170180
{
171181
name: "mount",
172-
args: []string{"--mount", "/foo", "--mount", "/bar:w"},
182+
args: []string{"--mount", "/foo", "--mount", "./bar:w"},
173183
newInstance: false,
174-
expected: []string{`.mounts += [{"location": "/foo", "writable": false},{"location": "/bar", "writable": true}] | .mounts |= unique_by(.location)`},
184+
expected: []string{`.mounts += [{"location": "` + expand("/foo") + `", "writable": false},{"location": "` + expand("./bar") + `", "writable": true}] | .mounts |= unique_by(.location)`},
175185
},
176186
{
177187
name: "mount-only",
178188
args: []string{"--mount-only", "/foo", "--mount-only", "/bar:w"},
179189
newInstance: false,
180-
expected: []string{`.mounts = [{"location": "/foo", "writable": false},{"location": "/bar", "writable": true}]`},
190+
expected: []string{`.mounts = [{"location": "` + expand("/foo") + `", "writable": false},{"location": "` + expand("/bar") + `", "writable": true}]`},
181191
},
182192
{
183193
name: "mixture of mount and mount-only",

pkg/limayaml/defaults.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -713,6 +713,15 @@ func FillDefault(ctx context.Context, y, d, o *limatype.LimaYAML, filePath strin
713713
mounts[i].NineP.Cache = ptr.Of(Default9pCacheForRO)
714714
}
715715
}
716+
// The location must be an absolute path or a path that begins with `~`.
717+
// This condition should have been validated before reaching here in Load().
718+
// FIXME: support Windows
719+
if runtime.GOOS != "windows" && !filepath.IsAbs(mount.Location) && !localpathutil.IsTildePath(mount.Location) {
720+
// NOTREACHED
721+
err := fmt.Errorf("mount location %q must be an absolute path or a path that begins with `~`", mount.Location)
722+
panic(err)
723+
}
724+
// Expand a path that begins with `~`. Relative paths are prohibited (see above, issue #3948).
716725
if location, err := localpathutil.Expand(mount.Location); err == nil {
717726
mounts[i].Location = location
718727
} else {

pkg/limayaml/load.go

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,14 @@ import (
99
"fmt"
1010
"os"
1111
"path/filepath"
12+
"runtime"
1213

1314
"github.com/sirupsen/logrus"
1415

1516
"github.com/lima-vm/lima/v2/pkg/limatype"
1617
"github.com/lima-vm/lima/v2/pkg/limatype/dirnames"
1718
"github.com/lima-vm/lima/v2/pkg/limatype/filenames"
19+
"github.com/lima-vm/lima/v2/pkg/localpathutil"
1820
)
1921

2022
// Load loads the yaml and fulfills unspecified fields with the default values.
@@ -69,6 +71,15 @@ func load(ctx context.Context, b []byte, filePath string, warn bool) (*limatype.
6971
return nil, err
7072
}
7173

74+
for _, mount := range y.Mounts {
75+
// The location must be an absolute path or a path that begins with `~`.
76+
// This has to be validated before calling FillDefault(), which may expand `~`.
77+
// FIXME: support Windows
78+
if runtime.GOOS != "windows" && !filepath.IsAbs(mount.Location) && !localpathutil.IsTildePath(mount.Location) {
79+
return nil, fmt.Errorf("mount location %q must be an absolute path or a path that begins with `~`", mount.Location)
80+
}
81+
}
82+
7283
FillDefault(ctx, &y, &d, &o, filePath, warn)
7384

7485
return &y, nil

pkg/limayaml/load_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package limayaml
55

66
import (
7+
"runtime"
78
"testing"
89

910
"gotest.tools/v3/assert"
@@ -68,3 +69,39 @@ additionalDisks:
6869
assert.Equal(t, y.AdditionalDisks[0].FSArgs[0], "-i")
6970
assert.Equal(t, y.AdditionalDisks[0].FSArgs[1], "size=512")
7071
}
72+
73+
func TestLoadMounts(t *testing.T) {
74+
tests := []struct {
75+
name string
76+
mounts string
77+
wantErr string
78+
}{
79+
{
80+
name: "Valid",
81+
mounts: `mounts: [{location: "/foo", writable: false}, {location: "~/foo", writable: true}]`,
82+
wantErr: "",
83+
},
84+
{
85+
name: "Invalid (relative)",
86+
mounts: `mounts: [{location: ".", writable: false}]`,
87+
wantErr: func() string {
88+
if runtime.GOOS == "windows" {
89+
// FIXME: support Windows paths
90+
return ""
91+
}
92+
return "must be an absolute path"
93+
}(),
94+
},
95+
}
96+
97+
for _, tt := range tests {
98+
t.Run(tt.name, func(t *testing.T) {
99+
_, err := Load(t.Context(), []byte(tt.mounts), "lima.yaml")
100+
if tt.wantErr != "" {
101+
assert.ErrorContains(t, err, tt.wantErr)
102+
} else {
103+
assert.NilError(t, err)
104+
}
105+
})
106+
}
107+
}

0 commit comments

Comments
 (0)