Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 14 additions & 8 deletions cmd/wfctl/infra_apply_sensitive_routing_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"testing"
"time"

"github.com/GoCodeAlone/workflow/iac/sensitive"
"github.com/GoCodeAlone/workflow/interfaces"
"github.com/GoCodeAlone/workflow/secrets"
)
Expand Down Expand Up @@ -121,21 +122,26 @@ func TestPersistResourceWithSecretRouting_RoutesSensitiveAndSanitizesState(t *te
t.Fatalf("expected 1 saved, got %d", len(store.saved))
}
state := store.saved[0]
if state.Outputs["secret_key"] != "secret_ref://myres_secret_key" {
secretKey := sensitive.SecretKey("myres", "secret_key")
accessKey := sensitive.SecretKey("myres", "access_key")
if state.Outputs["secret_key"] != sensitive.Placeholder("myres", "secret_key") {
t.Errorf("state secret_key not sanitized: %v", state.Outputs["secret_key"])
}
if state.Outputs["access_key"] != "secret_ref://myres_access_key" {
if state.Outputs["access_key"] != sensitive.Placeholder("myres", "access_key") {
t.Errorf("state access_key not sanitized: %v", state.Outputs["access_key"])
}
if state.Outputs["bucket"] != "b" {
t.Errorf("state bucket lost: %v", state.Outputs["bucket"])
}
if prov.values["myres_secret_key"] != "SK" {
if prov.values[secretKey] != "SK" {
t.Errorf("provider missing secret_key value")
}
if hydrated["myres_secret_key"] != "SK" {
if hydrated[secretKey] != "SK" {
t.Errorf("hydrated missing secret_key: %v", hydrated)
}
if prov.values[accessKey] != "AK" {
t.Errorf("provider missing access_key value")
}
}

func TestPersistResourceWithSecretRouting_NoProviderHardFails(t *testing.T) {
Expand Down Expand Up @@ -181,7 +187,7 @@ func TestPersistResourceWithSecretRouting_SaveFailureCompensatesWithDelete(t *te
if drv.deleteCalls[0].ProviderID != "AKIA" {
t.Errorf("compensating Delete used wrong ProviderID: %v", drv.deleteCalls[0])
}
if _, ok := prov.values["myres_secret_key"]; ok {
if _, ok := prov.values[sensitive.SecretKey("myres", "secret_key")]; ok {
t.Errorf("compensating Delete should have removed routed secret; got %v", prov.values)
}
}
Expand Down Expand Up @@ -223,7 +229,7 @@ func TestPersistResourceWithSecretRouting_Idempotent(t *testing.T) {
t.Fatalf("persist iter %d: %v", i, err)
}
}
if prov.values["myres_secret_key"] != "SK" {
if prov.values[sensitive.SecretKey("myres", "secret_key")] != "SK" {
t.Errorf("provider value lost on re-Apply: %v", prov.values)
}
if len(store.saved) != 2 {
Expand Down Expand Up @@ -264,7 +270,7 @@ func TestPersistResourceWithSecretRouting_ReadModeSanitizeOnly_PreservesPriorPla
// Pre-existing state has a placeholder
store := &stubInfraStore{
saved: []interfaces.ResourceState{
{Name: "myres", Outputs: map[string]any{"secret_key": "secret_ref://myres_secret_key", "bucket": "b"}},
{Name: "myres", Outputs: map[string]any{"secret_key": sensitive.Placeholder("myres", "secret_key"), "bucket": "b"}},
},
}
out := interfaces.ResourceOutput{
Expand All @@ -283,7 +289,7 @@ func TestPersistResourceWithSecretRouting_ReadModeSanitizeOnly_PreservesPriorPla
t.Fatalf("expected 2 saves (initial + this), got %d", len(store.saved))
}
latest := store.saved[1]
if latest.Outputs["secret_key"] != "secret_ref://myres_secret_key" {
if latest.Outputs["secret_key"] != sensitive.Placeholder("myres", "secret_key") {
t.Errorf("Read mode lost prior placeholder: %v", latest.Outputs["secret_key"])
}
if latest.Outputs["bucket"] != "b" {
Expand Down
26 changes: 14 additions & 12 deletions cmd/wfctl/infra_audit_state_secrets.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@ import (
// - a plaintext value matching secrets.DefaultSensitiveKeys() → flag legacy.
// - a "secret://<key>" string → flag mistaken config-reference in state.
//
// Then walks secrets.Provider.List() (when supported) for any
// "<resource>_<key>" name whose <resource> is NOT in IaCStateStore →
// orphan, candidate for prune.
// Then walks secrets.Provider.List() (when supported) for any name that is
// neither directly referenced by a state placeholder nor recoverable as a
// legacy "<resource>_<key>" routed secret for a state resource → orphan,
// candidate for prune.
//
// Exit codes:
//
Expand Down Expand Up @@ -103,15 +104,12 @@ func runAuditStateSecretsWithPrune(ctx context.Context, w io.Writer, store infra
defaultSensitive[k] = struct{}{}
}

// Collect the union of sensitive-output key names actually observed in
// state records (any Outputs[k] whose value is a secret_ref://
// placeholder). Drivers may declare sensitive keys that aren't in
// secrets.DefaultSensitiveKeys() — without harvesting these, orphan
// detection's stripKnownSensitiveSuffix would fail to recover the
// resource-name prefix and misflag valid routed secrets as orphans
// (and --prune would delete them). Defaults remain in the suffix set
// as a fallback for first-run scenarios where state hasn't recorded
// any placeholders yet.
// Collect live placeholder keys plus the union of sensitive-output key
// names observed in state records. New routed secrets are authoritative by
// placeholder value because SecretKey(resource,key) may include hashing and
// truncation. The suffix set remains only as a legacy fallback for older
// "<resource>_<key>" routed-secret names.
liveSecretNames := map[string]struct{}{}
observedSensitive := map[string]struct{}{}
for k := range defaultSensitive {
observedSensitive[k] = struct{}{}
Expand All @@ -120,6 +118,7 @@ func runAuditStateSecretsWithPrune(ctx context.Context, w io.Writer, store infra
for k, v := range states[i].Outputs {
if sensitive.IsPlaceholder(v) {
observedSensitive[k] = struct{}{}
liveSecretNames[strings.TrimPrefix(v.(string), sensitive.PlaceholderPrefix)] = struct{}{}
}
}
}
Expand Down Expand Up @@ -181,6 +180,9 @@ func runAuditStateSecretsWithPrune(ctx context.Context, w io.Writer, store infra
case err == nil:
sort.Strings(names)
for _, name := range names {
if _, ok := liveSecretNames[name]; ok {
continue
}
res := stripSensitiveSuffix(name, observedSensitive)
if _, ok := stateNames[res]; ok {
continue
Expand Down
27 changes: 27 additions & 0 deletions cmd/wfctl/infra_audit_state_secrets_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"strings"
"testing"

"github.com/GoCodeAlone/workflow/iac/sensitive"
"github.com/GoCodeAlone/workflow/interfaces"
"github.com/GoCodeAlone/workflow/secrets"
)
Expand Down Expand Up @@ -159,3 +160,29 @@ func TestAuditStateSecrets_ValidPlaceholderWithMatchingProvider_NoFinding(t *tes
t.Errorf("rc = %d, want 0 (placeholder + matching provider value); output:\n%s", rc, w.String())
}
}

func TestAuditStateSecrets_NewSecretKeyPlaceholderNotPruned(t *testing.T) {
liveKey := sensitive.SecretKey("live", "secret_key")
store := &stubInfraStore{
saved: []interfaces.ResourceState{
{Name: "live", Outputs: map[string]any{"secret_key": sensitive.Placeholder("live", "secret_key")}},
},
}
prov := newEnvTestProvider()
prov.values[liveKey] = "VALID"
prov.values["ghost_secret_key"] = "ORPHAN"
w := &bytes.Buffer{}
rc := runAuditStateSecretsWithPrune(context.Background(), w, store, prov, true)
if rc != 0 {
t.Errorf("rc = %d, want 0 after pruning only orphan; output:\n%s", rc, w.String())
}
if _, ok := prov.values[liveKey]; !ok {
t.Fatalf("live routed secret %q was pruned; remaining values: %v", liveKey, prov.values)
}
if _, ok := prov.values["ghost_secret_key"]; ok {
t.Fatalf("orphan secret was not pruned; remaining values: %v", prov.values)
}
if strings.Contains(w.String(), liveKey) {
t.Errorf("live routed secret should not be reported as orphan; output:\n%s", w.String())
}
}
103 changes: 90 additions & 13 deletions iac/sensitive/route.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
// - Route is invoked on Create/Update only. Read/Adoption/Refresh paths
// use Sanitize-only logic (not in this package — see
// cmd/wfctl/infra_apply.go) to prevent cache pollution.
// - The placeholder format "secret_ref://<resource>_<key>" is distinct
// from the user-supplied "secret://<key>" config-reference convention.
// - The placeholder format "secret_ref://<SecretKey(resource,key)>" is
// distinct from the user-supplied "secret://<key>" config-reference
// convention.
// - Routing trigger is exclusively out.Sensitive[k]==true (per-call
// dynamic). ResourceDriver.SensitiveKeys() is NOT consulted here;
// it remains a display-masking signal.
Expand All @@ -20,6 +21,8 @@ package sensitive

import (
"context"
"crypto/sha256"
"encoding/hex"
"errors"
"fmt"
"sort"
Expand All @@ -29,21 +32,40 @@ import (
"github.com/GoCodeAlone/workflow/secrets"
)

// PlaceholderPrefix is the URI scheme used in state.Outputs values to
// reference a routed secret stored in the configured secrets.Provider.
// Distinct from secrets.SecretPrefix ("secret://") which is for
// user-supplied config references.
const PlaceholderPrefix = "secret_ref://"
const (
// PlaceholderPrefix is the URI scheme used in state.Outputs values to
// reference a routed secret stored in the configured secrets.Provider.
// Distinct from secrets.SecretPrefix ("secret://") which is for
// user-supplied config references.
PlaceholderPrefix = "secret_ref://"

// secretKeyMaxLength follows the lowest common provider limit currently
// targeted by routed output secrets: GitHub Actions secret names.
secretKeyMaxLength = 100
secretKeyHashBytes = 16
)

// SecretKey returns the canonical secrets.Provider key for a resource's
// output: "<resourceName>_<outputKey>". Exported so audit-state-secrets
// and other consumers can reverse-engineer routed-secret names.
// output. The key is provider-safe and collision-resistant for distinct
// (resourceName, outputKey) pairs. Exported so audit-state-secrets and other
// consumers can recompute routed-secret names from known resource/output pairs.
func SecretKey(resourceName, outputKey string) string {
return resourceName + "_" + outputKey
raw := resourceName + "\x00" + outputKey
resourcePart := sanitizeSecretKeyPart(resourceName)
outputPart := sanitizeSecretKeyPart(outputKey)
hash := shortHash(raw)

prefix := ""
if strings.HasPrefix(strings.ToUpper(resourcePart+"__"+outputPart), "GITHUB_") {
prefix = "WF_"
}
maxPartsLength := secretKeyMaxLength - len(prefix) - len(hash) - len("__") - len("_")
resourcePart, outputPart = truncateSecretKeyParts(resourcePart, outputPart, maxPartsLength)
return prefix + resourcePart + "__" + outputPart + "_" + hash
Comment thread
intel352 marked this conversation as resolved.
}

// Placeholder returns the canonical "secret_ref://<resourceName>_<outputKey>"
// string that replaces a routed value in state.Outputs.
// Placeholder returns PlaceholderPrefix + SecretKey(resourceName, outputKey),
// replacing a routed value in state.Outputs.
func Placeholder(resourceName, outputKey string) string {
return PlaceholderPrefix + SecretKey(resourceName, outputKey)
}
Expand All @@ -59,7 +81,7 @@ func IsPlaceholder(v any) bool {
}

// Route routes sensitive fields from out through provider, keying each
// secret as "<resourceName>_<outputKey>". Returns:
// secret via SecretKey(resourceName, outputKey). Returns:
//
// - sanitized: a copy of out.Outputs with sensitive values replaced by
// PlaceholderPrefix + SecretKey(resourceName, k). Suitable for
Expand Down Expand Up @@ -154,6 +176,61 @@ func stringifyOutput(v any) (string, error) {
}
}

func sanitizeSecretKeyPart(part string) string {
var b strings.Builder
b.Grow(len(part))
for _, r := range part {
switch {
case r >= 'A' && r <= 'Z':
b.WriteRune(r)
case r >= 'a' && r <= 'z':
b.WriteRune(r)
case r >= '0' && r <= '9':
b.WriteRune(r)
case r == '_':
b.WriteRune(r)
default:
b.WriteByte('_')
}
}
out := b.String()
if out == "" {
out = "SECRET"
}
if out[0] >= '0' && out[0] <= '9' {
out = "_" + out
}
return out
}

func shortHash(value string) string {
sum := sha256.Sum256([]byte(value))
return strings.ToUpper(hex.EncodeToString(sum[:secretKeyHashBytes]))
}

func truncateSecretKeyParts(resourcePart, outputPart string, maxLength int) (string, string) {
if len(resourcePart)+len(outputPart) <= maxLength {
return resourcePart, outputPart
}
resourceBudget := maxLength / 2
outputBudget := maxLength - resourceBudget
if len(resourcePart) < resourceBudget {
outputBudget += resourceBudget - len(resourcePart)
resourceBudget = len(resourcePart)
}
if len(outputPart) < outputBudget {
resourceBudget += outputBudget - len(outputPart)
outputBudget = len(outputPart)
}
if len(resourcePart) > resourceBudget {
resourcePart = resourcePart[:resourceBudget]
}
if len(outputPart) > outputBudget {
outputPart = outputPart[:outputBudget]
}
return resourcePart, outputPart
}

// Revoke deletes routed secrets for resourceName. mergedKeys is the union
// of placeholder-derived keys (caller extracts from pre-delete
// state.Outputs) and any legacy heuristic keys. Errors from
Expand Down
Loading
Loading