Skip to content

Refactor secret handling to use byte arrays for values #444

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anirudhwarrier wants to merge 6 commits into
base: main
Choose a base branch
from
+50 −11
Open

Refactor secret handling to use byte arrays for values #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1e683f6
Refactor secret handling to use byte arrays for values
anirudhwarrier May 19, 2026
a1e658e
Merge branch 'main' into DEVSVCS-4954/zero-secret
anirudhwarrier May 22, 2026
13e0be4
Merge branch 'main' into DEVSVCS-4954/zero-secret
anirudhwarrier May 22, 2026
828da79
Review feedback
timothyF95 May 29, 2026
4fe326b
Feedback
timothyF95 May 29, 2026
9d6189c
Merge branch 'main' into DEVSVCS-4954/zero-secret
timothyF95 May 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 19 additions & 6 deletions cmd/secrets/common/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,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"`
}
Expand Down Expand Up @@ -168,9 +175,10 @@ func (h *Handler) ResolveInputs() (UpsertSecretsInputs, error) {
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: envVal,
Value: value,
Namespace: "main",
})

Expand Down Expand Up @@ -314,15 +322,18 @@ 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 {
return nil, err
}

encryptedSecrets := make([]*vault.EncryptedSecret, 0, len(rawSecrets))
for _, item := range rawSecrets {
for i := range rawSecrets {
item := &rawSecrets[i]
cipherHex, err := EncryptSecret(item.Value, pubKeyHex, owner)
clear(item.Value)
if err != nil {
return nil, fmt.Errorf("failed to encrypt secret (key=%s ns=%s): %w", item.ID, item.Namespace, err)
}
Expand All @@ -340,7 +351,7 @@ func (h *Handler) EncryptSecrets(rawSecrets UpsertSecretsInputs, owner string) (
}

// 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 {
Expand All @@ -350,7 +361,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)
}
Expand All @@ -362,7 +373,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
Expand Down Expand Up @@ -413,6 +424,8 @@ func (h *Handler) Execute(
duration time.Duration,
secretsAuth string,
) error {
defer ZeroUpsertSecretValues(inputs)

h.execCtx = ctx
if IsBrowserFlow(secretsAuth) {
return h.executeBrowserUpsert(ctx, inputs, method)
Expand Down
34 changes: 29 additions & 5 deletions cmd/secrets/common/handler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,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"
Expand All @@ -71,8 +91,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, "0xabc")
Expand All @@ -95,6 +115,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) {
Expand All @@ -104,7 +128,7 @@ func TestEncryptSecrets(t *testing.T) {
},
}

enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: "v", Namespace: "n"}}, "0xabc")
enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: []byte("v"), Namespace: "n"}}, "0xabc")
require.Error(t, err)
require.Nil(t, enc)
require.Contains(t, err.Error(), "gateway POST failed")
Expand All @@ -130,7 +154,7 @@ func TestEncryptSecrets(t *testing.T) {
},
}

enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: "v", Namespace: "n"}}, "0xabc")
enc, err := h.EncryptSecrets(UpsertSecretsInputs{{ID: "s", Value: []byte("v"), Namespace: "n"}}, "0xabc")
require.Error(t, err)
require.Nil(t, enc)
require.Contains(t, err.Error(), "vault public key fetch error")
Expand Down Expand Up @@ -229,7 +253,7 @@ func TestEncryptSecrets_UsesWorkflowOwnerAddress(t *testing.T) {
h.OwnerAddress = "0xabc"

enc, err := h.EncryptSecrets(UpsertSecretsInputs{
{ID: "secret-1", Value: "val1", Namespace: "main"},
{ID: "secret-1", Value: []byte("val1"), Namespace: "main"},
}, "0xabc")
require.NoError(t, err)
require.Len(t, enc, 1)
Expand Down
1 change: 1 addition & 0 deletions cmd/secrets/create/create.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions cmd/secrets/update/update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading