Repository: tetratelabs/wazero
Pull request #999 (2023-01-03 03:39:26)

Ensures writes aren't allowed when opening for read
This masks interfaces returned by `fs.File` so that we don't accidentally allow writes when opened for reading. This also adds `syscall.NewReadFS` which can enforce read-only access in general, such as would be ideal for tests that try to read files from the host root filesystem (e.g. /etc/passwd).

Note: this is intentionally not exposed as `experimental` yet. We should wait until user request, or the end of the month before deciding which surviving apis to expose.


Patch:
diff --git a/internal/syscallfs/dirfs.go b/internal/syscallfs/dirfs.go
index ca6e1358..1ed4f7b1 100644
--- a/internal/syscallfs/dirfs.go
+++ b/internal/syscallfs/dirfs.go
@@ -32,7 +32,16 @@ func (dir dirFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, err
 	if !fs.ValidPath(name) {
 		return nil, &fs.PathError{Op: "open", Path: name, Err: fs.ErrInvalid}
 	}
-	return os.OpenFile(path.Join(string(dir), name), flag, perm)
+
+	f, err := os.OpenFile(path.Join(string(dir), name), flag, perm)
+	if err != nil {
+		return nil, err
+	}
+
+	if flag == 0 || flag == os.O_RDONLY {
+		return maskForReads(f), nil
+	}
+	return f, nil
 }
 
 // Mkdir implements FS.Mkdir
diff --git a/internal/syscallfs/readfs.go b/internal/syscallfs/readfs.go
new file mode 100644
index 00000000..5d3a599c
--- /dev/null
+++ b/internal/syscallfs/readfs.go
@@ -0,0 +1,55 @@
+package syscallfs
+
+import (
+	"fmt"
+	"io/fs"
+	"os"
+	"syscall"
+)
+
+func NewReadFS(fs FS) FS {
+	if _, ok := fs.(*readFS); ok {
+		return fs
+	}
+	return &readFS{fs}
+}
+
+type readFS struct{ fs FS }
+
+// Open implements the same method as documented on fs.FS
+func (ro *readFS) Open(name string) (fs.File, error) {
+	panic(fmt.Errorf("unexpected to call fs.FS.Open(%s)", name))
+}
+
+// OpenFile implements FS.OpenFile
+func (ro *readFS) OpenFile(name string, flag int, perm fs.FileMode) (fs.File, error) {
+	if flag == 0 || flag == os.O_RDONLY {
+		return ro.fs.OpenFile(name, flag, perm)
+	}
+	return nil, syscall.ENOSYS
+}
+
+// Mkdir implements FS.Mkdir
+func (ro *readFS) Mkdir(name string, perm fs.FileMode) error {
+	return syscall.ENOSYS
+}
+
+// Rename implements FS.Rename
+func (ro *readFS) Rename(from, to string) error {
+	return syscall.ENOSYS
+}
+
+// Rmdir implements FS.Rmdir
+func (ro *readFS) Rmdir(name string) error {
+	return syscall.ENOSYS
+}
+
+// Unlink implements FS.Unlink
+func (ro *readFS) Unlink(name string) error {
+	return syscall.ENOSYS
+}
+
+// Utimes implements FS.Utimes
+func (ro *readFS) Utimes(name string, atimeNsec, mtimeNsec int64) error {
+	return ro.fs.Utimes(name, atimeNsec, mtimeNsec)
+}
diff --git a/internal/syscallfs/syscallfs.go b/internal/syscallfs/syscallfs.go
index 05aaacb8..7acbf0d0 100644
--- a/internal/syscallfs/syscallfs.go
+++ b/internal/syscallfs/syscallfs.go
@@ -1,6 +1,7 @@
 package syscallfs
 
 import (
+	"io"
 	"io/fs"
 )
 
@@ -85,3 +86,58 @@ type FS interface {
 	//     runtime.GOOS=js support changing it. Hence, ctime it is absent here.
 	Utimes(path string, atimeNsec, mtimeNsec int64) error
 }
