diff --git a/internal/metastructure/resolver/resolver.go b/internal/metastructure/resolver/resolver.go index 9091d2b45..131e6c996 100644 --- a/internal/metastructure/resolver/resolver.go +++ b/internal/metastructure/resolver/resolver.go @@ -695,6 +695,13 @@ func (pr *propertyResolver) resolveEmbedRef(properties json.RawMessage, ref pkgm return true }) envMap["$value"] = valueStr + // Same reasoning as resolveReference: restate the marker from the + // resolution rather than inheriting whatever the copy carried. + if ref.ResolvedValue.Hashed { + envMap["$hashed"] = true + } else { + delete(envMap, "$hashed") + } envJSON, err := json.Marshal(envMap) if err != nil { return properties, fmt.Errorf("embed: marshal updated envelope: %w", err) @@ -748,6 +755,18 @@ func (pr *propertyResolver) resolveReference(properties json.RawMessage, ref pkg valueToSet := pr.extractResolvedValue(ref) if valueToSet != nil { refObject["$value"] = valueToSet + // The envelope above was copied wholesale from the target, so any + // $hashed marker on it describes the value it used to hold, not the + // one just written over it. Restate the marker from the resolution + // instead of inheriting it. Leaving a stale true makes the terminal + // hashing pass skip the envelope and persist plaintext labelled as a + // digest; clearing a true that is still accurate would let a digest + // past the plugin-boundary guard and reach the provider as a secret. + if ref.ResolvedValue.Hashed { + refObject["$hashed"] = true + } else { + delete(refObject, "$hashed") + } } if ref.ResolvedValue.Strategy != "" { refObject["$strategy"] = ref.ResolvedValue.Strategy @@ -779,6 +798,12 @@ func (pr *propertyResolver) resolveReference(properties json.RawMessage, ref pkg func (pr *propertyResolver) setRefValue(uri pkgmodel.FormaeURI, value string) error { var actualValue string var inheritedVisibility, inheritedStrategy string + // Whether the value being resolved FROM is itself a stored digest rather + // than recoverable plaintext. It has to travel with the resolved value: + // the plugin-boundary guard refuses a write on the $hashed marker alone, + // so dropping it here would let a digest reach a provider as though it + // were the secret. + var inheritedHashed bool if parsed := gjson.Parse(value); parsed.IsObject() { if parsed.Get("$value").Exists() { @@ -789,6 +814,7 @@ func (pr *propertyResolver) setRefValue(uri pkgmodel.FormaeURI, value string) er inheritedVisibility = parsed.Get("$visibility").String() inheritedStrategy = parsed.Get("$strategy").String() + inheritedHashed = parsed.Get("$hashed").Bool() } else { actualValue = value } @@ -836,6 +862,7 @@ func (pr *propertyResolver) setRefValue(uri pkgmodel.FormaeURI, value string) er // erase the record the next plan compares against. newValue.JSONPath = ref.ResolvedValue.JSONPath newValue.ResolvedFrom = ref.ResolvedValue.ResolvedFrom + newValue.Hashed = inheritedHashed ref.ResolvedValue = newValue } diff --git a/internal/metastructure/resolver/resolver_test.go b/internal/metastructure/resolver/resolver_test.go index fe51dca58..21fde0c54 100644 --- a/internal/metastructure/resolver/resolver_test.go +++ b/internal/metastructure/resolver/resolver_test.go @@ -1697,3 +1697,43 @@ func TestResolvePropertyReferences_PreservesResolvedFrom(t *testing.T) { assert.Equal(t, digest, out.Get("$resolvedFrom").String(), "resolving a reference must not drop its provenance") } + +// Resolution restates the hashed marker from the value it resolved, rather than +// inheriting whatever the target envelope happened to carry. Both directions +// matter and they pull opposite ways: a stale true makes the terminal hashing +// pass skip an envelope and persist plaintext labelled as a digest, while +// clearing a true that is still accurate lets a digest past the plugin-boundary +// guard and reach a provider as though it were the secret. +func TestResolvePropertyReferences_HashedMarkerFollowsTheResolvedValue(t *testing.T) { + const uri = pkgmodel.FormaeURI("formae://2abcDEFghiJKLmnoPQRstuVWxyz#/SecretString") + + // An envelope that was hashed at rest, now being re-resolved. + target := json.RawMessage(`{ + "DbPassword": { + "$ref": "` + string(uri) + `", + "$value": "0000000000000000000000000000000000000000000000000000000000000000", + "$hashed": true, + "$visibility": "Opaque" + } + }`) + + t.Run("live plaintext clears a stale marker", func(t *testing.T) { + out, err := ResolvePropertyReferences(uri, target, "live-plaintext") + require.NoError(t, err) + assert.Equal(t, "live-plaintext", gjson.GetBytes(out, "DbPassword.$value").String()) + assert.False(t, gjson.GetBytes(out, "DbPassword.$hashed").Bool(), + "a live plaintext resolution must not stay marked hashed, or terminal hashing skips it") + assert.NoError(t, guardNoHashedValues(out), + "plaintext must be writable to a provider") + }) + + t.Run("a resolved digest keeps its marker so the write guard still fires", func(t *testing.T) { + digest := `{"$value":"1111111111111111111111111111111111111111111111111111111111111111","$hashed":true,"$visibility":"Opaque"}` + out, err := ResolvePropertyReferences(uri, target, digest) + require.NoError(t, err) + assert.True(t, gjson.GetBytes(out, "DbPassword.$hashed").Bool(), + "resolving from a stored digest must stay marked hashed") + assert.ErrorIs(t, guardNoHashedValues(out), ErrHashedValueNotWritable, + "a digest must never reach a provider as if it were the secret") + }) +} diff --git a/internal/workflow_tests/local/apply_forma/failed_command_secret_test.go b/internal/workflow_tests/local/apply_forma/failed_command_secret_test.go new file mode 100644 index 000000000..b939cb00c --- /dev/null +++ b/internal/workflow_tests/local/apply_forma/failed_command_secret_test.go @@ -0,0 +1,224 @@ +// © 2026 Platform Engineering Labs Inc. +// +// SPDX-License-Identifier: FSL-1.1-ALv2 + +//go:build unit + +package workflow_tests_local + +import ( + "encoding/json" + "fmt" + "sync/atomic" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/platform-engineering-labs/formae/internal/metastructure/config" + "github.com/platform-engineering-labs/formae/internal/metastructure/forma_command" + "github.com/platform-engineering-labs/formae/internal/metastructure/testutil" + "github.com/platform-engineering-labs/formae/internal/metastructure/util" + "github.com/platform-engineering-labs/formae/internal/workflow_tests/test_helpers" + pkgmodel "github.com/platform-engineering-labs/formae/pkg/model" + "github.com/platform-engineering-labs/formae/pkg/plugin" + "github.com/platform-engineering-labs/formae/pkg/plugin/resource" +) + +// failingConsumerOverrides mocks a consumer whose Create succeeds and whose +// Update always fails, so a command that plans a consumer update reaches a +// Failed terminal state after resolution has already substituted the live +// secret value into the consumer's desired state. +func failingConsumerOverrides(updateAttempts *atomic.Int32) *plugin.ResourcePluginOverrides { + return &plugin.ResourcePluginOverrides{ + Create: func(req *resource.CreateRequest) (*resource.CreateResult, error) { + if req.ResourceType != "FakeAWS::S3::Bucket" { + return nil, nil + } + return &resource.CreateResult{ + ProgressResult: &resource.ProgressResult{ + Operation: resource.OperationCreate, + OperationStatus: resource.OperationStatusSuccess, + RequestID: "consumer-create-1", + NativeID: "bucket-native-1", + }, + }, nil + }, + Update: func(req *resource.UpdateRequest) (*resource.UpdateResult, error) { + if req.ResourceType != "FakeAWS::S3::Bucket" { + return nil, nil + } + updateAttempts.Add(1) + return nil, fmt.Errorf("consumer update rejected by the provider") + }, + } +} + +// A command that fails after resolution substituted a secret's live value into +// a consumer's desired state must leave no plaintext at rest. Terminal-state +// hashing runs for every final state, Failed included, so the failed command's +// stored rows must hold digests rather than the value the consumer was about +// to be given. +func TestApplyForma_FailedCommand_LeavesNoConsumerPlaintextAtRest(t *testing.T) { + testutil.RunTestFromProjectRoot(t, func(t *testing.T) { + const secretV1 = "failed-command-secret-v1" + const secretV2 = "failed-command-secret-v2" + + logCapture := test_helpers.SetupTestLogger() + + var updateAttempts atomic.Int32 + overrides := failingConsumerOverrides(&updateAttempts) + + cfg := test_helpers.NewTestMetastructureConfig() + cfg.Agent.Synchronization.Enabled = false + m, def, err := test_helpers.NewTestMetastructureWithConfig(t, overrides, cfg) + defer def() + require.NoError(t, err) + + stack := "test-stack-" + util.NewID() + targets := []pkgmodel.Target{{Label: "test-target", Namespace: "test-namespace"}} + + createForma := &pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{secretResource(stack, "my-secret", secretV1), secretConsumer(stack, "my-secret")}, + Targets: targets, + } + _, err = m.ApplyForma(createForma, &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + waitForApplyComplete(t, m) + + cmds, err := m.Datastore.LoadFormaCommands() + require.NoError(t, err) + createCmd := findCommandByType(cmds, pkgmodel.CommandApply) + require.NotNil(t, createCmd) + require.Equal(t, forma_command.CommandStateSuccess, createCmd.State, + "precondition: the create apply must succeed") + + // Change only the secret's value. The consumer is planned because its + // reference resolves from a moved source, its plugin is handed the new + // value, and the plugin refuses, failing the command. + rotateForma := &pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{secretResource(stack, "my-secret", secretV2), secretConsumer(stack, "my-secret")}, + Targets: targets, + } + _, err = m.ApplyForma(rotateForma, &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + waitForApplyComplete(t, m) + + cmds, err = m.Datastore.LoadFormaCommands() + require.NoError(t, err) + var failedCmd *forma_command.FormaCommand + for _, c := range cmds { + if c.Command == pkgmodel.CommandApply && c.ID != createCmd.ID { + failedCmd = c + } + } + require.NotNil(t, failedCmd, "the second apply command must exist") + require.Greater(t, updateAttempts.Load(), int32(0), + "precondition: the consumer's plugin must have been called with the resolved value") + require.Equal(t, forma_command.CommandStateFailed, failedCmd.State, + "precondition: the consumer's refusal must fail the command") + + // The failed command's stored rows carry no plaintext, from either + // generation of the secret's value. + assertNoPlaintextInResourceUpdates(t, m, failedCmd.ID, secretV2) + assertNoPlaintextInResourceUpdates(t, m, failedCmd.ID, secretV1) + + // Nor does the command blob itself. + blob, err := json.Marshal(failedCmd) + require.NoError(t, err) + assert.NotContains(t, string(blob), secretV2, "the failed command blob leaked the resolved secret") + assert.NotContains(t, string(blob), secretV1, "the failed command blob leaked the prior secret") + + // Nor the resource rows. + resources, err := m.Datastore.LoadResourcesByStack(stack) + require.NoError(t, err) + for i := range resources { + assert.NotContains(t, string(resources[i].Properties), secretV2, + "resources.properties leaked plaintext for %s", resources[i].Label) + assert.NotContains(t, string(resources[i].Properties), secretV1, + "resources.properties leaked prior plaintext for %s", resources[i].Label) + } + + // Nor the logs. + for _, entry := range logCapture.GetEntries() { + assert.NotContains(t, entry, secretV1, "log entry leaked the prior secret") + assert.NotContains(t, entry, secretV2, "log entry leaked the resolved secret") + } + }) +} + +// Destroying a secret that a consumer still references fails the consumer, and +// the failed command must still leave no plaintext at rest: the consumer's +// stored desired state holds a digest, never the value it last resolved. +func TestApplyForma_DestroyedReferencedSecret_LeavesNoConsumerPlaintextAtRest(t *testing.T) { + testutil.RunTestFromProjectRoot(t, func(t *testing.T) { + const secretV1 = "destroyed-secret-v1" + + logCapture := test_helpers.SetupTestLogger() + + var calls atomic.Int32 + var props atomic.Value + overrides := secretConsumerOverrides(&calls, &props) + + cfg := test_helpers.NewTestMetastructureConfig() + cfg.Agent.Synchronization.Enabled = false + m, def, err := test_helpers.NewTestMetastructureWithConfig(t, overrides, cfg) + defer def() + require.NoError(t, err) + + stack := "test-stack-" + util.NewID() + targets := []pkgmodel.Target{{Label: "test-target", Namespace: "test-namespace"}} + + createForma := &pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{secretResource(stack, "my-secret", secretV1), secretConsumer(stack, "my-secret")}, + Targets: targets, + } + _, err = m.ApplyForma(createForma, &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + waitForApplyComplete(t, m) + + cmds, err := m.Datastore.LoadFormaCommands() + require.NoError(t, err) + createCmd := findCommandByType(cmds, pkgmodel.CommandApply) + require.NotNil(t, createCmd) + require.Equal(t, forma_command.CommandStateSuccess, createCmd.State, + "precondition: the create apply must succeed") + + // Reconcile with the secret dropped from the forma. It is destroyed + // while the consumer still references it. + dropForma := &pkgmodel.Forma{ + Stacks: []pkgmodel.Stack{{Label: stack}}, + Resources: []pkgmodel.Resource{secretConsumer(stack, "my-secret")}, + Targets: targets, + } + _, err = m.ApplyForma(dropForma, &config.FormaCommandConfig{Mode: pkgmodel.FormaApplyModeReconcile}, "test-client-id", "", "") + require.NoError(t, err) + waitForApplyComplete(t, m) + + cmds, err = m.Datastore.LoadFormaCommands() + require.NoError(t, err) + var dropCmd *forma_command.FormaCommand + for _, c := range cmds { + if c.Command == pkgmodel.CommandApply && c.ID != createCmd.ID { + dropCmd = c + } + } + require.NotNil(t, dropCmd, "the second apply command must exist") + + assertNoPlaintextInResourceUpdates(t, m, dropCmd.ID, secretV1) + + resources, err := m.Datastore.LoadResourcesByStack(stack) + require.NoError(t, err) + for i := range resources { + assert.NotContains(t, string(resources[i].Properties), secretV1, + "resources.properties leaked plaintext for %s", resources[i].Label) + } + + for _, entry := range logCapture.GetEntries() { + assert.NotContains(t, entry, secretV1, "log entry leaked the secret") + } + }) +}