From ee43ba7e78f388020c3ecf3594265764c4a988e6 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Fri, 26 Apr 2024 09:32:15 +0000 Subject: [PATCH 1/3] fix: avoid deleting root filesystem when KANIKO_DIR not set --- README.md | 5 +++++ envbuilder.go | 20 ++++++++++++++++-- integration/integration_test.go | 36 +++++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index d33f20a6..afa13fe8 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,11 @@ $ vim .devcontainer/Dockerfile Exit the container, and re-run the `docker run` command... after the build completes, `htop` should exist in the container! 🥳 +> **Note:** Envbuilder performs destructive filesystem operations! To guard against accidental data +> loss, it will refuse to run if it detects that KANIKO_DIR is not set to a specific value. +> If you need to bypass this behaviour for any reason, you can bypass this safety check by setting +> `FORCE_SAFE=true`. + ### Git Branch Selection Choose a branch using `GIT_URL` with a _ref/heads_ reference. For instance: diff --git a/envbuilder.go b/envbuilder.go index acc666f8..9ef1666e 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -427,8 +427,7 @@ func Run(ctx context.Context, options Options) error { // It's possible that the container will already have files in it, and // we don't want to merge a new container with the old one. - err = util.DeleteFilesystem() - if err != nil { + if err := maybeDeleteFilesystem(options.ForceSafe); err != nil { return nil, fmt.Errorf("delete filesystem: %w", err) } @@ -1063,3 +1062,20 @@ func findDevcontainerJSON(options Options) (string, string, error) { return "", "", errors.New("can't find devcontainer.json, is it a correct spec?") } + +// maybeDeleteFilesystem wraps util.DeleteFilesystem with a guard to hopefully stop +// folks from unwittingly deleting their entire root directory. +func maybeDeleteFilesystem(force bool) error { + kanikoDir, ok := os.LookupEnv("KANIKO_DIR") + if !ok || strings.TrimSpace(kanikoDir) != MagicDir { + if force { + _, _ = fmt.Fprintf(os.Stderr, "WARNING! BYPASSING SAFETY CHECK! THIS WILL DELETE %s!\n", kanikoDir) + } else { + _, _ = fmt.Fprintf(os.Stderr, "KANIKO_DIR is not set to %s. Bailing!\n", MagicDir) + _, _ = fmt.Fprintln(os.Stderr, "To bypass this check, set FORCE_SAFE=true.") + return errors.New("safety check failed") + } + } + + return util.DeleteFilesystem() +} diff --git a/integration/integration_test.go b/integration/integration_test.go index 869e4e67..7e41d600 100644 --- a/integration/integration_test.go +++ b/integration/integration_test.go @@ -44,6 +44,42 @@ const ( testImageUbuntu = "localhost:5000/envbuilder-test-ubuntu:latest" ) +func TestForceSafe(t *testing.T) { + t.Parallel() + + t.Run("Safe", func(t *testing.T) { + t.Parallel() + srv := createGitServer(t, gitServerOptions{ + files: map[string]string{ + "Dockerfile": "FROM " + testImageAlpine, + }, + }) + _, err := runEnvbuilder(t, options{env: []string{ + "GIT_URL=" + srv.URL, + "KANIKO_DIR=/not/envbuilder", + "DOCKERFILE_PATH=Dockerfile", + }}) + require.ErrorContains(t, err, "delete filesystem: safety check failed") + }) + + // Careful with this one! + t.Run("Unsafe", func(t *testing.T) { + t.Parallel() + srv := createGitServer(t, gitServerOptions{ + files: map[string]string{ + "Dockerfile": "FROM " + testImageAlpine, + }, + }) + _, err := runEnvbuilder(t, options{env: []string{ + "GIT_URL=" + srv.URL, + "KANIKO_DIR=/not/envbuilder", + "FORCE_SAFE=true", + "DOCKERFILE_PATH=Dockerfile", + }}) + require.NoError(t, err) + }) +} + func TestFailsGitAuth(t *testing.T) { t.Parallel() srv := createGitServer(t, gitServerOptions{ From 554da86dc681880a1ebdbc1a275b030c66af39bf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 29 Apr 2024 15:43:13 +0100 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Mathias Fredriksson --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index afa13fe8..964f7887 100644 --- a/README.md +++ b/README.md @@ -47,9 +47,10 @@ $ vim .devcontainer/Dockerfile Exit the container, and re-run the `docker run` command... after the build completes, `htop` should exist in the container! 🥳 -> **Note:** Envbuilder performs destructive filesystem operations! To guard against accidental data +> [!NOTE] +> Envbuilder performs destructive filesystem operations! To guard against accidental data > loss, it will refuse to run if it detects that KANIKO_DIR is not set to a specific value. -> If you need to bypass this behaviour for any reason, you can bypass this safety check by setting +> If you need to bypass this behavior for any reason, you can bypass this safety check by setting > `FORCE_SAFE=true`. ### Git Branch Selection From 5cd77549258ee8fb5f5e37f0c2076e43fbf4c396 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 29 Apr 2024 15:39:10 +0000 Subject: [PATCH 3/3] add countdown --- envbuilder.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/envbuilder.go b/envbuilder.go index 9ef1666e..92ffc84f 100644 --- a/envbuilder.go +++ b/envbuilder.go @@ -1069,7 +1069,14 @@ func maybeDeleteFilesystem(force bool) error { kanikoDir, ok := os.LookupEnv("KANIKO_DIR") if !ok || strings.TrimSpace(kanikoDir) != MagicDir { if force { - _, _ = fmt.Fprintf(os.Stderr, "WARNING! BYPASSING SAFETY CHECK! THIS WILL DELETE %s!\n", kanikoDir) + bailoutSecs := 10 + _, _ = fmt.Fprintln(os.Stderr, "WARNING! BYPASSING SAFETY CHECK! THIS WILL DELETE YOUR ROOT FILESYSTEM!") + _, _ = fmt.Fprintf(os.Stderr, "You have %d seconds to bail out", bailoutSecs) + for i := 0; i < bailoutSecs; i++ { + _, _ = fmt.Fprintf(os.Stderr, ".") + <-time.After(time.Second) + } + _, _ = fmt.Fprintf(os.Stderr, "\n") } else { _, _ = fmt.Fprintf(os.Stderr, "KANIKO_DIR is not set to %s. Bailing!\n", MagicDir) _, _ = fmt.Fprintln(os.Stderr, "To bypass this check, set FORCE_SAFE=true.")