+
+// maskForReads masks the file with read-only interfaces used by wazero.
+//
+// Note: This technique was adapted from similar code in zipkin-go.
+func maskForReads(f fs.File) fs.File {
+	// The below are the types wazero casts into.
+	// Note: os.File implements this even for normal files.
+	d, i0 := f.(fs.ReadDirFile)
+	ra, i1 := f.(io.ReaderAt)
+	s, i2 := f.(io.Seeker)
+
+	// Wrap any combination of the types above.
+	switch {
+	case !i0 && !i1 && !i2: // 0, 0, 0
+		return struct{ fs.File }{f}
+	case !i0 && !i1 && i2: // 0, 0, 1
+		return struct {
+			fs.File
+			io.Seeker
+		}{f, s}
+	case !i0 && i1 && !i2: // 0, 1, 0
+		return struct {
+			fs.File
+			io.ReaderAt
+		}{f, ra}
+	case !i0 && i1 && i2: // 0, 1, 1
+		return struct {
+			fs.File
+			io.ReaderAt
+			io.Seeker
+		}{f, ra, s}
+	case i0 && !i1 && !i2: // 1, 0, 0
+		return struct {
+			fs.ReadDirFile
+		}{d}
+	case i0 && !i1 && i2: // 1, 0, 1
+		return struct {
+			fs.ReadDirFile
+			io.Seeker
+		}{d, s}
+	case i0 && i1 && !i2: // 1, 1, 0
+		return struct {
+			fs.ReadDirFile
+			io.ReaderAt
+		}{d, ra}
+	case i0 && i1 && i2: // 1, 1, 1
+		return struct {
+			fs.ReadDirFile
+			io.ReaderAt
+			io.Seeker
+		}{d, ra, s}
+	default:
+		panic("BUG: unhandled pattern")
+	}
+}


