diff --git a/libs/dyn/convert/from_typed.go b/libs/dyn/convert/from_typed.go index 66451124293..d2c08773731 100644 --- a/libs/dyn/convert/from_typed.go +++ b/libs/dyn/convert/from_typed.go @@ -133,6 +133,15 @@ func fromTypedStruct(src reflect.Value, ref dyn.Value, options ...fromTypedOptio return dyn.InvalidValue, err } + // 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 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..20750a647a4 100644 --- a/libs/dyn/convert/from_typed_test.go +++ b/libs/dyn/convert/from_typed_test.go @@ -926,3 +926,29 @@ 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, 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/convert/struct_info.go b/libs/dyn/convert/struct_info.go index 69e8f868ed5..c78b49a0184 100644 --- a/libs/dyn/convert/struct_info.go +++ b/libs/dyn/convert/struct_info.go @@ -28,6 +28,10 @@ type structInfo struct { // Maps JSON-name of the field to Golang struct name GolangNames map[string]string + // 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. @@ -68,6 +72,7 @@ func buildStructInfo(typ reflect.Type) structInfo { 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. @@ -134,6 +139,11 @@ func buildStructInfo(typ reflect.Type) structInfo { } out.GolangNames[name] = sf.Name + btag := structtag.BundleTag(sf.Tag.Get("bundle")) + if btag.Sensitive() { + out.Sensitive[name] = true + } + // The field is declared directly in this struct, so it is governed by // this struct's ForceSendFields (if it has one). if forceSendFieldsIndex != nil { @@ -176,6 +186,25 @@ 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 +// 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 +} + // isForceSend reports whether the field named k is listed in the ForceSendFields // that governs it (see structInfo.ForceSendFieldsIndex). func (s *structInfo) isForceSend(v reflect.Value, k string) bool { 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..612f9c26fad 100644 --- a/libs/dyn/dynvar/resolve.go +++ b/libs/dyn/dynvar/resolve.go @@ -153,10 +153,18 @@ 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. // + // Preserve sensitivity: NewValue strips the secretString wrapper, so use + // NewSensitiveValue when the resolved value is a sensitive string. + if resolved[0].IsSensitive() { + s, _ := resolved[0].AsString() + return dyn.NewSensitiveValue(s, ref.Value.Locations()), nil + } return dyn.NewValue(resolved[0].Value(), ref.Value.Locations()), 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 +172,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 +192,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..3399f2057a6 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.NewSensitiveValue("top-secret", nil), + "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.NewSensitiveValue("password123", nil), + "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..cbeb5b28919 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(dyn.SensitiveValueRedacted) + 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..9866f46dc37 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.NewSensitiveValue("real-secret", nil) + b, err := Marshal(v) + if assert.NoError(t, err) { + assert.JSONEq(t, `"`+dyn.SensitiveValueRedacted+`"`, string(b)) + } +} + +func TestMarshal_SensitiveInMap(t *testing.T) { + m := dyn.NewMapping() + 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":"`+dyn.SensitiveValueRedacted+`","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/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 72803511b80..e6ea328f082 100644 --- a/libs/dyn/value.go +++ b/libs/dyn/value.go @@ -73,6 +73,24 @@ func (v Value) AppendLocationsFromValue(w Value) Value { } } +// 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 { + _, ok := v.v.(secretString) + return ok +} + +// 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 +} + func (v Value) Kind() Kind { return v.k } @@ -120,6 +138,10 @@ func (v Value) AsAny() any { case KindNil: return v.v case KindString: + // secretString holds a sensitive value; return the redaction placeholder. + if _, ok := v.v.(secretString); ok { + return SensitiveValueRedacted + } return v.v case KindBool: return v.v diff --git a/libs/dyn/value_test.go b/libs/dyn/value_test.go index 8717aae620e..a2edf4e0128 100644 --- a/libs/dyn/value_test.go +++ b/libs/dyn/value_test.go @@ -50,6 +50,34 @@ func TestValueIsValid(t *testing.T) { assert.True(t, intValue.IsValid()) } +func TestNewSensitiveValue(t *testing.T) { + v := dyn.NewSensitiveValue("secret-value", nil) + assert.True(t, v.IsSensitive()) + assert.Equal(t, dyn.KindString, v.Kind()) + + // AsAny returns the redaction placeholder. + assert.Equal(t, dyn.SensitiveValueRedacted, v.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()) +} + +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.NewSensitiveValue("secret", nil).WithLocations(locs) + assert.True(t, v.IsSensitive()) + assert.Equal(t, locs, v.Locations()) +} + 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/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/yamlsaver/saver.go b/libs/dyn/yamlsaver/saver.go index 4c302e26f0d..01b345923d2 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: dyn.SensitiveValueRedacted, Style: style}, nil + } + switch v.Kind() { case dyn.KindMap: m, _ := v.AsMap() 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()) }) } }