diff --git a/runsc/cmd/metric_server.go b/runsc/cmd/metric_server.go index 3fa05adf2..938e2ee5d 100644 --- a/runsc/cmd/metric_server.go +++ b/runsc/cmd/metric_server.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "io" + "io/ioutil" "math/rand" "net" "net/http" @@ -743,6 +744,11 @@ func (m *MetricServer) Execute(ctx context.Context, f *flag.FlagSet, args ...any if _, err := container.ListSandboxes(conf.RootDir); err != nil { return util.Errorf("Invalid root directory %q: tried to list sandboxes within it and got: %v", conf.RootDir, err) } + // container.ListSandboxes uses a glob pattern, which doesn't error out on + // permission errors. Double-check by actually listing the directory. + if _, err := ioutil.ReadDir(conf.RootDir); err != nil { + return util.Errorf("Invalid root directory %q: tried to list all entries within it and got: %v", conf.RootDir, err) + } m.startTime = time.Now() m.rand = rand.New(rand.NewSource(m.startTime.UnixNano())) m.rootDir = conf.RootDir diff --git a/runsc/container/metric_server_test.go b/runsc/container/metric_server_test.go index aeafaf1af..d1d3fe839 100644 --- a/runsc/container/metric_server_test.go +++ b/runsc/container/metric_server_test.go @@ -17,6 +17,7 @@ package container import ( "context" "fmt" + "io/ioutil" "os" "path/filepath" "testing" @@ -434,3 +435,28 @@ func TestContainerMetricsMultiple(t *testing.T) { t.Errorf("Unexpectedly found testmetric_fs_opens metric data for no-metrics container %s: %v", noMetricsCont.ID, val) } } + +func TestMetricServerChecksRootDirectoryAccess(t *testing.T) { + te, cleanup := setupMetrics(t) + defer cleanup() + if err := te.client.ShutdownServer(te.testCtx); err != nil { + t.Fatalf("Cannot stop metric server: %v", err) + } + prevStat, err := os.Lstat(te.sleepConf.RootDir) + if err != nil { + t.Fatalf("cannot stat %q: %v", te.sleepConf.RootDir, err) + } + if err := os.Chmod(te.sleepConf.RootDir, 0); err != nil { + t.Fatalf("cannot chmod %q as 000: %v", te.sleepConf.RootDir, err) + } + defer os.Chmod(te.sleepConf.RootDir, prevStat.Mode()) + if _, err := ioutil.ReadDir(te.sleepConf.RootDir); err == nil { + t.Logf("Can still read directory %v despite chmodding it to 0. Maybe we are running as root? Skipping test.", te.sleepConf.RootDir) + return + } + shorterCtx, shorterCtxCancel := context.WithTimeout(te.testCtx, time.Second) + defer shorterCtxCancel() + if err := te.client.SpawnServer(shorterCtx, te.sleepConf); err == nil { + t.Error("Metric server was successfully able to be spawned despite not having access to the root directory") + } +}