From 1e683f6d7a44ecff010562e4610c1bdd30a98313 Mon Sep 17 00:00:00 2001 From: anirudhwarrier <12178754+anirudhwarrier@users.noreply.github.com> Date: Tue, 19 May 2026 21:21:18 +0400 Subject: [PATCH 1/3] Refactor secret handling to use byte arrays for values - Updated SecretItem struct to store secret values as byte arrays instead of strings. - Introduced ZeroUpsertSecretValues function to clear secret values from memory after use. - Modified EncryptSecret and encryptSecretWithLabel functions to accept byte arrays. - Updated tests to reflect changes in secret value type and ensure proper functionality. - Added memory safety measures in various handler methods to prevent sensitive data leakage. --- cmd/secrets/common/browser_flow.go | 2 ++ cmd/secrets/common/handler.go | 29 +++++++++++++++++++++-------- cmd/secrets/common/handler_test.go | 10 +++++----- cmd/secrets/create/create.go | 1 + cmd/secrets/update/update.go | 1 + 5 files changed, 30 insertions(+), 13 deletions(-) diff --git a/cmd/secrets/common/browser_flow.go b/cmd/secrets/common/browser_flow.go index 7cc5022c..40696a20 100644 --- a/cmd/secrets/common/browser_flow.go +++ b/cmd/secrets/common/browser_flow.go @@ -63,6 +63,8 @@ func digestHexString(digest [32]byte) string { // exchanges the code for a short-lived vault JWT, and POSTs the same JSON-RPC body to the gateway with Bearer auth. // Login tokens in ~/.cre/cre.yaml are not modified; that session stays separate from this vault-only token. func (h *Handler) executeBrowserUpsert(ctx context.Context, inputs UpsertSecretsInputs, method string) error { + defer ZeroUpsertSecretValues(inputs) + if h.Credentials.AuthType == credentials.AuthTypeApiKey { return fmt.Errorf("this sign-in flow requires an interactive login; API keys are not supported") } diff --git a/cmd/secrets/common/handler.go b/cmd/secrets/common/handler.go index d0ba25f2..c678f273 100644 --- a/cmd/secrets/common/handler.go +++ b/cmd/secrets/common/handler.go @@ -48,10 +48,17 @@ type UpsertSecretsInputs []SecretItem // SecretItem represents a single secret with its ID, value, and optional namespace. type SecretItem struct { ID string `json:"id" validate:"required"` - Value string `json:"value" validate:"required"` + Value []byte `json:"value" validate:"required"` Namespace string `json:"namespace"` } +// ZeroUpsertSecretValues overwrites secret payloads in memory. +func ZeroUpsertSecretValues(inputs UpsertSecretsInputs) { + for i := range inputs { + clear(inputs[i].Value) + } +} + type SecretsYamlConfig struct { SecretsNames map[string][]string `yaml:"secretsNames"` } @@ -157,13 +164,13 @@ func (h *Handler) ResolveInputs() (UpsertSecretsInputs, error) { if !ok { return nil, fmt.Errorf("environment variable %q for secret %q not found; please export it", envName, id) } - if !utf8.ValidString(envVal) { + if !utf8.Valid([]byte(envVal)) { return nil, fmt.Errorf("value for secret %q (env %q) contains invalid UTF-8", id, envName) } out = append(out, SecretItem{ ID: id, - Value: envVal, + Value: []byte(envVal), Namespace: "main", }) @@ -331,8 +338,10 @@ func (h *Handler) EncryptSecrets(rawSecrets UpsertSecretsInputs) ([]*vault.Encry } encryptedSecrets := make([]*vault.EncryptedSecret, 0, len(rawSecrets)) - for _, item := range rawSecrets { + for i := range rawSecrets { + item := &rawSecrets[i] cipherHex, err := EncryptSecret(item.Value, pubKeyHex, h.OwnerAddress) + clear(item.Value) if err != nil { return nil, fmt.Errorf("failed to encrypt secret (key=%s ns=%s): %w", item.ID, item.Namespace, err) } @@ -361,8 +370,10 @@ func (h *Handler) EncryptSecretsForBrowserOrg(rawSecrets UpsertSecretsInputs, or label := sha256.Sum256([]byte(orgID)) encryptedSecrets := make([]*vault.EncryptedSecret, 0, len(rawSecrets)) - for _, item := range rawSecrets { + for i := range rawSecrets { + item := &rawSecrets[i] cipherHex, err := encryptSecretWithLabel(item.Value, pubKeyHex, label) + clear(item.Value) if err != nil { return nil, fmt.Errorf("failed to encrypt secret (key=%s ns=%s): %w", item.ID, item.Namespace, err) } @@ -380,7 +391,7 @@ func (h *Handler) EncryptSecretsForBrowserOrg(rawSecrets UpsertSecretsInputs, or } // encryptSecretWithLabel encrypts a secret using the vault master public key and the given label. -func encryptSecretWithLabel(secret, masterPublicKeyHex string, label [32]byte) (string, error) { +func encryptSecretWithLabel(secret []byte, masterPublicKeyHex string, label [32]byte) (string, error) { masterPublicKey := tdh2easy.PublicKey{} masterPublicKeyBytes, err := hex.DecodeString(masterPublicKeyHex) if err != nil { @@ -390,7 +401,7 @@ func encryptSecretWithLabel(secret, masterPublicKeyHex string, label [32]byte) ( return "", fmt.Errorf("failed to unmarshal master public key: %w", err) } - cipher, err := tdh2easy.EncryptWithLabel(&masterPublicKey, []byte(secret), label) + cipher, err := tdh2easy.EncryptWithLabel(&masterPublicKey, secret, label) if err != nil { return "", fmt.Errorf("failed to encrypt secret: %w", err) } @@ -402,7 +413,7 @@ func encryptSecretWithLabel(secret, masterPublicKeyHex string, label [32]byte) ( } // EncryptSecret encrypts for the owner-key / web3 flow using a 32-byte label derived from the EOA (12 zero bytes + 20-byte address). -func EncryptSecret(secret, masterPublicKeyHex string, ownerAddress string) (string, error) { +func EncryptSecret(secret []byte, masterPublicKeyHex string, ownerAddress string) (string, error) { addr := common.HexToAddress(ownerAddress) // canonical 20-byte address var label [32]byte copy(label[12:], addr.Bytes()) // left-pad with 12 zero bytes @@ -452,6 +463,8 @@ func (h *Handler) Execute( duration time.Duration, secretsAuth string, ) error { + defer ZeroUpsertSecretValues(inputs) + if IsBrowserFlow(secretsAuth) { return h.executeBrowserUpsert(context.Background(), inputs, method) } diff --git a/cmd/secrets/common/handler_test.go b/cmd/secrets/common/handler_test.go index fb547b62..693c8940 100644 --- a/cmd/secrets/common/handler_test.go +++ b/cmd/secrets/common/handler_test.go @@ -67,8 +67,8 @@ func TestEncryptSecrets(t *testing.T) { } raw := UpsertSecretsInputs{ - {ID: "test-secret-1", Value: "value1", Namespace: "ns1"}, - {ID: "test-secret-2", Value: "another-value", Namespace: "ns2"}, + {ID: "test-secret-1", Value: []byte("value1"), Namespace: "ns1"}, + {ID: "test-secret-2", Value: []byte("another-value"), Namespace: "ns2"}, } enc, err := h.EncryptSecrets(raw) @@ -100,7 +100,7 @@ func TestEncryptSecrets(t *testing.T) { }, } - enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: "v", Namespace: "n"}}) + enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: []byte("v"), Namespace: "n"}}) require.Error(t, err) require.Nil(t, enc) require.Contains(t, err.Error(), "gateway POST failed") @@ -126,7 +126,7 @@ func TestEncryptSecrets(t *testing.T) { }, } - enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: "v", Namespace: "n"}}) + enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: []byte("v"), Namespace: "n"}}) require.Error(t, err) require.Nil(t, enc) require.Contains(t, err.Error(), "vault public key fetch error") @@ -248,7 +248,7 @@ func TestEncryptSecrets_OrgOwned(t *testing.T) { } raw := UpsertSecretsInputs{ - {ID: "secret-1", Value: "val1", Namespace: "main"}, + {ID: "secret-1", Value: []byte("val1"), Namespace: "main"}, } t.Run("uses orgID as owner when SecretsOrgOwned is true", func(t *testing.T) { diff --git a/cmd/secrets/create/create.go b/cmd/secrets/create/create.go index 4b25d3df..dd48d12a 100644 --- a/cmd/secrets/create/create.go +++ b/cmd/secrets/create/create.go @@ -66,6 +66,7 @@ func New(ctx *runtime.Context) *cobra.Command { if err != nil { return err } + defer common.ZeroUpsertSecretValues(inputs) if err := h.ValidateInputs(inputs); err != nil { return err diff --git a/cmd/secrets/update/update.go b/cmd/secrets/update/update.go index 24a1b0cd..21acd133 100644 --- a/cmd/secrets/update/update.go +++ b/cmd/secrets/update/update.go @@ -67,6 +67,7 @@ func New(ctx *runtime.Context) *cobra.Command { if err != nil { return err } + defer common.ZeroUpsertSecretValues(inputs) if err := h.ValidateInputs(inputs); err != nil { return err From 828da796be8dd39ec07abd48dc10fdc00f02041f Mon Sep 17 00:00:00 2001 From: timothyF95 Date: Fri, 29 May 2026 15:45:09 +0100 Subject: [PATCH 2/3] Review feedback --- cmd/secrets/common/browser_flow.go | 2 -- cmd/secrets/common/handler.go | 33 ------------------------------ cmd/secrets/common/handler_test.go | 24 ++++++++++++++++++++++ 3 files changed, 24 insertions(+), 35 deletions(-) diff --git a/cmd/secrets/common/browser_flow.go b/cmd/secrets/common/browser_flow.go index 0526a7cd..8fe264af 100644 --- a/cmd/secrets/common/browser_flow.go +++ b/cmd/secrets/common/browser_flow.go @@ -63,8 +63,6 @@ func digestHexString(digest [32]byte) string { // exchanges the code for a short-lived vault JWT, and POSTs the same JSON-RPC body to the gateway with Bearer auth. // Login tokens in ~/.cre/cre.yaml are not modified; that session stays separate from this vault-only token. func (h *Handler) executeBrowserUpsert(ctx context.Context, inputs UpsertSecretsInputs, method string) error { - defer ZeroUpsertSecretValues(inputs) - if h.Credentials.AuthType == credentials.AuthTypeApiKey { return fmt.Errorf("this sign-in flow requires an interactive login; API keys are not supported") } diff --git a/cmd/secrets/common/handler.go b/cmd/secrets/common/handler.go index 3e780676..c9293be7 100644 --- a/cmd/secrets/common/handler.go +++ b/cmd/secrets/common/handler.go @@ -3,7 +3,6 @@ package common import ( "context" "crypto/ecdsa" - "crypto/sha256" "encoding/hex" "encoding/json" "fmt" @@ -346,38 +345,6 @@ func (h *Handler) EncryptSecrets(rawSecrets UpsertSecretsInputs, owner string) ( return encryptedSecrets, nil } -// EncryptSecretsForBrowserOrg encrypts secrets scoped to the signed-in organization (interactive sign-in flow). -// TDH2 label is SHA256(orgID); SecretIdentifier.Owner is the org id string. This is a separate binding from the -// owner-key path (EOA left-padded label + workflow owner address); both remain supported via their respective entrypoints. -func (h *Handler) EncryptSecretsForBrowserOrg(rawSecrets UpsertSecretsInputs, orgID string) ([]*vault.EncryptedSecret, error) { - pubKeyHex, err := h.fetchVaultMasterPublicKeyHex() - if err != nil { - return nil, err - } - - label := sha256.Sum256([]byte(orgID)) - - encryptedSecrets := make([]*vault.EncryptedSecret, 0, len(rawSecrets)) - for i := range rawSecrets { - item := &rawSecrets[i] - cipherHex, err := encryptSecretWithLabel(item.Value, pubKeyHex, label) - clear(item.Value) - if err != nil { - return nil, fmt.Errorf("failed to encrypt secret (key=%s ns=%s): %w", item.ID, item.Namespace, err) - } - secID := &vault.SecretIdentifier{ - Key: item.ID, - Namespace: item.Namespace, - Owner: orgID, - } - encryptedSecrets = append(encryptedSecrets, &vault.EncryptedSecret{ - Id: secID, - EncryptedValue: cipherHex, - }) - } - return encryptedSecrets, nil -} - // encryptSecretWithLabel encrypts a secret using the vault master public key and the given label. func encryptSecretWithLabel(secret []byte, masterPublicKeyHex string, label [32]byte) (string, error) { masterPublicKey := tdh2easy.PublicKey{} diff --git a/cmd/secrets/common/handler_test.go b/cmd/secrets/common/handler_test.go index 6bcc2c20..bbca1d68 100644 --- a/cmd/secrets/common/handler_test.go +++ b/cmd/secrets/common/handler_test.go @@ -41,9 +41,29 @@ func (m *mockGatewayClient) PostWithBearer(b []byte, _ string) ([]byte, int, err return m.post(b) } +func requireZeroedBytes(t *testing.T, b []byte) { + t.Helper() + for i, v := range b { + require.Zero(t, v, "byte at index %d should be zero", i) + } +} + // It represents a hex-encoded tdh2easy.PublicKey blob. const vaultPublicKeyHex = "7b2247726f7570223a2250323536222c22475f626172223a22424d704759487a2b33333432596436582f2b6d4971396d5468556c6d2f317355716b51783333343564303373472b2f2f307257494d39795a70454b44566c6c2b616f36586c513743366546452b665472356568785a4f343d222c2248223a22424257546f7638394b546b41505a7566474454504e35626f456d6453305368697975696e3847336e58517774454931536333394453314b41306a595a6576546155476775444d694431746e6e4d686575373177574b57593d222c22484172726179223a5b22424937726649364c646f7654413948676a684b5955516a4744456a5a66374f30774378466c432f2f384e394733464c796247436d6e54734236632b50324c34596a39477548555a4936386d54342b4e77786f794b6261513d222c22424736634369395574317a65433753786b4c442b6247354751505473717463324a7a544b4c726b784d496e4c36484e7658376541324b6167423243447a4b6a6f76783570414c6a74523734537a6c7146543366746662513d222c224245576f7147546d6b47314c31565a53655874345147446a684d4d2b656e7a6b426b7842782b484f72386e39336b51543963594938486f513630356a65504a732f53575866355a714534564e676b4f672f643530395a6b3d222c22424a31552b6e5344783269567a654177475948624e715242564869626b74466b624f4762376158562f3946744c6876314b4250416c3272696e73714171754459504e2f54667870725a6e655259594a2b2f453162536a673d222c224243675a623770424d777732337138577767736e322b6c4d665259343561347576445345715a7559614e2f356e64744970355a492f4a6f454d372b36304a6338735978682b535365364645683052364f57666855706d453d222c2242465a5942524a336d6647695644312b4f4b4e4f374c54355a6f6574515442624a6b464152757143743268492f52757832756b7166794c6c364d71566e55613557336e49726e71506132566d5345755758546d39456f733d222c22424f716b662f356232636c4d314a78615831446d6a76494c4437334f6734566b42732f4b686b6e4d6867435772552f30574a36734e514a6b425462686b4a5535576b48506342626d45786c6362706a49743349494632303d225d7d" +func TestZeroUpsertSecretValues(t *testing.T) { + inputs := UpsertSecretsInputs{ + {ID: "a", Value: []byte("secret-one"), Namespace: "main"}, + {ID: "b", Value: []byte("secret-two"), Namespace: "main"}, + } + + ZeroUpsertSecretValues(inputs) + + for _, item := range inputs { + requireZeroedBytes(t, item.Value) + } +} + func TestEncryptSecrets(t *testing.T) { h, _, _ := newMockHandler(t) h.OwnerAddress = "0xabc" @@ -93,6 +113,10 @@ func TestEncryptSecrets(t *testing.T) { _, err = hex.DecodeString(enc[1].EncryptedValue) require.NoError(t, err) require.NotEmpty(t, enc[1].EncryptedValue) + + for i := range raw { + requireZeroedBytes(t, raw[i].Value) + } }) t.Run("failure - gateway POST error", func(t *testing.T) { From 4fe326bac70efae0af66136cfc3c26ecac71925e Mon Sep 17 00:00:00 2001 From: timothyF95 Date: Fri, 29 May 2026 15:48:22 +0100 Subject: [PATCH 3/3] Feedback --- cmd/secrets/common/handler.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/cmd/secrets/common/handler.go b/cmd/secrets/common/handler.go index c9293be7..a4288b93 100644 --- a/cmd/secrets/common/handler.go +++ b/cmd/secrets/common/handler.go @@ -168,13 +168,14 @@ func (h *Handler) ResolveInputs() (UpsertSecretsInputs, error) { if !ok { return nil, fmt.Errorf("environment variable %q for secret %q not found; please export it", envName, id) } - if !utf8.Valid([]byte(envVal)) { + if !utf8.ValidString(envVal) { return nil, fmt.Errorf("value for secret %q (env %q) contains invalid UTF-8", id, envName) } + value := []byte(envVal) out = append(out, SecretItem{ ID: id, - Value: []byte(envVal), + Value: value, Namespace: "main", }) @@ -318,6 +319,7 @@ func (h *Handler) ResolveVaultIdentifierOwnerForAuth(secretsAuth string) (string // EncryptSecrets encrypts secrets for the given workflow owner address. // TDH2 label is the workflow owner address left-padded to 32 bytes; SecretIdentifier.Owner is the same hex address string. +// Each item's Value slice is zeroed in place as it is encrypted; callers must not reuse rawSecrets afterward. func (h *Handler) EncryptSecrets(rawSecrets UpsertSecretsInputs, owner string) ([]*vault.EncryptedSecret, error) { pubKeyHex, err := h.fetchVaultMasterPublicKeyHex() if err != nil {