Test patch:
diff --git a/imports/wasi_snapshot_preview1/fs_test.go b/imports/wasi_snapshot_preview1/fs_test.go
index 03c21667..2433e72f 100644
--- a/imports/wasi_snapshot_preview1/fs_test.go
+++ b/imports/wasi_snapshot_preview1/fs_test.go
@@ -2243,35 +2243,29 @@ func Test_pathLink(t *testing.T) {
 }
 
 func Test_pathOpen(t *testing.T) {
-	osDir := t.TempDir()      // open before loop to ensure no locking problems.
-	writefsDir := t.TempDir() // open before loop to ensure no locking problems.
-	os := os.DirFS(osDir)
-	writeFS, err := syscallfs.NewDirFS(writefsDir)
+	dir := t.TempDir() // open before loop to ensure no locking problems.
+	writeFS, err := syscallfs.NewDirFS(dir)
 	require.NoError(t, err)
+	readFS := syscallfs.NewReadFS(writeFS)
 
 	fileName := "file"
 	fileContents := []byte("012")
-	writeFile(t, osDir, fileName, fileContents)
-	writeFile(t, writefsDir, fileName, fileContents)
+	writeFile(t, dir, fileName, fileContents)
 
 	appendName := "append"
 	appendContents := []byte("345")
-	writeFile(t, osDir, appendName, appendContents)
-	writeFile(t, writefsDir, appendName, appendContents)
+	writeFile(t, dir, appendName, appendContents)
 
 	truncName := "trunc"
 	truncContents := []byte("678")
-	writeFile(t, osDir, truncName, truncContents)
-	writeFile(t, writefsDir, truncName, truncContents)
+	writeFile(t, dir, truncName, truncContents)
 
 	dirName := "dir"
-	mkdir(t, osDir, dirName)
-	mkdir(t, writefsDir, dirName)
+	mkdir(t, dir, dirName)
 
 	dirFileName := path.Join(dirName, fileName)
 	dirFileContents := []byte("def")
-	writeFile(t, osDir, dirFileName, dirFileContents)
-	writeFile(t, writefsDir, dirFileName, dirFileContents)
+	writeFile(t, dir, dirFileName, dirFileContents)
 
 	expectedOpenedFd := sys.FdRoot + 1
 
@@ -2286,8 +2280,8 @@ func Test_pathOpen(t *testing.T) {
 		expectedLog   string
 	}{
 		{
-			name: "os.DirFS",
-			fs:   os,
+			name: "syscallfs.ReadFS",
+			fs:   readFS,
 			path: func(*testing.T) string { return fileName },
 			expected: func(t *testing.T, fsc *sys.FSContext) {
 				requireContents(t, fsc, expectedOpenedFd, fileName, fileContents)
@@ -2310,8 +2304,8 @@ func Test_pathOpen(t *testing.T) {
 `,
 		},
 		{
-			name:          "os.DirFS FD_APPEND",
-			fs:            os,
+			name:          "syscallfs.ReadFS FD_APPEND",
+			fs:            readFS,
 			fdflags:       FD_APPEND,
 			path:          func(t *testing.T) (file string) { return appendName },
 			expectedErrno: ErrnoNosys,
@@ -2332,7 +2326,7 @@ func Test_pathOpen(t *testing.T) {
 				require.True(t, fsc.CloseFile(expectedOpenedFd))
 
 				// verify the contents were appended
-				b := readFile(t, writefsDir, appendName)
+				b := readFile(t, dir, appendName)
 				require.Equal(t, append(appendContents, contents...), b)
 			},
 			expectedLog: `
@@ -2341,8 +2335,8 @@ func Test_pathOpen(t *testing.T) {
 `,
 		},
 		{
-			name:          "os.DirFS O_CREAT",
-			fs:            os,
+			name:          "syscallfs.ReadFS O_CREAT",
+			fs:            readFS,
 			oflags:        O_CREAT,
 			expectedErrno: ErrnoNosys,
 			path:          func(*testing.T) string { return "creat" },
@@ -2364,7 +2358,7 @@ func Test_pathOpen(t *testing.T) {
 				require.True(t, fsc.CloseFile(expectedOpenedFd))
 
 				// verify the contents were written
-				b := readFile(t, writefsDir, "creat")
+				b := readFile(t, dir, "creat")
 				require.Equal(t, contents, b)
 			},
 			expectedLog: `
@@ -2373,8 +2367,8 @@ func Test_pathOpen(t *testing.T) {
 `,
 		},
 		{
-			name:          "os.DirFS O_CREAT O_TRUNC",
-			fs:            os,
+			name:          "syscallfs.ReadFS O_CREAT O_TRUNC",
+			fs:            readFS,
 			oflags:        O_CREAT | O_TRUNC,
 			expectedErrno: ErrnoNosys,
 			path:          func(t *testing.T) (file string) { return path.Join(dirName, "O_CREAT-O_TRUNC") },
@@ -2396,7 +2390,7 @@ func Test_pathOpen(t *testing.T) {
 				require.True(t, fsc.CloseFile(expectedOpenedFd))
 
 				// verify the contents were written
-				b := readFile(t, writefsDir, path.Join(dirName, "O_CREAT-O_TRUNC"))
+				b := readFile(t, dir, path.Join(dirName, "O_CREAT-O_TRUNC"))
 				require.Equal(t, contents, b)
 			},
 			expectedLog: `
@@ -2405,8 +2399,8 @@ func Test_pathOpen(t *testing.T) {
 `,
 		},
 		{
-			name:   "os.DirFS O_DIRECTORY",
-			fs:     os,
+			name:   "syscallfs.ReadFS O_DIRECTORY",
+			fs:     readFS,
 			oflags: O_DIRECTORY,
 			path:   func(*testing.T) string { return dirName },
 			expected: func(t *testing.T, fsc *sys.FSContext) {
@@ -2435,8 +2429,8 @@ func Test_pathOpen(t *testing.T) {
 `,
 		},
 		{
-			name:          "os.DirFS O_TRUNC",
-			fs:            os,
+			name:          "syscallfs.ReadFS O_TRUNC",
+			fs:            readFS,
 			oflags:        O_TRUNC,
 			expectedErrno: ErrnoNosys,
 			path:          func(*testing.T) string { return "trunc" },
@@ -2457,7 +2451,7 @@ func Test_pathOpen(t *testing.T) {
 				require.True(t, fsc.CloseFile(expectedOpenedFd))
 
 				// verify the contents were truncated
-				b := readFile(t, writefsDir, "trunc")
+				b := readFile(t, dir, "trunc")
 				require.Equal(t, contents, b)
 			},
 			expectedLog: `
diff --git a/internal/syscallfs/dirfs_test.go b/internal/syscallfs/dirfs_test.go
index e002136e..4c09e590 100644
--- a/internal/syscallfs/dirfs_test.go
+++ b/internal/syscallfs/dirfs_test.go
@@ -8,9 +8,7 @@ import (
 	"runtime"
 	"syscall"
 	"testing"
-	"time"
 
-	"github.com/tetratelabs/wazero/internal/platform"
 	"github.com/tetratelabs/wazero/internal/testing/require"
 )
 
@@ -319,78 +317,13 @@ func TestDirFS_Utimes(t *testing.T) {
 
 	testFS := dirFS(tmpDir)
 
-	file := "file"
-	err := os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
-	require.NoError(t, err)
-
-	dir := "dir"
-	err = os.Mkdir(path.Join(tmpDir, dir), 0o700)
-	require.NoError(t, err)
-
-	t.Run("doesn't exist", func(t *testing.T) {
-		err := testFS.Utimes("nope",
-			time.Unix(123, 4*1e3).UnixNano(),
-			time.Unix(567, 8*1e3).UnixNano())
-		require.Equal(t, syscall.ENOENT, err)
-	})
+	testFS_Utimes(t, tmpDir, testFS)
+}
 
-	type test struct {
-		name                 string
-		path                 string
-		atimeNsec, mtimeNsec int64
-	}
-
-	// Note: This sets microsecond granularity because Windows doesn't support
-	// nanosecond.
-	tests := []test{
-		{
-			name:      "file positive",
-			path:      file,
-			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
-			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
-		},
-		{
-			name:      "dir positive",
-			path:      dir,
-			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
-			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
-		},
-		{name: "file zero", path: file},
-		{name: "dir zero", path: dir},
-	}
-
-	// linux and freebsd report inaccurate results when the input ts is negative.
-	if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
-		tests = append(tests,
-			test{
-				name:      "file negative",
-				path:      file,
-				atimeNsec: time.Unix(-123, -4*1e3).UnixNano(),
-				mtimeNsec: time.Unix(-567, -8*1e3).UnixNano(),
-			},
-			test{
-				name:      "dir negative",
-				path:      dir,
-				atimeNsec: time.Unix(-123, -4*1e3).UnixNano(),
-				mtimeNsec: time.Unix(-567, -8*1e3).UnixNano(),
-			},
-		)
-	}
-
-	for _, tt := range tests {
-		tc := tt
-		t.Run(tc.name, func(t *testing.T) {
-			err := testFS.Utimes(tc.path, tc.atimeNsec, tc.mtimeNsec)
-			require.NoError(t, err)
+func TestDirFS_Open_Read(t *testing.T) {
+	tmpDir := t.TempDir()
 
-			stat, err := os.Stat(path.Join(tmpDir, tc.path))
-			require.NoError(t, err)
+	testFS := dirFS(tmpDir)
 
-			atimeNsec, mtimeNsec, _ := platform.StatTimes(stat)
-			if platform.CompilerSupported() {
-				require.Equal(t, atimeNsec, tc.atimeNsec)
-			} // else only mtimes will return.
-			require.Equal(t, mtimeNsec, tc.mtimeNsec)
-		})
-	}
+	testFS_Open_Read(t, tmpDir, testFS)
 }
diff --git a/internal/syscallfs/readfs_test.go b/internal/syscallfs/readfs_test.go
new file mode 100644
index 00000000..e08d4b9f
--- /dev/null
+++ b/internal/syscallfs/readfs_test.go
@@ -0,0 +1,82 @@
+package syscallfs
+
+import (
+	"io/fs"
+	"os"
+	"path"
+	"syscall"
+	"testing"
+
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func TestReadFS_MkDir(t *testing.T) {
+	dir := t.TempDir()
+
+	testFS := NewReadFS(dirFS(dir))
+
+	err := testFS.Mkdir("mkdir", fs.ModeDir)
+	require.Equal(t, syscall.ENOSYS, err)
+}
+
+func TestReadFS_Rename(t *testing.T) {
+	tmpDir := t.TempDir()
+	testFS := NewReadFS(dirFS(tmpDir))
+
+	file1 := "file1"
+	file1Path := path.Join(tmpDir, file1)
+	file1Contents := []byte{1}
+	err := os.WriteFile(file1Path, file1Contents, 0o600)
+	require.NoError(t, err)
+
+	file2 := "file2"
+	file2Path := path.Join(tmpDir, file2)
+	file2Contents := []byte{2}
+	err = os.WriteFile(file2Path, file2Contents, 0o600)
+	require.NoError(t, err)
+
+	err = testFS.Rename(file1, file2)
+	require.Equal(t, syscall.ENOSYS, err)
+}
+
+func TestReadFS_Rmdir(t *testing.T) {
+	dir := t.TempDir()
+
+	testFS := NewReadFS(dirFS(dir))
+
+	name := "rmdir"
+	realPath := path.Join(dir, name)
+	require.NoError(t, os.Mkdir(realPath, 0o700))
+
+	err := testFS.Rmdir(name)
+	require.Equal(t, syscall.ENOSYS, err)
+}
+
+func TestReadFS_Unlink(t *testing.T) {
+	dir := t.TempDir()
+
+	testFS := NewReadFS(dirFS(dir))
+
+	name := "unlink"
+	realPath := path.Join(dir, name)
+	require.NoError(t, os.WriteFile(realPath, []byte{}, 0o600))
+
+	err := testFS.Unlink(name)
+	require.Equal(t, syscall.ENOSYS, err)
+}
+
+func TestReadFS_Utimes(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	testFS := NewReadFS(dirFS(tmpDir))
+
+	testFS_Utimes(t, tmpDir, testFS)
+}
+
+func TestReadFS_Open_Read(t *testing.T) {
+	tmpDir := t.TempDir()
+
+	testFS := NewReadFS(dirFS(tmpDir))
+
+	testFS_Open_Read(t, tmpDir, testFS)
+}
diff --git a/internal/syscallfs/syscallfs_test.go b/internal/syscallfs/syscallfs_test.go
new file mode 100644
index 00000000..eb473c52
--- /dev/null
+++ b/internal/syscallfs/syscallfs_test.go
@@ -0,0 +1,165 @@
+package syscallfs
+
+import (
+	"errors"
+	"io"
+	"io/fs"
+	"os"
+	"path"
+	"runtime"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/tetratelabs/wazero/internal/platform"
+	"github.com/tetratelabs/wazero/internal/testing/require"
+)
+
+func testFS_Open_Read(t *testing.T, tmpDir string, testFS FS) {
+	file := "file"
+	fileContents := []byte{1, 2, 3, 4}
+	err := os.WriteFile(path.Join(tmpDir, file), fileContents, 0o700)
+	require.NoError(t, err)
+
+	dir := "dir"
+	dirRealPath := path.Join(tmpDir, dir)
+	err = os.Mkdir(dirRealPath, 0o700)
+	require.NoError(t, err)
+
+	file1 := "file1"
+	fileInDir := path.Join(dirRealPath, file1)
+	require.NoError(t, os.WriteFile(fileInDir, []byte{2}, 0o600))
+
+	t.Run("doesn't exist", func(t *testing.T) {
+		_, err := testFS.OpenFile("nope", os.O_RDONLY, 0)
+
+		// We currently follow os.Open not syscall.Open, so the error is wrapped.
+		require.Equal(t, syscall.ENOENT, errors.Unwrap(err))
+	})
+
+	t.Run("dir exists", func(t *testing.T) {
+		f, err := testFS.OpenFile(dir, os.O_RDONLY, 0)
+		require.NoError(t, err)
+		defer f.Close()
+
+		// Ensure it implements fs.ReadDirFile
+		d, ok := f.(fs.ReadDirFile)
+		require.True(t, ok)
+		e, err := d.ReadDir(-1)
+		require.NoError(t, err)
+		require.Equal(t, 1, len(e))
+		require.False(t, e[0].IsDir())
+		require.Equal(t, file1, e[0].Name())
+
+		// Ensure it doesn't implement io.Writer
+		_, ok = f.(io.Writer)
+		require.False(t, ok)
+	})
+
+	t.Run("file exists", func(t *testing.T) {
+		f, err := testFS.OpenFile(file, os.O_RDONLY, 0)
+		require.NoError(t, err)
+		defer f.Close()
+
+		// Ensure it implements io.ReaderAt
+		r, ok := f.(io.ReaderAt)
+		require.True(t, ok)
+		lenToRead := len(fileContents) - 1
+		buf := make([]byte, lenToRead)
+		n, err := r.ReadAt(buf, 1)
+		require.NoError(t, err)
+		require.Equal(t, lenToRead, n)
+		require.Equal(t, fileContents[1:], buf)
+
+		// Ensure it implements io.Seeker
+		s, ok := f.(io.Seeker)
+		require.True(t, ok)
+		offset, err := s.Seek(1, io.SeekStart)
+		require.NoError(t, err)
+		require.Equal(t, int64(1), offset)
+		b, err := io.ReadAll(f)
+		require.NoError(t, err)
+		require.Equal(t, fileContents[1:], b)
+
+		// Ensure it doesn't implement io.Writer
+		_, ok = f.(io.Writer)
+		require.False(t, ok)
+	})
+}
+
+func testFS_Utimes(t *testing.T, tmpDir string, testFS FS) {
+	file := "file"
+	err := os.WriteFile(path.Join(tmpDir, file), []byte{}, 0o700)
+	require.NoError(t, err)
+
+	dir := "dir"
+	err = os.Mkdir(path.Join(tmpDir, dir), 0o700)
+	require.NoError(t, err)
+
+	t.Run("doesn't exist", func(t *testing.T) {
+		err := testFS.Utimes("nope",
+			time.Unix(123, 4*1e3).UnixNano(),
+			time.Unix(567, 8*1e3).UnixNano())
+		require.Equal(t, syscall.ENOENT, err)
+	})
+
+	type test struct {
+		name                 string
+		path                 string
+		atimeNsec, mtimeNsec int64
+	}
+
+	// Note: This sets microsecond granularity because Windows doesn't support
+	// nanosecond.
+	tests := []test{
+		{
+			name:      "file positive",
+			path:      file,
+			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
+			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
+		},
+		{
+			name:      "dir positive",
+			path:      dir,
+			atimeNsec: time.Unix(123, 4*1e3).UnixNano(),
+			mtimeNsec: time.Unix(567, 8*1e3).UnixNano(),
+		},
+		{name: "file zero", path: file},
+		{name: "dir zero", path: dir},
+	}
+
+	// linux and freebsd report inaccurate results when the input ts is negative.
+	if runtime.GOOS == "windows" || runtime.GOOS == "darwin" {
+		tests = append(tests,
+			test{
+				name:      "file negative",
+				path:      file,
+				atimeNsec: time.Unix(-123, -4*1e3).UnixNano(),
+				mtimeNsec: time.Unix(-567, -8*1e3).UnixNano(),
+			},
+			test{
+				name:      "dir negative",
+				path:      dir,
+				atimeNsec: time.Unix(-123, -4*1e3).UnixNano(),
+				mtimeNsec: time.Unix(-567, -8*1e3).UnixNano(),
+			},
+		)
+	}
+
+	for _, tt := range tests {
+		tc := tt
+		t.Run(tc.name, func(t *testing.T) {
+			err := testFS.Utimes(tc.path, tc.atimeNsec, tc.mtimeNsec)
+			require.NoError(t, err)
+
+			stat, err := os.Stat(path.Join(tmpDir, tc.path))
+			require.NoError(t, err)
+
+			atimeNsec, mtimeNsec, _ := platform.StatTimes(stat)
+			if platform.CompilerSupported() {
+				require.Equal(t, atimeNsec, tc.atimeNsec)
+			} // else only mtimes will return.
+			require.Equal(t, mtimeNsec, tc.mtimeNsec)
+		})
+	}
+}