From 464404e431d21d5b328bff432bb43adb27fece84 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Fri, 12 Jun 2026 15:18:53 +0000 Subject: [PATCH 1/5] Add libs/inputonly to strip INPUT_ONLY fields before render MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit New package with Strip(v any, paths []string) (any, error) that round-trips v through encoding/json into a generic representation, removes the dotted paths from the tree, and returns the masked value for the caller to marshal in its preferred format. Used by generated CLI commands (cmd/account/**, cmd/workspace/**) to drop fields the OpenAPI spec marks INPUT_ONLY before the response flows into cmdio.Render. The SDK transport struct serializes these fields unconditionally (REQUIRED on the request side, no omitempty), so the CLI sees them as empty strings even though the server never populates them on responses; the stripped any rendering matches what the spec promises. Path semantics: arrays are traversed transparently (each element visited), and dynamically-keyed maps (proto map) are too — when no literal key matches the next path component, every value is visited with the same path. This handles list responses and map-valued fields with the same path expression as singletons. Co-authored-by: Isaac --- libs/inputonly/inputonly.go | 93 +++++++++++++++++++++ libs/inputonly/inputonly_test.go | 137 +++++++++++++++++++++++++++++++ 2 files changed, 230 insertions(+) create mode 100644 libs/inputonly/inputonly.go create mode 100644 libs/inputonly/inputonly_test.go diff --git a/libs/inputonly/inputonly.go b/libs/inputonly/inputonly.go new file mode 100644 index 0000000000..25e5701797 --- /dev/null +++ b/libs/inputonly/inputonly.go @@ -0,0 +1,93 @@ +// Package inputonly drops fields marked INPUT_ONLY in the OpenAPI spec from +// SDK response values before they are rendered to the user. +// +// The Databricks Go SDK uses a single struct per resource for both request and +// response (transport-layer pattern). Some fields are REQUIRED on the request +// side — so their JSON tags have no omitempty — but INPUT_ONLY on the response +// side, meaning the server never populates them. When the CLI hands such a +// struct straight to encoding/json it emits the zero value (`"foo": ""`), +// which leaks API surface that isn't meant to round-trip. +// +// This package is consumed by the generated CLI command code in cmd/account/** +// and cmd/workspace/**: cligen reads the schemas in .codegen/cli.json, walks +// the response type, and emits a Strip call before the existing cmdio.Render. +// Keeping the logic out of libs/cmdio matches @pietern's guidance — cmdio +// stays a generic rendering pipeline, and the filtering policy lives where the +// metadata (cli.json) does. +package inputonly + +import ( + "encoding/json" + "fmt" + "strings" +) + +// Strip returns v with the listed dotted paths removed. If paths is empty, v +// is returned unchanged. Otherwise v is round-tripped through JSON into a +// generic representation, the listed paths are deleted, and the masked value +// is returned for the caller to marshal in its preferred format. +// +// Paths use dotted notation (e.g. "stable_url.initial_workspace_id"). Arrays +// and dynamically-keyed maps (e.g. proto map) are traversed +// transparently: a single path applies to every element of an array, and to +// every value of a map when no literal key matches the next path component. +// List responses and map-valued fields therefore share the same path +// expression as singletons. +func Strip(v any, paths []string) (any, error) { + if len(paths) == 0 { + return v, nil + } + b, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("inputonly: marshal: %w", err) + } + var out any + if err := json.Unmarshal(b, &out); err != nil { + return nil, fmt.Errorf("inputonly: unmarshal: %w", err) + } + for _, p := range paths { + deletePath(out, strings.Split(p, ".")) + } + return out, nil +} + +// deletePath walks v according to keys and removes the leaf key from any +// object it lands on. +// +// Both arrays and dynamically-keyed maps are traversed transparently: +// +// - When v is a []any, every element is visited with the same key list. +// - When v is a map[string]any but the next key is not a literal match, +// every value is visited with the same key list — this handles proto +// map fields, whose JSON keys are user-provided strings and +// whose values carry the field name from the path. +// +// Both struct fields and proto map surface as map[string]any after +// json.Unmarshal, so a single corner case remains: if a map's user-provided +// key happens to equal an inner field name, the literal match wins and that +// entry is removed instead of the field inside each value. cligen emits paths +// from the schema, so this only fires for real-world key collisions and +// matches the expected behavior for any path the schema actually targets. +func deletePath(v any, keys []string) { + if len(keys) == 0 { + return + } + switch t := v.(type) { + case map[string]any: + if child, ok := t[keys[0]]; ok { + if len(keys) == 1 { + delete(t, keys[0]) + } else { + deletePath(child, keys[1:]) + } + return + } + for _, child := range t { + deletePath(child, keys) + } + case []any: + for _, el := range t { + deletePath(el, keys) + } + } +} diff --git a/libs/inputonly/inputonly_test.go b/libs/inputonly/inputonly_test.go new file mode 100644 index 0000000000..3bb2625190 --- /dev/null +++ b/libs/inputonly/inputonly_test.go @@ -0,0 +1,137 @@ +package inputonly + +import ( + "encoding/json" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +type stableURL struct { + Name string `json:"name"` + InitialWorkspaceID string `json:"initial_workspace_id"` + URL string `json:"url,omitempty"` +} + +type stableURLList struct { + StableURLs []stableURL `json:"stable_urls"` +} + +type wrapper struct { + StableURL stableURL `json:"stable_url"` +} + +type taggedStableURLs struct { + Tags map[string]stableURL `json:"tags"` +} + +type nestedMapWrapper struct { + Spec struct { + Tags map[string]stableURL `json:"tags"` + } `json:"spec"` +} + +func TestStripEmptyPathsReturnsValueUnchanged(t *testing.T) { + in := stableURL{Name: "n", InitialWorkspaceID: "w"} + out, err := Strip(in, nil) + require.NoError(t, err) + assert.Equal(t, in, out) +} + +func TestStripFlatField(t *testing.T) { + in := stableURL{Name: "n", InitialWorkspaceID: "w", URL: "u"} + out, err := Strip(in, []string{"initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"name":"n","url":"u"}`, string(b)) +} + +func TestStripFieldAbsentIsNoop(t *testing.T) { + in := stableURL{Name: "n"} + out, err := Strip(in, []string{"missing_field"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + // Name retained; missing path silently ignored. InitialWorkspaceID stays + // present at "" because the struct tag has no omitempty. + assert.JSONEq(t, `{"name":"n","initial_workspace_id":""}`, string(b)) +} + +func TestStripNested(t *testing.T) { + in := wrapper{StableURL: stableURL{Name: "n", InitialWorkspaceID: "w"}} + out, err := Strip(in, []string{"stable_url.initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"stable_url":{"name":"n"}}`, string(b)) +} + +func TestStripSliceElements(t *testing.T) { + in := stableURLList{StableURLs: []stableURL{ + {Name: "a", InitialWorkspaceID: "1"}, + {Name: "b", InitialWorkspaceID: "2"}, + }} + out, err := Strip(in, []string{"stable_urls.initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"stable_urls":[{"name":"a"},{"name":"b"}]}`, string(b)) +} + +func TestStripMapValues(t *testing.T) { + in := taggedStableURLs{Tags: map[string]stableURL{ + "env": {Name: "a", InitialWorkspaceID: "1"}, + "prod": {Name: "b", InitialWorkspaceID: "2"}, + }} + out, err := Strip(in, []string{"tags.initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + + var got struct { + Tags map[string]map[string]any `json:"tags"` + } + require.NoError(t, json.Unmarshal(b, &got)) + require.Len(t, got.Tags, 2) + for k, v := range got.Tags { + assert.NotContains(t, v, "initial_workspace_id", "tag %q should have initial_workspace_id stripped", k) + assert.Contains(t, v, "name", "tag %q should retain name", k) + } +} + +func TestStripNestedInMapValue(t *testing.T) { + // Path lands inside a map at the second segment, then descends two more + // levels into the map's value. Confirms map transparency composes with + // regular literal-key descent. + var in nestedMapWrapper + in.Spec.Tags = map[string]stableURL{ + "a": {Name: "x", InitialWorkspaceID: "1"}, + "b": {Name: "y", InitialWorkspaceID: "2"}, + } + out, err := Strip(in, []string{"spec.tags.initial_workspace_id"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + + var got struct { + Spec struct { + Tags map[string]map[string]any `json:"tags"` + } `json:"spec"` + } + require.NoError(t, json.Unmarshal(b, &got)) + require.Len(t, got.Spec.Tags, 2) + for k, v := range got.Spec.Tags { + assert.NotContains(t, v, "initial_workspace_id", "spec.tags[%q] should have initial_workspace_id stripped", k) + } +} + +func TestStripMultiplePaths(t *testing.T) { + in := stableURL{Name: "n", InitialWorkspaceID: "w", URL: "u"} + out, err := Strip(in, []string{"initial_workspace_id", "url"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"name":"n"}`, string(b)) +} From 341cfb9823e393fed0c42538e96fa7d829d271fe Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Fri, 12 Jun 2026 15:19:11 +0000 Subject: [PATCH 2/5] cligen: emit inputonly.Strip for fields marked INPUT_ONLY in cli.json MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cliv1's .codegen/cli.json already carries x-databricks-field-behaviors in its schemas block, so the CLI generator can pull the INPUT_ONLY paths per method without any genkit-side change. (Cf. discussion at go/slack DECO-27296.) What's added: - internal/cligen/input_only.go: walk the schema graph rooted at the method's response type, return sorted dotted paths of every field with the INPUT_ONLY behavior. Singleton message refs are followed; array/map element types aren't (cli.json's SchemaFieldJSON carries only one ref slot, populated for singleton fields). A field that is itself INPUT_ONLY emits a single path and stops — the whole subtree is stripped at runtime by libs/inputonly.Strip. - internal/cligen/cligen.go: populateInputOnlyPaths runs after Resolve(); it skips paginated, LRO, wait, byte-stream, and empty responses because each renders a different shape and needs its own path source (deferred follow-up). - internal/cligen/model.go: MethodJSON gains InputOnlyPaths []string (transient, not in JSON); ServiceJSON gains HasInputOnlyPaths() so the template can gate the libs/inputonly import. - templates/service.go.tmpl: at the standard render emit site, emit inputonly.Strip(response, paths) into a `masked` local and pass that to the existing cmdio.Render. cmdio stays untouched. If paths is empty the existing call is byte-identical. Tests cover flat, nested-via-ref, INPUT_ONLY-message (no recursion), cycle, and unknown-root cases. Co-authored-by: Isaac --- internal/cligen/cligen.go | 1 + internal/cligen/input_only.go | 105 ++++++++++++++++++ internal/cligen/input_only_test.go | 126 ++++++++++++++++++++++ internal/cligen/model.go | 19 ++++ internal/cligen/templates/service.go.tmpl | 9 ++ 5 files changed, 260 insertions(+) create mode 100644 internal/cligen/input_only.go create mode 100644 internal/cligen/input_only_test.go diff --git a/internal/cligen/cligen.go b/internal/cligen/cligen.go index 200392f8b6..a536f59367 100644 --- a/internal/cligen/cligen.go +++ b/internal/cligen/cligen.go @@ -71,6 +71,7 @@ func Generate(jsonPath, targetDir string) ([]string, error) { if err := batch.Resolve(); err != nil { return nil, fmt.Errorf("%s: %w", jsonPath, err) } + populateInputOnlyPaths(batch, doc.Schemas) var filenames []string diff --git a/internal/cligen/input_only.go b/internal/cligen/input_only.go new file mode 100644 index 0000000000..02e009b31b --- /dev/null +++ b/internal/cligen/input_only.go @@ -0,0 +1,105 @@ +package main + +import ( + "slices" + "strings" + + "github.com/databricks/cli/internal/clijson" +) + +// inputOnlyBehavior is the field-behavior token in cli.json that marks a field +// the server reads on writes but never returns on responses. The constant lives +// here rather than as an enum because cli.json carries Behaviors as []string; +// matching by value keeps the contract dependency one-way. +const inputOnlyBehavior = "INPUT_ONLY" + +// populateInputOnlyPaths fills MethodJSON.InputOnlyPaths for every method +// whose response is a singleton message and that takes the normal sync render +// path (no pagination / LRO / wait / byte stream). The paths drive an +// inputonly.Strip call emitted by service.go.tmpl before cmdio.Render so the +// SDK transport struct's INPUT_ONLY fields don't leak into JSON output. +// +// Methods on the LRO / wait / pagination / byte-stream branches still render +// directly via cmdio.Render — extending those is a follow-up because each +// branch's rendered shape differs (operation type, iterator element, byte +// stream) and cli.json does not yet surface iterator element types. +func populateInputOnlyPaths(batch *CommandsBlock, schemas map[string]*clijson.SchemaJSON) { + if len(schemas) == 0 { + return + } + for _, s := range batch.Services { + if s.Package == nil { + continue + } + for _, m := range s.Methods { + if !eligibleForInputOnly(m) { + continue + } + rootName := s.Package.Name + "." + m.Response.PascalName + m.InputOnlyPaths = inputOnlyPaths(schemas, rootName) + } + } +} + +// eligibleForInputOnly picks the methods whose render site is the standard +// `cmdio.Render(ctx, response)` call in the method-call template. +func eligibleForInputOnly(m *MethodJSON) bool { + if m.Response == nil || m.Response.PascalName == "" || m.Response.IsEmptyResponse { + return false + } + if m.IsResponseByteStream { + return false + } + if m.Pagination != nil || m.Wait != nil || m.LongRunningOperation != nil { + return false + } + return true +} + +// inputOnlyPaths walks the schema graph rooted at rootName and returns the +// sorted dotted JSON paths of every INPUT_ONLY field reachable via direct +// message-typed refs. +// +// Array and map element types are not followed: cli.json's SchemaFieldJSON +// carries one ref slot, populated for singleton message-typed fields only — +// `collaborators` (an array of CleanRoomCollaborator) has no ref, so a field +// like `invite_recipient_workspace_id` reachable only via that array is not +// caught here. A field that is itself INPUT_ONLY at the container level is +// still emitted as a single path; the runtime in libs/inputonly traverses +// arrays and maps transparently. +// +// Cycles in the schema graph are pruned via a per-descent set: the same +// schema can still be reached from multiple parent paths and emit different +// paths for each, but recursion never loops. +func inputOnlyPaths(schemas map[string]*clijson.SchemaJSON, rootName string) []string { + var paths []string + onPath := map[string]bool{} + var walk func(prefix []string, schemaName string) + walk = func(prefix []string, schemaName string) { + s := schemas[schemaName] + if s == nil || onPath[schemaName] { + return + } + onPath[schemaName] = true + defer delete(onPath, schemaName) + names := make([]string, 0, len(s.Fields)) + for name := range s.Fields { + names = append(names, name) + } + slices.Sort(names) + for _, name := range names { + f := s.Fields[name] + childPath := append(slices.Clone(prefix), name) + if slices.Contains(f.Behaviors, inputOnlyBehavior) { + paths = append(paths, strings.Join(childPath, ".")) + continue + } + if f.Ref != "" { + walk(childPath, f.Ref) + } + } + } + walk(nil, rootName) + slices.Sort(paths) + return paths +} diff --git a/internal/cligen/input_only_test.go b/internal/cligen/input_only_test.go new file mode 100644 index 0000000000..3a4bf7a004 --- /dev/null +++ b/internal/cligen/input_only_test.go @@ -0,0 +1,126 @@ +package main + +import ( + "testing" + + "github.com/databricks/cli/internal/clijson" + "github.com/stretchr/testify/assert" +) + +func TestInputOnlyPathsFlat(t *testing.T) { + schemas := map[string]*clijson.SchemaJSON{ + "dr.StableUrl": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "name": {}, + "initial_workspace_id": {Behaviors: []string{"IMMUTABLE", "INPUT_ONLY"}}, + "url": {Behaviors: []string{"OUTPUT_ONLY"}}, + }, + }, + } + assert.Equal(t, []string{"initial_workspace_id"}, inputOnlyPaths(schemas, "dr.StableUrl")) +} + +func TestInputOnlyPathsNestedMessage(t *testing.T) { + // A singleton message ref is followed; an INPUT_ONLY field on the + // referenced type emits a dotted path. + schemas := map[string]*clijson.SchemaJSON{ + "x.Wrapper": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "inner": {Ref: "x.Inner"}, + }, + }, + "x.Inner": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "secret": {Behaviors: []string{"INPUT_ONLY"}}, + "name": {}, + }, + }, + } + assert.Equal(t, []string{"inner.secret"}, inputOnlyPaths(schemas, "x.Wrapper")) +} + +func TestInputOnlyPathsInputOnlyMessageNotRecursedInto(t *testing.T) { + // A field whose type is itself INPUT_ONLY emits a single path; the + // whole subtree is stripped at runtime so its inner fields don't need + // their own paths. + schemas := map[string]*clijson.SchemaJSON{ + "x.Outer": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "creds": {Ref: "x.Creds", Behaviors: []string{"INPUT_ONLY"}}, + }, + }, + "x.Creds": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "password": {Behaviors: []string{"INPUT_ONLY"}}, + }, + }, + } + assert.Equal(t, []string{"creds"}, inputOnlyPaths(schemas, "x.Outer")) +} + +func TestInputOnlyPathsCycle(t *testing.T) { + // A tree-shaped resource that references itself: the walker doesn't + // loop and still picks up the INPUT_ONLY field at this level. + schemas := map[string]*clijson.SchemaJSON{ + "x.Tree": { + Fields: map[string]*clijson.SchemaFieldJSON{ + "secret": {Behaviors: []string{"INPUT_ONLY"}}, + "parent": {Ref: "x.Tree"}, + }, + }, + } + assert.Equal(t, []string{"secret"}, inputOnlyPaths(schemas, "x.Tree")) +} + +func TestInputOnlyPathsUnknownRoot(t *testing.T) { + assert.Nil(t, inputOnlyPaths(map[string]*clijson.SchemaJSON{}, "x.Missing")) +} + +func TestEligibleForInputOnly(t *testing.T) { + cases := []struct { + name string + method *MethodJSON + want bool + }{ + { + name: "standard sync method", + method: &MethodJSON{Response: &EntityJSON{PascalName: "StableUrl"}}, + want: true, + }, + { + name: "empty response", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Empty", IsEmptyResponse: true}}, + want: false, + }, + { + name: "no response", + method: &MethodJSON{}, + want: false, + }, + { + name: "paginated", + method: &MethodJSON{Response: &EntityJSON{PascalName: "ListResponse"}, Pagination: &PaginationJSON{}}, + want: false, + }, + { + name: "byte stream", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Body"}, IsResponseByteStream: true}, + want: false, + }, + { + name: "long-running", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Operation"}, LongRunningOperation: &LROJSON{}}, + want: false, + }, + { + name: "wait", + method: &MethodJSON{Response: &EntityJSON{PascalName: "Resp"}, Wait: &WaitJSON{}}, + want: false, + }, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + assert.Equal(t, c.want, eligibleForInputOnly(c.method)) + }) + } +} diff --git a/internal/cligen/model.go b/internal/cligen/model.go index ae5ce44dd6..3b6dc12ab7 100644 --- a/internal/cligen/model.go +++ b/internal/cligen/model.go @@ -126,6 +126,19 @@ func (s *ServiceJSON) Comment(prefix string, maxLen int) string { return commentWrap(s.Description, prefix, maxLen) } +// HasInputOnlyPaths is true when any of the service's methods has a non-empty +// InputOnlyPaths. service.go.tmpl uses it to gate the libs/inputonly import: +// importing it unconditionally would leave it unused on services that don't +// strip anything. +func (s *ServiceJSON) HasInputOnlyPaths() bool { + for _, m := range s.Methods { + if len(m.InputOnlyPaths) > 0 { + return true + } + } + return false +} + // MethodJSON is the render context for one command (a method of a service). type MethodJSON struct { Name string `json:"name,omitempty"` @@ -163,6 +176,12 @@ type MethodJSON struct { Pagination *PaginationJSON `json:"pagination,omitempty"` Wait *WaitJSON `json:"wait,omitempty"` LongRunningOperation *LROJSON `json:"long_running_operation,omitempty"` + + // InputOnlyPaths is the sorted set of dotted JSON paths in the method's + // response type that are annotated INPUT_ONLY in cli.json's schemas + // block. Populated by populateInputOnlyPaths after Resolve(); empty for + // methods whose render site is not the standard sync path. + InputOnlyPaths []string `json:"-"` } func (m *MethodJSON) KebabName() string { return kebabName(m.Name) } diff --git a/internal/cligen/templates/service.go.tmpl b/internal/cligen/templates/service.go.tmpl index 21b634f457..c2aa04b5b1 100644 --- a/internal/cligen/templates/service.go.tmpl +++ b/internal/cligen/templates/service.go.tmpl @@ -11,6 +11,9 @@ import ( "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/flags" + {{ if .HasInputOnlyPaths -}} + "github.com/databricks/cli/libs/inputonly" + {{ end -}} "github.com/databricks/cli/cmd/root" "github.com/databricks/databricks-sdk-go/experimental/api" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" @@ -545,6 +548,12 @@ func new{{.PascalName}}() *cobra.Command { {{ if $method.IsResponseByteStream -}} defer response.{{$method.ResponseBodyField.PascalName}}.Close() return cmdio.Render{{ if $method.Pagination}}Iterator{{end}}(ctx, response.{{$method.ResponseBodyField.PascalName}}) + {{- else if $method.InputOnlyPaths }} + masked, err := inputonly.Strip(response, []string{ {{- range $i, $p := $method.InputOnlyPaths}}{{if $i}}, {{end}}"{{$p}}"{{end -}} }) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) {{- else }} return cmdio.Render{{ if $method.Pagination}}Iterator{{end}}(ctx, response) {{- end -}} From 03d0a67366ce126b32b7e1a97f63c70ce23b26d6 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Fri, 12 Jun 2026 15:19:24 +0000 Subject: [PATCH 3/5] Regenerate cmd/** to apply inputonly.Strip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Run of ./task generate-cligen after the previous commit. Six service files change; everything else is byte-identical (most response types have no INPUT_ONLY fields). Each modified service has at least one method whose response type declares an INPUT_ONLY field in cli.json's schemas block. Examples: - disaster-recovery: `get-stable-url` / `create-stable-url` strip `initial_workspace_id`; failover-group methods strip `initial_primary_region`. - clean-rooms: nested paths like `remote_detailed_info.creator.invite_recipient_workspace_id`. - database / postgres / secrets-uc / quality-monitor-v2: multi-field paths including container fields whose entire subtree is INPUT_ONLY (e.g. `spec` on secrets-uc). Paginated list methods (e.g. `list-stable-urls`) still go through cmdio.RenderIterator unchanged. cli.json does not currently surface the iterator element type, so per-element path computation is a follow-up — the leak persists on those commands until then. Co-authored-by: Isaac --- .../disaster-recovery/disaster-recovery.go | 37 +++++++++++-- cmd/workspace/clean-rooms/clean-rooms.go | 13 ++++- cmd/workspace/database/database.go | 55 ++++++++++++++++--- cmd/workspace/postgres/postgres.go | 43 ++++++++++++--- .../quality-monitor-v2/quality-monitor-v2.go | 19 ++++++- cmd/workspace/secrets-uc/secrets-uc.go | 19 ++++++- 6 files changed, 156 insertions(+), 30 deletions(-) diff --git a/cmd/account/disaster-recovery/disaster-recovery.go b/cmd/account/disaster-recovery/disaster-recovery.go index 156d3baea4..abdefd4680 100644 --- a/cmd/account/disaster-recovery/disaster-recovery.go +++ b/cmd/account/disaster-recovery/disaster-recovery.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" "github.com/databricks/databricks-sdk-go/service/disasterrecovery" "github.com/spf13/cobra" @@ -151,7 +152,11 @@ func newCreateFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -245,7 +250,11 @@ func newCreateStableUrl() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -464,7 +473,11 @@ func newFailoverFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -524,7 +537,11 @@ func newGetFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -584,7 +601,11 @@ func newGetStableUrl() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -847,7 +868,11 @@ func newUpdateFailoverGroup() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_primary_region"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/clean-rooms/clean-rooms.go b/cmd/workspace/clean-rooms/clean-rooms.go index 6ce4f30def..803dfca463 100644 --- a/cmd/workspace/clean-rooms/clean-rooms.go +++ b/cmd/workspace/clean-rooms/clean-rooms.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/service/cleanrooms" "github.com/spf13/cobra" ) @@ -331,7 +332,11 @@ func newGet() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"remote_detailed_info.creator.invite_recipient_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -481,7 +486,11 @@ func newUpdate() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"remote_detailed_info.creator.invite_recipient_workspace_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/database/database.go b/cmd/workspace/database/database.go index 92aa1408bf..b723135f1a 100644 --- a/cmd/workspace/database/database.go +++ b/cmd/workspace/database/database.go @@ -10,6 +10,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/service/database" "github.com/spf13/cobra" ) @@ -146,7 +147,11 @@ Create a Database Catalog. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"create_database_if_not_exists"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -535,7 +540,11 @@ Create a Synced Database Table. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"database_instance_name", "logical_database_name", "spec.create_database_objects_if_missing", "spec.existing_pipeline_id", "spec.new_pipeline_spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -881,7 +890,11 @@ Find a Database Instance by uid.` return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"custom_tags", "enable_pg_native_login", "enable_readable_secondaries", "node_count", "parent_instance_ref.lsn", "retention_window_in_days", "stopped", "usage_policy_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1011,7 +1024,11 @@ Get a Database Catalog.` return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"create_database_if_not_exists"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1070,7 +1087,11 @@ Get a Database Instance. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"custom_tags", "enable_pg_native_login", "enable_readable_secondaries", "node_count", "parent_instance_ref.lsn", "retention_window_in_days", "stopped", "usage_policy_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1242,7 +1263,11 @@ Get a Synced Database Table.` return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"database_instance_name", "logical_database_name", "spec.create_database_objects_if_missing", "spec.existing_pipeline_id", "spec.new_pipeline_spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1644,7 +1669,11 @@ func newUpdateDatabaseCatalog() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"create_database_if_not_exists"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1735,7 +1764,11 @@ Update a Database Instance. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"custom_tags", "enable_pg_native_login", "enable_readable_secondaries", "node_count", "parent_instance_ref.lsn", "retention_window_in_days", "stopped", "usage_policy_id"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1820,7 +1853,11 @@ func newUpdateSyncedDatabaseTable() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"database_instance_name", "logical_database_name", "spec.create_database_objects_if_missing", "spec.existing_pipeline_id", "spec.new_pipeline_spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/postgres/postgres.go b/cmd/workspace/postgres/postgres.go index 9522a6d774..8d1b4360c8 100644 --- a/cmd/workspace/postgres/postgres.go +++ b/cmd/workspace/postgres/postgres.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" "github.com/databricks/databricks-sdk-go/experimental/api" "github.com/databricks/databricks-sdk-go/service/postgres" @@ -1851,7 +1852,11 @@ Get a Branch. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1912,7 +1917,11 @@ Get a Database Catalog. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -1972,7 +1981,11 @@ Get a Database. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2035,7 +2048,11 @@ Get an Endpoint. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2158,7 +2175,11 @@ Get a Project. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"initial_endpoint_spec", "spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2221,7 +2242,11 @@ Get a Postgres Role for a Branch. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -2284,7 +2309,11 @@ Get a Synced Database Table. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"spec"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go b/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go index 8290fbf92a..45d7995194 100644 --- a/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go +++ b/cmd/workspace/quality-monitor-v2/quality-monitor-v2.go @@ -9,6 +9,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/service/qualitymonitorv2" "github.com/spf13/cobra" ) @@ -128,7 +129,11 @@ Create a quality monitor. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"validity_check_configurations"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -255,7 +260,11 @@ Read a quality monitor. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"validity_check_configurations"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -427,7 +436,11 @@ Update a quality monitor. return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"validity_check_configurations"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. diff --git a/cmd/workspace/secrets-uc/secrets-uc.go b/cmd/workspace/secrets-uc/secrets-uc.go index 006a5f5d33..6fdcc9c711 100644 --- a/cmd/workspace/secrets-uc/secrets-uc.go +++ b/cmd/workspace/secrets-uc/secrets-uc.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/libs/cmdctx" "github.com/databricks/cli/libs/cmdio" "github.com/databricks/cli/libs/flags" + "github.com/databricks/cli/libs/inputonly" "github.com/databricks/databricks-sdk-go/common/types/fieldmask" sdktime "github.com/databricks/databricks-sdk-go/common/types/time" "github.com/databricks/databricks-sdk-go/service/catalog" @@ -164,7 +165,11 @@ func newCreateSecret() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"owner", "value"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -294,7 +299,11 @@ func newGetSecret() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"owner", "value"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. @@ -505,7 +514,11 @@ func newUpdateSecret() *cobra.Command { return err } - return cmdio.Render(ctx, response) + masked, err := inputonly.Strip(response, []string{"owner", "value"}) + if err != nil { + return err + } + return cmdio.Render(ctx, masked) } // Disable completions since they are not applicable. From 39db81c28a48fe0d22dd20f280443363f9c1b242 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Fri, 12 Jun 2026 15:42:28 +0000 Subject: [PATCH 4/5] acc: update update-database-instance fixture to drop stopped DatabaseInstance.stopped is INPUT_ONLY in cli.json, so the regenerated update-database-instance command now strips it from the response. The fixture was captured before that change and still expected `stopped` in the rendered JSON. Regenerated via ./task test-update. Co-authored-by: Isaac --- .../cmd/workspace/database/update-database-instance/output.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/acceptance/cmd/workspace/database/update-database-instance/output.txt b/acceptance/cmd/workspace/database/update-database-instance/output.txt index 463b128eaf..3bebd05980 100644 --- a/acceptance/cmd/workspace/database/update-database-instance/output.txt +++ b/acceptance/cmd/workspace/database/update-database-instance/output.txt @@ -27,6 +27,5 @@ Exit code: 1 >>> [CLI] database update-database-instance test-db * --json {"stopped": true} { - "name": "test-db", - "stopped": true + "name": "test-db" } From 7b43980ada2e54d85aa5c2f2dafc9f99804cd063 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Fri, 12 Jun 2026 16:17:03 +0000 Subject: [PATCH 5/5] inputonly: preserve int64 precision and drop match-anywhere fallback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in the original Strip implementation, both pointed out on the prior PR's review and inherited here: 1. The json.Marshal -> json.Unmarshal(any) round-trip decoded every number as float64, silently losing precision above 2^53 — i.e. on real SDK fields like spark_context_id (int64). Switch to json.NewDecoder(...).UseNumber() so numbers stay as json.Number strings and re-marshal verbatim. 2. The map-handling branch in deletePath fell through to iterate every value when the literal key didn't match. For INPUT_ONLY fields the server always omits the field at its expected location, so the literal miss was the common case, and the fallback would silently recurse into every nested object and strip any same-named leaf — turning anchored paths into match-anywhere expressions. Repro: path "name" against {"id":"123","details":{"name":"x"}} stripped details.name. The same fallback also created a map-key collision foot-gun (a proto-map entry literally named the leaf would be removed, leaving the mask unapplied elsewhere). Fix: strict literal lookup on maps, return on miss. Arrays stay transparent because []any is type-distinguishable at runtime so the path semantic there is unambiguous. cligen does not emit paths that descend through proto maps today; if cli.json grows map value refs later, the path language should grow an explicit marker ("tags.*.field") rather than reintroducing implicit fallback. Adds two regression tests: - TestStripPreservesLargeInt64: value 2^53+1 survives a strip on a sibling field. - TestStripDoesNotMatchAnywhere: path "name" leaves details.name untouched. Drops two tests from the previous commit that relied on the map fallback (TestStripMapValues, TestStripNestedInMapValue) and replaces them with TestStripDoesNotDescendIntoMaps, which documents the strict behavior. Also notes in the doc comment that filtering reorders JSON keys alphabetically (map[string]any vs struct declaration order) so downstream consumers and acceptance fixtures expect that. Co-authored-by: Isaac --- libs/inputonly/inputonly.go | 75 ++++++++++++---------- libs/inputonly/inputonly_test.go | 104 ++++++++++++++++++------------- 2 files changed, 103 insertions(+), 76 deletions(-) diff --git a/libs/inputonly/inputonly.go b/libs/inputonly/inputonly.go index 25e5701797..be77557afc 100644 --- a/libs/inputonly/inputonly.go +++ b/libs/inputonly/inputonly.go @@ -11,12 +11,10 @@ // This package is consumed by the generated CLI command code in cmd/account/** // and cmd/workspace/**: cligen reads the schemas in .codegen/cli.json, walks // the response type, and emits a Strip call before the existing cmdio.Render. -// Keeping the logic out of libs/cmdio matches @pietern's guidance — cmdio -// stays a generic rendering pipeline, and the filtering policy lives where the -// metadata (cli.json) does. package inputonly import ( + "bytes" "encoding/json" "fmt" "strings" @@ -27,12 +25,19 @@ import ( // generic representation, the listed paths are deleted, and the masked value // is returned for the caller to marshal in its preferred format. // -// Paths use dotted notation (e.g. "stable_url.initial_workspace_id"). Arrays -// and dynamically-keyed maps (e.g. proto map) are traversed -// transparently: a single path applies to every element of an array, and to -// every value of a map when no literal key matches the next path component. -// List responses and map-valued fields therefore share the same path -// expression as singletons. +// Paths use dotted notation (e.g. "stable_url.initial_workspace_id") and are +// matched literally at every segment. Arrays are traversed transparently: a +// path is applied to every element of any []any encountered along the way. +// Maps are not — see deletePath for the reasoning. +// +// Numbers are decoded via json.Number rather than float64 so values above +// 2^53 (e.g. SDK fields typed int64 like spark_context_id) re-marshal +// verbatim instead of silently losing precision. +// +// Side note: encoding/json marshals struct fields in declaration order but +// map[string]any keys alphabetically. Filtered responses therefore render +// with sorted JSON keys, while unfiltered ones keep SDK-struct order. +// Acceptance fixtures and downstream consumers should be tolerant of that. func Strip(v any, paths []string) (any, error) { if len(paths) == 0 { return v, nil @@ -41,8 +46,10 @@ func Strip(v any, paths []string) (any, error) { if err != nil { return nil, fmt.Errorf("inputonly: marshal: %w", err) } + dec := json.NewDecoder(bytes.NewReader(b)) + dec.UseNumber() var out any - if err := json.Unmarshal(b, &out); err != nil { + if err := dec.Decode(&out); err != nil { return nil, fmt.Errorf("inputonly: unmarshal: %w", err) } for _, p := range paths { @@ -51,40 +58,42 @@ func Strip(v any, paths []string) (any, error) { return out, nil } -// deletePath walks v according to keys and removes the leaf key from any -// object it lands on. +// deletePath walks v according to keys and removes the leaf key from the +// object it lands on. Each segment is matched literally; if no literal match +// exists at a given level the path simply does not apply, and we return. // -// Both arrays and dynamically-keyed maps are traversed transparently: +// Arrays are traversed transparently. After json.Unmarshal a JSON array is +// always []any, type-distinguishable from map[string]any, so descending into +// every element with the same key list is unambiguous. // -// - When v is a []any, every element is visited with the same key list. -// - When v is a map[string]any but the next key is not a literal match, -// every value is visited with the same key list — this handles proto -// map fields, whose JSON keys are user-provided strings and -// whose values carry the field name from the path. -// -// Both struct fields and proto map surface as map[string]any after -// json.Unmarshal, so a single corner case remains: if a map's user-provided -// key happens to equal an inner field name, the literal match wins and that -// entry is removed instead of the field inside each value. cligen emits paths -// from the schema, so this only fires for real-world key collisions and -// matches the expected behavior for any path the schema actually targets. +// Maps are not traversed transparently, even though proto map +// surfaces as map[string]any just like a struct does. Falling back to +// match-anywhere when the literal misses turns an anchored path into a +// match-anywhere expression: e.g. path "name" against +// {"id":"123","details":{"name":"x"}} would strip "details.name" instead of +// no-oping. Since INPUT_ONLY fields are always omitted by the server, the +// literal miss is the normal case and the fallback would fire on every +// Strip call; the failure mode is silent over-stripping. cligen emits +// paths anchored to specific schema locations and does not currently emit +// paths that descend through proto maps (cli.json carries one ref slot, +// populated for singleton message fields only). If that contract grows +// map value refs later, the path language should grow an explicit map +// marker (e.g. "*") rather than reintroducing implicit fallback. func deletePath(v any, keys []string) { if len(keys) == 0 { return } switch t := v.(type) { case map[string]any: - if child, ok := t[keys[0]]; ok { - if len(keys) == 1 { - delete(t, keys[0]) - } else { - deletePath(child, keys[1:]) - } + child, ok := t[keys[0]] + if !ok { return } - for _, child := range t { - deletePath(child, keys) + if len(keys) == 1 { + delete(t, keys[0]) + return } + deletePath(child, keys[1:]) case []any: for _, el := range t { deletePath(el, keys) diff --git a/libs/inputonly/inputonly_test.go b/libs/inputonly/inputonly_test.go index 3bb2625190..ff1af4fc2a 100644 --- a/libs/inputonly/inputonly_test.go +++ b/libs/inputonly/inputonly_test.go @@ -26,12 +26,6 @@ type taggedStableURLs struct { Tags map[string]stableURL `json:"tags"` } -type nestedMapWrapper struct { - Spec struct { - Tags map[string]stableURL `json:"tags"` - } `json:"spec"` -} - func TestStripEmptyPathsReturnsValueUnchanged(t *testing.T) { in := stableURL{Name: "n", InitialWorkspaceID: "w"} out, err := Strip(in, nil) @@ -80,58 +74,82 @@ func TestStripSliceElements(t *testing.T) { assert.JSONEq(t, `{"stable_urls":[{"name":"a"},{"name":"b"}]}`, string(b)) } -func TestStripMapValues(t *testing.T) { - in := taggedStableURLs{Tags: map[string]stableURL{ - "env": {Name: "a", InitialWorkspaceID: "1"}, - "prod": {Name: "b", InitialWorkspaceID: "2"}, - }} - out, err := Strip(in, []string{"tags.initial_workspace_id"}) +func TestStripMultiplePaths(t *testing.T) { + in := stableURL{Name: "n", InitialWorkspaceID: "w", URL: "u"} + out, err := Strip(in, []string{"initial_workspace_id", "url"}) require.NoError(t, err) b, err := json.Marshal(out) require.NoError(t, err) - - var got struct { - Tags map[string]map[string]any `json:"tags"` - } - require.NoError(t, json.Unmarshal(b, &got)) - require.Len(t, got.Tags, 2) - for k, v := range got.Tags { - assert.NotContains(t, v, "initial_workspace_id", "tag %q should have initial_workspace_id stripped", k) - assert.Contains(t, v, "name", "tag %q should retain name", k) - } + assert.JSONEq(t, `{"name":"n"}`, string(b)) } -func TestStripNestedInMapValue(t *testing.T) { - // Path lands inside a map at the second segment, then descends two more - // levels into the map's value. Confirms map transparency composes with - // regular literal-key descent. - var in nestedMapWrapper - in.Spec.Tags = map[string]stableURL{ - "a": {Name: "x", InitialWorkspaceID: "1"}, - "b": {Name: "y", InitialWorkspaceID: "2"}, +// TestStripPreservesLargeInt64 guards against the float64 round-trip pitfall: +// encoding/json decodes JSON numbers into float64 (53-bit mantissa) when the +// destination is `any`, which silently loses precision for SDK fields like +// spark_context_id (int64) above 2^53. Strip uses json.Number to side-step +// that. +func TestStripPreservesLargeInt64(t *testing.T) { + type clusterResponse struct { + ClusterName string `json:"cluster_name"` + SparkContextID int64 `json:"spark_context_id"` + InitialWorkspaceID string `json:"initial_workspace_id"` + } + in := clusterResponse{ + ClusterName: "c", + SparkContextID: 9007199254740993, // 2^53 + 1, unrepresentable as float64 + InitialWorkspaceID: "ws-1", } - out, err := Strip(in, []string{"spec.tags.initial_workspace_id"}) + out, err := Strip(in, []string{"initial_workspace_id"}) require.NoError(t, err) b, err := json.Marshal(out) require.NoError(t, err) + assert.Contains(t, string(b), `"spark_context_id":9007199254740993`) + assert.NotContains(t, string(b), "initial_workspace_id") +} - var got struct { - Spec struct { - Tags map[string]map[string]any `json:"tags"` - } `json:"spec"` +// TestStripDoesNotMatchAnywhere guards against the implicit-fallback pitfall: +// an anchored path that doesn't match at the level it targets must NOT +// silently descend into other objects and strip same-named fields elsewhere. +// Since INPUT_ONLY fields are always omitted by the server, the literal miss +// at the anchored level is the common case — a fallback descent would fire +// every time and quietly strip legitimate fields with the same leaf name. +func TestStripDoesNotMatchAnywhere(t *testing.T) { + type details struct { + Name string `json:"name"` + Size string `json:"size"` } - require.NoError(t, json.Unmarshal(b, &got)) - require.Len(t, got.Spec.Tags, 2) - for k, v := range got.Spec.Tags { - assert.NotContains(t, v, "initial_workspace_id", "spec.tags[%q] should have initial_workspace_id stripped", k) + type response struct { + ID string `json:"id"` + Details details `json:"details"` } + in := response{ID: "123", Details: details{Name: "keep-me", Size: "L"}} + out, err := Strip(in, []string{"name"}) + require.NoError(t, err) + b, err := json.Marshal(out) + require.NoError(t, err) + assert.JSONEq(t, `{"id":"123","details":{"name":"keep-me","size":"L"}}`, string(b)) } -func TestStripMultiplePaths(t *testing.T) { - in := stableURL{Name: "n", InitialWorkspaceID: "w", URL: "u"} - out, err := Strip(in, []string{"initial_workspace_id", "url"}) +// TestStripDoesNotDescendIntoMaps documents the strict-map behavior: a path +// that doesn't literally match a map key is a no-op, never a fallback into +// every value. cligen does not emit paths that descend through proto maps +// today (cli.json carries no map value refs); the runtime matches that +// contract instead of guessing. +func TestStripDoesNotDescendIntoMaps(t *testing.T) { + in := taggedStableURLs{Tags: map[string]stableURL{ + "env": {Name: "a", InitialWorkspaceID: "1"}, + "prod": {Name: "b", InitialWorkspaceID: "2"}, + }} + out, err := Strip(in, []string{"tags.initial_workspace_id"}) require.NoError(t, err) b, err := json.Marshal(out) require.NoError(t, err) - assert.JSONEq(t, `{"name":"n"}`, string(b)) + + var got struct { + Tags map[string]map[string]any `json:"tags"` + } + require.NoError(t, json.Unmarshal(b, &got)) + require.Len(t, got.Tags, 2) + assert.Equal(t, "1", got.Tags["env"]["initial_workspace_id"]) + assert.Equal(t, "2", got.Tags["prod"]["initial_workspace_id"]) }