fix(secretsmanager): prevent binary secret plaintext at rest and strengthen enrichment test

Author: JeroenSoeters

pkg/cfres/secretsmanager/secret.go

@@ -98,9 +98,12 @@ func (s *Secret) readWithClients(ctx context.Context, ccxClient ccxReader, smCli
 	if secret.SecretString != nil {
 		props["SecretString"] = *secret.SecretString
 	}
-	if secret.SecretBinary != nil {
-		props["SecretBinary"] = secret.SecretBinary
-	}
+	// SecretBinary is intentionally not enriched here. The agent's opaque-hashing
+	// table (persist_value_transformer.knownOpaqueFields) only covers SecretString
+	// for this resource type; returning SecretBinary as a plain []byte would store
+	// the raw binary secret as base64 plaintext at rest with no hashing. Binary
+	// secrets remain unresolvable via formae until the agent core and this plugin's
+	// schema are updated in concert to add SecretBinary opaque coverage.
 
 	completeProps, err := json.Marshal(props)
 	if err != nil {

pkg/cfres/secretsmanager/secret_test.go

@@ -50,7 +50,9 @@ func (m *mockSMClient) GetSecretValue(ctx context.Context, input *secretsmanager
 
 func TestSecret_Read_AlwaysEnrichesSecretValue(t *testing.T) {
 	// Read unconditionally enriches: the secret value is always fetched and
-	// merged into the result properties.
+	// merged into the result properties regardless of the RedactSensitive flag.
+	// Setting RedactSensitive=true proves the plugin no longer short-circuits on
+	// that flag (the whole point of this branch's removal).
 	ctx := context.Background()
 	ccxMock := &mockCCXReader{}
 	smMock := &mockSMClient{}
@@ -68,9 +70,12 @@ func TestSecret_Read_AlwaysEnrichesSecretValue(t *testing.T) {
 	}, nil)
 
 	s := &Secret{cfg: &config.Config{}}
+	// RedactSensitive=true is the flag the old code used to skip enrichment.
+	// With the flag removed, enrichment must still fire.
 	result, err := s.readWithClients(ctx, ccxMock, smMock, &resource.ReadRequest{
-		NativeID:     "my-secret-id",
-		ResourceType: "AWS::SecretsManager::Secret",
+		NativeID:        "my-secret-id",
+		ResourceType:    "AWS::SecretsManager::Secret",
+		RedactSensitive: true,
 	})
 
 	require.NoError(t, err)
@@ -84,6 +89,45 @@ func TestSecret_Read_AlwaysEnrichesSecretValue(t *testing.T) {
 	smMock.AssertExpectations(t)
 }
 
+func TestSecret_Read_SecretBinaryNotEnriched(t *testing.T) {
+	// SecretBinary is intentionally excluded from enrichment: the agent's
+	// opaque-hashing table only covers SecretString for this resource type.
+	// Returning raw binary data would persist it as base64 plaintext at rest.
+	// This test asserts the absence of SecretBinary in the result properties
+	// when the AWS API returns a binary secret.
+	ctx := context.Background()
+	ccxMock := &mockCCXReader{}
+	smMock := &mockSMClient{}
+
+	ccxMock.On("ReadResource", ctx, mock.Anything).Return(&resource.ReadResult{
+		ResourceType: "AWS::SecretsManager::Secret",
+		Properties:   `{"Name":"binary-secret"}`,
+	}, nil)
+
+	smMock.On("GetSecretValue", ctx, mock.Anything).Return(&secretsmanager.GetSecretValueOutput{
+		SecretBinary: []byte("binary-payload"),
+	}, nil)
+
+	s := &Secret{cfg: &config.Config{}}
+	result, err := s.readWithClients(ctx, ccxMock, smMock, &resource.ReadRequest{
+		NativeID:     "binary-secret-id",
+		ResourceType: "AWS::SecretsManager::Secret",
+	})
+
+	require.NoError(t, err)
+	require.NotNil(t, result)
+
+	var props map[string]any
+	require.NoError(t, json.Unmarshal([]byte(result.Properties), &props))
+	_, hasSecretBinary := props["SecretBinary"]
+	assert.False(t, hasSecretBinary, "SecretBinary must not be present in result (no opaque coverage in agent)")
+	_, hasSecretString := props["SecretString"]
+	assert.False(t, hasSecretString, "SecretString must not be present when the secret has no string value")
+
+	ccxMock.AssertExpectations(t)
+	smMock.AssertExpectations(t)
+}
+
 func TestSecret_Read_EnrichesEvenWhenCCXReturnsEmptyProperties(t *testing.T) {
 	// Enrichment works even when Cloud Control returns an empty properties blob.
 	ctx := context.Background()