From 0bb37495f9714ef93dba0d155a5ef130f9fad940 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 20 Jun 2025 15:03:50 +0200 Subject: [PATCH 1/8] Error when bundle root contains glob --- .../glob_in_root_path/[abc]/databricks.yml | 2 ++ .../glob_in_root_path/[abc]/resource.yml | 0 .../includes/glob_in_root_path/output.txt | 10 +++++++ .../bundle/includes/glob_in_root_path/script | 2 ++ bundle/config/loader/process_root_includes.go | 29 +++++++++++++++++++ 5 files changed, 43 insertions(+) create mode 100644 acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml create mode 100644 acceptance/bundle/includes/glob_in_root_path/[abc]/resource.yml create mode 100644 acceptance/bundle/includes/glob_in_root_path/output.txt create mode 100644 acceptance/bundle/includes/glob_in_root_path/script 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..88bdc20114 --- /dev/null +++ b/acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml @@ -0,0 +1,2 @@ +include: + - "*.yml" diff --git a/acceptance/bundle/includes/glob_in_root_path/[abc]/resource.yml b/acceptance/bundle/includes/glob_in_root_path/[abc]/resource.yml new file mode 100644 index 0000000000..e69de29bb2 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/bundle/config/loader/process_root_includes.go b/bundle/config/loader/process_root_includes.go index 191f5a0f3c..52bbd278c1 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,23 @@ 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 from a relative path, + // so if we want to support this, we'll need to implement our own 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. From 73843210d35a65316c1fd8f0f312f7ba740bf897 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 20 Jun 2025 15:08:01 +0200 Subject: [PATCH 2/8] add unit test --- .../loader/process_root_includes_test.go | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/bundle/config/loader/process_root_includes_test.go b/bundle/config/loader/process_root_includes_test.go index 27ff9b05fe..f1fbf28ff2 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,61 @@ 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: "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: "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.`, + }, + }, + } + + 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]) + }) + } +} From cda2b3ef0743d84a8fdb7fbba2b6ade6582b073a Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 20 Jun 2025 15:27:19 +0200 Subject: [PATCH 3/8] fix windows test --- .../bundle/includes/glob_in_root_path/test.toml | 3 +++ .../config/loader/process_root_includes_test.go | 17 +++++++++++++---- 2 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 acceptance/bundle/includes/glob_in_root_path/test.toml 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..b4cd43ce31 --- /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_test.go b/bundle/config/loader/process_root_includes_test.go index f1fbf28ff2..da418dfcc3 100644 --- a/bundle/config/loader/process_root_includes_test.go +++ b/bundle/config/loader/process_root_includes_test.go @@ -138,16 +138,16 @@ func TestProcessRootIncludesGlobInRootPath(t *testing.T) { }, }, { - name: "bracket", - root: "[ab]", + 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.`, + Detail: `The path to the bundle root [ab contains glob pattern character "[". Please remove the character from this path to use bundle commands.`, }, }, { - name: "bracket", + name: "right bracket", root: "ab]/bax", diag: diag.Diagnostic{ Severity: diag.Error, @@ -155,6 +155,15 @@ func TestProcessRootIncludesGlobInRootPath(t *testing.T) { 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 { From 2ca85cc692db7e37a0671dd111dcb3b61759198c Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Fri, 20 Jun 2025 16:55:20 +0200 Subject: [PATCH 4/8] fix test --- acceptance/bundle/includes/glob_in_root_path/test.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/acceptance/bundle/includes/glob_in_root_path/test.toml b/acceptance/bundle/includes/glob_in_root_path/test.toml index b4cd43ce31..a298217f21 100644 --- a/acceptance/bundle/includes/glob_in_root_path/test.toml +++ b/acceptance/bundle/includes/glob_in_root_path/test.toml @@ -1,3 +1,3 @@ [[Repls]] -Old = "\\" -New = "/" +Old = '\\' +New = '/' From ebf2652e8f3bf6ac88682985c0f305bda818e921 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Sat, 21 Jun 2025 08:47:44 +0200 Subject: [PATCH 5/8] add changelog --- NEXT_CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 9465c99263..0265aa28b5 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -3,6 +3,7 @@ ## Release v0.257.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)) ### Dependency updates From 227f6fda084d48babde2118806950e520085dc64 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 2 Jul 2025 11:19:57 +0200 Subject: [PATCH 6/8] - --- bundle/config/loader/process_root_includes.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/bundle/config/loader/process_root_includes.go b/bundle/config/loader/process_root_includes.go index 52bbd278c1..6b767341b3 100644 --- a/bundle/config/loader/process_root_includes.go +++ b/bundle/config/loader/process_root_includes.go @@ -55,9 +55,10 @@ func (m *processRootIncludes) Apply(ctx context.Context, b *bundle.Bundle) diag. // parsed by [filepath.Glob] as being glob patterns and thus can cause // unexpected behavior. // - // The standard library does not support globbing from a relative path, - // so if we want to support this, we'll need to implement our own globbing - // function. We can use [filepath.Match] to do so. + // 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, From 624e98ae474cf1d58dd847562765f353742bb51f Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Mon, 7 Jul 2025 11:02:11 +0200 Subject: [PATCH 7/8] simplify test --- .../bundle/includes/glob_in_root_path/[abc]/databricks.yml | 2 -- acceptance/bundle/includes/glob_in_root_path/[abc]/resource.yml | 0 2 files changed, 2 deletions(-) delete mode 100644 acceptance/bundle/includes/glob_in_root_path/[abc]/resource.yml diff --git a/acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml b/acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml index 88bdc20114..e69de29bb2 100644 --- a/acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml +++ b/acceptance/bundle/includes/glob_in_root_path/[abc]/databricks.yml @@ -1,2 +0,0 @@ -include: - - "*.yml" diff --git a/acceptance/bundle/includes/glob_in_root_path/[abc]/resource.yml b/acceptance/bundle/includes/glob_in_root_path/[abc]/resource.yml deleted file mode 100644 index e69de29bb2..0000000000 From 49d99da9e18aa84298a76b8eba5b6e55d0e25037 Mon Sep 17 00:00:00 2001 From: Shreyas Goenka Date: Wed, 9 Jul 2025 11:27:58 +0200 Subject: [PATCH 8/8] update tests --- acceptance/bundle/includes/glob_in_root_path/out.test.toml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 acceptance/bundle/includes/glob_in_root_path/out.test.toml 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"]