diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index fcb69af8cc..fa38303d72 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -3,6 +3,7 @@ ## Release v0.259.0 ### Notable Changes +* Error when the absolute path to `databricks.yml` contains a glob character. These are: `*`, `?`, `[`, `]` and `^`. If the path to the `databricks.yml` file on your local filesystem contains one of these characters, that could lead to incorrect computation of glob patterns for the `includes` block and might cause resources to be deleted. After this patch users will not be at risk for unexpected deletions due to this issue. ([#3096](https://github.com/databricks/cli/pull/3096)) * Diagnostics messages are no longer buffered to be printed at the end of command, flushed after every mutator ([#3175](https://github.com/databricks/cli/pull/3175)) * Diagnostics are now always rendered with forward slashes in file paths, even on Windows ([#3175](https://github.com/databricks/cli/pull/3175)) * "bundle summary" now prints diagnostics to stderr instead of stdout in text output mode ([#3175](https://github.com/databricks/cli/pull/3175)) diff --git a/acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml b/acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/acceptance/bundle/includes/glob_in_root_path/out.test.toml b/acceptance/bundle/includes/glob_in_root_path/out.test.toml new file mode 100644 index 0000000000..8f3575be7b --- /dev/null +++ b/acceptance/bundle/includes/glob_in_root_path/out.test.toml @@ -0,0 +1,5 @@ +Local = true +Cloud = false + +[EnvMatrix] + DATABRICKS_CLI_DEPLOYMENT = ["terraform", "direct-exp"] diff --git a/acceptance/bundle/includes/glob_in_root_path/output.txt b/acceptance/bundle/includes/glob_in_root_path/output.txt new file mode 100644 index 0000000000..5012e71f2e --- /dev/null +++ b/acceptance/bundle/includes/glob_in_root_path/output.txt @@ -0,0 +1,10 @@ + +>>> [CLI] bundle validate +Error: Bundle root path contains glob pattern characters + +The path to the bundle root [TEST_TMP_DIR]/[abc] contains glob pattern character "[". Please remove the character from this path to use bundle commands. + + +Found 1 error + +Exit code: 1 diff --git a/acceptance/bundle/includes/glob_in_root_path/script b/acceptance/bundle/includes/glob_in_root_path/script new file mode 100644 index 0000000000..6c6fd9414c --- /dev/null +++ b/acceptance/bundle/includes/glob_in_root_path/script @@ -0,0 +1,2 @@ +cd [abc] +trace $CLI bundle validate diff --git a/acceptance/bundle/includes/glob_in_root_path/test.toml b/acceptance/bundle/includes/glob_in_root_path/test.toml new file mode 100644 index 0000000000..a298217f21 --- /dev/null +++ b/acceptance/bundle/includes/glob_in_root_path/test.toml @@ -0,0 +1,3 @@ +[[Repls]] +Old = '\\' +New = '/' diff --git a/bundle/config/loader/process_root_includes.go b/bundle/config/loader/process_root_includes.go index 46768fb539..934c777a90 100644 --- a/bundle/config/loader/process_root_includes.go +++ b/bundle/config/loader/process_root_includes.go @@ -24,6 +24,18 @@ func (m *processRootIncludes) Name() string { return "ProcessRootIncludes" } +// hasGlobCharacters checks if a path contains any glob characters. +func hasGlobCharacters(path string) (string, bool) { + // List of glob characters supported by the filepath package in [filepath.Match] + globCharacters := []string{"*", "?", "[", "]", "^"} + for _, char := range globCharacters { + if strings.Contains(path, char) { + return char, true + } + } + return "", false +} + func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { var out []bundle.Mutator @@ -39,6 +51,24 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. var files []string var diags diag.Diagnostics + // We error on glob characters in the bundle root path since they are + // parsed by [filepath.Glob] as being glob patterns and thus can cause + // unexpected behavior. + // + // The standard library does not support globbing relative paths from a specified + // base directory. To support this, we can either: + // 1. Change CWD to the bundle root path before calling [filepath.Glob] + // 2. Implement our own custom globbing function. We can use [filepath.Match] to do so. + if char, ok := hasGlobCharacters(b.BundleRootPath); ok { + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: "Bundle root path contains glob pattern characters", + Detail: fmt.Sprintf("The path to the bundle root %s contains glob pattern character %q. Please remove the character from this path to use bundle commands.", b.BundleRootPath, char), + }) + + return diags + } + // For each glob, find all files to load. // Ordering of the list of globs is maintained in the output. // For matches that appear in multiple globs, only the first is kept. diff --git a/bundle/config/loader/process_root_includes_test.go b/bundle/config/loader/process_root_includes_test.go index 27ff9b05fe..da418dfcc3 100644 --- a/bundle/config/loader/process_root_includes_test.go +++ b/bundle/config/loader/process_root_includes_test.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/config/loader" "github.com/databricks/cli/internal/testutil" + "github.com/databricks/cli/libs/diag" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -111,3 +112,70 @@ func TestProcessRootIncludesNotExists(t *testing.T) { require.True(t, diags.HasError()) assert.ErrorContains(t, diags.Error(), "notexist.yml defined in 'include' section does not match any files") } + +func TestProcessRootIncludesGlobInRootPath(t *testing.T) { + tests := []struct { + name string + root string + diag diag.Diagnostic + }{ + { + name: "star", + root: "foo/a*", + diag: diag.Diagnostic{ + Severity: diag.Error, + Summary: "Bundle root path contains glob pattern characters", + Detail: `The path to the bundle root foo/a* contains glob pattern character "*". Please remove the character from this path to use bundle commands.`, + }, + }, + { + name: "question mark", + root: "bar/?b", + diag: diag.Diagnostic{ + Severity: diag.Error, + Summary: "Bundle root path contains glob pattern characters", + Detail: `The path to the bundle root bar/?b contains glob pattern character "?". Please remove the character from this path to use bundle commands.`, + }, + }, + { + name: "left bracket", + root: "[ab", + diag: diag.Diagnostic{ + Severity: diag.Error, + Summary: "Bundle root path contains glob pattern characters", + Detail: `The path to the bundle root [ab contains glob pattern character "[". Please remove the character from this path to use bundle commands.`, + }, + }, + { + name: "right bracket", + root: "ab]/bax", + diag: diag.Diagnostic{ + Severity: diag.Error, + Summary: "Bundle root path contains glob pattern characters", + Detail: `The path to the bundle root ab]/bax contains glob pattern character "]". Please remove the character from this path to use bundle commands.`, + }, + }, + { + name: "hat", + root: "ab^bax", + diag: diag.Diagnostic{ + Severity: diag.Error, + Summary: "Bundle root path contains glob pattern characters", + Detail: `The path to the bundle root ab^bax contains glob pattern character "^". Please remove the character from this path to use bundle commands.`, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + b := &bundle.Bundle{ + BundleRootPath: test.root, + } + + diags := bundle.Apply(context.Background(), b, loader.ProcessRootIncludes()) + require.True(t, diags.HasError()) + assert.Len(t, diags, 1) + assert.Equal(t, test.diag, diags[0]) + }) + } +}