From bd24482180899824b6a19e70370f1256c206726d Mon Sep 17 00:00:00 2001 From: Husni Adil Makmur Date: Sun, 2 Aug 2026 13:10:59 +0700 Subject: [PATCH] fix: preserve JSON value types when building environment variables MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Secrets whose payload is JSON were rendered with fmt.Sprintf("%v"), which is Go's debug formatting rather than JSON. The value a process received was then not the value that was stored: 1754110382 -> "1.754110382e+09" ["a","b"] -> "[a b]" {"token":"secret"} -> "map[token:secret]" The first case is the damaging one. JSON has no integer type, so every number decodes as float64, and %v prints large float64 values in scientific notation. Any large integer in a secret — a Unix timestamp, an account id, a port — arrived corrupted, with no error to indicate it. Nothing surfaced this because the conversion cannot fail: %v accepts any value and always produces a string, so a type it handles badly still looks like a successful fetch. The failure appears later, in the process that receives the value. DecodeSecretJSON keeps numbers as json.Number so their original text survives, and StringifyValue re-encodes arrays and objects as JSON so the receiving process can parse them back. Scalars keep their literal form. Four providers decode JSON payloads and are affected: aws_secretsmanager, gcloud_secretmanager, azure_keyvault, and bitwarden in 'note' format. 1password, infisical and bitwarden_sm build string-only maps from their SDKs, so %v was already a no-op there; they move to the shared helper so the behaviour cannot drift apart later. Payloads that are not JSON still fall back to being treated as a single value, including the case of trailing content after a JSON object, which json.Unmarshal rejected and a bare json.Decoder would not. CONFIGURATION.md gains a Value Types table under Key Mappings, since the conversion applies to every provider that parses JSON rather than to one. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01Hmh2p2Bg6kmxxvzpFDW2WL --- CONFIGURATION.md | 18 +++ internal/provider/aws/secretsmanager.go | 6 +- .../provider/azurekeyvault/azurekeyvault.go | 6 +- internal/provider/bitwarden/bitwarden.go | 6 +- internal/provider/bitwarden/bitwarden_sm.go | 2 +- internal/provider/gcsm/gcsm.go | 6 +- internal/provider/infisical/infisical.go | 2 +- internal/provider/onepassword/onepassword.go | 2 +- internal/provider/value.go | 63 ++++++++++ internal/provider/value_test.go | 110 ++++++++++++++++++ tests/end2end/json_value_types_test.go | 88 ++++++++++++++ tests/end2end/testhelpers.go | 13 ++- 12 files changed, 306 insertions(+), 16 deletions(-) create mode 100644 internal/provider/value.go create mode 100644 internal/provider/value_test.go create mode 100644 tests/end2end/json_value_types_test.go diff --git a/CONFIGURATION.md b/CONFIGURATION.md index af48f3b..39ce841 100644 --- a/CONFIGURATION.md +++ b/CONFIGURATION.md @@ -737,6 +737,24 @@ keys: - Use `==` to keep the source key name as the target name - Keys are case-sensitive +### Value Types + +Environment variables are always strings, so values from a JSON secret are +converted: + +| JSON value | Environment variable | +|---|---| +| `"token"` | `token` | +| `1754110382` | `1754110382` | +| `1.5` | `1.5` | +| `true` | `true` | +| `["read","write"]` | `["read","write"]` | +| `{"token":"secret"}` | `{"token":"secret"}` | +| `null` | (empty string) | + +Numbers keep the digits they were written with. Arrays and objects are passed +through as JSON, so the receiving process can parse them back. + ## Environment Inheritance By default, sstart inherits all system environment variables and adds secrets on top. To create a clean environment with only secrets (no system environment variables), set `inherit: false`: diff --git a/internal/provider/aws/secretsmanager.go b/internal/provider/aws/secretsmanager.go index 915f622..a9d69a1 100644 --- a/internal/provider/aws/secretsmanager.go +++ b/internal/provider/aws/secretsmanager.go @@ -88,8 +88,8 @@ func (p *SecretsManagerProvider) Fetch(secretContext provider.SecretContext, map } // Parse the secret value (assuming JSON format) - var secretData map[string]interface{} - if err := json.Unmarshal([]byte(*result.SecretString), &secretData); err != nil { + secretData, err := provider.DecodeSecretJSON([]byte(*result.SecretString)) + if err != nil { // If not JSON, treat as a single value secretKey := strings.ToUpper(strings.ReplaceAll(mapID, "-", "_")) + "_SECRET" log.Printf("WARN: Secret from provider '%s' is not JSON format. Secret loaded to %s", mapID, secretKey) @@ -118,7 +118,7 @@ func (p *SecretsManagerProvider) Fetch(secretContext provider.SecretContext, map continue } - value := fmt.Sprintf("%v", v) + value := provider.StringifyValue(v) kvs = append(kvs, provider.KeyValue{ Key: targetKey, Value: value, diff --git a/internal/provider/azurekeyvault/azurekeyvault.go b/internal/provider/azurekeyvault/azurekeyvault.go index b086483..6e1f421 100644 --- a/internal/provider/azurekeyvault/azurekeyvault.go +++ b/internal/provider/azurekeyvault/azurekeyvault.go @@ -88,8 +88,8 @@ func (p *AzureKeyVaultProvider) Fetch(secretContext provider.SecretContext, mapI } // Try to parse as JSON first - var secretData map[string]interface{} - if err := json.Unmarshal([]byte(secretValue), &secretData); err != nil { + secretData, err := provider.DecodeSecretJSON([]byte(secretValue)) + if err != nil { // If not JSON, treat as a single value secretKey := strings.ToUpper(strings.ReplaceAll(mapID, "-", "_")) + "_SECRET" log.Printf("WARN: Secret from provider '%s' is not JSON format. Secret loaded to %s", mapID, secretKey) @@ -118,7 +118,7 @@ func (p *AzureKeyVaultProvider) Fetch(secretContext provider.SecretContext, mapI continue } - value := fmt.Sprintf("%v", v) + value := provider.StringifyValue(v) kvs = append(kvs, provider.KeyValue{ Key: targetKey, Value: value, diff --git a/internal/provider/bitwarden/bitwarden.go b/internal/provider/bitwarden/bitwarden.go index fa133c7..fc3b642 100644 --- a/internal/provider/bitwarden/bitwarden.go +++ b/internal/provider/bitwarden/bitwarden.go @@ -195,9 +195,11 @@ func (p *BitwardenProvider) Fetch(secretContext provider.SecretContext, mapID st if item.Notes == "" { return nil, fmt.Errorf("bitwarden item '%s' has no notes for 'note' format", cfg.ItemID) } - if err := json.Unmarshal([]byte(item.Notes), &secretData); err != nil { + parsedNotes, err := provider.DecodeSecretJSON([]byte(item.Notes)) + if err != nil { return nil, fmt.Errorf("failed to parse notes as JSON for bitwarden item '%s': %w", cfg.ItemID, err) } + secretData = parsedNotes case "login": // Extract login credentials if item.Login == nil { @@ -301,7 +303,7 @@ func (p *BitwardenProvider) Fetch(secretContext provider.SecretContext, mapID st continue } - value := fmt.Sprintf("%v", v) + value := provider.StringifyValue(v) kvs = append(kvs, provider.KeyValue{ Key: targetKey, Value: value, diff --git a/internal/provider/bitwarden/bitwarden_sm.go b/internal/provider/bitwarden/bitwarden_sm.go index abb842b..3baac24 100644 --- a/internal/provider/bitwarden/bitwarden_sm.go +++ b/internal/provider/bitwarden/bitwarden_sm.go @@ -140,7 +140,7 @@ func (p *BitwardenSMProvider) Fetch(secretContext provider.SecretContext, mapID continue } - value := fmt.Sprintf("%v", v) + value := provider.StringifyValue(v) kvs = append(kvs, provider.KeyValue{ Key: targetKey, Value: value, diff --git a/internal/provider/gcsm/gcsm.go b/internal/provider/gcsm/gcsm.go index aea5047..70ed6a1 100644 --- a/internal/provider/gcsm/gcsm.go +++ b/internal/provider/gcsm/gcsm.go @@ -84,9 +84,9 @@ func (p *GCSMProvider) Fetch(secretContext provider.SecretContext, mapID string, } // Parse the secret value (assuming JSON format) - secretData := make(map[string]interface{}) secretString := string(result.Payload.Data) - if err := json.Unmarshal([]byte(secretString), &secretData); err != nil { + secretData, err := provider.DecodeSecretJSON([]byte(secretString)) + if err != nil { // If not JSON, treat as a single value secretKey := strings.ToUpper(strings.ReplaceAll(mapID, "-", "_")) + "_SECRET" log.Printf("WARN: Secret from provider '%s' is not JSON format. Secret loaded to %s", mapID, secretKey) @@ -115,7 +115,7 @@ func (p *GCSMProvider) Fetch(secretContext provider.SecretContext, mapID string, continue } - value := fmt.Sprintf("%v", v) + value := provider.StringifyValue(v) kvs = append(kvs, provider.KeyValue{ Key: targetKey, Value: value, diff --git a/internal/provider/infisical/infisical.go b/internal/provider/infisical/infisical.go index b40db5b..9c59d9e 100644 --- a/internal/provider/infisical/infisical.go +++ b/internal/provider/infisical/infisical.go @@ -126,7 +126,7 @@ func (p *InfisicalProvider) Fetch(secretContext provider.SecretContext, mapID st continue } - value := fmt.Sprintf("%v", v) + value := provider.StringifyValue(v) kvs = append(kvs, provider.KeyValue{ Key: targetKey, Value: value, diff --git a/internal/provider/onepassword/onepassword.go b/internal/provider/onepassword/onepassword.go index 6fc139d..c9912bc 100644 --- a/internal/provider/onepassword/onepassword.go +++ b/internal/provider/onepassword/onepassword.go @@ -399,7 +399,7 @@ func mapSecretKeys(secretData map[string]interface{}, keys map[string]string) [] continue } - value := fmt.Sprintf("%v", v) + value := provider.StringifyValue(v) kvs = append(kvs, provider.KeyValue{ Key: targetKey, Value: value, diff --git a/internal/provider/value.go b/internal/provider/value.go new file mode 100644 index 0000000..09f1a8d --- /dev/null +++ b/internal/provider/value.go @@ -0,0 +1,63 @@ +package provider + +import ( + "bytes" + "encoding/json" + "fmt" + "strconv" +) + +// DecodeSecretJSON parses a secret payload into a generic map, keeping numbers +// as json.Number so that their original text survives the round trip. +// +// Decoding into interface{} the usual way turns every JSON number into a +// float64, and large integers such as Unix timestamps then stringify as +// scientific notation. +func DecodeSecretJSON(data []byte) (map[string]interface{}, error) { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + + var parsed map[string]interface{} + if err := dec.Decode(&parsed); err != nil { + return nil, err + } + + // json.Unmarshal rejects trailing content; Decoder does not. Keep the + // stricter behaviour so a payload that is only partly JSON still falls + // back to being treated as a plain value. + if dec.More() { + return nil, fmt.Errorf("unexpected trailing content after JSON value") + } + + return parsed, nil +} + +// StringifyValue renders a decoded JSON value as the string that goes into an +// environment variable. +// +// Scalars keep their literal JSON form. Arrays and objects are re-encoded as +// JSON so the value stays parseable by the receiving process; Go's default +// formatting would emit map[k:v] instead. +func StringifyValue(v interface{}) string { + switch value := v.(type) { + case nil: + return "" + case string: + return value + case json.Number: + return value.String() + case bool: + return strconv.FormatBool(value) + case float64: + return strconv.FormatFloat(value, 'f', -1, 64) + case int: + return strconv.Itoa(value) + case int64: + return strconv.FormatInt(value, 10) + default: + if encoded, err := json.Marshal(value); err == nil { + return string(encoded) + } + return fmt.Sprintf("%v", value) + } +} diff --git a/internal/provider/value_test.go b/internal/provider/value_test.go new file mode 100644 index 0000000..dcfd647 --- /dev/null +++ b/internal/provider/value_test.go @@ -0,0 +1,110 @@ +package provider + +import ( + "encoding/json" + "testing" +) + +func TestStringifyValue(t *testing.T) { + tests := []struct { + name string + value interface{} + want string + }{ + {"string", "plain", "plain"}, + {"empty string", "", ""}, + {"nil", nil, ""}, + {"bool true", true, "true"}, + {"bool false", false, "false"}, + {"json number int", json.Number("42"), "42"}, + {"json number float", json.Number("1.5"), "1.5"}, + {"float64", float64(1.5), "1.5"}, + {"int", 7, "7"}, + {"int64", int64(7), "7"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := StringifyValue(tt.value); got != tt.want { + t.Errorf("StringifyValue(%v) = %q, want %q", tt.value, got, tt.want) + } + }) + } +} + +// A Unix timestamp is the case that motivated this helper: decoded as float64 +// and rendered with %v it becomes "1.754110382e+09". +func TestStringifyValue_LargeIntegerKeepsItsDigits(t *testing.T) { + parsed, err := DecodeSecretJSON([]byte(`{"expiresAt":1754110382}`)) + if err != nil { + t.Fatalf("DecodeSecretJSON() error = %v", err) + } + + if got := StringifyValue(parsed["expiresAt"]); got != "1754110382" { + t.Errorf("expiresAt = %q, want %q", got, "1754110382") + } +} + +func TestStringifyValue_ContainersStayParseableJSON(t *testing.T) { + parsed, err := DecodeSecretJSON([]byte(`{"list":["a","b"],"nested":{"token":"sk-secret"}}`)) + if err != nil { + t.Fatalf("DecodeSecretJSON() error = %v", err) + } + + tests := []struct { + key string + want string + }{ + {"list", `["a","b"]`}, + {"nested", `{"token":"sk-secret"}`}, + } + + for _, tt := range tests { + t.Run(tt.key, func(t *testing.T) { + got := StringifyValue(parsed[tt.key]) + if got != tt.want { + t.Errorf("%s = %q, want %q", tt.key, got, tt.want) + } + // The point of re-encoding is that the receiver can parse it back. + var back interface{} + if err := json.Unmarshal([]byte(got), &back); err != nil { + t.Errorf("%s is not valid JSON: %v", tt.key, err) + } + }) + } +} + +func TestDecodeSecretJSON(t *testing.T) { + tests := []struct { + name string + payload string + wantErr bool + }{ + {"object", `{"a":"b"}`, false}, + {"empty object", `{}`, false}, + {"plain string is not an object", `hunter2`, true}, + {"array is not an object", `["a"]`, true}, + {"truncated", `{"a":`, true}, + {"trailing content", `{"a":"b"} extra`, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := DecodeSecretJSON([]byte(tt.payload)) + if (err != nil) != tt.wantErr { + t.Errorf("DecodeSecretJSON(%q) error = %v, wantErr %v", tt.payload, err, tt.wantErr) + } + }) + } +} + +func TestDecodeSecretJSON_NumbersAreNotFloats(t *testing.T) { + parsed, err := DecodeSecretJSON([]byte(`{"n":1754110382}`)) + if err != nil { + t.Fatalf("DecodeSecretJSON() error = %v", err) + } + + if _, ok := parsed["n"].(json.Number); !ok { + t.Errorf("n decoded as %T, want json.Number", parsed["n"]) + } +} diff --git a/tests/end2end/json_value_types_test.go b/tests/end2end/json_value_types_test.go new file mode 100644 index 0000000..4f8eadd --- /dev/null +++ b/tests/end2end/json_value_types_test.go @@ -0,0 +1,88 @@ +package end2end + +import ( + "context" + "fmt" + "os" + "path/filepath" + "testing" + + "github.com/dirathea/sstart/internal/config" + _ "github.com/dirathea/sstart/internal/provider/aws" + "github.com/dirathea/sstart/internal/secrets" +) + +// TestE2E_JSONValueTypes covers secret payloads whose values are not strings. +// +// Rendering decoded JSON with %v turned large integers into scientific +// notation and containers into Go map syntax, so the value that reached the +// child process was not the value that was stored. +func TestE2E_JSONValueTypes(t *testing.T) { + ctx := context.Background() + + localstack := SetupLocalStack(ctx, t) + defer func() { + if err := localstack.Cleanup(); err != nil { + t.Errorf("Failed to terminate localstack container: %v", err) + } + }() + + secretName := "test/myapp/mixed-types" + payload := `{ + "API_KEY": "plain-string", + "EXPIRES_AT": 1754110382, + "PORT": 5432, + "RATIO": 1.5, + "ENABLED": true, + "SCOPES": ["read", "write"], + "NESTED": {"token": "sk-secret"} + }` + SetupAWSSecretRaw(ctx, t, localstack, secretName, payload) + + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, ".sstart.yml") + + configYAML := fmt.Sprintf(` +providers: + - kind: aws_secretsmanager + id: aws-types + secret_id: %s + region: us-east-1 + endpoint: %s +`, secretName, localstack.Endpoint) + + if err := os.WriteFile(configFile, []byte(configYAML), 0644); err != nil { + t.Fatalf("Failed to write config file: %v", err) + } + + cfg, err := config.Load(configFile) + if err != nil { + t.Fatalf("Failed to load config: %v", err) + } + + collected, err := secrets.NewCollector(cfg).Collect(ctx, nil) + if err != nil { + t.Fatalf("Failed to collect secrets: %v", err) + } + + expected := map[string]string{ + "API_KEY": "plain-string", + "EXPIRES_AT": "1754110382", + "PORT": "5432", + "RATIO": "1.5", + "ENABLED": "true", + "SCOPES": `["read","write"]`, + "NESTED": `{"token":"sk-secret"}`, + } + + for key, want := range expected { + got, exists := collected[key] + if !exists { + t.Errorf("Expected secret '%s' not found", key) + continue + } + if got != want { + t.Errorf("Secret '%s': expected '%s', got '%s'", key, want, got) + } + } +} diff --git a/tests/end2end/testhelpers.go b/tests/end2end/testhelpers.go index 319ab78..43c3e3f 100644 --- a/tests/end2end/testhelpers.go +++ b/tests/end2end/testhelpers.go @@ -151,12 +151,21 @@ func SetupAllContainers(ctx context.Context, t *testing.T) (*LocalStackContainer func SetupAWSSecret(ctx context.Context, t *testing.T, localstack *LocalStackContainer, secretName string, secretData map[string]string) { t.Helper() - awsRegion := "us-east-1" secretJSON, err := json.Marshal(secretData) if err != nil { t.Fatalf("Failed to marshal secret data: %v", err) } + SetupAWSSecretRaw(ctx, t, localstack, secretName, string(secretJSON)) +} + +// SetupAWSSecretRaw stores a secret payload verbatim, for cases where the +// payload is not a flat map of strings. +func SetupAWSSecretRaw(ctx context.Context, t *testing.T, localstack *LocalStackContainer, secretName string, payload string) { + t.Helper() + + awsRegion := "us-east-1" + awsCfg, err := awsconfig.LoadDefaultConfig(ctx, awsconfig.WithRegion(awsRegion), awsconfig.WithCredentialsProvider(credentials.NewStaticCredentialsProvider("test", "test", "")), @@ -171,7 +180,7 @@ func SetupAWSSecret(ctx context.Context, t *testing.T, localstack *LocalStackCon _, err = secretsManagerClient.CreateSecret(ctx, &secretsmanager.CreateSecretInput{ Name: aws.String(secretName), - SecretString: aws.String(string(secretJSON)), + SecretString: aws.String(payload), }) if err != nil { t.Fatalf("Failed to create secret in AWS Secrets Manager: %v", err)