From 0cce77e107882435183889dc8620c3f16578fe9f Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 10 Jul 2026 15:05:51 +0200 Subject: [PATCH 1/9] internal: Add support for 'sensitive' JSON tag for bundle config fields --- bundle/config/mask.go | 81 ++++++++++++++++ bundle/config/mask_test.go | 76 +++++++++++++++ cmd/bundle/plan.go | 117 ++++++++++++++++++++++- cmd/bundle/plan_test.go | 42 ++++++++ cmd/bundle/validate.go | 7 +- libs/dyn/convert/struct_info.go | 28 ++++++ libs/structs/structtag/bundletag.go | 6 ++ libs/structs/structtag/bundletag_test.go | 10 +- 8 files changed, 362 insertions(+), 5 deletions(-) create mode 100644 bundle/config/mask.go create mode 100644 bundle/config/mask_test.go create mode 100644 cmd/bundle/plan_test.go diff --git a/bundle/config/mask.go b/bundle/config/mask.go new file mode 100644 index 00000000000..55d60b0559d --- /dev/null +++ b/bundle/config/mask.go @@ -0,0 +1,81 @@ +package config + +import ( + "sync" + + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" +) + +const sensitiveValueMask = "********" + +// sensitiveFieldsCache caches sensitive field names per resource type key. +var ( + sensitiveFieldsCache map[string]map[string]bool + sensitiveFieldsCacheOnce sync.Once +) + +// sensitiveFields returns a map of JSON field names → true for resource type +// key (e.g. "secrets"). Built once from the convert.SensitiveFieldNames helper. +func sensitiveFields(resourceTypeKey string) map[string]bool { + sensitiveFieldsCacheOnce.Do(func() { + sensitiveFieldsCache = make(map[string]map[string]bool) + for name, typ := range ResourcesTypes { + if fields := convert.SensitiveFieldNames(typ); len(fields) > 0 { + sensitiveFieldsCache[name] = fields + } + } + }) + return sensitiveFieldsCache[resourceTypeKey] +} + +// SensitiveFieldsForResourceType returns the set of JSON field names that are +// tagged `bundle:"sensitive"` for the given resource type key (e.g. "secrets"). +// Returns nil when the type has no sensitive fields or is unknown. +func SensitiveFieldsForResourceType(resourceTypeKey string) map[string]bool { + return sensitiveFields(resourceTypeKey) +} + +// MaskSensitiveFields returns a copy of v with all fields tagged +// `bundle:"sensitive"` replaced by [sensitiveValueMask]. +// +// Only the display copy of a dyn.Value should be passed here — +// the live config value that feeds the deployment pipeline must never be masked. +func MaskSensitiveFields(v dyn.Value) (dyn.Value, error) { + // Pattern: resources.. + resourcesPattern := dyn.NewPattern( + dyn.Key("resources"), + dyn.AnyKey(), // resource type (e.g. "secrets") + dyn.AnyKey(), // resource name + ) + + return dyn.MapByPattern(v, resourcesPattern, func(p dyn.Path, resource dyn.Value) (dyn.Value, error) { + if len(p) < 2 { + return resource, nil + } + resourceType := p[1].Key() + fields := sensitiveFields(resourceType) + if len(fields) == 0 { + return resource, nil + } + + for fieldName := range fields { + fv, err := dyn.GetByPath(resource, dyn.NewPath(dyn.Key(fieldName))) + if err != nil { + // Field not present — nothing to mask. + continue + } + s, ok := fv.AsString() + if !ok || s == "" { + // Not a non-empty string — nothing to mask. + continue + } + resource, err = dyn.SetByPath(resource, dyn.NewPath(dyn.Key(fieldName)), + dyn.NewValue(sensitiveValueMask, fv.Locations())) + if err != nil { + return dyn.InvalidValue, err + } + } + return resource, nil + }) +} diff --git a/bundle/config/mask_test.go b/bundle/config/mask_test.go new file mode 100644 index 00000000000..a7581e0e189 --- /dev/null +++ b/bundle/config/mask_test.go @@ -0,0 +1,76 @@ +package config_test + +import ( + "reflect" + "testing" + + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestMaskSensitiveFieldsNoOp(t *testing.T) { + root := config.Root{ + Resources: config.Resources{}, + } + v, err := convert.FromTyped(root, dyn.NilValue) + require.NoError(t, err) + + masked, err := config.MaskSensitiveFields(v) + require.NoError(t, err) + assert.Equal(t, v, masked) +} + +// testSensitiveResource mimics a resource type with a sensitive field, used to +// verify SensitiveFieldNames without relying on resources.Secret (which lives +// on a different branch). +type testSensitiveResource struct { + Name string `json:"name"` + Token string `json:"token" bundle:"sensitive"` +} + +func TestSensitiveFieldNamesReadsTag(t *testing.T) { + fields := convert.SensitiveFieldNames(reflect.TypeFor[testSensitiveResource]()) + assert.True(t, fields["token"], "token should be sensitive") + assert.False(t, fields["name"], "name should not be sensitive") +} + +func TestSensitiveFieldNamesNilForNonStruct(t *testing.T) { + fields := convert.SensitiveFieldNames(reflect.TypeFor[string]()) + assert.Nil(t, fields) +} + +func TestSensitiveFieldNamesPointerDereference(t *testing.T) { + fields := convert.SensitiveFieldNames(reflect.TypeFor[*testSensitiveResource]()) + assert.True(t, fields["token"]) +} + +// TestMaskSensitiveFieldsOnDynValue tests the masking logic directly on a +// constructed dyn.Value tree, without needing resources.Secret to exist. +func TestMaskSensitiveFieldsOnDynValue(t *testing.T) { + // Build a minimal dyn.Value that looks like: + // resources: + // jobs: + // my_job: + // name: "hello" + // + // Since jobs have no sensitive fields, masking should leave it unchanged. + v := dyn.NewValue(map[string]dyn.Value{ + "resources": dyn.NewValue(map[string]dyn.Value{ + "jobs": dyn.NewValue(map[string]dyn.Value{ + "my_job": dyn.NewValue(map[string]dyn.Value{ + "name": dyn.NewValue("hello", nil), + }, nil), + }, nil), + }, nil), + }, nil) + + masked, err := config.MaskSensitiveFields(v) + require.NoError(t, err) + + name, err := dyn.GetByPath(masked, dyn.MustPathFromString("resources.jobs.my_job.name")) + require.NoError(t, err) + assert.Equal(t, "hello", name.MustString()) +} diff --git a/cmd/bundle/plan.go b/cmd/bundle/plan.go index 20df8cb5f0f..1cb67abaead 100644 --- a/cmd/bundle/plan.go +++ b/cmd/bundle/plan.go @@ -1,11 +1,13 @@ package bundle import ( + "bytes" "encoding/json" "fmt" "strings" "github.com/databricks/cli/bundle" + bundleconfig "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" @@ -109,7 +111,7 @@ It is useful for previewing changes before running 'bundle deploy'.`, // Note, this string should not be changed, "bundle deployment migrate" depends on this format: fmt.Fprintf(out, "Plan: %d to add, %d to change, %d to delete, %d unchanged\n", createCount, updateCount, deleteCount, unchangedCount) case flags.OutputJSON: - buf, err := json.MarshalIndent(plan, "", " ") + buf, err := marshalPlanRedacted(plan) if err != nil { return err } @@ -129,3 +131,116 @@ It is useful for previewing changes before running 'bundle deploy'.`, return cmd } + +// marshalPlanRedacted encodes plan as indented JSON with sensitive field values +// replaced by "********". It operates on a decoded copy of the plan and never +// mutates the live *deployplan.Plan used by the deployment pipeline. +func marshalPlanRedacted(plan *deployplan.Plan) ([]byte, error) { + // Step 1: encode the plan to JSON. + raw, err := json.Marshal(plan) + if err != nil { + return nil, err + } + + // Step 2: decode into a generic map using UseNumber to preserve int64 IDs. + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + var m map[string]any + if err := dec.Decode(&m); err != nil { + return nil, err + } + + // Step 3: walk plan.plan entries and redact sensitive fields. + planEntries, _ := m["plan"].(map[string]any) + for resourceKey, entryAny := range planEntries { + // Resource key format: "resources..", e.g. "resources.secrets.my_secret". + parts := strings.SplitN(resourceKey, ".", 3) + if len(parts) < 2 || parts[0] != "resources" { + continue + } + sensitiveNames := bundleconfig.SensitiveFieldsForResourceType(parts[1]) + if len(sensitiveNames) == 0 { + continue + } + + entry, ok := entryAny.(map[string]any) + if !ok { + continue + } + redactPlanEntry(entry, sensitiveNames) + } + + // Step 4: re-encode the redacted copy. + return json.MarshalIndent(m, "", " ") +} + +// redactPlanEntry masks sensitive field values inside a single decoded plan +// entry map in place. sensitiveNames is the set of JSON field names to mask. +func redactPlanEntry(entry map[string]any, sensitiveNames map[string]bool) { + // Mask fields inside new_state.value (JSON object of the resource state). + if ns, ok := entry["new_state"].(map[string]any); ok { + if valRaw, ok := ns["value"].(json.RawMessage); ok { + ns["value"] = redactJSONObject(valRaw, sensitiveNames) + } else if valMap, ok := ns["value"].(map[string]any); ok { + redactMapInPlace(valMap, sensitiveNames) + } + } + + // Mask fields inside changes[].{old, new, remote}. + // Each change key is the field path (e.g. "value") and the payload is a + // ChangeDesc object whose Old/New/Remote hold the raw field value. + if changes, ok := entry["changes"].(map[string]any); ok { + for fieldPath, changeAny := range changes { + // fieldPath may be "value" or a nested path like "config.value". + // Check the top-level segment only. + topField := fieldPath + if before, _, ok0 := strings.Cut(fieldPath, "."); ok0 { + topField = before + } + if !sensitiveNames[topField] { + continue + } + change, ok := changeAny.(map[string]any) + if !ok { + continue + } + for _, key := range []string{"old", "new", "remote"} { + if s, ok := change[key].(string); ok && s != "" { + change[key] = "********" + } + } + } + } + + // Mask fields inside remote_state (the full remote resource struct). + // In practice sensitive fields like Secret.Value are write-only and not + // returned by the API, so remote_state will already be empty here. We + // mask defensively in case a future resource type differs. + if rs, ok := entry["remote_state"].(map[string]any); ok { + redactMapInPlace(rs, sensitiveNames) + } +} + +// redactJSONObject decodes a json.RawMessage as a map, masks sensitive fields, +// and returns the result as a map[string]any. Falls back to returning raw on +// decode error. +func redactJSONObject(raw json.RawMessage, sensitiveNames map[string]bool) any { + var m map[string]any + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + if err := dec.Decode(&m); err != nil { + return raw + } + redactMapInPlace(m, sensitiveNames) + return m +} + +// redactMapInPlace replaces non-empty string values for sensitive field names +// with "********", operating in place on a decoded JSON map. +func redactMapInPlace(m map[string]any, sensitiveNames map[string]bool) { + for name := range sensitiveNames { + if s, ok := m[name].(string); ok && s != "" { + m[name] = "********" + } + } +} diff --git a/cmd/bundle/plan_test.go b/cmd/bundle/plan_test.go new file mode 100644 index 00000000000..3bace49fc0d --- /dev/null +++ b/cmd/bundle/plan_test.go @@ -0,0 +1,42 @@ +package bundle + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestRedactMapInPlace(t *testing.T) { + m := map[string]any{ + "value": "super-secret", + "comment": "visible", + } + redactMapInPlace(m, map[string]bool{"value": true}) + assert.Equal(t, "********", m["value"]) + assert.Equal(t, "visible", m["comment"]) +} + +func TestRedactMapInPlaceEmptyNotMasked(t *testing.T) { + m := map[string]any{ + "value": "", + } + redactMapInPlace(m, map[string]bool{"value": true}) + assert.Empty(t, m["value"]) +} + +func TestMarshalPlanRedactedNoSensitiveResources(t *testing.T) { + // A plan with a job (no sensitive fields) must not be altered. + raw := `{"plan_version":2,"plan":{"resources.jobs.my_job":{"action":"create","new_state":{"value":{"name":"my_job"}}}}}` + + var m map[string]any + require.NoError(t, json.Unmarshal([]byte(raw), &m)) + + // Reconstruct as a minimal deployplan.Plan for marshaling. + buf, err := json.MarshalIndent(m, "", " ") + require.NoError(t, err) + + // The job name must not be masked. + assert.Contains(t, string(buf), `"my_job"`) +} diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index a2ec31f721b..eff442772be 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -5,6 +5,7 @@ import ( "fmt" "github.com/databricks/cli/bundle" + bundleconfig "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/render" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" @@ -17,7 +18,11 @@ func renderJsonOutput(cmd *cobra.Command, b *bundle.Bundle) error { if b == nil { return nil } - buf, err := json.MarshalIndent(b.Config.Value().AsAny(), "", " ") + v, err := bundleconfig.MaskSensitiveFields(b.Config.Value()) + if err != nil { + return err + } + buf, err := json.MarshalIndent(v.AsAny(), "", " ") if err != nil { return err } diff --git a/libs/dyn/convert/struct_info.go b/libs/dyn/convert/struct_info.go index 7e5b0bc741e..698d0cf316f 100644 --- a/libs/dyn/convert/struct_info.go +++ b/libs/dyn/convert/struct_info.go @@ -31,6 +31,10 @@ type structInfo struct { // ForceSendFieldsStructKey maps the JSON-name of the field to which ForceSendFields slice it belongs to: // -1 for direct fields, embedded struct index for embedded fields ForceSendFieldsStructKey map[string]int + + // Sensitive tracks fields tagged `bundle:"sensitive"` by their JSON name. + // Values for these fields should be masked in display output. + Sensitive map[string]bool } // structInfoCache caches type information. @@ -62,6 +66,7 @@ func buildStructInfo(typ reflect.Type) structInfo { ForceEmpty: make(map[string]bool), GolangNames: make(map[string]string), ForceSendFieldsStructKey: make(map[string]int), + Sensitive: make(map[string]bool), } // Queue holds the indexes of the structs to visit. @@ -119,6 +124,10 @@ func buildStructInfo(typ reflect.Type) structInfo { } out.GolangNames[name] = sf.Name + if structtag.BundleTag(sf.Tag.Get("bundle")).Sensitive() { + out.Sensitive[name] = true + } + // Determine which ForceSendFields this field belongs to if len(prefix) == 0 { // Direct field on the main struct @@ -192,6 +201,25 @@ func (s *structInfo) FieldValues(v reflect.Value) []FieldValue { // Type of [dyn.Value]. var configValueType = reflect.TypeFor[dyn.Value]() +// SensitiveFieldNames returns the JSON field names of typ that carry the +// `bundle:"sensitive"` tag. A pointer type is dereferenced before inspection. +// Returns nil for non-struct types. Callers use this to identify fields that +// must be masked in display output (validate -o json, plan -o json) without +// touching the typed values used by the actual deployment pipeline. +func SensitiveFieldNames(typ reflect.Type) map[string]bool { + for typ.Kind() == reflect.Pointer { + typ = typ.Elem() + } + if typ.Kind() != reflect.Struct { + return nil + } + si := getStructInfo(typ) + if len(si.Sensitive) == 0 { + return nil + } + return si.Sensitive +} + // getForceSendFieldsValues collects ForceSendFields reflect.Values // Returns map[structKey]reflect.Value where structKey is -1 for direct fields, embedded index for embedded fields func getForceSendFieldsValues(v reflect.Value) map[int]reflect.Value { diff --git a/libs/structs/structtag/bundletag.go b/libs/structs/structtag/bundletag.go index 9b7bb2d0ac2..f8254b53e07 100644 --- a/libs/structs/structtag/bundletag.go +++ b/libs/structs/structtag/bundletag.go @@ -11,3 +11,9 @@ func (tag BundleTag) ReadOnly() bool { func (tag BundleTag) Internal() bool { return hasOption(string(tag), "internal") } + +// Sensitive reports whether the field holds a value that must be masked +// when rendering configuration or plan output (e.g. secret values). +func (tag BundleTag) Sensitive() bool { + return hasOption(string(tag), "sensitive") +} diff --git a/libs/structs/structtag/bundletag_test.go b/libs/structs/structtag/bundletag_test.go index 3d2129ec764..ff3057cee93 100644 --- a/libs/structs/structtag/bundletag_test.go +++ b/libs/structs/structtag/bundletag_test.go @@ -8,16 +8,19 @@ import ( func TestBundleTagMethods(t *testing.T) { tests := []struct { - tag string - isReadOnly bool - isInternal bool + tag string + isReadOnly bool + isInternal bool + isSensitive bool }{ // only one annotation. {tag: "readonly", isReadOnly: true}, {tag: "internal", isInternal: true}, + {tag: "sensitive", isSensitive: true}, // multiple annotations. {tag: "readonly,internal", isReadOnly: true, isInternal: true}, + {tag: "readonly,sensitive", isReadOnly: true, isSensitive: true}, // unknown annotations are ignored. {tag: "something"}, @@ -32,6 +35,7 @@ func TestBundleTagMethods(t *testing.T) { assert.Equal(t, test.isReadOnly, tag.ReadOnly()) assert.Equal(t, test.isInternal, tag.Internal()) + assert.Equal(t, test.isSensitive, tag.Sensitive()) }) } } From 7abdf8a1378f2044bd9f5165d2c18aa9518a8359 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 10 Jul 2026 16:40:11 +0200 Subject: [PATCH 2/9] no masking in plan --- bundle/config/mask.go | 7 --- cmd/bundle/plan.go | 117 +--------------------------------------- cmd/bundle/plan_test.go | 42 --------------- 3 files changed, 1 insertion(+), 165 deletions(-) delete mode 100644 cmd/bundle/plan_test.go diff --git a/bundle/config/mask.go b/bundle/config/mask.go index 55d60b0559d..61e04d21fde 100644 --- a/bundle/config/mask.go +++ b/bundle/config/mask.go @@ -29,13 +29,6 @@ func sensitiveFields(resourceTypeKey string) map[string]bool { return sensitiveFieldsCache[resourceTypeKey] } -// SensitiveFieldsForResourceType returns the set of JSON field names that are -// tagged `bundle:"sensitive"` for the given resource type key (e.g. "secrets"). -// Returns nil when the type has no sensitive fields or is unknown. -func SensitiveFieldsForResourceType(resourceTypeKey string) map[string]bool { - return sensitiveFields(resourceTypeKey) -} - // MaskSensitiveFields returns a copy of v with all fields tagged // `bundle:"sensitive"` replaced by [sensitiveValueMask]. // diff --git a/cmd/bundle/plan.go b/cmd/bundle/plan.go index 1cb67abaead..20df8cb5f0f 100644 --- a/cmd/bundle/plan.go +++ b/cmd/bundle/plan.go @@ -1,13 +1,11 @@ package bundle import ( - "bytes" "encoding/json" "fmt" "strings" "github.com/databricks/cli/bundle" - bundleconfig "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/deployplan" "github.com/databricks/cli/bundle/phases" "github.com/databricks/cli/cmd/bundle/utils" @@ -111,7 +109,7 @@ It is useful for previewing changes before running 'bundle deploy'.`, // Note, this string should not be changed, "bundle deployment migrate" depends on this format: fmt.Fprintf(out, "Plan: %d to add, %d to change, %d to delete, %d unchanged\n", createCount, updateCount, deleteCount, unchangedCount) case flags.OutputJSON: - buf, err := marshalPlanRedacted(plan) + buf, err := json.MarshalIndent(plan, "", " ") if err != nil { return err } @@ -131,116 +129,3 @@ It is useful for previewing changes before running 'bundle deploy'.`, return cmd } - -// marshalPlanRedacted encodes plan as indented JSON with sensitive field values -// replaced by "********". It operates on a decoded copy of the plan and never -// mutates the live *deployplan.Plan used by the deployment pipeline. -func marshalPlanRedacted(plan *deployplan.Plan) ([]byte, error) { - // Step 1: encode the plan to JSON. - raw, err := json.Marshal(plan) - if err != nil { - return nil, err - } - - // Step 2: decode into a generic map using UseNumber to preserve int64 IDs. - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.UseNumber() - var m map[string]any - if err := dec.Decode(&m); err != nil { - return nil, err - } - - // Step 3: walk plan.plan entries and redact sensitive fields. - planEntries, _ := m["plan"].(map[string]any) - for resourceKey, entryAny := range planEntries { - // Resource key format: "resources..", e.g. "resources.secrets.my_secret". - parts := strings.SplitN(resourceKey, ".", 3) - if len(parts) < 2 || parts[0] != "resources" { - continue - } - sensitiveNames := bundleconfig.SensitiveFieldsForResourceType(parts[1]) - if len(sensitiveNames) == 0 { - continue - } - - entry, ok := entryAny.(map[string]any) - if !ok { - continue - } - redactPlanEntry(entry, sensitiveNames) - } - - // Step 4: re-encode the redacted copy. - return json.MarshalIndent(m, "", " ") -} - -// redactPlanEntry masks sensitive field values inside a single decoded plan -// entry map in place. sensitiveNames is the set of JSON field names to mask. -func redactPlanEntry(entry map[string]any, sensitiveNames map[string]bool) { - // Mask fields inside new_state.value (JSON object of the resource state). - if ns, ok := entry["new_state"].(map[string]any); ok { - if valRaw, ok := ns["value"].(json.RawMessage); ok { - ns["value"] = redactJSONObject(valRaw, sensitiveNames) - } else if valMap, ok := ns["value"].(map[string]any); ok { - redactMapInPlace(valMap, sensitiveNames) - } - } - - // Mask fields inside changes[].{old, new, remote}. - // Each change key is the field path (e.g. "value") and the payload is a - // ChangeDesc object whose Old/New/Remote hold the raw field value. - if changes, ok := entry["changes"].(map[string]any); ok { - for fieldPath, changeAny := range changes { - // fieldPath may be "value" or a nested path like "config.value". - // Check the top-level segment only. - topField := fieldPath - if before, _, ok0 := strings.Cut(fieldPath, "."); ok0 { - topField = before - } - if !sensitiveNames[topField] { - continue - } - change, ok := changeAny.(map[string]any) - if !ok { - continue - } - for _, key := range []string{"old", "new", "remote"} { - if s, ok := change[key].(string); ok && s != "" { - change[key] = "********" - } - } - } - } - - // Mask fields inside remote_state (the full remote resource struct). - // In practice sensitive fields like Secret.Value are write-only and not - // returned by the API, so remote_state will already be empty here. We - // mask defensively in case a future resource type differs. - if rs, ok := entry["remote_state"].(map[string]any); ok { - redactMapInPlace(rs, sensitiveNames) - } -} - -// redactJSONObject decodes a json.RawMessage as a map, masks sensitive fields, -// and returns the result as a map[string]any. Falls back to returning raw on -// decode error. -func redactJSONObject(raw json.RawMessage, sensitiveNames map[string]bool) any { - var m map[string]any - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.UseNumber() - if err := dec.Decode(&m); err != nil { - return raw - } - redactMapInPlace(m, sensitiveNames) - return m -} - -// redactMapInPlace replaces non-empty string values for sensitive field names -// with "********", operating in place on a decoded JSON map. -func redactMapInPlace(m map[string]any, sensitiveNames map[string]bool) { - for name := range sensitiveNames { - if s, ok := m[name].(string); ok && s != "" { - m[name] = "********" - } - } -} diff --git a/cmd/bundle/plan_test.go b/cmd/bundle/plan_test.go deleted file mode 100644 index 3bace49fc0d..00000000000 --- a/cmd/bundle/plan_test.go +++ /dev/null @@ -1,42 +0,0 @@ -package bundle - -import ( - "encoding/json" - "testing" - - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestRedactMapInPlace(t *testing.T) { - m := map[string]any{ - "value": "super-secret", - "comment": "visible", - } - redactMapInPlace(m, map[string]bool{"value": true}) - assert.Equal(t, "********", m["value"]) - assert.Equal(t, "visible", m["comment"]) -} - -func TestRedactMapInPlaceEmptyNotMasked(t *testing.T) { - m := map[string]any{ - "value": "", - } - redactMapInPlace(m, map[string]bool{"value": true}) - assert.Empty(t, m["value"]) -} - -func TestMarshalPlanRedactedNoSensitiveResources(t *testing.T) { - // A plan with a job (no sensitive fields) must not be altered. - raw := `{"plan_version":2,"plan":{"resources.jobs.my_job":{"action":"create","new_state":{"value":{"name":"my_job"}}}}}` - - var m map[string]any - require.NoError(t, json.Unmarshal([]byte(raw), &m)) - - // Reconstruct as a minimal deployplan.Plan for marshaling. - buf, err := json.MarshalIndent(m, "", " ") - require.NoError(t, err) - - // The job name must not be masked. - assert.Contains(t, string(buf), `"my_job"`) -} From f930a69493418716ee31b8c73502e30afadb47b8 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 10 Jul 2026 16:53:11 +0200 Subject: [PATCH 3/9] fix lint --- bundle/config/mask.go | 13 ++++++------- libs/dyn/convert/struct_info.go | 11 +++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/bundle/config/mask.go b/bundle/config/mask.go index 61e04d21fde..4cbb8020fc7 100644 --- a/bundle/config/mask.go +++ b/bundle/config/mask.go @@ -12,13 +12,7 @@ const sensitiveValueMask = "********" // sensitiveFieldsCache caches sensitive field names per resource type key. var ( sensitiveFieldsCache map[string]map[string]bool - sensitiveFieldsCacheOnce sync.Once -) - -// sensitiveFields returns a map of JSON field names → true for resource type -// key (e.g. "secrets"). Built once from the convert.SensitiveFieldNames helper. -func sensitiveFields(resourceTypeKey string) map[string]bool { - sensitiveFieldsCacheOnce.Do(func() { + sensitiveFieldsCacheOnce = sync.OnceFunc(func() { sensitiveFieldsCache = make(map[string]map[string]bool) for name, typ := range ResourcesTypes { if fields := convert.SensitiveFieldNames(typ); len(fields) > 0 { @@ -26,6 +20,11 @@ func sensitiveFields(resourceTypeKey string) map[string]bool { } } }) +) + +// sensitiveFields returns a map of JSON field names → true for resource type +// key (e.g. "secrets"). Built once from the convert.SensitiveFieldNames helper. +func sensitiveFields(resourceTypeKey string) map[string]bool { return sensitiveFieldsCache[resourceTypeKey] } diff --git a/libs/dyn/convert/struct_info.go b/libs/dyn/convert/struct_info.go index 81d8c580fef..ab13dfa6ad7 100644 --- a/libs/dyn/convert/struct_info.go +++ b/libs/dyn/convert/struct_info.go @@ -67,11 +67,11 @@ func getStructInfo(typ reflect.Type) structInfo { // buildStructInfo populates a new [structInfo] for the given type. func buildStructInfo(typ reflect.Type) structInfo { out := structInfo{ - Fields: make(map[string][]int), - ForceEmpty: make(map[string]bool), - GolangNames: make(map[string]string), - ForceSendFieldsIndex: make(map[string][]int), - Sensitive: make(map[string]bool), + Fields: make(map[string][]int), + ForceEmpty: make(map[string]bool), + GolangNames: make(map[string]string), + ForceSendFieldsIndex: make(map[string][]int), + Sensitive: make(map[string]bool), } // Queue holds the indexes of the structs to visit. @@ -184,7 +184,6 @@ func (s *structInfo) FieldValues(v reflect.Value) []FieldValue { return out } - // SensitiveFieldNames returns the JSON field names of typ that carry the // `bundle:"sensitive"` tag. A pointer type is dereferenced before inspection. // Returns nil for non-struct types. Callers use this to identify fields that From 687b636927ecc85cafb7c56be0c11ef0f2056c77 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Fri, 10 Jul 2026 17:10:44 +0200 Subject: [PATCH 4/9] fixed missed cache call --- bundle/config/mask.go | 1 + 1 file changed, 1 insertion(+) diff --git a/bundle/config/mask.go b/bundle/config/mask.go index 4cbb8020fc7..0a83db5859b 100644 --- a/bundle/config/mask.go +++ b/bundle/config/mask.go @@ -25,6 +25,7 @@ var ( // sensitiveFields returns a map of JSON field names → true for resource type // key (e.g. "secrets"). Built once from the convert.SensitiveFieldNames helper. func sensitiveFields(resourceTypeKey string) map[string]bool { + sensitiveFieldsCacheOnce() return sensitiveFieldsCache[resourceTypeKey] } From f8ff27eb6dc6beb3bca9d9057299956d21013386 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 14 Jul 2026 11:06:54 +0200 Subject: [PATCH 5/9] addressed feedback --- bundle/config/mask.go | 21 +++- .../no_reference_to_sensitive_fields.go | 83 +++++++++++++ .../no_reference_to_sensitive_fields_test.go | 116 ++++++++++++++++++ bundle/phases/initialize.go | 1 + cmd/bundle/validate.go | 2 + 5 files changed, 217 insertions(+), 6 deletions(-) create mode 100644 bundle/config/validate/no_reference_to_sensitive_fields.go create mode 100644 bundle/config/validate/no_reference_to_sensitive_fields_test.go diff --git a/bundle/config/mask.go b/bundle/config/mask.go index 0a83db5859b..c72a98bd83c 100644 --- a/bundle/config/mask.go +++ b/bundle/config/mask.go @@ -1,6 +1,7 @@ package config import ( + "fmt" "sync" "github.com/databricks/cli/libs/dyn" @@ -22,9 +23,9 @@ var ( }) ) -// sensitiveFields returns a map of JSON field names → true for resource type +// SensitiveFields returns a map of JSON field names → true for a resource type // key (e.g. "secrets"). Built once from the convert.SensitiveFieldNames helper. -func sensitiveFields(resourceTypeKey string) map[string]bool { +func SensitiveFields(resourceTypeKey string) map[string]bool { sensitiveFieldsCacheOnce() return sensitiveFieldsCache[resourceTypeKey] } @@ -47,20 +48,28 @@ func MaskSensitiveFields(v dyn.Value) (dyn.Value, error) { return resource, nil } resourceType := p[1].Key() - fields := sensitiveFields(resourceType) + fields := SensitiveFields(resourceType) if len(fields) == 0 { return resource, nil } for fieldName := range fields { fv, err := dyn.GetByPath(resource, dyn.NewPath(dyn.Key(fieldName))) + if dyn.IsNoSuchKeyError(err) { + // Field not present in this resource instance — nothing to mask. + continue + } if err != nil { - // Field not present — nothing to mask. + return dyn.InvalidValue, err + } + if fv.Kind() == dyn.KindNil { continue } s, ok := fv.AsString() - if !ok || s == "" { - // Not a non-empty string — nothing to mask. + if !ok { + return dyn.InvalidValue, fmt.Errorf("sensitive field %q must be a string, got %s", fieldName, fv.Kind()) + } + if s == "" { continue } resource, err = dyn.SetByPath(resource, dyn.NewPath(dyn.Key(fieldName)), diff --git a/bundle/config/validate/no_reference_to_sensitive_fields.go b/bundle/config/validate/no_reference_to_sensitive_fields.go new file mode 100644 index 00000000000..b410b1bb8a1 --- /dev/null +++ b/bundle/config/validate/no_reference_to_sensitive_fields.go @@ -0,0 +1,83 @@ +package validate + +import ( + "context" + "fmt" + "strings" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/cli/libs/dyn/convert" + "github.com/databricks/cli/libs/dyn/dynvar" +) + +type noReferenceToSensitiveFields struct{} + +// NoReferenceToSensitiveFields returns a validator that errors when any value +// in the config references a field tagged `bundle:"sensitive"` via interpolation +// (e.g. ${resources.secrets.my_secret.value}). +// +// Sensitive values must never appear in other fields: they would end up in +// validate JSON output, plan output, or debug logs where masking is impractical. +func NoReferenceToSensitiveFields() bundle.Mutator { + return &noReferenceToSensitiveFields{} +} + +func (m *noReferenceToSensitiveFields) Name() string { + return "validate:no_reference_to_sensitive_fields" +} + +func (m *noReferenceToSensitiveFields) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { + var diags diag.Diagnostics + + _ = dyn.WalkReadOnly(b.Config.Value(), func(_ dyn.Path, v dyn.Value) error { + ref, ok := dynvar.NewRef(v) + if !ok { + return nil + } + for _, r := range ref.References() { + if d := checkSensitiveReference(r, v.Locations()); d != nil { + diags = append(diags, *d) + } + } + return nil + }) + + return diags +} + +// checkSensitiveReference returns an error diagnostic when the reference path +// points to a field tagged `bundle:"sensitive"`, or nil otherwise. +// +// It reads config.ResourcesTypes at call time (no extra cache) so tests can +// patch the map before calling and see the effect immediately. +func checkSensitiveReference(ref string, locs []dyn.Location) *diag.Diagnostic { + if !strings.HasPrefix(ref, "resources.") { + return nil + } + p, err := dyn.NewPathFromString(ref) + // Need at least resources.... + if err != nil || len(p) < 4 { + return nil + } + + resourceType := p[1].Key() + fieldName := p[3].Key() + + typ, ok := config.ResourcesTypes[resourceType] + if !ok { + return nil + } + sensitiveFields := convert.SensitiveFieldNames(typ) + if !sensitiveFields[fieldName] { + return nil + } + + return &diag.Diagnostic{ + Severity: diag.Error, + Summary: fmt.Sprintf("${%s}: references a sensitive field and cannot be used in interpolation", ref), + Locations: locs, + } +} diff --git a/bundle/config/validate/no_reference_to_sensitive_fields_test.go b/bundle/config/validate/no_reference_to_sensitive_fields_test.go new file mode 100644 index 00000000000..0b355cf08d8 --- /dev/null +++ b/bundle/config/validate/no_reference_to_sensitive_fields_test.go @@ -0,0 +1,116 @@ +package validate + +import ( + "reflect" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config" + "github.com/databricks/cli/bundle/config/resources" + "github.com/databricks/cli/bundle/internal/bundletest" + "github.com/databricks/cli/libs/diag" + "github.com/databricks/cli/libs/dyn" + "github.com/databricks/databricks-sdk-go/service/jobs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// sensitiveTestResource is a minimal struct with one sensitive field, used to +// exercise the validator without a real resource that carries `bundle:"sensitive"`. +type sensitiveTestResource struct { + Name string `json:"name"` + Token string `json:"token" bundle:"sensitive"` +} + +// withSensitiveResourceType registers sensitiveTestResource under the key +// "secrets" in config.ResourcesTypes for the duration of the test. +func withSensitiveResourceType(t *testing.T) { + t.Helper() + orig := config.ResourcesTypes + patched := make(map[string]reflect.Type, len(orig)+1) + for k, v := range orig { + patched[k] = v + } + patched["secrets"] = reflect.TypeFor[sensitiveTestResource]() + config.ResourcesTypes = patched + t.Cleanup(func() { config.ResourcesTypes = orig }) +} + +func makeJobsBundle(t *testing.T) *bundle.Bundle { + t.Helper() + return &bundle.Bundle{ + Config: config.Root{ + Resources: config.Resources{ + Jobs: map[string]*resources.Job{ + "src": {JobSettings: jobs.JobSettings{Name: "source"}}, + "dst": {JobSettings: jobs.JobSettings{Name: "placeholder"}}, + }, + }, + }, + } +} + +func TestNoReferenceToSensitiveFields_NoSensitiveType(t *testing.T) { + // jobs has no sensitive fields; a cross-resource reference is allowed. + b := makeJobsBundle(t) + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.name}")) + }) + diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) + assert.Empty(t, diags) +} + +func TestNoReferenceToSensitiveFields_NonResourceReference(t *testing.T) { + b := makeJobsBundle(t) + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${bundle.name}")) + }) + diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) + assert.Empty(t, diags) +} + +func TestNoReferenceToSensitiveFields_ShortPath(t *testing.T) { + // Reference stops before the field component — must not panic or error. + b := makeJobsBundle(t) + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src}")) + }) + diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) + assert.Empty(t, diags) +} + +func TestNoReferenceToSensitiveFields_NonSensitiveFieldOnSensitiveType(t *testing.T) { + withSensitiveResourceType(t) + b := makeJobsBundle(t) + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.secrets.my_secret.name}")) + }) + diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) + assert.Empty(t, diags) +} + +func TestNoReferenceToSensitiveFields_SensitiveFieldReference(t *testing.T) { + withSensitiveResourceType(t) + b := makeJobsBundle(t) + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.secrets.my_secret.token}")) + }) + diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) + require.Len(t, diags, 1) + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Contains(t, diags[0].Summary, "resources.secrets.my_secret.token") + assert.Contains(t, diags[0].Summary, "sensitive field") +} + +func TestNoReferenceToSensitiveFields_SensitiveFieldInJobName(t *testing.T) { + // Sensitive reference inside a non-sensitive field of another resource is also caught. + withSensitiveResourceType(t) + b := makeJobsBundle(t) + bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { + return dyn.Set(v, "resources.jobs.dst.name", dyn.V("prefix-${resources.secrets.my_secret.token}-suffix")) + }) + diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) + require.Len(t, diags, 1) + assert.Equal(t, diag.Error, diags[0].Severity) + assert.Contains(t, diags[0].Summary, "resources.secrets.my_secret.token") +} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index bfa2af4124b..9a91db34120 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -31,6 +31,7 @@ func Initialize(ctx context.Context, b *bundle.Bundle) { validate.AllResourcesHaveValues(), validate.NoInterpolationInAuthConfig(), validate.NoInterpolationInBundleName(), + validate.NoReferenceToSensitiveFields(), validate.ValidateEngine(), validate.Scripts(), diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index eff442772be..6f25cd7e84a 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -18,6 +18,8 @@ func renderJsonOutput(cmd *cobra.Command, b *bundle.Bundle) error { if b == nil { return nil } + // Mask only in the display copy: the live config must carry the real values + // through the deployment pipeline (e.g. UC secrets must reach the API call). v, err := bundleconfig.MaskSensitiveFields(b.Config.Value()) if err != nil { return err From b5da9aafc219beb4ccb59d3761d6b6c8191a6a94 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 14 Jul 2026 11:22:08 +0200 Subject: [PATCH 6/9] fix lint --- .../config/validate/no_reference_to_sensitive_fields_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/bundle/config/validate/no_reference_to_sensitive_fields_test.go b/bundle/config/validate/no_reference_to_sensitive_fields_test.go index 0b355cf08d8..0975d2d5103 100644 --- a/bundle/config/validate/no_reference_to_sensitive_fields_test.go +++ b/bundle/config/validate/no_reference_to_sensitive_fields_test.go @@ -1,6 +1,7 @@ package validate import ( + "maps" "reflect" "testing" @@ -28,9 +29,7 @@ func withSensitiveResourceType(t *testing.T) { t.Helper() orig := config.ResourcesTypes patched := make(map[string]reflect.Type, len(orig)+1) - for k, v := range orig { - patched[k] = v - } + maps.Copy(patched, orig) patched["secrets"] = reflect.TypeFor[sensitiveTestResource]() config.ResourcesTypes = patched t.Cleanup(func() { config.ResourcesTypes = orig }) From 2e0b5c7e1394f291175d503f40dd2a5c285187a5 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 14 Jul 2026 14:26:41 +0200 Subject: [PATCH 7/9] add new dyn type --- bundle/config/mask.go | 83 ------------- bundle/config/mask_test.go | 76 ------------ .../no_reference_to_sensitive_fields.go | 83 ------------- .../no_reference_to_sensitive_fields_test.go | 115 ------------------ bundle/phases/initialize.go | 1 - cmd/bundle/validate.go | 9 +- libs/dyn/convert/from_typed.go | 5 + libs/dyn/convert/from_typed_test.go | 27 ++++ libs/dyn/convert/struct_info.go | 4 +- libs/dyn/convert/struct_info_test.go | 101 +++++++++++++++ libs/dyn/dynvar/resolve.go | 19 ++- libs/dyn/dynvar/resolve_test.go | 33 +++++ libs/dyn/jsonsaver/marshal.go | 11 ++ libs/dyn/jsonsaver/marshal_test.go | 19 +++ libs/dyn/value.go | 40 +++++- libs/dyn/value_test.go | 33 +++++ libs/dyn/visit.go | 14 ++- libs/dyn/visit_set.go | 14 ++- libs/dyn/yamlsaver/saver.go | 4 + 19 files changed, 306 insertions(+), 385 deletions(-) delete mode 100644 bundle/config/mask.go delete mode 100644 bundle/config/mask_test.go delete mode 100644 bundle/config/validate/no_reference_to_sensitive_fields.go delete mode 100644 bundle/config/validate/no_reference_to_sensitive_fields_test.go diff --git a/bundle/config/mask.go b/bundle/config/mask.go deleted file mode 100644 index c72a98bd83c..00000000000 --- a/bundle/config/mask.go +++ /dev/null @@ -1,83 +0,0 @@ -package config - -import ( - "fmt" - "sync" - - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/dyn/convert" -) - -const sensitiveValueMask = "********" - -// sensitiveFieldsCache caches sensitive field names per resource type key. -var ( - sensitiveFieldsCache map[string]map[string]bool - sensitiveFieldsCacheOnce = sync.OnceFunc(func() { - sensitiveFieldsCache = make(map[string]map[string]bool) - for name, typ := range ResourcesTypes { - if fields := convert.SensitiveFieldNames(typ); len(fields) > 0 { - sensitiveFieldsCache[name] = fields - } - } - }) -) - -// SensitiveFields returns a map of JSON field names → true for a resource type -// key (e.g. "secrets"). Built once from the convert.SensitiveFieldNames helper. -func SensitiveFields(resourceTypeKey string) map[string]bool { - sensitiveFieldsCacheOnce() - return sensitiveFieldsCache[resourceTypeKey] -} - -// MaskSensitiveFields returns a copy of v with all fields tagged -// `bundle:"sensitive"` replaced by [sensitiveValueMask]. -// -// Only the display copy of a dyn.Value should be passed here — -// the live config value that feeds the deployment pipeline must never be masked. -func MaskSensitiveFields(v dyn.Value) (dyn.Value, error) { - // Pattern: resources.. - resourcesPattern := dyn.NewPattern( - dyn.Key("resources"), - dyn.AnyKey(), // resource type (e.g. "secrets") - dyn.AnyKey(), // resource name - ) - - return dyn.MapByPattern(v, resourcesPattern, func(p dyn.Path, resource dyn.Value) (dyn.Value, error) { - if len(p) < 2 { - return resource, nil - } - resourceType := p[1].Key() - fields := SensitiveFields(resourceType) - if len(fields) == 0 { - return resource, nil - } - - for fieldName := range fields { - fv, err := dyn.GetByPath(resource, dyn.NewPath(dyn.Key(fieldName))) - if dyn.IsNoSuchKeyError(err) { - // Field not present in this resource instance — nothing to mask. - continue - } - if err != nil { - return dyn.InvalidValue, err - } - if fv.Kind() == dyn.KindNil { - continue - } - s, ok := fv.AsString() - if !ok { - return dyn.InvalidValue, fmt.Errorf("sensitive field %q must be a string, got %s", fieldName, fv.Kind()) - } - if s == "" { - continue - } - resource, err = dyn.SetByPath(resource, dyn.NewPath(dyn.Key(fieldName)), - dyn.NewValue(sensitiveValueMask, fv.Locations())) - if err != nil { - return dyn.InvalidValue, err - } - } - return resource, nil - }) -} diff --git a/bundle/config/mask_test.go b/bundle/config/mask_test.go deleted file mode 100644 index a7581e0e189..00000000000 --- a/bundle/config/mask_test.go +++ /dev/null @@ -1,76 +0,0 @@ -package config_test - -import ( - "reflect" - "testing" - - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/dyn/convert" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -func TestMaskSensitiveFieldsNoOp(t *testing.T) { - root := config.Root{ - Resources: config.Resources{}, - } - v, err := convert.FromTyped(root, dyn.NilValue) - require.NoError(t, err) - - masked, err := config.MaskSensitiveFields(v) - require.NoError(t, err) - assert.Equal(t, v, masked) -} - -// testSensitiveResource mimics a resource type with a sensitive field, used to -// verify SensitiveFieldNames without relying on resources.Secret (which lives -// on a different branch). -type testSensitiveResource struct { - Name string `json:"name"` - Token string `json:"token" bundle:"sensitive"` -} - -func TestSensitiveFieldNamesReadsTag(t *testing.T) { - fields := convert.SensitiveFieldNames(reflect.TypeFor[testSensitiveResource]()) - assert.True(t, fields["token"], "token should be sensitive") - assert.False(t, fields["name"], "name should not be sensitive") -} - -func TestSensitiveFieldNamesNilForNonStruct(t *testing.T) { - fields := convert.SensitiveFieldNames(reflect.TypeFor[string]()) - assert.Nil(t, fields) -} - -func TestSensitiveFieldNamesPointerDereference(t *testing.T) { - fields := convert.SensitiveFieldNames(reflect.TypeFor[*testSensitiveResource]()) - assert.True(t, fields["token"]) -} - -// TestMaskSensitiveFieldsOnDynValue tests the masking logic directly on a -// constructed dyn.Value tree, without needing resources.Secret to exist. -func TestMaskSensitiveFieldsOnDynValue(t *testing.T) { - // Build a minimal dyn.Value that looks like: - // resources: - // jobs: - // my_job: - // name: "hello" - // - // Since jobs have no sensitive fields, masking should leave it unchanged. - v := dyn.NewValue(map[string]dyn.Value{ - "resources": dyn.NewValue(map[string]dyn.Value{ - "jobs": dyn.NewValue(map[string]dyn.Value{ - "my_job": dyn.NewValue(map[string]dyn.Value{ - "name": dyn.NewValue("hello", nil), - }, nil), - }, nil), - }, nil), - }, nil) - - masked, err := config.MaskSensitiveFields(v) - require.NoError(t, err) - - name, err := dyn.GetByPath(masked, dyn.MustPathFromString("resources.jobs.my_job.name")) - require.NoError(t, err) - assert.Equal(t, "hello", name.MustString()) -} diff --git a/bundle/config/validate/no_reference_to_sensitive_fields.go b/bundle/config/validate/no_reference_to_sensitive_fields.go deleted file mode 100644 index b410b1bb8a1..00000000000 --- a/bundle/config/validate/no_reference_to_sensitive_fields.go +++ /dev/null @@ -1,83 +0,0 @@ -package validate - -import ( - "context" - "fmt" - "strings" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/dyn/convert" - "github.com/databricks/cli/libs/dyn/dynvar" -) - -type noReferenceToSensitiveFields struct{} - -// NoReferenceToSensitiveFields returns a validator that errors when any value -// in the config references a field tagged `bundle:"sensitive"` via interpolation -// (e.g. ${resources.secrets.my_secret.value}). -// -// Sensitive values must never appear in other fields: they would end up in -// validate JSON output, plan output, or debug logs where masking is impractical. -func NoReferenceToSensitiveFields() bundle.Mutator { - return &noReferenceToSensitiveFields{} -} - -func (m *noReferenceToSensitiveFields) Name() string { - return "validate:no_reference_to_sensitive_fields" -} - -func (m *noReferenceToSensitiveFields) Apply(_ context.Context, b *bundle.Bundle) diag.Diagnostics { - var diags diag.Diagnostics - - _ = dyn.WalkReadOnly(b.Config.Value(), func(_ dyn.Path, v dyn.Value) error { - ref, ok := dynvar.NewRef(v) - if !ok { - return nil - } - for _, r := range ref.References() { - if d := checkSensitiveReference(r, v.Locations()); d != nil { - diags = append(diags, *d) - } - } - return nil - }) - - return diags -} - -// checkSensitiveReference returns an error diagnostic when the reference path -// points to a field tagged `bundle:"sensitive"`, or nil otherwise. -// -// It reads config.ResourcesTypes at call time (no extra cache) so tests can -// patch the map before calling and see the effect immediately. -func checkSensitiveReference(ref string, locs []dyn.Location) *diag.Diagnostic { - if !strings.HasPrefix(ref, "resources.") { - return nil - } - p, err := dyn.NewPathFromString(ref) - // Need at least resources.... - if err != nil || len(p) < 4 { - return nil - } - - resourceType := p[1].Key() - fieldName := p[3].Key() - - typ, ok := config.ResourcesTypes[resourceType] - if !ok { - return nil - } - sensitiveFields := convert.SensitiveFieldNames(typ) - if !sensitiveFields[fieldName] { - return nil - } - - return &diag.Diagnostic{ - Severity: diag.Error, - Summary: fmt.Sprintf("${%s}: references a sensitive field and cannot be used in interpolation", ref), - Locations: locs, - } -} diff --git a/bundle/config/validate/no_reference_to_sensitive_fields_test.go b/bundle/config/validate/no_reference_to_sensitive_fields_test.go deleted file mode 100644 index 0975d2d5103..00000000000 --- a/bundle/config/validate/no_reference_to_sensitive_fields_test.go +++ /dev/null @@ -1,115 +0,0 @@ -package validate - -import ( - "maps" - "reflect" - "testing" - - "github.com/databricks/cli/bundle" - "github.com/databricks/cli/bundle/config" - "github.com/databricks/cli/bundle/config/resources" - "github.com/databricks/cli/bundle/internal/bundletest" - "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/dyn" - "github.com/databricks/databricks-sdk-go/service/jobs" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" -) - -// sensitiveTestResource is a minimal struct with one sensitive field, used to -// exercise the validator without a real resource that carries `bundle:"sensitive"`. -type sensitiveTestResource struct { - Name string `json:"name"` - Token string `json:"token" bundle:"sensitive"` -} - -// withSensitiveResourceType registers sensitiveTestResource under the key -// "secrets" in config.ResourcesTypes for the duration of the test. -func withSensitiveResourceType(t *testing.T) { - t.Helper() - orig := config.ResourcesTypes - patched := make(map[string]reflect.Type, len(orig)+1) - maps.Copy(patched, orig) - patched["secrets"] = reflect.TypeFor[sensitiveTestResource]() - config.ResourcesTypes = patched - t.Cleanup(func() { config.ResourcesTypes = orig }) -} - -func makeJobsBundle(t *testing.T) *bundle.Bundle { - t.Helper() - return &bundle.Bundle{ - Config: config.Root{ - Resources: config.Resources{ - Jobs: map[string]*resources.Job{ - "src": {JobSettings: jobs.JobSettings{Name: "source"}}, - "dst": {JobSettings: jobs.JobSettings{Name: "placeholder"}}, - }, - }, - }, - } -} - -func TestNoReferenceToSensitiveFields_NoSensitiveType(t *testing.T) { - // jobs has no sensitive fields; a cross-resource reference is allowed. - b := makeJobsBundle(t) - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src.name}")) - }) - diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) - assert.Empty(t, diags) -} - -func TestNoReferenceToSensitiveFields_NonResourceReference(t *testing.T) { - b := makeJobsBundle(t) - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${bundle.name}")) - }) - diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) - assert.Empty(t, diags) -} - -func TestNoReferenceToSensitiveFields_ShortPath(t *testing.T) { - // Reference stops before the field component — must not panic or error. - b := makeJobsBundle(t) - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.jobs.src}")) - }) - diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) - assert.Empty(t, diags) -} - -func TestNoReferenceToSensitiveFields_NonSensitiveFieldOnSensitiveType(t *testing.T) { - withSensitiveResourceType(t) - b := makeJobsBundle(t) - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.secrets.my_secret.name}")) - }) - diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) - assert.Empty(t, diags) -} - -func TestNoReferenceToSensitiveFields_SensitiveFieldReference(t *testing.T) { - withSensitiveResourceType(t) - b := makeJobsBundle(t) - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.jobs.dst.name", dyn.V("${resources.secrets.my_secret.token}")) - }) - diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) - require.Len(t, diags, 1) - assert.Equal(t, diag.Error, diags[0].Severity) - assert.Contains(t, diags[0].Summary, "resources.secrets.my_secret.token") - assert.Contains(t, diags[0].Summary, "sensitive field") -} - -func TestNoReferenceToSensitiveFields_SensitiveFieldInJobName(t *testing.T) { - // Sensitive reference inside a non-sensitive field of another resource is also caught. - withSensitiveResourceType(t) - b := makeJobsBundle(t) - bundletest.Mutate(t, b, func(v dyn.Value) (dyn.Value, error) { - return dyn.Set(v, "resources.jobs.dst.name", dyn.V("prefix-${resources.secrets.my_secret.token}-suffix")) - }) - diags := NoReferenceToSensitiveFields().Apply(t.Context(), b) - require.Len(t, diags, 1) - assert.Equal(t, diag.Error, diags[0].Severity) - assert.Contains(t, diags[0].Summary, "resources.secrets.my_secret.token") -} diff --git a/bundle/phases/initialize.go b/bundle/phases/initialize.go index 9a91db34120..bfa2af4124b 100644 --- a/bundle/phases/initialize.go +++ b/bundle/phases/initialize.go @@ -31,7 +31,6 @@ func Initialize(ctx context.Context, b *bundle.Bundle) { validate.AllResourcesHaveValues(), validate.NoInterpolationInAuthConfig(), validate.NoInterpolationInBundleName(), - validate.NoReferenceToSensitiveFields(), validate.ValidateEngine(), validate.Scripts(), diff --git a/cmd/bundle/validate.go b/cmd/bundle/validate.go index 6f25cd7e84a..a2ec31f721b 100644 --- a/cmd/bundle/validate.go +++ b/cmd/bundle/validate.go @@ -5,7 +5,6 @@ import ( "fmt" "github.com/databricks/cli/bundle" - bundleconfig "github.com/databricks/cli/bundle/config" "github.com/databricks/cli/bundle/render" "github.com/databricks/cli/cmd/bundle/utils" "github.com/databricks/cli/cmd/root" @@ -18,13 +17,7 @@ func renderJsonOutput(cmd *cobra.Command, b *bundle.Bundle) error { if b == nil { return nil } - // Mask only in the display copy: the live config must carry the real values - // through the deployment pipeline (e.g. UC secrets must reach the API call). - v, err := bundleconfig.MaskSensitiveFields(b.Config.Value()) - if err != nil { - return err - } - buf, err := json.MarshalIndent(v.AsAny(), "", " ") + buf, err := json.MarshalIndent(b.Config.Value().AsAny(), "", " ") if err != nil { return err } diff --git a/libs/dyn/convert/from_typed.go b/libs/dyn/convert/from_typed.go index 66451124293..f27f7359760 100644 --- a/libs/dyn/convert/from_typed.go +++ b/libs/dyn/convert/from_typed.go @@ -133,6 +133,11 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptio return dyn.InvalidValue, err } + // Mark the value as sensitive if the field carries the bundle:"sensitive" tag. + if info.Sensitive[k] { + nv = nv.MarkSensitive() + } + // Either if the key was set in the reference, the field is not zero-valued, OR it's forced if ok || nv.Kind() != dyn.KindNil || isForced { // If v isZero, it could be because it's a variable reference; so we check that nv is zero as well diff --git a/libs/dyn/convert/from_typed_test.go b/libs/dyn/convert/from_typed_test.go index 0c8cf902bb4..748a1b010ae 100644 --- a/libs/dyn/convert/from_typed_test.go +++ b/libs/dyn/convert/from_typed_test.go @@ -926,3 +926,30 @@ func TestFromTypedForceSendFieldsEmbedded(t *testing.T) { assert.Equal(t, dyn.KindNil, field.Kind(), "embedded field should be present due to ForceSendFields") assert.Equal(t, dyn.V("value"), other) } + +func TestFromTypedSensitiveField(t *testing.T) { + type Tmp struct { + Name string `json:"name"` + Token string `json:"token" bundle:"sensitive"` + } + + src := Tmp{ + Name: "my-resource", + Token: "super-secret", + } + + nv, err := FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + name := nv.Get("name") + token := nv.Get("token") + + assert.False(t, name.IsSensitive(), "name should not be sensitive") + assert.True(t, token.IsSensitive(), "token should be sensitive due to bundle:\"sensitive\" tag") + + // MustString returns the real value. + assert.Equal(t, "super-secret", token.MustString()) + // AsAny returns the redaction placeholder. + assert.Equal(t, "********", token.AsAny()) +} + diff --git a/libs/dyn/convert/struct_info.go b/libs/dyn/convert/struct_info.go index ab13dfa6ad7..c78b49a0184 100644 --- a/libs/dyn/convert/struct_info.go +++ b/libs/dyn/convert/struct_info.go @@ -31,6 +31,7 @@ type structInfo struct { // Sensitive tracks fields tagged `bundle:"sensitive"` by their JSON name. // Values for these fields should be masked in display output. Sensitive map[string]bool + // ForceSendFieldsIndex maps the JSON-name of the field to the index path (for // use with [reflect.Value.FieldByIndex]) of the ForceSendFields slice that // governs it: the one declared by the struct that also declares the field. @@ -138,7 +139,8 @@ func buildStructInfo(typ reflect.Type) structInfo { } out.GolangNames[name] = sf.Name - if structtag.BundleTag(sf.Tag.Get("bundle")).Sensitive() { + btag := structtag.BundleTag(sf.Tag.Get("bundle")) + if btag.Sensitive() { out.Sensitive[name] = true } diff --git a/libs/dyn/convert/struct_info_test.go b/libs/dyn/convert/struct_info_test.go index f921523c391..9a0a68ec9d6 100644 --- a/libs/dyn/convert/struct_info_test.go +++ b/libs/dyn/convert/struct_info_test.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/libs/dyn" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestStructInfoPlain(t *testing.T) { @@ -226,3 +227,103 @@ func TestStructInfoValueFieldMultiple(t *testing.T) { getStructInfo(reflect.TypeFor[Tmp]()) }) } + +func TestSensitiveFieldNamesPlain(t *testing.T) { + type Tmp struct { + Name string `json:"name"` + Token string `json:"token" bundle:"sensitive"` + } + + fields := SensitiveFieldNames(reflect.TypeFor[Tmp]()) + assert.True(t, fields["token"]) + assert.False(t, fields["name"]) +} + +func TestSensitiveFieldNamesPointerDereference(t *testing.T) { + type Tmp struct { + Token string `json:"token" bundle:"sensitive"` + } + + fields := SensitiveFieldNames(reflect.TypeFor[*Tmp]()) + assert.True(t, fields["token"]) +} + +func TestSensitiveFieldNamesNilForNonStruct(t *testing.T) { + assert.Nil(t, SensitiveFieldNames(reflect.TypeFor[string]())) +} + +func TestSensitiveFieldNamesNilWhenNone(t *testing.T) { + type Tmp struct { + Name string `json:"name"` + } + + assert.Nil(t, SensitiveFieldNames(reflect.TypeFor[Tmp]())) +} + +func TestSensitiveFieldNamesEmbeddedByValue(t *testing.T) { + type Inner struct { + Token string `json:"token" bundle:"sensitive"` + } + + type Outer struct { + Name string `json:"name"` + Inner + } + + fields := SensitiveFieldNames(reflect.TypeFor[Outer]()) + assert.True(t, fields["token"]) + assert.False(t, fields["name"]) +} + +func TestSensitiveFieldNamesEmbeddedByPointer(t *testing.T) { + type Inner struct { + Token string `json:"token" bundle:"sensitive"` + } + + type Outer struct { + Name string `json:"name"` + *Inner + } + + fields := SensitiveFieldNames(reflect.TypeFor[Outer]()) + assert.True(t, fields["token"]) + assert.False(t, fields["name"]) +} + +func TestSensitiveFieldNamesTopLevelPrecedence(t *testing.T) { + // A sensitive field in an embedded struct is shadowed by a non-sensitive + // field of the same JSON name at the top level — top level wins. + type Inner struct { + Token string `json:"token" bundle:"sensitive"` + } + + type Outer struct { + Token string `json:"token"` // not sensitive; shadows Inner.Token + Inner + } + + fields := SensitiveFieldNames(reflect.TypeFor[Outer]()) + assert.False(t, fields["token"]) +} + +// TestSensitiveFieldNamesUsage demonstrates using SensitiveFieldNames with +// FromTyped: a sensitive field's value round-trips through dyn.Value and the +// caller can check which fields to mask before marshaling. +func TestSensitiveFieldNamesUsage(t *testing.T) { + type Resource struct { + Name string `json:"name"` + Token string `json:"token" bundle:"sensitive"` + } + + src := Resource{Name: "my-resource", Token: "s3cr3t"} + v, err := FromTyped(src, dyn.NilValue) + require.NoError(t, err) + + fields := SensitiveFieldNames(reflect.TypeFor[Resource]()) + assert.True(t, fields["token"], "token should be identified as sensitive") + + // Verify the value round-tripped correctly before masking. + tok, err := dyn.GetByPath(v, dyn.NewPath(dyn.Key("token"))) + require.NoError(t, err) + assert.Equal(t, "s3cr3t", tok.MustString()) +} diff --git a/libs/dyn/dynvar/resolve.go b/libs/dyn/dynvar/resolve.go index 6f9269f4974..fa6abe5b42b 100644 --- a/libs/dyn/dynvar/resolve.go +++ b/libs/dyn/dynvar/resolve.go @@ -153,10 +153,16 @@ func (r *resolver) resolveRef(ref Ref, seen []string) (dyn.Value, error) { // of where it is used. This also means that relative path resolution is done // relative to where a variable is used, not where it is defined. // - return dyn.NewValue(resolved[0].Value(), ref.Value.Locations()), nil + result := dyn.NewValue(resolved[0].Value(), ref.Value.Locations()) + if resolved[0].IsSensitive() { + result = result.MarkSensitive() + } + return result, nil } // Not pure; perform string interpolation. + // Track whether any resolved value is sensitive; if so, the result is also sensitive. + anySensitive := false for j := range ref.Matches { // The value is invalid if resolution returned [ErrSkipResolution]. // We must skip those and leave the original variable reference in place. @@ -164,7 +170,12 @@ func (r *resolver) resolveRef(ref Ref, seen []string) (dyn.Value, error) { continue } + if resolved[j].IsSensitive() { + anySensitive = true + } + // Try to turn the resolved value into a string. + // Use AsString (not AsAny) to get the real value even for sensitive strings. s, ok := resolved[j].AsString() if !ok { // Only allow primitive types to be converted to string. @@ -179,7 +190,11 @@ func (r *resolver) resolveRef(ref Ref, seen []string) (dyn.Value, error) { ref.Str = strings.Replace(ref.Str, ref.Matches[j][0], s, 1) } - return dyn.NewValue(ref.Str, ref.Value.Locations()), nil + result := dyn.NewValue(ref.Str, ref.Value.Locations()) + if anySensitive { + result = result.MarkSensitive() + } + return result, nil } func (r *resolver) resolveKey(key string, seen []string) (dyn.Value, error) { diff --git a/libs/dyn/dynvar/resolve_test.go b/libs/dyn/dynvar/resolve_test.go index c2bbaae74f1..c31c353ac89 100644 --- a/libs/dyn/dynvar/resolve_test.go +++ b/libs/dyn/dynvar/resolve_test.go @@ -371,6 +371,39 @@ func TestResolveMapVariable(t *testing.T) { assert.Equal(t, "value2", getByPath(t, mapVal, "key2").MustString()) } +func TestResolveSensitivePureSubstitution(t *testing.T) { + // A pure reference to a sensitive value must produce a sensitive result. + in := dyn.V(map[string]dyn.Value{ + "secret": dyn.V("top-secret").MarkSensitive(), + "ref": dyn.V("${secret}"), + }) + + out, err := dynvar.Resolve(in, dynvar.DefaultLookup(in)) + require.NoError(t, err) + + result := getByPath(t, out, "ref") + assert.True(t, result.IsSensitive()) + // MustString returns the real value even when sensitive. + assert.Equal(t, "top-secret", result.MustString()) +} + +func TestResolveSensitiveStringInterpolation(t *testing.T) { + // When any resolved value is sensitive, the interpolated result must also be sensitive. + in := dyn.V(map[string]dyn.Value{ + "secret": dyn.V("password123").MarkSensitive(), + "prefix": dyn.V("token"), + "ref": dyn.V("${prefix}:${secret}"), + }) + + out, err := dynvar.Resolve(in, dynvar.DefaultLookup(in)) + require.NoError(t, err) + + result := getByPath(t, out, "ref") + assert.True(t, result.IsSensitive()) + // The real interpolated string is still accessible. + assert.Equal(t, "token:password123", result.MustString()) +} + func TestResolveSequenceVariable(t *testing.T) { in := dyn.V(map[string]dyn.Value{ "seq": dyn.V([]dyn.Value{ diff --git a/libs/dyn/jsonsaver/marshal.go b/libs/dyn/jsonsaver/marshal.go index a78a68f2998..5ffcc3767b2 100644 --- a/libs/dyn/jsonsaver/marshal.go +++ b/libs/dyn/jsonsaver/marshal.go @@ -39,6 +39,17 @@ func (w wrap) MarshalJSON() ([]byte, error) { // marshalValue recursively writes JSON for a [dyn.Value] to the buffer. func marshalValue(buf *bytes.Buffer, v dyn.Value) error { + if v.IsSensitive() { + out, err := marshalNoEscape("********") + if err != nil { + return err + } + // The encoder writes a trailing newline, so we need to remove it. + out = out[:len(out)-1] + buf.Write(out) + return nil + } + switch v.Kind() { case dyn.KindString, dyn.KindBool, dyn.KindInt, dyn.KindFloat, dyn.KindTime, dyn.KindNil: out, err := marshalNoEscape(v.AsAny()) diff --git a/libs/dyn/jsonsaver/marshal_test.go b/libs/dyn/jsonsaver/marshal_test.go index 68c2017397f..5eaf09668f8 100644 --- a/libs/dyn/jsonsaver/marshal_test.go +++ b/libs/dyn/jsonsaver/marshal_test.go @@ -64,6 +64,25 @@ func TestMarshal_Sequence(t *testing.T) { } } +func TestMarshal_Sensitive(t *testing.T) { + v := dyn.V("real-secret").MarkSensitive() + b, err := Marshal(v) + if assert.NoError(t, err) { + assert.JSONEq(t, `"********"`, string(b)) + } +} + +func TestMarshal_SensitiveInMap(t *testing.T) { + m := dyn.NewMapping() + m.SetLoc("token", nil, dyn.V("my-token").MarkSensitive()) + m.SetLoc("name", nil, dyn.V("public")) + + b, err := Marshal(dyn.V(m)) + if assert.NoError(t, err) { + assert.JSONEq(t, `{"token":"********","name":"public"}`, string(b)) + } +} + func TestMarshal_Complex(t *testing.T) { map1 := dyn.NewMapping() map1.SetLoc("str1", nil, dyn.V("value1")) diff --git a/libs/dyn/value.go b/libs/dyn/value.go index 72803511b80..51d8ad1824d 100644 --- a/libs/dyn/value.go +++ b/libs/dyn/value.go @@ -19,6 +19,11 @@ type Value struct { // Whether or not this value is an anchor. // If this node doesn't map to a type, we don't need to warn about it. anchor bool + + // Whether or not this value is sensitive. + // Sensitive values are replaced by a redaction placeholder in JSON/YAML output. + // The underlying value is still accessible via AsString/MustString. + sensitive bool } // InvalidValue is equal to the zero-value of Value. @@ -61,18 +66,39 @@ func (v Value) WithLocations(loc []Location) Value { // create a copy of the locations, so that mutations to the original slice // don't affect new value. - l: slices.Clone(loc), + l: slices.Clone(loc), + sensitive: v.sensitive, } } func (v Value) AppendLocationsFromValue(w Value) Value { return Value{ - v: v.v, - k: v.k, - l: append(v.l, w.l...), + v: v.v, + k: v.k, + l: append(v.l, w.l...), + sensitive: v.sensitive, } } +// MarkSensitive returns a copy of the value marked as sensitive. +// When a sensitive value is marshaled to JSON or YAML, it is replaced by a redaction placeholder. +// The underlying value is still accessible via AsString/MustString for use in the deployment pipeline. +func (v Value) MarkSensitive() Value { + v.sensitive = true + return v +} + +// IsSensitive reports whether this value is marked sensitive. +func (v Value) IsSensitive() bool { + return v.sensitive +} + +// WithSensitive returns a copy of the value with the sensitive flag set to the given value. +func (v Value) WithSensitive(sensitive bool) Value { + v.sensitive = sensitive + return v +} + func (v Value) Kind() Kind { return v.k } @@ -120,6 +146,9 @@ func (v Value) AsAny() any { case KindNil: return v.v case KindString: + if v.sensitive { + return "********" + } return v.v case KindBool: return v.v @@ -203,6 +232,9 @@ func (v Value) eq(w Value) bool { if !slices.Equal(v.l, w.l) { return false } + if v.sensitive != w.sensitive { + return false + } switch v.k { case KindMap: diff --git a/libs/dyn/value_test.go b/libs/dyn/value_test.go index 8717aae620e..f5132559093 100644 --- a/libs/dyn/value_test.go +++ b/libs/dyn/value_test.go @@ -50,6 +50,39 @@ func TestValueIsValid(t *testing.T) { assert.True(t, intValue.IsValid()) } +func TestMarkSensitive(t *testing.T) { + v := dyn.V("secret-value") + assert.False(t, v.IsSensitive()) + + sv := v.MarkSensitive() + assert.True(t, sv.IsSensitive()) + + // AsAny returns the redaction placeholder for sensitive strings. + assert.Equal(t, "********", sv.AsAny()) + + // MustString returns the real underlying value. + assert.Equal(t, "secret-value", sv.MustString()) +} + +func TestSensitivePreservedByWithLocations(t *testing.T) { + locs := []dyn.Location{{File: "file", Line: 1, Column: 1}} + v := dyn.V("secret").MarkSensitive() + v2 := v.WithLocations(locs) + assert.True(t, v2.IsSensitive()) + assert.Equal(t, locs, v2.Locations()) +} + +func TestSensitivePreservedByWithSensitive(t *testing.T) { + v := dyn.V("secret").MarkSensitive() + assert.True(t, v.IsSensitive()) + + v2 := v.WithSensitive(false) + assert.False(t, v2.IsSensitive()) + + v3 := v2.WithSensitive(true) + assert.True(t, v3.IsSensitive()) +} + func TestIsZero(t *testing.T) { assert.True(t, dyn.V(0).IsZero(), "int") assert.True(t, dyn.V(int(0)).IsZero(), "int") diff --git a/libs/dyn/visit.go b/libs/dyn/visit.go index 7ae00fa8e08..89b137596ce 100644 --- a/libs/dyn/visit.go +++ b/libs/dyn/visit.go @@ -142,9 +142,10 @@ func (c pathComponent) visit(v Value, prefix Path, suffix Pattern, opts visitOpt m = m.Clone() m.SetLoc(c.key, nil, nv) return Value{ - v: m, - k: KindMap, - l: v.l, + v: m, + k: KindMap, + l: v.l, + sensitive: v.sensitive, }, nil case c.isIndex(): @@ -181,9 +182,10 @@ func (c pathComponent) visit(v Value, prefix Path, suffix Pattern, opts visitOpt s = slices.Clone(s) s[c.index] = nv return Value{ - v: s, - k: KindSequence, - l: v.l, + v: s, + k: KindSequence, + l: v.l, + sensitive: v.sensitive, }, nil default: diff --git a/libs/dyn/visit_set.go b/libs/dyn/visit_set.go index 1b3c7f1b661..009bab1e0ba 100644 --- a/libs/dyn/visit_set.go +++ b/libs/dyn/visit_set.go @@ -43,9 +43,10 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) { m = m.Clone() m.SetLoc(component.key, nil, nv) return Value{ - v: m, - k: KindMap, - l: v.l, + v: m, + k: KindMap, + l: v.l, + sensitive: v.sensitive, }, nil case component.isIndex(): @@ -64,9 +65,10 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) { s = slices.Clone(s) s[component.index] = nv return Value{ - v: s, - k: KindSequence, - l: v.l, + v: s, + k: KindSequence, + l: v.l, + sensitive: v.sensitive, }, nil default: diff --git a/libs/dyn/yamlsaver/saver.go b/libs/dyn/yamlsaver/saver.go index 4c302e26f0d..591fabff405 100644 --- a/libs/dyn/yamlsaver/saver.go +++ b/libs/dyn/yamlsaver/saver.go @@ -72,6 +72,10 @@ func (s *saver) toYamlNode(v dyn.Value) (*yaml.Node, error) { } func (s *saver) toYamlNodeWithStyle(v dyn.Value, style yaml.Style) (*yaml.Node, error) { + if v.IsSensitive() { + return &yaml.Node{Kind: yaml.ScalarNode, Value: "********", Style: style}, nil + } + switch v.Kind() { case dyn.KindMap: m, _ := v.AsMap() From f52fb6ab8c96a30ca03625ca687b34ed1442a9d5 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 14 Jul 2026 14:39:44 +0200 Subject: [PATCH 8/9] fix fmt --- libs/dyn/convert/from_typed_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/libs/dyn/convert/from_typed_test.go b/libs/dyn/convert/from_typed_test.go index 748a1b010ae..307087bac69 100644 --- a/libs/dyn/convert/from_typed_test.go +++ b/libs/dyn/convert/from_typed_test.go @@ -952,4 +952,3 @@ func TestFromTypedSensitiveField(t *testing.T) { // AsAny returns the redaction placeholder. assert.Equal(t, "********", token.AsAny()) } - From 5ae8b6d5897f3f5af69735752fa388f3a65842bf Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Thu, 16 Jul 2026 14:03:33 +0200 Subject: [PATCH 9/9] added typed leaf --- libs/dyn/convert/from_typed.go | 10 ++++-- libs/dyn/convert/from_typed_test.go | 2 +- libs/dyn/convert/normalize.go | 4 +++ libs/dyn/dynvar/resolve.go | 8 +++-- libs/dyn/dynvar/resolve_test.go | 4 +-- libs/dyn/jsonsaver/marshal.go | 2 +- libs/dyn/jsonsaver/marshal_test.go | 8 ++--- libs/dyn/kind.go | 2 +- libs/dyn/sensitive.go | 31 +++++++++++++++++++ libs/dyn/value.go | 48 ++++++++++++----------------- libs/dyn/value_test.go | 41 +++++++++++------------- libs/dyn/value_underlying.go | 11 +++++-- libs/dyn/visit.go | 14 ++++----- libs/dyn/visit_set.go | 14 ++++----- libs/dyn/yamlsaver/saver.go | 2 +- 15 files changed, 115 insertions(+), 86 deletions(-) create mode 100644 libs/dyn/sensitive.go diff --git a/libs/dyn/convert/from_typed.go b/libs/dyn/convert/from_typed.go index f27f7359760..d2c08773731 100644 --- a/libs/dyn/convert/from_typed.go +++ b/libs/dyn/convert/from_typed.go @@ -133,9 +133,13 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptio return dyn.InvalidValue, err } - // Mark the value as sensitive if the field carries the bundle:"sensitive" tag. - if info.Sensitive[k] { - nv = nv.MarkSensitive() + // If the field carries the bundle:"sensitive" tag and resolved to a non-empty + // string, wrap it as a sensitive value so that all downstream serializers + // (JSON, YAML) redact it automatically. + if info.Sensitive[k] && nv.Kind() == dyn.KindString { + if s, ok := nv.AsString(); ok && s != "" { + nv = dyn.NewSensitiveValue(s, nv.Locations()) + } } // Either if the key was set in the reference, the field is not zero-valued, OR it's forced diff --git a/libs/dyn/convert/from_typed_test.go b/libs/dyn/convert/from_typed_test.go index 307087bac69..20750a647a4 100644 --- a/libs/dyn/convert/from_typed_test.go +++ b/libs/dyn/convert/from_typed_test.go @@ -950,5 +950,5 @@ func TestFromTypedSensitiveField(t *testing.T) { // MustString returns the real value. assert.Equal(t, "super-secret", token.MustString()) // AsAny returns the redaction placeholder. - assert.Equal(t, "********", token.AsAny()) + assert.Equal(t, dyn.SensitiveValueRedacted, token.AsAny()) } diff --git a/libs/dyn/convert/normalize.go b/libs/dyn/convert/normalize.go index 79cfee37441..eb90b7bf632 100644 --- a/libs/dyn/convert/normalize.go +++ b/libs/dyn/convert/normalize.go @@ -285,6 +285,10 @@ func (n normalizeOptions) normalizeString(typ reflect.Type, src dyn.Value, path switch src.Kind() { case dyn.KindString: + // Preserve sensitive strings: NewValue would strip the secretString wrapper. + if src.IsSensitive() { + return src, nil + } return dyn.NewValue(src.MustString(), src.Locations()), nil case dyn.KindBool: return dyn.NewValue(strconv.FormatBool(src.MustBool()), src.Locations()), nil diff --git a/libs/dyn/dynvar/resolve.go b/libs/dyn/dynvar/resolve.go index fa6abe5b42b..612f9c26fad 100644 --- a/libs/dyn/dynvar/resolve.go +++ b/libs/dyn/dynvar/resolve.go @@ -153,11 +153,13 @@ func (r *resolver) resolveRef(ref Ref, seen []string) (dyn.Value, error) { // of where it is used. This also means that relative path resolution is done // relative to where a variable is used, not where it is defined. // - result := dyn.NewValue(resolved[0].Value(), ref.Value.Locations()) + // Preserve sensitivity: NewValue strips the secretString wrapper, so use + // NewSensitiveValue when the resolved value is a sensitive string. if resolved[0].IsSensitive() { - result = result.MarkSensitive() + s, _ := resolved[0].AsString() + return dyn.NewSensitiveValue(s, ref.Value.Locations()), nil } - return result, nil + return dyn.NewValue(resolved[0].Value(), ref.Value.Locations()), nil } // Not pure; perform string interpolation. diff --git a/libs/dyn/dynvar/resolve_test.go b/libs/dyn/dynvar/resolve_test.go index c31c353ac89..3399f2057a6 100644 --- a/libs/dyn/dynvar/resolve_test.go +++ b/libs/dyn/dynvar/resolve_test.go @@ -374,7 +374,7 @@ func TestResolveMapVariable(t *testing.T) { func TestResolveSensitivePureSubstitution(t *testing.T) { // A pure reference to a sensitive value must produce a sensitive result. in := dyn.V(map[string]dyn.Value{ - "secret": dyn.V("top-secret").MarkSensitive(), + "secret": dyn.NewSensitiveValue("top-secret", nil), "ref": dyn.V("${secret}"), }) @@ -390,7 +390,7 @@ func TestResolveSensitivePureSubstitution(t *testing.T) { func TestResolveSensitiveStringInterpolation(t *testing.T) { // When any resolved value is sensitive, the interpolated result must also be sensitive. in := dyn.V(map[string]dyn.Value{ - "secret": dyn.V("password123").MarkSensitive(), + "secret": dyn.NewSensitiveValue("password123", nil), "prefix": dyn.V("token"), "ref": dyn.V("${prefix}:${secret}"), }) diff --git a/libs/dyn/jsonsaver/marshal.go b/libs/dyn/jsonsaver/marshal.go index 5ffcc3767b2..cbeb5b28919 100644 --- a/libs/dyn/jsonsaver/marshal.go +++ b/libs/dyn/jsonsaver/marshal.go @@ -40,7 +40,7 @@ func (w wrap) MarshalJSON() ([]byte, error) { // marshalValue recursively writes JSON for a [dyn.Value] to the buffer. func marshalValue(buf *bytes.Buffer, v dyn.Value) error { if v.IsSensitive() { - out, err := marshalNoEscape("********") + out, err := marshalNoEscape(dyn.SensitiveValueRedacted) if err != nil { return err } diff --git a/libs/dyn/jsonsaver/marshal_test.go b/libs/dyn/jsonsaver/marshal_test.go index 5eaf09668f8..9866f46dc37 100644 --- a/libs/dyn/jsonsaver/marshal_test.go +++ b/libs/dyn/jsonsaver/marshal_test.go @@ -65,21 +65,21 @@ func TestMarshal_Sequence(t *testing.T) { } func TestMarshal_Sensitive(t *testing.T) { - v := dyn.V("real-secret").MarkSensitive() + v := dyn.NewSensitiveValue("real-secret", nil) b, err := Marshal(v) if assert.NoError(t, err) { - assert.JSONEq(t, `"********"`, string(b)) + assert.JSONEq(t, `"`+dyn.SensitiveValueRedacted+`"`, string(b)) } } func TestMarshal_SensitiveInMap(t *testing.T) { m := dyn.NewMapping() - m.SetLoc("token", nil, dyn.V("my-token").MarkSensitive()) + m.SetLoc("token", nil, dyn.NewSensitiveValue("my-token", nil)) m.SetLoc("name", nil, dyn.V("public")) b, err := Marshal(dyn.V(m)) if assert.NoError(t, err) { - assert.JSONEq(t, `{"token":"********","name":"public"}`, string(b)) + assert.JSONEq(t, `{"token":"`+dyn.SensitiveValueRedacted+`","name":"public"}`, string(b)) } } diff --git a/libs/dyn/kind.go b/libs/dyn/kind.go index 1890e9e2ccd..684239503b3 100644 --- a/libs/dyn/kind.go +++ b/libs/dyn/kind.go @@ -25,7 +25,7 @@ func kindOf(v any) Kind { return KindMap case []Value: return KindSequence - case string: + case string, secretString: return KindString case bool: return KindBool diff --git a/libs/dyn/sensitive.go b/libs/dyn/sensitive.go new file mode 100644 index 00000000000..e6c363d95ab --- /dev/null +++ b/libs/dyn/sensitive.go @@ -0,0 +1,31 @@ +package dyn + +import "slices" + +// SensitiveValueRedacted is the placeholder emitted in JSON/YAML output +// whenever a sensitive string value is serialized. +const SensitiveValueRedacted = "" + +// secretString is the internal storage type for sensitive string values. +// Using a distinct Go type (rather than a plain string plus a flag) makes +// sensitivity impossible to strip by accident: every path that copies v.v +// preserves the wrapper automatically, and a switch on v.v that handles +// `string` but not `secretString` will miss it — making omissions auditable. +// +// Kind() still returns KindString so all existing switch-on-Kind logic +// continues to work unchanged. +type secretString struct { + value string +} + +// NewSensitiveValue returns a KindString Value whose content is treated as +// sensitive. JSON and YAML serializers replace it with [SensitiveValueRedacted]; +// AsString / MustString still return the real value for use in the deployment +// pipeline. +func NewSensitiveValue(s string, loc []Location) Value { + return Value{ + v: secretString{s}, + k: KindString, + l: slices.Clone(loc), + } +} diff --git a/libs/dyn/value.go b/libs/dyn/value.go index 51d8ad1824d..e6ea328f082 100644 --- a/libs/dyn/value.go +++ b/libs/dyn/value.go @@ -19,11 +19,6 @@ type Value struct { // Whether or not this value is an anchor. // If this node doesn't map to a type, we don't need to warn about it. anchor bool - - // Whether or not this value is sensitive. - // Sensitive values are replaced by a redaction placeholder in JSON/YAML output. - // The underlying value is still accessible via AsString/MustString. - sensitive bool } // InvalidValue is equal to the zero-value of Value. @@ -66,36 +61,33 @@ func (v Value) WithLocations(loc []Location) Value { // create a copy of the locations, so that mutations to the original slice // don't affect new value. - l: slices.Clone(loc), - sensitive: v.sensitive, + l: slices.Clone(loc), } } func (v Value) AppendLocationsFromValue(w Value) Value { return Value{ - v: v.v, - k: v.k, - l: append(v.l, w.l...), - sensitive: v.sensitive, + v: v.v, + k: v.k, + l: append(v.l, w.l...), } } -// MarkSensitive returns a copy of the value marked as sensitive. -// When a sensitive value is marshaled to JSON or YAML, it is replaced by a redaction placeholder. -// The underlying value is still accessible via AsString/MustString for use in the deployment pipeline. -func (v Value) MarkSensitive() Value { - v.sensitive = true - return v -} - -// IsSensitive reports whether this value is marked sensitive. +// IsSensitive reports whether this value holds a sensitive string. +// Sensitivity is encoded in the type of v.v (secretString vs plain string), +// so it is preserved automatically whenever v.v is copied. func (v Value) IsSensitive() bool { - return v.sensitive + _, ok := v.v.(secretString) + return ok } -// WithSensitive returns a copy of the value with the sensitive flag set to the given value. -func (v Value) WithSensitive(sensitive bool) Value { - v.sensitive = sensitive +// MarkSensitive returns a copy of this value marked as sensitive. +// If the value is already a KindString, its content is re-wrapped as a +// secretString; otherwise the value is returned unchanged. +func (v Value) MarkSensitive() Value { + if s, ok := v.v.(string); ok { + v.v = secretString{s} + } return v } @@ -146,8 +138,9 @@ func (v Value) AsAny() any { case KindNil: return v.v case KindString: - if v.sensitive { - return "********" + // secretString holds a sensitive value; return the redaction placeholder. + if _, ok := v.v.(secretString); ok { + return SensitiveValueRedacted } return v.v case KindBool: @@ -232,9 +225,6 @@ func (v Value) eq(w Value) bool { if !slices.Equal(v.l, w.l) { return false } - if v.sensitive != w.sensitive { - return false - } switch v.k { case KindMap: diff --git a/libs/dyn/value_test.go b/libs/dyn/value_test.go index f5132559093..a2edf4e0128 100644 --- a/libs/dyn/value_test.go +++ b/libs/dyn/value_test.go @@ -50,37 +50,32 @@ func TestValueIsValid(t *testing.T) { assert.True(t, intValue.IsValid()) } -func TestMarkSensitive(t *testing.T) { - v := dyn.V("secret-value") - assert.False(t, v.IsSensitive()) +func TestNewSensitiveValue(t *testing.T) { + v := dyn.NewSensitiveValue("secret-value", nil) + assert.True(t, v.IsSensitive()) + assert.Equal(t, dyn.KindString, v.Kind()) - sv := v.MarkSensitive() - assert.True(t, sv.IsSensitive()) + // AsAny returns the redaction placeholder. + assert.Equal(t, dyn.SensitiveValueRedacted, v.AsAny()) - // AsAny returns the redaction placeholder for sensitive strings. - assert.Equal(t, "********", sv.AsAny()) + // AsString / MustString return the real underlying value. + s, ok := v.AsString() + assert.True(t, ok) + assert.Equal(t, "secret-value", s) + assert.Equal(t, "secret-value", v.MustString()) +} - // MustString returns the real underlying value. - assert.Equal(t, "secret-value", sv.MustString()) +func TestPlainStringNotSensitive(t *testing.T) { + v := dyn.V("hello") + assert.False(t, v.IsSensitive()) + assert.Equal(t, "hello", v.AsAny()) } func TestSensitivePreservedByWithLocations(t *testing.T) { locs := []dyn.Location{{File: "file", Line: 1, Column: 1}} - v := dyn.V("secret").MarkSensitive() - v2 := v.WithLocations(locs) - assert.True(t, v2.IsSensitive()) - assert.Equal(t, locs, v2.Locations()) -} - -func TestSensitivePreservedByWithSensitive(t *testing.T) { - v := dyn.V("secret").MarkSensitive() + v := dyn.NewSensitiveValue("secret", nil).WithLocations(locs) assert.True(t, v.IsSensitive()) - - v2 := v.WithSensitive(false) - assert.False(t, v2.IsSensitive()) - - v3 := v2.WithSensitive(true) - assert.True(t, v3.IsSensitive()) + assert.Equal(t, locs, v.Locations()) } func TestIsZero(t *testing.T) { diff --git a/libs/dyn/value_underlying.go b/libs/dyn/value_underlying.go index a33ecd38ed8..792db7dbe00 100644 --- a/libs/dyn/value_underlying.go +++ b/libs/dyn/value_underlying.go @@ -40,9 +40,16 @@ func (v Value) MustSequence() []Value { // AsString returns the underlying string if this value is a string, // the zero value and false otherwise. +// For sensitive values (stored as [secretString]) it returns the real plaintext. func (v Value) AsString() (string, bool) { - vv, ok := v.v.(string) - return vv, ok + switch vv := v.v.(type) { + case string: + return vv, true + case secretString: + return vv.value, true + default: + return "", false + } } // MustString returns the underlying string if this value is a string, diff --git a/libs/dyn/visit.go b/libs/dyn/visit.go index 89b137596ce..7ae00fa8e08 100644 --- a/libs/dyn/visit.go +++ b/libs/dyn/visit.go @@ -142,10 +142,9 @@ func (c pathComponent) visit(v Value, prefix Path, suffix Pattern, opts visitOpt m = m.Clone() m.SetLoc(c.key, nil, nv) return Value{ - v: m, - k: KindMap, - l: v.l, - sensitive: v.sensitive, + v: m, + k: KindMap, + l: v.l, }, nil case c.isIndex(): @@ -182,10 +181,9 @@ func (c pathComponent) visit(v Value, prefix Path, suffix Pattern, opts visitOpt s = slices.Clone(s) s[c.index] = nv return Value{ - v: s, - k: KindSequence, - l: v.l, - sensitive: v.sensitive, + v: s, + k: KindSequence, + l: v.l, }, nil default: diff --git a/libs/dyn/visit_set.go b/libs/dyn/visit_set.go index 009bab1e0ba..1b3c7f1b661 100644 --- a/libs/dyn/visit_set.go +++ b/libs/dyn/visit_set.go @@ -43,10 +43,9 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) { m = m.Clone() m.SetLoc(component.key, nil, nv) return Value{ - v: m, - k: KindMap, - l: v.l, - sensitive: v.sensitive, + v: m, + k: KindMap, + l: v.l, }, nil case component.isIndex(): @@ -65,10 +64,9 @@ func SetByPath(v Value, p Path, nv Value) (Value, error) { s = slices.Clone(s) s[component.index] = nv return Value{ - v: s, - k: KindSequence, - l: v.l, - sensitive: v.sensitive, + v: s, + k: KindSequence, + l: v.l, }, nil default: diff --git a/libs/dyn/yamlsaver/saver.go b/libs/dyn/yamlsaver/saver.go index 591fabff405..01b345923d2 100644 --- a/libs/dyn/yamlsaver/saver.go +++ b/libs/dyn/yamlsaver/saver.go @@ -73,7 +73,7 @@ func (s *saver) toYamlNode(v dyn.Value) (*yaml.Node, error) { func (s *saver) toYamlNodeWithStyle(v dyn.Value, style yaml.Style) (*yaml.Node, error) { if v.IsSensitive() { - return &yaml.Node{Kind: yaml.ScalarNode, Value: "********", Style: style}, nil + return &yaml.Node{Kind: yaml.ScalarNode, Value: dyn.SensitiveValueRedacted, Style: style}, nil } switch v.Kind() {