From 35b4201149f547ea8ea32d540dfb94b09acd4cdb Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Mon, 10 Aug 2026 13:50:11 +0000 Subject: [PATCH 1/3] Reject secret scope permissions with no principal A secret scope permission that names no principal is accepted by `bundle validate` today, but the backend rejects a secret ACL with an empty principal. The direct engine only fails later while collapsing permissions, and Terraform creates the scope before the ACL call fails, leaving a partial deploy that `destroy` cannot clean up. Reject the input at validation time so both engines fail early with a located, actionable error. A principal of the wrong type counts as missing: normalization only warns and drops the value, so the permission would otherwise still reach the backend without a principal. --- ...ret-scope-permission-principal-required.md | 1 + .../databricks.yml | 27 +++++++++ .../out.test.toml | 2 + .../output.txt | 57 ++++++++++++++++++ .../secret_scope_required_principal/script | 7 +++ .../secret_scope_required_principal/test.toml | 1 + bundle/config/validate/required.go | 58 ++++++++++++++++++- 7 files changed, 151 insertions(+), 2 deletions(-) create mode 100644 .nextchanges/bundles/secret-scope-permission-principal-required.md create mode 100644 acceptance/bundle/validate/secret_scope_required_principal/databricks.yml create mode 100644 acceptance/bundle/validate/secret_scope_required_principal/out.test.toml create mode 100644 acceptance/bundle/validate/secret_scope_required_principal/output.txt create mode 100644 acceptance/bundle/validate/secret_scope_required_principal/script create mode 100644 acceptance/bundle/validate/secret_scope_required_principal/test.toml diff --git a/.nextchanges/bundles/secret-scope-permission-principal-required.md b/.nextchanges/bundles/secret-scope-permission-principal-required.md new file mode 100644 index 00000000000..171c007448b --- /dev/null +++ b/.nextchanges/bundles/secret-scope-permission-principal-required.md @@ -0,0 +1 @@ +Reject secret scope permissions that name no principal, instead of failing after the scope is created. diff --git a/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml b/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml new file mode 100644 index 00000000000..49c9548505f --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml @@ -0,0 +1,27 @@ +bundle: + name: test-bundle + +resources: + secret_scopes: + no_principal: + name: test-scope-no-principal + permissions: + # Missing principal: rejected. + - level: READ + empty_principal: + name: test-scope-empty-principal + permissions: + # Empty principal: rejected. + - level: READ + group_name: "" + wrong_type_principal: + name: test-scope-wrong-type-principal + permissions: + # Normalization only warns and drops the value, leaving no principal: rejected. + - level: READ + group_name: [] + valid: + name: test-scope-valid + permissions: + - level: READ + group_name: users diff --git a/acceptance/bundle/validate/secret_scope_required_principal/out.test.toml b/acceptance/bundle/validate/secret_scope_required_principal/out.test.toml new file mode 100644 index 00000000000..98ea5040486 --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/out.test.toml @@ -0,0 +1,2 @@ +Cloud = false +EnvMatrix.DATABRICKS_BUNDLE_ENGINE = ["terraform", "direct"] diff --git a/acceptance/bundle/validate/secret_scope_required_principal/output.txt b/acceptance/bundle/validate/secret_scope_required_principal/output.txt new file mode 100644 index 00000000000..987ce601cb6 --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/output.txt @@ -0,0 +1,57 @@ + +>>> [CLI] bundle validate +Warning: expected string, found sequence + at resources.secret_scopes.wrong_type_principal.permissions[0].group_name + in databricks.yml:22:23 + +Error: secret scope permission principal is required + at resources.secret_scopes.empty_principal.permissions[0] + in databricks.yml:12:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.wrong_type_principal.permissions[0] + in databricks.yml:18:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.no_principal.permissions[0] + in databricks.yml:7:7 + +Set one of user_name, group_name or service_principal_name + +Name: test-bundle +Target: default +Workspace: + User: [USERNAME] + Path: /Workspace/Users/[USERNAME]/.bundle/test-bundle/default + +Found 3 errors and 1 warning + +>>> [CLI] bundle deploy +Warning: expected string, found sequence + at resources.secret_scopes.wrong_type_principal.permissions[0].group_name + in databricks.yml:22:23 + +Error: secret scope permission principal is required + at resources.secret_scopes.empty_principal.permissions[0] + in databricks.yml:12:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.wrong_type_principal.permissions[0] + in databricks.yml:18:7 + +Set one of user_name, group_name or service_principal_name + +Error: secret scope permission principal is required + at resources.secret_scopes.no_principal.permissions[0] + in databricks.yml:7:7 + +Set one of user_name, group_name or service_principal_name + + +>>> print_requests.py //secrets diff --git a/acceptance/bundle/validate/secret_scope_required_principal/script b/acceptance/bundle/validate/secret_scope_required_principal/script new file mode 100644 index 00000000000..ffc24566291 --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/script @@ -0,0 +1,7 @@ +musterr trace $CLI bundle validate + +# Deploy must abort at validation: no scope is created, so no partial deploy. +musterr trace $CLI bundle deploy +trace print_requests.py //secrets + +rm -f out.requests.txt diff --git a/acceptance/bundle/validate/secret_scope_required_principal/test.toml b/acceptance/bundle/validate/secret_scope_required_principal/test.toml new file mode 100644 index 00000000000..159efe02696 --- /dev/null +++ b/acceptance/bundle/validate/secret_scope_required_principal/test.toml @@ -0,0 +1 @@ +RecordRequests = true diff --git a/bundle/config/validate/required.go b/bundle/config/validate/required.go index 26223630e77..7f9bf198a6b 100644 --- a/bundle/config/validate/required.go +++ b/bundle/config/validate/required.go @@ -83,8 +83,14 @@ func sortDiagnostics(diags diag.Diagnostics) { return n } - // Finally sort by locations as a tie breaker if summaries are the same. - return cmp.Compare(fmt.Sprintf("%v", a.Locations), fmt.Sprintf("%v", b.Locations)) + // Then sort by locations as a tie breaker if summaries are the same. + if n := cmp.Compare(fmt.Sprintf("%v", a.Locations), fmt.Sprintf("%v", b.Locations)); n != 0 { + return n + } + + // Diagnostics for sibling entries of one resource share a location, so fall back to + // the path to keep the order stable. + return cmp.Compare(fmt.Sprintf("%v", a.Paths), fmt.Sprintf("%v", b.Paths)) }) } @@ -184,6 +190,53 @@ func errorForInvalidGrants(ctx context.Context, b *bundle.Bundle) diag.Diagnosti return diags } +// errorForInvalidSecretScopePermissions errors for secret scope permissions that name no +// principal. The backend rejects a secret ACL without one, but nothing rejects the input: +// the direct engine only fails later while collapsing permissions, and Terraform creates +// the scope before the ACL call fails, leaving a partial deploy. +func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { + diags := diag.Diagnostics{} + + _, err := dyn.MapByPattern( + b.Config.Value(), + dyn.NewPattern(dyn.Key("resources"), dyn.Key("secret_scopes"), dyn.AnyKey(), dyn.Key("permissions"), dyn.AnyIndex()), + func(p dyn.Path, v dyn.Value) (dyn.Value, error) { + if hasSecretScopePrincipal(v) { + return v, nil + } + // ApplyBundlePermissions rebuilds the permissions sequence through + // convert.FromTyped, which drops the entries' locations, so point at the scope. + diags = diags.Append(diag.Diagnostic{ + Severity: diag.Error, + Summary: "secret scope permission principal is required", + Detail: "Set one of user_name, group_name or service_principal_name", + Locations: b.Config.GetLocations("resources.secret_scopes." + p[2].Key()), + Paths: []dyn.Path{slices.Clone(p)}, + }) + return v, nil + }, + ) + if err != nil { + return diag.FromErr(err) + } + + sortDiagnostics(diags) + + return diags +} + +// hasSecretScopePrincipal reports whether the permission names a principal. A value of the +// wrong type counts as missing: normalization only warns and drops it, so the permission +// would still reach the backend without a principal. +func hasSecretScopePrincipal(v dyn.Value) bool { + for _, field := range []string{"user_name", "group_name", "service_principal_name"} { + if s, ok := v.Get(field).AsString(); ok && s != "" { + return true + } + } + return false +} + // isMissingOrEmptyString reports whether v is unset, null, or an empty string. func isMissingOrEmptyString(v dyn.Value) bool { switch v.Kind() { @@ -211,6 +264,7 @@ func isMissingOrEmptySequence(v dyn.Value) bool { func (f *required) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags := errorForMissingFields(ctx, b) diags = diags.Extend(errorForInvalidGrants(ctx, b)) + diags = diags.Extend(errorForInvalidSecretScopePermissions(ctx, b)) if diags.HasError() { return diags } From 5c8ba96efba16aba60aa279e305d2d882c9d52b0 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Tue, 11 Aug 2026 08:21:16 +0000 Subject: [PATCH 2/3] Tighten comments on secret scope permission validation --- .../databricks.yml | 2 +- bundle/config/validate/required.go | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml b/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml index 49c9548505f..0e33e50deca 100644 --- a/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml +++ b/acceptance/bundle/validate/secret_scope_required_principal/databricks.yml @@ -17,7 +17,7 @@ resources: wrong_type_principal: name: test-scope-wrong-type-principal permissions: - # Normalization only warns and drops the value, leaving no principal: rejected. + # Wrong-typed principal: normalization drops it, so treated as missing. - level: READ group_name: [] valid: diff --git a/bundle/config/validate/required.go b/bundle/config/validate/required.go index 7f9bf198a6b..bd3c8eb2b5e 100644 --- a/bundle/config/validate/required.go +++ b/bundle/config/validate/required.go @@ -88,8 +88,7 @@ func sortDiagnostics(diags diag.Diagnostics) { return n } - // Diagnostics for sibling entries of one resource share a location, so fall back to - // the path to keep the order stable. + // Sibling entries can share a location; fall back to path for a stable order. return cmp.Compare(fmt.Sprintf("%v", a.Paths), fmt.Sprintf("%v", b.Paths)) }) } @@ -190,10 +189,9 @@ func errorForInvalidGrants(ctx context.Context, b *bundle.Bundle) diag.Diagnosti return diags } -// errorForInvalidSecretScopePermissions errors for secret scope permissions that name no -// principal. The backend rejects a secret ACL without one, but nothing rejects the input: -// the direct engine only fails later while collapsing permissions, and Terraform creates -// the scope before the ACL call fails, leaving a partial deploy. +// errorForInvalidSecretScopePermissions errors when a permission names no principal. +// The backend rejects that ACL; erroring here avoids a partial deploy where Terraform +// creates the scope before the ACL call fails. func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags := diag.Diagnostics{} @@ -204,8 +202,8 @@ func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle if hasSecretScopePrincipal(v) { return v, nil } - // ApplyBundlePermissions rebuilds the permissions sequence through - // convert.FromTyped, which drops the entries' locations, so point at the scope. + // ApplyBundlePermissions rebuilds permissions via convert.FromTyped and drops + // per-entry locations, so point at the scope. diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "secret scope permission principal is required", @@ -225,9 +223,8 @@ func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle return diags } -// hasSecretScopePrincipal reports whether the permission names a principal. A value of the -// wrong type counts as missing: normalization only warns and drops it, so the permission -// would still reach the backend without a principal. +// hasSecretScopePrincipal reports whether a principal is set. Wrong-typed values count as +// missing: normalization only warns and drops them. func hasSecretScopePrincipal(v dyn.Value) bool { for _, field := range []string{"user_name", "group_name", "service_principal_name"} { if s, ok := v.Get(field).AsString(); ok && s != "" { From 65171ea0862abe43b3a679ebca5dc85181165e86 Mon Sep 17 00:00:00 2001 From: Rada Kamysheva Date: Tue, 11 Aug 2026 11:20:42 +0000 Subject: [PATCH 3/3] Validate secret scope principals via typed config Drop dyn.MapByPattern in favor of iterating SecretScopes, and keep comments to the non-obvious bits (wrong-typed drop, without-fix behavior). --- .../secret_scope_required_principal/script | 2 ++ bundle/config/validate/required.go | 35 +++++-------------- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/acceptance/bundle/validate/secret_scope_required_principal/script b/acceptance/bundle/validate/secret_scope_required_principal/script index ffc24566291..056ede5170d 100644 --- a/acceptance/bundle/validate/secret_scope_required_principal/script +++ b/acceptance/bundle/validate/secret_scope_required_principal/script @@ -1,3 +1,5 @@ +# Without the fix, validate only warns; deploy fails later in SecretScopeFixups (direct) +# or leaves empty-principal ACLs that destroy cannot remove (terraform). musterr trace $CLI bundle validate # Deploy must abort at validation: no scope is created, so no partial deploy. diff --git a/bundle/config/validate/required.go b/bundle/config/validate/required.go index bd3c8eb2b5e..b886c2c1d73 100644 --- a/bundle/config/validate/required.go +++ b/bundle/config/validate/required.go @@ -190,32 +190,26 @@ func errorForInvalidGrants(ctx context.Context, b *bundle.Bundle) diag.Diagnosti } // errorForInvalidSecretScopePermissions errors when a permission names no principal. -// The backend rejects that ACL; erroring here avoids a partial deploy where Terraform -// creates the scope before the ACL call fails. +// Wrong-typed values are already empty here (normalization only warns and drops them). func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle) diag.Diagnostics { diags := diag.Diagnostics{} - _, err := dyn.MapByPattern( - b.Config.Value(), - dyn.NewPattern(dyn.Key("resources"), dyn.Key("secret_scopes"), dyn.AnyKey(), dyn.Key("permissions"), dyn.AnyIndex()), - func(p dyn.Path, v dyn.Value) (dyn.Value, error) { - if hasSecretScopePrincipal(v) { - return v, nil + for key, scope := range b.Config.Resources.SecretScopes { + for i, perm := range scope.Permissions { + if perm.UserName != "" || perm.GroupName != "" || perm.ServicePrincipalName != "" { + continue } + path := fmt.Sprintf("resources.secret_scopes.%s.permissions[%d]", key, i) // ApplyBundlePermissions rebuilds permissions via convert.FromTyped and drops // per-entry locations, so point at the scope. diags = diags.Append(diag.Diagnostic{ Severity: diag.Error, Summary: "secret scope permission principal is required", Detail: "Set one of user_name, group_name or service_principal_name", - Locations: b.Config.GetLocations("resources.secret_scopes." + p[2].Key()), - Paths: []dyn.Path{slices.Clone(p)}, + Locations: b.Config.GetLocations("resources.secret_scopes." + key), + Paths: []dyn.Path{dyn.MustPathFromString(path)}, }) - return v, nil - }, - ) - if err != nil { - return diag.FromErr(err) + } } sortDiagnostics(diags) @@ -223,17 +217,6 @@ func errorForInvalidSecretScopePermissions(ctx context.Context, b *bundle.Bundle return diags } -// hasSecretScopePrincipal reports whether a principal is set. Wrong-typed values count as -// missing: normalization only warns and drops them. -func hasSecretScopePrincipal(v dyn.Value) bool { - for _, field := range []string{"user_name", "group_name", "service_principal_name"} { - if s, ok := v.Get(field).AsString(); ok && s != "" { - return true - } - } - return false -} - // isMissingOrEmptyString reports whether v is unset, null, or an empty string. func isMissingOrEmptyString(v dyn.Value) bool { switch v.Kind() {