diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 1f46a0a57c6..6343eb4a1f4 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -5,6 +5,7 @@ ### Notable Changes ### CLI +* `workspace export-dir` no longer aborts when a workspace object's name is not a legal local filename (e.g. a notebook named `New Notebook 2026-05-04 13:54:24` whose `:` is illegal on Windows). Such files are now skipped with a warning and the export completes ([#5171](https://github.com/databricks/cli/issues/5171)). ### Bundles * `bundle run` now prints the modern job run URL (`/jobs//runs/`) so that non-admin users permitted to view the run are taken to the run instead of the workspace homepage. diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/notebook.py b/acceptance/cmd/workspace/export-dir-illegal-filename/notebook.py new file mode 100644 index 00000000000..4914a7436d9 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/notebook.py @@ -0,0 +1,2 @@ +# Databricks notebook source +print("hello") diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/out.test.toml b/acceptance/cmd/workspace/export-dir-illegal-filename/out.test.toml new file mode 100644 index 00000000000..fee96cc336e --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/out.test.toml @@ -0,0 +1,6 @@ +Local = true +Cloud = false +GOOS.darwin = false +GOOS.linux = false +GOOS.windows = true +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/output.txt b/acceptance/cmd/workspace/export-dir-illegal-filename/output.txt new file mode 100644 index 00000000000..c234e37825d --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/output.txt @@ -0,0 +1,5 @@ + +>>> [CLI] workspace export-dir /test-dir [TEST_TMP_DIR]/export +Exporting files from /test-dir +/test-dir/New Notebook [TIMESTAMP] (skipped; invalid name for local file system) +Export complete diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/script b/acceptance/cmd/workspace/export-dir-illegal-filename/script new file mode 100644 index 00000000000..37b90688fd1 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/script @@ -0,0 +1,7 @@ +$CLI workspace import "/test-dir/New Notebook 2026-05-04 13:54:24.py" --file notebook.py --format AUTO --language PYTHON + +mkdir -p "$TEST_TMP_DIR/export" + +# On Windows the notebook name contains ':' which is illegal in a local filename. +# export-dir should skip it with a warning and still complete (databricks/cli#5171). +trace $CLI workspace export-dir /test-dir "$TEST_TMP_DIR/export" diff --git a/acceptance/cmd/workspace/export-dir-illegal-filename/test.toml b/acceptance/cmd/workspace/export-dir-illegal-filename/test.toml new file mode 100644 index 00000000000..c8c749d4f76 --- /dev/null +++ b/acceptance/cmd/workspace/export-dir-illegal-filename/test.toml @@ -0,0 +1,28 @@ +Local = true +Cloud = false + +# This reproduces databricks/cli#5171, which only manifests on Windows: a ':' in +# the notebook name is a legal filename character on Linux/macOS but illegal on +# Windows, so os.Create rejects the local target path there. +[GOOS] +darwin = false +linux = false +windows = true + +[Env] +MSYS_NO_PATHCONV = "1" + +[[Server]] +Pattern = "GET /api/2.0/workspace/list" +Response.Body = ''' +{ + "objects": [ + { + "path": "/test-dir/New Notebook 2026-05-04 13:54:24", + "object_type": "NOTEBOOK", + "language": "PYTHON", + "object_id": 123 + } + ] +} +''' diff --git a/cmd/workspace/workspace/export_dir.go b/cmd/workspace/workspace/export_dir.go index e7965bcda91..f06d7d16b2f 100644 --- a/cmd/workspace/workspace/export_dir.go +++ b/cmd/workspace/workspace/export_dir.go @@ -125,6 +125,13 @@ func (opts *exportDirOptions) callback(ctx context.Context, workspaceFiler filer // create the file f, err := os.Create(targetPath) if err != nil { + // A workspace name can be illegal as a local filename (e.g. a ':' + // in "New Notebook 2026-05-04 13:54:24" on Windows). Skip those with + // a warning rather than aborting the whole export (#5171). + if isInvalidLocalNameError(err) { + cmdio.LogString(ctx, sourcePath+" (skipped; invalid name for local file system)") + return nil + } return err } defer f.Close() diff --git a/cmd/workspace/workspace/export_dir_other.go b/cmd/workspace/workspace/export_dir_other.go new file mode 100644 index 00000000000..e2cd1741d17 --- /dev/null +++ b/cmd/workspace/workspace/export_dir_other.go @@ -0,0 +1,11 @@ +//go:build !windows + +package workspace + +// isInvalidLocalNameError reports whether err means the workspace object could +// not be written because its name is not a legal filename on the local OS. On +// non-Windows platforms the only bytes illegal in a filename are '/' and NUL, +// neither of which can appear in a workspace object name, so this never fires. +func isInvalidLocalNameError(err error) bool { + return false +} diff --git a/cmd/workspace/workspace/export_dir_windows.go b/cmd/workspace/workspace/export_dir_windows.go new file mode 100644 index 00000000000..d1276ce2b3f --- /dev/null +++ b/cmd/workspace/workspace/export_dir_windows.go @@ -0,0 +1,21 @@ +//go:build windows + +package workspace + +import ( + "errors" + "syscall" +) + +// errorInvalidName is the Windows ERROR_INVALID_NAME code. The file APIs return +// it when a path contains characters that are illegal in a local filename, such +// as the ':' in a notebook named "New Notebook 2026-05-04 13:54:24". It is not +// declared in the standard syscall package, so we use the well-known code. +// https://learn.microsoft.com/en-us/windows/win32/debug/system-error-codes--0-499- +const errorInvalidName = syscall.Errno(0x7b) + +// isInvalidLocalNameError reports whether err means the workspace object could +// not be written because its name is not a legal filename on the local OS. +func isInvalidLocalNameError(err error) bool { + return errors.Is(err, errorInvalidName) +} diff --git a/cmd/workspace/workspace/export_dir_windows_test.go b/cmd/workspace/workspace/export_dir_windows_test.go new file mode 100644 index 00000000000..aba1e735aa4 --- /dev/null +++ b/cmd/workspace/workspace/export_dir_windows_test.go @@ -0,0 +1,56 @@ +//go:build windows + +package workspace + +import ( + "errors" + "io/fs" + "os" + "syscall" + "testing" + + "github.com/stretchr/testify/assert" +) + +// The narrow contract: only the Windows "invalid file name" error is treated as +// skippable. Genuine failures (permission, missing path, anything else) must not +// be swallowed, otherwise export-dir would silently drop files on real errors. +func TestIsInvalidLocalNameError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "invalid name wrapped in PathError", + err: &os.PathError{Op: "open", Path: `C:\tmp\New Notebook 13:54:24.py`, Err: syscall.Errno(0x7b)}, + want: true, + }, + { + name: "permission denied is not skipped", + err: fs.ErrPermission, + want: false, + }, + { + name: "not exist is not skipped", + err: fs.ErrNotExist, + want: false, + }, + { + name: "generic error is not skipped", + err: errors.New("boom"), + want: false, + }, + { + name: "nil is not skipped", + err: nil, + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + assert.Equal(t, tt.want, isInvalidLocalNameError(tt.err)) + }) + } +}