From 9b21af59fc3877603662fb295c86a0ceef50b157 Mon Sep 17 00:00:00 2001 From: Noam Loewenstern Date: Thu, 17 Sep 2026 22:40:04 +0300 Subject: [PATCH 1/6] fix(output): redact sensitive fields in json and pretty formatters JSONFormatter and PrettyFormatter marshaled structs directly, ignoring the `sensitive:"true"` struct tag that TableFormatter already honors. Any command using --format json/pretty printed real secrets (instance tokens, SSH private keys, webhook secrets, DB env values, etc.) regardless of --show-sensitive. Add a shared reflection-based redactor (internal/output/redact.go) that deep-walks structs, pointers, slices, and maps, replacing sensitive fields with SensitiveOverlay unless ShowSensitive is set, and wire it into both formatters. TableFormatter's own per-field check now reuses the same isSensitiveField helper instead of duplicating the tag lookup. Struct output is rendered through an order-preserving map so existing JSON key ordering is unaffected. Co-Authored-By: Claude Sonnet 5 --- internal/output/json.go | 2 +- internal/output/pretty.go | 2 +- internal/output/redact.go | 197 +++++++++++++++++++++++++++++++++ internal/output/redact_test.go | 149 +++++++++++++++++++++++++ internal/output/table.go | 3 +- 5 files changed, 349 insertions(+), 4 deletions(-) create mode 100644 internal/output/redact.go create mode 100644 internal/output/redact_test.go diff --git a/internal/output/json.go b/internal/output/json.go index 323dcaa..609ea23 100644 --- a/internal/output/json.go +++ b/internal/output/json.go @@ -17,5 +17,5 @@ func NewJSONFormatter(opts Options) *JSONFormatter { // Format formats the data as compact JSON func (f *JSONFormatter) Format(data interface{}) error { encoder := json.NewEncoder(f.opts.Writer) - return encoder.Encode(data) + return encoder.Encode(redactSensitive(data, f.opts.ShowSensitive)) } diff --git a/internal/output/pretty.go b/internal/output/pretty.go index 6940ca5..0f7576f 100644 --- a/internal/output/pretty.go +++ b/internal/output/pretty.go @@ -18,5 +18,5 @@ func NewPrettyFormatter(opts Options) *PrettyFormatter { func (f *PrettyFormatter) Format(data interface{}) error { encoder := json.NewEncoder(f.opts.Writer) encoder.SetIndent("", " ") - return encoder.Encode(data) + return encoder.Encode(redactSensitive(data, f.opts.ShowSensitive)) } diff --git a/internal/output/redact.go b/internal/output/redact.go new file mode 100644 index 0000000..a89be99 --- /dev/null +++ b/internal/output/redact.go @@ -0,0 +1,197 @@ +package output + +import ( + "bytes" + "encoding/json" + "fmt" + "reflect" + "strings" +) + +// redactSensitive returns a copy of data safe to serialize, with any field +// tagged `sensitive:"true"` replaced by SensitiveOverlay unless showSensitive +// is true. The original value is never mutated. Struct fields are walked +// recursively through pointers, slices, arrays, and maps, mirroring the +// traversal TableFormatter uses for its own per-field sensitive check. +func redactSensitive(data any, showSensitive bool) any { + if showSensitive { + return data + } + return redactValue(reflect.ValueOf(data)) +} + +// isSensitiveField reports whether a struct field is tagged `sensitive:"true"`. +func isSensitiveField(field reflect.StructField) bool { + return field.Tag.Get("sensitive") == "true" +} + +func redactValue(v reflect.Value) any { + if !v.IsValid() { + return nil + } + + // Respect existing custom JSON encoding (e.g. time.Time) instead of + // walking into its unexported internals. + if v.CanInterface() { + if _, ok := v.Interface().(json.Marshaler); ok { + return v.Interface() + } + } + + switch v.Kind() { + case reflect.Pointer, reflect.Interface: + if v.IsNil() { + return nil + } + return redactValue(v.Elem()) + case reflect.Struct: + return redactStruct(v) + case reflect.Slice, reflect.Array: + if v.Kind() == reflect.Slice && v.IsNil() { + return nil + } + items := make([]any, v.Len()) + for i := 0; i < v.Len(); i++ { + items[i] = redactValue(v.Index(i)) + } + return items + case reflect.Map: + if v.IsNil() { + return nil + } + m := newOrderedMap(v.Len()) + iter := v.MapRange() + for iter.Next() { + m.set(fmt.Sprint(iter.Key().Interface()), redactValue(iter.Value())) + } + return m + default: + return v.Interface() + } +} + +// redactStruct converts a struct into an orderedMap so field order in the +// rendered JSON matches the struct's declared field order, exactly like +// marshaling the struct directly would. +func redactStruct(v reflect.Value) *orderedMap { + t := v.Type() + result := newOrderedMap(t.NumField()) + + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + if !field.IsExported() { + continue + } + + name, omitempty, skip := jsonFieldName(field) + if skip { + continue + } + + fv := v.Field(i) + + if isSensitiveField(field) { + if omitempty && isEmptyValue(fv) { + continue + } + result.set(name, SensitiveOverlay) + continue + } + + if omitempty && isEmptyValue(fv) { + continue + } + result.set(name, redactValue(fv)) + } + + return result +} + +// jsonFieldName mirrors encoding/json's handling of the `json` struct tag. +func jsonFieldName(field reflect.StructField) (name string, omitempty, skip bool) { + name = field.Name + + tag := field.Tag.Get("json") + if tag == "" { + return name, false, false + } + + parts := strings.Split(tag, ",") + if parts[0] == "-" { + return "", false, true + } + if parts[0] != "" { + name = parts[0] + } + for _, opt := range parts[1:] { + if opt == "omitempty" { + omitempty = true + } + } + + return name, omitempty, false +} + +// isEmptyValue mirrors encoding/json's definition of "empty" for omitempty. +func isEmptyValue(v reflect.Value) bool { + switch v.Kind() { + case reflect.Pointer, reflect.Interface: + return v.IsNil() + case reflect.Slice, reflect.Map: + return v.IsNil() || v.Len() == 0 + case reflect.Array, reflect.String: + return v.Len() == 0 + case reflect.Bool: + return !v.Bool() + case reflect.Int, reflect.Int8, reflect.Int16, reflect.Int32, reflect.Int64: + return v.Int() == 0 + case reflect.Uint, reflect.Uint8, reflect.Uint16, reflect.Uint32, reflect.Uint64: + return v.Uint() == 0 + case reflect.Float32, reflect.Float64: + return v.Float() == 0 + default: + return false + } +} + +// orderedMap marshals to a JSON object preserving insertion order, since +// map[string]any would otherwise sort keys alphabetically. +type orderedMap struct { + keys []string + values []any +} + +func newOrderedMap(capacity int) *orderedMap { + return &orderedMap{ + keys: make([]string, 0, capacity), + values: make([]any, 0, capacity), + } +} + +func (m *orderedMap) set(key string, value any) { + m.keys = append(m.keys, key) + m.values = append(m.values, value) +} + +func (m *orderedMap) MarshalJSON() ([]byte, error) { + var buf bytes.Buffer + buf.WriteByte('{') + for i, key := range m.keys { + if i > 0 { + buf.WriteByte(',') + } + keyBytes, err := json.Marshal(key) + if err != nil { + return nil, fmt.Errorf("failed to marshal key %q: %w", key, err) + } + buf.Write(keyBytes) + buf.WriteByte(':') + valueBytes, err := json.Marshal(m.values[i]) + if err != nil { + return nil, fmt.Errorf("failed to marshal value for key %q: %w", key, err) + } + buf.Write(valueBytes) + } + buf.WriteByte('}') + return buf.Bytes(), nil +} diff --git a/internal/output/redact_test.go b/internal/output/redact_test.go new file mode 100644 index 0000000..bbdfdc5 --- /dev/null +++ b/internal/output/redact_test.go @@ -0,0 +1,149 @@ +package output + +import ( + "bytes" + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/coollabsio/coolify-cli/internal/config" + "github.com/coollabsio/coolify-cli/internal/models" +) + +const dummySecret = "dummy-secret-abc123" + +type sensitiveNested struct { + Label string `json:"label"` + Token string `json:"token" sensitive:"true"` +} + +type sensitiveStruct struct { + Name string `json:"name"` + Secret string `json:"secret" sensitive:"true"` + PtrKey *string `json:"ptr_key,omitempty" sensitive:"true"` + Nested sensitiveNested `json:"nested"` + Nests []sensitiveNested `json:"nests"` + NestPtr *sensitiveNested `json:"nest_ptr,omitempty"` + Tags map[string]string `json:"tags"` +} + +func renderAllFormats(t *testing.T, data any, showSensitive bool) map[string]string { + t.Helper() + out := make(map[string]string) + for _, format := range []string{FormatTable, FormatJSON, FormatPretty} { + var buf bytes.Buffer + formatter, err := NewFormatter(format, Options{Writer: &buf, ShowSensitive: showSensitive}) + require.NoError(t, err) + require.NoError(t, formatter.Format(data)) + out[format] = buf.String() + } + return out +} + +func TestRedactSensitive_HidesTopLevelAndNestedFields(t *testing.T) { + ptrVal := dummySecret + data := sensitiveStruct{ + Name: "widget", + Secret: dummySecret, + PtrKey: &ptrVal, + Nested: sensitiveNested{Label: "n1", Token: dummySecret}, + Nests: []sensitiveNested{ + {Label: "n2", Token: dummySecret}, + }, + NestPtr: &sensitiveNested{Label: "n3", Token: dummySecret}, + Tags: map[string]string{"env": "prod"}, + } + + outputs := renderAllFormats(t, data, false) + for format, rendered := range outputs { + assert.NotContains(t, rendered, dummySecret, "format %s leaked secret", format) + } + + // JSON/pretty must still be valid, structurally-correct JSON with the overlay in place. + for _, format := range []string{FormatJSON, FormatPretty} { + var decoded map[string]any + require.NoError(t, json.Unmarshal([]byte(outputs[format]), &decoded), "format %s produced invalid JSON", format) + assert.Equal(t, SensitiveOverlay, decoded["secret"]) + assert.Equal(t, SensitiveOverlay, decoded["ptr_key"]) + nested, ok := decoded["nested"].(map[string]any) + require.True(t, ok, "nested field missing in %s output", format) + assert.Equal(t, SensitiveOverlay, nested["token"]) + assert.Equal(t, "n1", nested["label"]) + + nests, ok := decoded["nests"].([]any) + require.True(t, ok) + require.Len(t, nests, 1) + nestItem, ok := nests[0].(map[string]any) + require.True(t, ok) + assert.Equal(t, SensitiveOverlay, nestItem["token"]) + + nestPtr, ok := decoded["nest_ptr"].(map[string]any) + require.True(t, ok) + assert.Equal(t, SensitiveOverlay, nestPtr["token"]) + + // Non-sensitive data must round-trip untouched. + assert.Equal(t, "widget", decoded["name"]) + } +} + +func TestRedactSensitive_ShowSensitiveRevealsRealValues(t *testing.T) { + ptrVal := dummySecret + data := sensitiveStruct{ + Name: "widget", + Secret: dummySecret, + PtrKey: &ptrVal, + Nested: sensitiveNested{Label: "n1", Token: dummySecret}, + } + + outputs := renderAllFormats(t, data, true) + for format, rendered := range outputs { + assert.Contains(t, rendered, dummySecret, "format %s should reveal secret with ShowSensitive", format) + } +} + +func TestRedactSensitive_DoesNotMutateOriginal(t *testing.T) { + data := sensitiveStruct{Name: "widget", Secret: dummySecret} + + var buf bytes.Buffer + formatter, err := NewFormatter(FormatJSON, Options{Writer: &buf, ShowSensitive: false}) + require.NoError(t, err) + require.NoError(t, formatter.Format(data)) + + assert.Equal(t, dummySecret, data.Secret, "original struct must not be mutated by redaction") +} + +func TestRedactSensitive_PrivateKeyList(t *testing.T) { + keys := []models.PrivateKey{ + {UUID: "uuid-1", Name: "key-one", PublicKey: "ssh-ed25519 " + dummySecret, PrivateKey: "-----BEGIN OPENSSH PRIVATE KEY-----\n" + dummySecret}, + } + + hidden := renderAllFormats(t, keys, false) + for format, rendered := range hidden { + assert.NotContains(t, rendered, dummySecret, "format %s leaked private key material", format) + } + + shown := renderAllFormats(t, keys, true) + for format, rendered := range shown { + assert.Contains(t, rendered, dummySecret, "format %s should reveal key material with ShowSensitive", format) + } +} + +func TestRedactSensitive_InstanceToken(t *testing.T) { + instances := []config.Instance{ + {Name: "prod", FQDN: "https://coolify.example.com", Token: dummySecret, Default: true}, + } + + hidden := renderAllFormats(t, instances, false) + for format, rendered := range hidden { + assert.NotContains(t, rendered, dummySecret, "format %s leaked instance token", format) + assert.NotContains(t, strings.ToLower(rendered), strings.ToLower(dummySecret)) + } + + shown := renderAllFormats(t, instances, true) + for format, rendered := range shown { + assert.Contains(t, rendered, dummySecret, "format %s should reveal token with ShowSensitive", format) + } +} diff --git a/internal/output/table.go b/internal/output/table.go index b2ee2de..9e8376c 100644 --- a/internal/output/table.go +++ b/internal/output/table.go @@ -193,8 +193,7 @@ func (f *TableFormatter) formatStructRow(val reflect.Value) []string { value := val.Field(i) // Check if field is marked as sensitive - isSensitive := field.Tag.Get("sensitive") == "true" - if isSensitive && !f.opts.ShowSensitive { + if isSensitiveField(field) && !f.opts.ShowSensitive { row = append(row, SensitiveOverlay) } else { row = append(row, f.formatValue(value)) From a6216679ba6c9cac7e0cb2a301b89292ba776039 Mon Sep 17 00:00:00 2001 From: Noam Loewenstern Date: Thu, 17 Sep 2026 22:58:19 +0300 Subject: [PATCH 2/6] fix(api): redact all sensitive fields in --debug request/response logs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The debug HTTP logger only masked the literal JSON key "token" before printing request/response bodies to stderr. Every other sensitive field (SSH private/public keys, webhook secrets, OAuth client secrets, DB/service env values, server IP/user, etc.) was printed in full whenever --debug was used, independent of --format or --show-sensitive. Extend the key set to cover every JSON field name backing a `sensitive:"true"` struct tag in internal/models and internal/config. This layer decodes arbitrary JSON (request bodies can be untyped maps, not just tagged structs), so it can't walk Go struct tags like internal/output's formatters do — a name-based set is the equivalent for that shape. Co-Authored-By: Claude Sonnet 5 --- internal/api/client.go | 31 ++++++++++++++++++++++++++- internal/api/client_test.go | 42 +++++++++++++++++++++++++++++++++++++ 2 files changed, 72 insertions(+), 1 deletion(-) diff --git a/internal/api/client.go b/internal/api/client.go index 3f01d5c..1aa4e6e 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -215,6 +215,35 @@ func (c *Client) doRequestOnce(ctx context.Context, method, path string, body, r return nil } +// sensitiveDebugLogKeys are the JSON field names of every struct field +// tagged `sensitive:"true"` across internal/models and internal/config. +// Debug logging works on decoded, untyped JSON (request/response bodies can +// be arbitrary maps, not just tagged structs), so it can't walk Go struct +// tags directly like internal/output's formatters do; this set is the +// closest equivalent for that generic shape. Keep in sync with the +// `sensitive:"true"` tags in those packages. +var sensitiveDebugLogKeys = map[string]struct{}{ + "token": {}, + "value": {}, + "real_value": {}, + "key": {}, + "secret": {}, + "script": {}, + "client_secret": {}, + "webhook_secret": {}, + "webhook_token": {}, + "manual_webhook_secret_github": {}, + "manual_webhook_secret_gitlab": {}, + "manual_webhook_secret_bitbucket": {}, + "manual_webhook_secret_gitea": {}, + "public_key": {}, + "private_key": {}, + "email": {}, + "ip": {}, + "user": {}, + "port": {}, +} + func redactJSONForLog(jsonBody []byte) string { var value any if err := json.Unmarshal(jsonBody, &value); err != nil { @@ -232,7 +261,7 @@ func redactSensitiveFields(value any) { switch typed := value.(type) { case map[string]any: for key, child := range typed { - if key == "token" { + if _, sensitive := sensitiveDebugLogKeys[key]; sensitive { typed[key] = "********" continue } diff --git a/internal/api/client_test.go b/internal/api/client_test.go index 676ae0a..8686025 100644 --- a/internal/api/client_test.go +++ b/internal/api/client_test.go @@ -52,6 +52,48 @@ func TestClient_DebugRedactsSensitiveResponseFields(t *testing.T) { assert.Contains(t, logs.String(), `"token":"********"`) } +func TestClient_DebugRedactsPrivateKeyMaterial(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"uuid":"key-1","public_key":"ssh-ed25519 dummy-public","private_key":"-----BEGIN OPENSSH PRIVATE KEY-----\ndummy-private-material"}`)) + })) + defer server.Close() + + var logs bytes.Buffer + previousWriter := log.Writer() + log.SetOutput(&logs) + t.Cleanup(func() { log.SetOutput(previousWriter) }) + + client := NewClient(server.URL, "coolify-token", WithDebug(true), WithRetries(0)) + var response map[string]any + err := client.Get(context.Background(), "security/keys/key-1", &response) + require.NoError(t, err) + assert.NotContains(t, logs.String(), "dummy-public") + assert.NotContains(t, logs.String(), "dummy-private-material") + assert.Contains(t, logs.String(), `"private_key":"********"`) + assert.Contains(t, logs.String(), `"public_key":"********"`) +} + +func TestClient_DebugRedactsWebhookSecret(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + _, _ = w.Write([]byte(`{"uuid":"app-1"}`)) + })) + defer server.Close() + + var logs bytes.Buffer + previousWriter := log.Writer() + log.SetOutput(&logs) + t.Cleanup(func() { log.SetOutput(previousWriter) }) + + client := NewClient(server.URL, "coolify-token", WithDebug(true), WithRetries(0)) + var response map[string]any + err := client.Post(context.Background(), "github-apps", map[string]string{"client_secret": "dummy-client-secret", "webhook_secret": "dummy-webhook-secret"}, &response) + require.NoError(t, err) + assert.NotContains(t, logs.String(), "dummy-client-secret") + assert.NotContains(t, logs.String(), "dummy-webhook-secret") + assert.Contains(t, logs.String(), `"client_secret":"********"`) + assert.Contains(t, logs.String(), `"webhook_secret":"********"`) +} + func TestNewClient(t *testing.T) { t.Run("creates client with defaults", func(t *testing.T) { client := NewClient("https://app.coolify.io", "test-token") From 9067c0a880d12bdc482d276b9f8036127c7a4500 Mon Sep 17 00:00:00 2001 From: Noam Loewenstern Date: Thu, 17 Sep 2026 23:00:25 +0300 Subject: [PATCH 3/6] test(context): cover context get for the sensitive-field json/pretty leak `context get` renders the real config.Instance (including its token), unlike `context list` which already had a narrow point-fix stripping Token entirely. It was still fully exposed under --format json/pretty until the internal/output redaction fix landed. Add a regression test so this specific command stays covered. Co-Authored-By: Claude Sonnet 5 --- cmd/context/get_test.go | 75 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 cmd/context/get_test.go diff --git a/cmd/context/get_test.go b/cmd/context/get_test.go new file mode 100644 index 0000000..0059279 --- /dev/null +++ b/cmd/context/get_test.go @@ -0,0 +1,75 @@ +package context + +import ( + "bytes" + "io" + "os" + "strings" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/viper" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const getSentinelToken = "sentinel-secret-token" + +func executeGetCommand(t *testing.T, format string, showSensitive bool) string { + t.Helper() + viper.Reset() + t.Cleanup(viper.Reset) + viper.Set("instances", []any{map[string]any{ + "name": "production", + "fqdn": "https://coolify.example.com", + "token": getSentinelToken, + "default": true, + }}) + + root := &cobra.Command{Use: "test"} + root.PersistentFlags().String("format", "table", "") + root.PersistentFlags().Bool("show-sensitive", false, "") + root.AddCommand(NewGetCommand()) + args := []string{"get", "production", "--format", format} + if showSensitive { + args = append(args, "--show-sensitive") + } + root.SetArgs(args) + + read, write, err := os.Pipe() + require.NoError(t, err) + originalStdout := os.Stdout + os.Stdout = write + t.Cleanup(func() { os.Stdout = originalStdout }) + + var output bytes.Buffer + require.NoError(t, root.Execute()) + require.NoError(t, write.Close()) + os.Stdout = originalStdout + _, err = io.Copy(&output, read) + require.NoError(t, err) + require.NoError(t, read.Close()) + return output.String() +} + +// TestGetCommand_NeverExposesTokenByDefault guards against the general +// json/pretty sensitive-field leak (fixed in internal/output/redact.go) +// for the one context command that renders the real config.Instance +// (including its token) rather than a stripped-down summary. +func TestGetCommand_NeverExposesTokenByDefault(t *testing.T) { + for _, format := range []string{"table", "json", "pretty"} { + t.Run(format, func(t *testing.T) { + rendered := executeGetCommand(t, format, false) + assert.NotContains(t, rendered, getSentinelToken) + }) + } +} + +func TestGetCommand_ShowSensitiveRevealsToken(t *testing.T) { + for _, format := range []string{"table", "json", "pretty"} { + t.Run(format, func(t *testing.T) { + rendered := executeGetCommand(t, format, true) + assert.True(t, strings.Contains(rendered, getSentinelToken), "expected token to be revealed with --show-sensitive") + }) + } +} From 89b4087e5d7788f5b3c298f9bca91631872854bb Mon Sep 17 00:00:00 2001 From: Noam Loewenstern Date: Thu, 17 Sep 2026 23:36:25 +0300 Subject: [PATCH 4/6] fix(output): keep non-sensitive json output identical to encoding/json The reflection-based redactor rebuilt every value it walked, so it also reimplemented parts of encoding/json it never meant to change. Four divergences, each reproduced against json.Marshal of the same value: - []byte rendered as an array of numbers instead of base64. - Maps rendered through the insertion-ordered map used for structs, which exposed Go's randomized map iteration order where encoding/json sorts keys. Live on `server provider

