Skip to content
8 changes: 6 additions & 2 deletions bundle/direct/dstate/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ import (
"github.com/databricks/cli/bundle/deployplan"
"github.com/databricks/cli/bundle/statemgmt/resourcestate"
"github.com/databricks/cli/internal/build"
"github.com/databricks/cli/libs/dyn"
"github.com/databricks/cli/libs/log"
"github.com/databricks/cli/libs/structs/structwalk"
"github.com/google/uuid"
)

Expand Down Expand Up @@ -89,8 +91,10 @@ func (db *DeploymentState) SaveState(key, newID string, state any, dependsOn []d
db.Data.State = make(map[string]ResourceEntry)
}

// don't indent so that every WAL entry remains on a single line
jsonMessage, err := json.Marshal(state)
// Redact sensitive fields before persisting: secrets must not appear on disk
// in plaintext. The original struct is not modified; the plan uses the
// unredacted in-memory value for API calls.
jsonMessage, err := structwalk.RedactSensitiveFields(state, dyn.SensitiveValueRedacted)
if err != nil {
return err
}
Expand Down
7 changes: 7 additions & 0 deletions libs/dyn/convert/from_typed.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,13 @@ 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 string
// (including empty), wrap it as a sensitive value so that all downstream
// serializers (JSON, YAML) redact it automatically.
if info.Sensitive[k] && nv.Kind() == dyn.KindString {
nv = dyn.NewSensitiveValue(nv.MustString(), nv.Locations())
}

Comment thread
pietern marked this conversation as resolved.
// 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
Expand Down
52 changes: 52 additions & 0 deletions libs/dyn/convert/from_typed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -926,3 +926,55 @@ 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",
Comment thread
andrewnester marked this conversation as resolved.
}

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 field should be sensitive")

// MustString returns the real value.
assert.Equal(t, "super-secret", token.MustString())
// AsAny returns the redaction placeholder.
assert.Equal(t, dyn.SensitiveValueRedacted, token.AsAny())
}

func TestFromTypedSensitiveFieldEmpty(t *testing.T) {
// An empty sensitive string must still be wrapped as sensitive.
type Tmp struct {
Token string `json:"token" bundle:"sensitive"`
}

nv, err := FromTyped(Tmp{Token: ""}, dyn.NilValue)
require.NoError(t, err)

token := nv.Get("token")
// Empty sensitive fields resolve to KindNil when the struct omits them
// (omitempty is the default). When the field IS present (e.g. forced via
// a non-nil pointer or ForceSendFields), it should be sensitive.
// Use a pointer-to-struct to force the zero value through.
src := &Tmp{Token: ""}
nv2, err := FromTyped(src, dyn.NilValue)
require.NoError(t, err)

token2 := nv2.Get("token")
if token2.Kind() == dyn.KindString {
assert.True(t, token2.IsSensitive(), "empty sensitive string should still be sensitive")
assert.Empty(t, token2.MustString())
}
_ = token // non-pointer form may be KindNil, which is fine
}
9 changes: 9 additions & 0 deletions libs/dyn/convert/normalize.go
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,15 @@ func (n normalizeOptions) normalizeString(typ reflect.Type, src dyn.Value, path

switch src.Kind() {
case dyn.KindString:
// A sensitive string must be returned as-is: dyn.NewValue(MustString(), ...)
// would construct a plain string and strip the secretString wrapper. Type
// normalization (e.g. coercing a bool "true" to string) cannot apply to a
// sensitive value because its Kind is already KindString, so returning early
// here is correct. Updating the secret value is done via FromTyped, not
// Normalize: FromTyped re-wraps the new value as NewSensitiveValue.
if src.IsSensitive() {
return src, nil

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means it is not possible to update the secret value in the typed tree.

If expected, please include this expectation in the comment.

}
return dyn.NewValue(src.MustString(), src.Locations()), nil
case dyn.KindBool:
return dyn.NewValue(strconv.FormatBool(src.MustBool()), src.Locations()), nil
Expand Down
29 changes: 29 additions & 0 deletions libs/dyn/convert/struct_info.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
andrewnester marked this conversation as resolved.

// 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.
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a few unit tests here in dyn/convert that demonstrates that this works in cases with embedded structs, anonymous structs, etc. Would also be good to include a user of these fields in the same package.

// 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 {
Expand Down
101 changes: 101 additions & 0 deletions libs/dyn/convert/struct_info_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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())
}
19 changes: 18 additions & 1 deletion libs/dyn/dynvar/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,18 +153,31 @@ 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.
if !resolved[j].IsValid() {
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.
Expand All @@ -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) {
Expand Down
33 changes: 33 additions & 0 deletions libs/dyn/dynvar/resolve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
11 changes: 11 additions & 0 deletions libs/dyn/jsonsaver/marshal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
Loading
Loading