regions --format json`, whose models.ProviderOption is a map[string]any: key order changed per run. - Embedded struct fields were nested under the embedded type's name instead of promoted into the parent object, or dropped entirely when the embedded type is unexported. - A type implementing json.Marshaler short-circuited the walk, so a sensitive field reachable only through it was printed in full. - A self-referential value recursed until the goroutine stack overflowed, which is fatal and unrecoverable, where encoding/json returns an error. Fix the shape by not rebuilding what doesn't need rebuilding: walk only types that can contain a `sensitive:"true"` field (containsSensitive, memoized) and hand everything else back untouched for encoding/json to marshal natively. That makes non-sensitive output identical by construction rather than by reimplementation, and it removes the need for the json.Marshaler special case, since types with no sensitive fields are already passed through. Within a walked subtree: promote embedded struct fields, render maps as plain maps so encoding/json sorts their keys, redact through a custom MarshalJSON rather than deferring to it, and guard pointer cycles. Note that SensitiveOverlay is a string, so a redacted non-string field (models.Server.Port) renders as a JSON string. Documented on redactSensitive; --show-sensitive returns the correctly typed value. Co-Authored-By: Claude Opus 5 --- internal/output/redact.go | 164 ++++++++++++++++++++++++----- internal/output/redact_test.go | 185 +++++++++++++++++++++++++++++++++ 2 files changed, 321 insertions(+), 28 deletions(-) diff --git a/internal/output/redact.go b/internal/output/redact.go index a89be99..d5f4c29 100644 --- a/internal/output/redact.go +++ b/internal/output/redact.go @@ -6,18 +6,27 @@ import ( "fmt" "reflect" "strings" + "sync" ) // redactSensitive returns a copy of data safe to serialize, with any field // tagged `sensitive:"true"` replaced by SensitiveOverlay unless showSensitive -// is true. The original value is never mutated. Struct fields are walked -// recursively through pointers, slices, arrays, and maps, mirroring the -// traversal TableFormatter uses for its own per-field sensitive check. +// is true. The original value is never mutated. +// +// Any value whose type cannot contain a sensitive field is handed back +// untouched so encoding/json marshals it natively. That keeps non-sensitive +// output byte-for-byte identical to marshaling the original value directly, +// including behavior this package does not reimplement ([]byte as base64, +// json.Marshaler, encoding.TextMarshaler, sorted map keys). +// +// Note: SensitiveOverlay is a string, so a redacted field of a non-string type +// (e.g. models.Server.Port) is rendered as a JSON string rather than its +// original type. Pass --show-sensitive to get the real, correctly typed value. func redactSensitive(data any, showSensitive bool) any { if showSensitive { return data } - return redactValue(reflect.ValueOf(data)) + return redactValue(reflect.ValueOf(data), nil) } // isSensitiveField reports whether a struct field is tagged `sensitive:"true"`. @@ -25,44 +34,59 @@ func isSensitiveField(field reflect.StructField) bool { return field.Tag.Get("sensitive") == "true" } -func redactValue(v reflect.Value) any { - if !v.IsValid() { +// redactValue walks v, rebuilding only the parts that can contain a sensitive +// field. visited holds the pointers on the current path so a self-referential +// value degrades to null instead of overflowing the stack. +func redactValue(v reflect.Value, visited map[uintptr]bool) any { + if !v.IsValid() || !v.CanInterface() { return nil } - // Respect existing custom JSON encoding (e.g. time.Time) instead of - // walking into its unexported internals. - if v.CanInterface() { - if _, ok := v.Interface().(json.Marshaler); ok { - return v.Interface() - } + if !containsSensitive(v.Type()) { + return v.Interface() } switch v.Kind() { - case reflect.Pointer, reflect.Interface: + case reflect.Pointer: + if v.IsNil() { + return nil + } + address := v.Pointer() + if visited[address] { + return nil + } + if visited == nil { + visited = make(map[uintptr]bool) + } + visited[address] = true + defer delete(visited, address) + return redactValue(v.Elem(), visited) + case reflect.Interface: if v.IsNil() { return nil } - return redactValue(v.Elem()) + return redactValue(v.Elem(), visited) case reflect.Struct: - return redactStruct(v) + return redactStruct(v, visited) case reflect.Slice, reflect.Array: if v.Kind() == reflect.Slice && v.IsNil() { return nil } items := make([]any, v.Len()) for i := 0; i < v.Len(); i++ { - items[i] = redactValue(v.Index(i)) + items[i] = redactValue(v.Index(i), visited) } return items case reflect.Map: if v.IsNil() { return nil } - m := newOrderedMap(v.Len()) + // A plain map keeps encoding/json's own key sorting, which a + // insertion-ordered map would replace with Go's randomized map order. + m := make(map[string]any, v.Len()) iter := v.MapRange() for iter.Next() { - m.set(fmt.Sprint(iter.Key().Interface()), redactValue(iter.Value())) + m[fmt.Sprint(iter.Key().Interface())] = redactValue(iter.Value(), visited) } return m default: @@ -73,12 +97,35 @@ func redactValue(v reflect.Value) any { // redactStruct converts a struct into an orderedMap so field order in the // rendered JSON matches the struct's declared field order, exactly like // marshaling the struct directly would. -func redactStruct(v reflect.Value) *orderedMap { +func redactStruct(v reflect.Value, visited map[uintptr]bool) *orderedMap { + result := newOrderedMap(v.Type().NumField()) + appendStructFields(v, result, visited) + return result +} + +func appendStructFields(v reflect.Value, result *orderedMap, visited map[uintptr]bool) { t := v.Type() - result := newOrderedMap(t.NumField()) for i := 0; i < t.NumField(); i++ { field := t.Field(i) + tag := field.Tag.Get("json") + + // encoding/json promotes an embedded struct's fields into the parent + // object unless the embedded field carries an explicit JSON name. + if field.Anonymous && embeddedName(tag) == "" { + fv := v.Field(i) + if fv.Kind() == reflect.Pointer { + if fv.IsNil() { + continue + } + fv = fv.Elem() + } + if fv.Kind() == reflect.Struct { + appendStructFields(fv, result, visited) + continue + } + } + if !field.IsExported() { continue } @@ -89,22 +136,31 @@ func redactStruct(v reflect.Value) *orderedMap { } fv := v.Field(i) + if omitempty && isEmptyValue(fv) { + continue + } if isSensitiveField(field) { - if omitempty && isEmptyValue(fv) { - continue - } result.set(name, SensitiveOverlay) continue } - if omitempty && isEmptyValue(fv) { - continue - } - result.set(name, redactValue(fv)) + result.set(name, redactValue(fv, visited)) } +} - return result +// embeddedName returns the explicit JSON name on an embedded field's tag, which +// stops encoding/json from promoting that field's contents into the parent. +func embeddedName(tag string) string { + if tag == "" { + return "" + } + name := strings.Split(tag, ",")[0] + if name == "-" { + // `json:"-"` drops the field; treat it as named so it is not promoted. + return "-" + } + return name } // jsonFieldName mirrors encoding/json's handling of the `json` struct tag. @@ -154,6 +210,58 @@ func isEmptyValue(v reflect.Value) bool { } } +var sensitiveTypeCache sync.Map // reflect.Type -> bool + +// containsSensitive reports whether t, or anything reachable from it, has a +// field tagged `sensitive:"true"`. An interface type counts as sensitive +// because its dynamic value is only knowable at runtime. +func containsSensitive(t reflect.Type) bool { + if cached, ok := sensitiveTypeCache.Load(t); ok { + return cached.(bool) + } + result := typeContainsSensitive(t, map[reflect.Type]bool{}) + sensitiveTypeCache.Store(t, result) + return result +} + +func typeContainsSensitive(t reflect.Type, inProgress map[reflect.Type]bool) bool { + if cached, ok := sensitiveTypeCache.Load(t); ok { + return cached.(bool) + } + if inProgress[t] { + // Recursive type: this edge adds nothing on its own. + return false + } + inProgress[t] = true + defer delete(inProgress, t) + + switch t.Kind() { + case reflect.Pointer, reflect.Slice, reflect.Array, reflect.Map: + return typeContainsSensitive(t.Elem(), inProgress) + case reflect.Interface: + return true + case reflect.Struct: + for i := 0; i < t.NumField(); i++ { + field := t.Field(i) + if strings.Split(field.Tag.Get("json"), ",")[0] == "-" { + continue + } + if !field.IsExported() && !field.Anonymous { + continue + } + if isSensitiveField(field) { + return true + } + if typeContainsSensitive(field.Type, inProgress) { + return true + } + } + return false + default: + return false + } +} + // orderedMap marshals to a JSON object preserving insertion order, since // map[string]any would otherwise sort keys alphabetically. type orderedMap struct { diff --git a/internal/output/redact_test.go b/internal/output/redact_test.go index bbdfdc5..89fb552 100644 --- a/internal/output/redact_test.go +++ b/internal/output/redact_test.go @@ -5,6 +5,7 @@ import ( "encoding/json" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -147,3 +148,187 @@ func TestRedactSensitive_InstanceToken(t *testing.T) { assert.Contains(t, rendered, dummySecret, "format %s should reveal token with ShowSensitive", format) } } + +// parityCarrier mixes a sensitive field with the encoding/json edge cases the +// redactor must not disturb: base64 byte slices, a json.Marshaler, nil vs empty +// slices and maps, and every omitempty flavour. +type parityCarrier struct { + Secret string `json:"secret" sensitive:"true"` + Str string `json:"str"` + Zero int `json:"zero"` + ZeroOmit int `json:"zero_omit,omitempty"` + BoolFalse bool `json:"bool_false"` + BoolOmit bool `json:"bool_omit,omitempty"` + NilPtr *string `json:"nil_ptr"` + NilSlice []string `json:"nil_slice"` + EmptySl []string `json:"empty_sl"` + NilMap map[string]string `json:"nil_map"` + EmptyMap map[string]string `json:"empty_map"` + EmptyArr [0]int `json:"empty_arr,omitempty"` + Arr [2]int `json:"arr"` + Iface any `json:"iface"` + Bytes []byte `json:"bytes"` + When time.Time `json:"when"` + Float float64 `json:"float"` + Excluded string `json:"-"` +} + +// TestRedactSensitive_LeavesNonSensitiveOutputByteIdentical is the guard +// against the redactor quietly reshaping output it only meant to pass through. +// Everything except the redacted field must marshal exactly as encoding/json +// would have marshaled it. +func TestRedactSensitive_LeavesNonSensitiveOutputByteIdentical(t *testing.T) { + data := parityCarrier{ + Secret: dummySecret, + Str: "x", + Arr: [2]int{1, 2}, + EmptySl: []string{}, + Bytes: []byte("hello"), + When: time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC), + Float: 1.5, + Excluded: dummySecret, + } + + expectation := data + expectation.Secret = SensitiveOverlay + want, err := json.Marshal(expectation) + require.NoError(t, err) + + got, err := json.Marshal(redactSensitive(data, false)) + require.NoError(t, err) + + assert.JSONEq(t, string(want), string(got)) + assert.Equal(t, string(want), string(got), "redaction must not reshape non-sensitive output") +} + +// TestRedactSensitive_MapKeysStayStablyOrdered guards against rendering maps +// through an insertion-ordered container, which would expose Go's randomized +// map iteration order instead of encoding/json's sorted keys. +func TestRedactSensitive_MapKeysStayStablyOrdered(t *testing.T) { + type carrier struct { + Options map[string]string `json:"options"` + Secret string `json:"secret" sensitive:"true"` + } + data := carrier{ + Options: map[string]string{"a": "1", "b": "2", "c": "3", "d": "4", "e": "5", "f": "6", "g": "7", "h": "8"}, + Secret: dummySecret, + } + + first, err := json.Marshal(redactSensitive(data, false)) + require.NoError(t, err) + + for i := 0; i < 50; i++ { + again, err := json.Marshal(redactSensitive(data, false)) + require.NoError(t, err) + require.Equal(t, string(first), string(again), "map key order must be stable across renders") + } + + expectation := data + expectation.Secret = SensitiveOverlay + want, err := json.Marshal(expectation) + require.NoError(t, err) + assert.Equal(t, string(want), string(first)) +} + +type embeddedBase struct { + Shared string `json:"shared"` + BaseToken string `json:"base_token" sensitive:"true"` +} + +type embeddingOuter struct { + embeddedBase + Name string `json:"name"` +} + +// TestRedactSensitive_PromotesEmbeddedFields covers encoding/json's promotion +// of embedded struct fields into the parent object. Nesting them under the type +// name (or dropping them, when the embedded type is unexported) would silently +// change the shape of any model that gains an embedded struct. +func TestRedactSensitive_PromotesEmbeddedFields(t *testing.T) { + data := embeddingOuter{ + embeddedBase: embeddedBase{Shared: "visible", BaseToken: dummySecret}, + Name: "outer", + } + + rendered, err := json.Marshal(redactSensitive(data, false)) + require.NoError(t, err) + + assert.NotContains(t, string(rendered), dummySecret) + assert.Equal(t, `{"shared":"visible","base_token":"********","name":"outer"}`, string(rendered)) +} + +type marshalerCarrier struct { + Inner sensitiveNested +} + +func (m marshalerCarrier) MarshalJSON() ([]byte, error) { return json.Marshal(m.Inner) } + +// TestRedactSensitive_RedactsThroughCustomMarshaler ensures a type's own +// MarshalJSON cannot be used to smuggle a sensitive field past the redactor. +func TestRedactSensitive_RedactsThroughCustomMarshaler(t *testing.T) { + data := struct { + Wrapped marshalerCarrier `json:"wrapped"` + }{Wrapped: marshalerCarrier{Inner: sensitiveNested{Label: "l", Token: dummySecret}}} + + rendered, err := json.Marshal(redactSensitive(data, false)) + require.NoError(t, err) + assert.NotContains(t, string(rendered), dummySecret) +} + +type selfReferential struct { + Name string `json:"name"` + Secret string `json:"secret" sensitive:"true"` + Self *selfReferential `json:"self,omitempty"` +} + +// TestRedactSensitive_SurvivesSelfReferentialValue pins the cycle guard. Without +// it the walk recurses until the goroutine stack overflows, which is a fatal +// error Go cannot recover from — it kills the CLI outright. +func TestRedactSensitive_SurvivesSelfReferentialValue(t *testing.T) { + data := &selfReferential{Name: "a", Secret: dummySecret} + data.Self = data + + rendered, err := json.Marshal(redactSensitive(data, false)) + require.NoError(t, err) + assert.NotContains(t, string(rendered), dummySecret) +} + +// TestRedactSensitive_DatabaseCredentials covers the database credentials that +// `database get`/`database list` return on the real model. +func TestRedactSensitive_DatabaseCredentials(t *testing.T) { + password := dummySecret + databases := []models.Database{{ + UUID: "db-1", + Name: "primary", + PostgresPassword: &password, + MysqlRootPassword: &password, + MysqlPassword: &password, + MariadbRootPassword: &password, + MariadbPassword: &password, + MongoInitdbRootPassword: &password, + RedisPassword: &password, + KeydbPassword: &password, + ClickhouseAdminPassword: &password, + DragonflyPassword: &password, + }} + + hidden := renderAllFormats(t, databases, false) + for format, rendered := range hidden { + assert.NotContains(t, rendered, dummySecret, "format %s leaked a database password", format) + } + + // Table output excludes these columns entirely (`table:"-"`), so only the + // structured formats can reveal them. + shown := renderAllFormats(t, databases, true) + for _, format := range []string{FormatJSON, FormatPretty} { + assert.Contains(t, shown[format], dummySecret, "format %s should reveal database passwords with ShowSensitive", format) + } +} + +// TestRedactSensitive_OmitsEmptySensitivePointer keeps an unset optional secret +// out of the output entirely rather than claiming a value exists. +func TestRedactSensitive_OmitsEmptySensitivePointer(t *testing.T) { + rendered, err := json.Marshal(redactSensitive(sensitiveStruct{Name: "widget"}, false)) + require.NoError(t, err) + assert.NotContains(t, string(rendered), "ptr_key") +} From f5234b8088f3a681d128e9e31a78c0320af6eb96 Mon Sep 17 00:00:00 2001 From: Noam Loewenstern Date: Thu, 17 Sep 2026 23:36:32 +0300 Subject: [PATCH 5/6] fix(models): tag database credentials as sensitive models.Database carries ten password fields (postgres, mysql, mariadb, mongo, redis, keydb, clickhouse, dragonfly) that had `table:"-"` but no `sensitive:"true"`. The tag-driven redaction added for json/pretty output cannot see an untagged field, so `database get`/`database list` printed them in plaintext under --format json and --format pretty: {"uuid":"...","name":"db","postgres_password":""} They were likewise absent from the debug logger's key set, so --debug printed them too. Tag all ten and add their JSON names to sensitiveDebugLogKeys. Only the Database model is tagged here; the create/update request types carry the same field names and are covered on the debug path by the key set, which is the only path that renders them. Co-Authored-By: Claude Opus 5 --- internal/api/client.go | 16 ++++++++++++++-- internal/models/database.go | 20 ++++++++++---------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/internal/api/client.go b/internal/api/client.go index 1aa4e6e..e3cb571 100644 --- a/internal/api/client.go +++ b/internal/api/client.go @@ -220,8 +220,10 @@ func (c *Client) doRequestOnce(ctx context.Context, method, path string, body, r // Debug logging works on decoded, untyped JSON (request/response bodies can // be arbitrary maps, not just tagged structs), so it can't walk Go struct // tags directly like internal/output's formatters do; this set is the -// closest equivalent for that generic shape. Keep in sync with the -// `sensitive:"true"` tags in those packages. +// closest equivalent for that generic shape. +// +// TestSensitiveDebugLogKeys_CoversEveryTaggedField fails if a new +// `sensitive:"true"` tag lands without its JSON name being added here. var sensitiveDebugLogKeys = map[string]struct{}{ "token": {}, "value": {}, @@ -242,6 +244,16 @@ var sensitiveDebugLogKeys = map[string]struct{}{ "ip": {}, "user": {}, "port": {}, + "postgres_password": {}, + "mysql_root_password": {}, + "mysql_password": {}, + "mariadb_root_password": {}, + "mariadb_password": {}, + "mongo_initdb_root_password": {}, + "redis_password": {}, + "keydb_password": {}, + "clickhouse_admin_password": {}, + "dragonfly_password": {}, } func redactJSONForLog(jsonBody []byte) string { diff --git a/internal/models/database.go b/internal/models/database.go index 37d2072..a05578e 100644 --- a/internal/models/database.go +++ b/internal/models/database.go @@ -25,46 +25,46 @@ type Database struct { // PostgreSQL specific PostgresUser *string `json:"postgres_user,omitempty" table:"-"` - PostgresPassword *string `json:"postgres_password,omitempty" table:"-"` + PostgresPassword *string `json:"postgres_password,omitempty" table:"-" sensitive:"true"` PostgresDB *string `json:"postgres_db,omitempty" table:"-"` PostgresInitdbArgs *string `json:"postgres_initdb_args,omitempty" table:"-"` PostgresHostAuthMethod *string `json:"postgres_host_auth_method,omitempty" table:"-"` PostgresConf *string `json:"postgres_conf,omitempty" table:"-"` // MySQL specific - MysqlRootPassword *string `json:"mysql_root_password,omitempty" table:"-"` - MysqlPassword *string `json:"mysql_password,omitempty" table:"-"` + MysqlRootPassword *string `json:"mysql_root_password,omitempty" table:"-" sensitive:"true"` + MysqlPassword *string `json:"mysql_password,omitempty" table:"-" sensitive:"true"` MysqlUser *string `json:"mysql_user,omitempty" table:"-"` MysqlDatabase *string `json:"mysql_database,omitempty" table:"-"` MysqlConf *string `json:"mysql_conf,omitempty" table:"-"` // MariaDB specific - MariadbRootPassword *string `json:"mariadb_root_password,omitempty" table:"-"` - MariadbPassword *string `json:"mariadb_password,omitempty" table:"-"` + MariadbRootPassword *string `json:"mariadb_root_password,omitempty" table:"-" sensitive:"true"` + MariadbPassword *string `json:"mariadb_password,omitempty" table:"-" sensitive:"true"` MariadbUser *string `json:"mariadb_user,omitempty" table:"-"` MariadbDatabase *string `json:"mariadb_database,omitempty" table:"-"` MariadbConf *string `json:"mariadb_conf,omitempty" table:"-"` // MongoDB specific MongoInitdbRootUsername *string `json:"mongo_initdb_root_username,omitempty" table:"-"` - MongoInitdbRootPassword *string `json:"mongo_initdb_root_password,omitempty" table:"-"` + MongoInitdbRootPassword *string `json:"mongo_initdb_root_password,omitempty" table:"-" sensitive:"true"` MongoInitdbDatabase *string `json:"mongo_initdb_database,omitempty" table:"-"` MongoConf *string `json:"mongo_conf,omitempty" table:"-"` // Redis specific - RedisPassword *string `json:"redis_password,omitempty" table:"-"` + RedisPassword *string `json:"redis_password,omitempty" table:"-" sensitive:"true"` RedisConf *string `json:"redis_conf,omitempty" table:"-"` // KeyDB specific - KeydbPassword *string `json:"keydb_password,omitempty" table:"-"` + KeydbPassword *string `json:"keydb_password,omitempty" table:"-" sensitive:"true"` KeydbConf *string `json:"keydb_conf,omitempty" table:"-"` // Clickhouse specific ClickhouseAdminUser *string `json:"clickhouse_admin_user,omitempty" table:"-"` - ClickhouseAdminPassword *string `json:"clickhouse_admin_password,omitempty" table:"-"` + ClickhouseAdminPassword *string `json:"clickhouse_admin_password,omitempty" table:"-" sensitive:"true"` // Dragonfly specific - DragonflyPassword *string `json:"dragonfly_password,omitempty" table:"-"` + DragonflyPassword *string `json:"dragonfly_password,omitempty" table:"-" sensitive:"true"` // Relationship IDs - internal database IDs (hidden from output) ServerID *int `json:"-" table:"-"` From c73e3e184f9175c05c71c18ebd2a73678b92439d Mon Sep 17 00:00:00 2001 From: Noam Loewenstern Date: Thu, 17 Sep 2026 23:36:39 +0300 Subject: [PATCH 6/6] test(api): fail when a sensitive tag is missing from the debug log key set sensitiveDebugLogKeys is a hand-maintained list of JSON field names with no automated link to the `sensitive:"true"` tags it mirrors. The debug logger works on untyped JSON bodies, so it cannot discover new tags on its own: a tag added to a model next month would silently keep being logged in full, with nothing failing. Parse internal/models and internal/config, collect every tagged field's JSON name, and assert the set covers them. Verified by deleting one key and watching it name the exact field: Database.RedisPassword is tagged sensitive:"true" but its JSON name "redis_password" is missing from sensitiveDebugLogKeys in client.go Co-Authored-By: Claude Opus 5 --- internal/api/sensitive_keys_test.go | 85 +++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 internal/api/sensitive_keys_test.go diff --git a/internal/api/sensitive_keys_test.go b/internal/api/sensitive_keys_test.go new file mode 100644 index 0000000..0d5b546 --- /dev/null +++ b/internal/api/sensitive_keys_test.go @@ -0,0 +1,85 @@ +package api + +import ( + "go/ast" + "go/parser" + "go/token" + "os" + "reflect" + "strconv" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestSensitiveDebugLogKeys_CoversEveryTaggedField is the guard that keeps +// sensitiveDebugLogKeys from drifting. The debug logger redacts by JSON key +// name because it works on untyped bodies, so it cannot discover new +// `sensitive:"true"` tags on its own. This test parses the model and config +// sources and fails if any tagged field's JSON name is missing from the set, +// turning a silent leak into a build failure. +func TestSensitiveDebugLogKeys_CoversEveryTaggedField(t *testing.T) { + tagged := sensitiveJSONNamesInPackages(t, "../models", "../config") + require.NotEmpty(t, tagged, "found no sensitive:\"true\" tags — the source scan is broken, not the code") + + for name, where := range tagged { + if _, ok := sensitiveDebugLogKeys[name]; !ok { + t.Errorf("%s is tagged sensitive:\"true\" but its JSON name %q is missing from sensitiveDebugLogKeys in client.go — --debug would log it in full", where, name) + } + } +} + +// sensitiveJSONNamesInPackages returns JSON name -> "Type.Field" for every +// struct field tagged `sensitive:"true"` in the given source directories. +func sensitiveJSONNamesInPackages(t *testing.T, dirs ...string) map[string]string { + t.Helper() + found := make(map[string]string) + + for _, dir := range dirs { + packages, err := parser.ParseDir(token.NewFileSet(), dir, func(info os.FileInfo) bool { + return !strings.HasSuffix(info.Name(), "_test.go") + }, 0) + require.NoError(t, err, "parsing %s", dir) + + for _, pkg := range packages { + for _, file := range pkg.Files { + ast.Inspect(file, func(node ast.Node) bool { + typeSpec, ok := node.(*ast.TypeSpec) + if !ok { + return true + } + structType, ok := typeSpec.Type.(*ast.StructType) + if !ok { + return true + } + for _, field := range structType.Fields.List { + if field.Tag == nil { + continue + } + tag, err := strconv.Unquote(field.Tag.Value) + if err != nil { + continue + } + structTag := reflect.StructTag(tag) + if structTag.Get("sensitive") != "true" { + continue + } + name := strings.Split(structTag.Get("json"), ",")[0] + if name == "" || name == "-" { + continue + } + fieldName := name + if len(field.Names) > 0 { + fieldName = field.Names[0].Name + } + found[name] = typeSpec.Name.Name + "." + fieldName + } + return true + }) + } + } + } + + return found +}