fix(wfctl): bootstrapSecrets returns rotation results for force-rotated keys (v0.27.3)

cmd/wfctl/infra_bootstrap.go

@@ -509,6 +509,26 @@ var bootstrapSecrets = func(ctx context.Context, provider secrets.Provider, cfg
 	if len(revoker) > 0 {
 		credRevoker = revoker[0]
 	}
+	// Strict-contract guard: every forceRotate entry MUST match exactly one
+	// secrets.generate[].Key. A mismatch (e.g. caller passed a cloud-side
+	// resource Name without translating to gen.Key) means the force-rotate
+	// code path will be silently bypassed for that entry — the loop below
+	// keys forceRotate[gen.Key], so the rotation never runs and the rotations
+	// slice stays empty. This produced the staging-dispatch false-negative
+	// "expected 1 rotation result, got 0" surfaced 2026-05-09 (run 25616807427).
+	// Caller MUST translate name→key (see buildForceRotateSet in this file
+	// and buildRotateAndPruneForceRotateSet in infra_rotate_and_prune.go).
+	if len(forceRotate) > 0 && cfg != nil {
+		known := make(map[string]bool, len(cfg.Generate))
+		for _, gen := range cfg.Generate {
+			known[gen.Key] = true
+		}
+		for k := range forceRotate {
+			if !known[k] {
+				return nil, nil, fmt.Errorf("forceRotate entry %q does not match any secrets.generate[].key — caller must translate cloud-side names to canonical keys before calling bootstrapSecrets", k)
+			}
+		}
+	}
 	generated := map[string]string{}
 	var rotations []RotationResult
 

cmd/wfctl/infra_bootstrap_rotation_count_test.go

@@ -0,0 +1,195 @@
+package main
+
+import (
+	"bytes"
+	"context"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/GoCodeAlone/workflow/interfaces"
+)
+
+// TestRotateAndPrune_KeyNameMismatch_ReturnsRotationResult
+// pins the contract that surfaced as a real-world bug on 2026-05-09 in
+// core-dump rotate-spaces-key staging dispatch run 25616807427:
+//
+//	Step 1: rotating credential "coredump-deploy-key" (type infra.spaces_key)...
+//	  secret "SPACES_access_key": created
+//	WFCTL_NEW_KEY_CREATED_AT=2026-05-10T01:41:58Z
+//	[external-plugins] shutting down plugin "digitalocean"
+//	rotate-and-prune: expected 1 rotation result, got 0
+//	error: rotate-and-prune exited with code 1
+//
+// Root cause: `wfctl infra rotate-and-prune --name <RESOURCE_NAME>` builds
+// `forceRotate := map[string]bool{name: true}` keyed by the cloud-side
+// resource name (per ADR 0023 + docs/runbooks/spaces-key-prune.md), but
+// `bootstrapSecrets` checks `forceRotate[gen.Key]` keyed by the canonical
+// generator key. Mismatch → force-rotate code path silently bypassed →
+// rotations slice never appended to → "expected 1 rotation result, got 0"
+// AFTER the cloud and GH Secrets side effects already committed.
+//
+// This test exercises the full runInfraRotateAndPrune path with a config
+// where `key != name` — the exact shape of core-dump's infra.yaml on the
+// failing run. Without the fix, runInfraRotateAndPrune returns code=1 with
+// "expected 1 rotation result, got 0" even though bootstrapSecrets minted
+// the new credential. With the fix, the CLI translates --name→gen.Key
+// at the buildRotateAndPruneForceRotateSet boundary; bootstrapSecrets
+// strictly requires forceRotate keyed by gen.Key and rejects unknown
+// entries (no name-fallback on the bootstrap side).
+func TestRotateAndPrune_KeyNameMismatch_ReturnsRotationResult(t *testing.T) {
+	t.Setenv("WFCTL_CONFIRM_PRUNE", "1")
+	tmpDir := t.TempDir()
+	t.Setenv("WFCTL_STATE_DIR", tmpDir)
+
+	// Write a config matching core-dump's shape: secrets.generate[].key=SPACES,
+	// secrets.generate[].name=coredump-deploy-key. The CLI takes
+	// --name coredump-deploy-key (cloud-side resource name).
+	cfgPath := filepath.Join(tmpDir, "infra.yaml")
+	body := `secrets:
+  provider: env
+  config:
+    prefix: WFCTL_TEST_
+  generate:
+    - key: SPACES
+      type: provider_credential
+      source: digitalocean.spaces
+      name: coredump-deploy-key
+`
+	if err := os.WriteFile(cfgPath, []byte(body), 0600); err != nil {
+		t.Fatalf("write fixture infra.yaml: %v", err)
+	}
+
+	// Stub the underlying generator so we don't reach DO. Returns a JSON
+	// blob shaped like generateDOSpacesKey's real output. The stub also
+	// proves the rotation HAPPENED (it was called) so any "got 0" failure
+	// is purely the keying-mismatch bug, not a missing call.
+	withStubGenerator(t, func(_ context.Context, _ string, _ map[string]any) (string, error) {
+		return `{"access_key":"AK_NEW_FROM_STUB","secret_key":"SK_NEW_FROM_STUB","created_at":"2026-05-10T01:41:58Z"}`, nil
+	})
+
+	// Pre-populate the env-provider's backing values so the force-rotate
+	// path (which reads OLD access_key for revocation) finds a value to
+	// "revoke" without being a hard requirement. Env provider reads from
+	// process env via prefix WFCTL_TEST_<KEY>.
+	t.Setenv("WFCTL_TEST_SPACES_access_key", "AK_OLD")
+	t.Setenv("WFCTL_TEST_SPACES_secret_key", "SK_OLD")
+
+	// fakeProviderEnumerableDriver returns the OLD canonical key as an
+	// enumerable resource so the prune step has something to look at and
+	// runs without the EnumerateAll-empty escape.
+	fakeProv := &fakeProviderEnumerableDriver{
+		outputs: []*interfaces.ResourceOutput{
+			{
+				Name:       "coredump-deploy-key",
+				Type:       "infra.spaces_key",
+				ProviderID: "AK_OLD",
+				Outputs: map[string]any{
+					"access_key": "AK_OLD",
+					"created_at": "2026-05-01T00:00:00Z",
+					"name":       "coredump-deploy-key",
+				},
+			},
+			{
+				Name:       "coredump-deploy-key",
+				Type:       "infra.spaces_key",
+				ProviderID: "AK_NEW_FROM_STUB",
+				Outputs: map[string]any{
+					"access_key": "AK_NEW_FROM_STUB",
+					"created_at": "2026-05-10T01:41:58Z",
+					"name":       "coredump-deploy-key",
+				},
+			},
+		},
+	}
+
+	var out bytes.Buffer
+	code := runInfraRotateAndPrune([]string{
+		"--type", "infra.spaces_key",
+		"--name", "coredump-deploy-key",
+		"--config", cfgPath,
+		"--confirm", "--non-interactive",
+		// --prune-first=false to keep the test focused on the rotate step's
+		// rotation-result contract; pre-prune is exercised by other tests.
+		"--prune-first=false",
+	}, fakeProv, &out)
+
+	if code != 0 {
+		t.Fatalf("rotate-and-prune failed: code=%d, out=%q\n\nThis is the regression: the rotation succeeded (stub generator was called and minted AK_NEW_FROM_STUB) but bootstrapSecrets returned an empty rotations slice because forceRotate was keyed by --name (\"coredump-deploy-key\") while bootstrapSecrets checks forceRotate[gen.Key] (\"SPACES\"). The CLI must translate --name→gen.Key.", code, out.String())
+	}
+
+	// Sanity: output should reflect rotation success (not skipped/created path).
+	got := out.String()
+	if !strings.Contains(got, "AK_NEW_FROM_STUB") {
+		t.Errorf("output should report new access_key from rotation; got: %s", got)
+	}
+}
+
+// TestBootstrapSecrets_ForceRotate_NameOnlyMatch_StillRotatesAndAppends
+// is the unit-level lock for the keying contract: when force-rotate
+// is keyed by secrets.generate[].Key for a successful provider_credential
+// force-rotate that hits an empty store, rotation must run and a
+// RotationResult must be appended. Without this, the rotate-and-prune
+// integration is fragile to internal CLI changes.
+//
+// Strict contract enforced by this PR:
+//   - forceRotate MUST be keyed by secrets.generate[].Key (canonical key).
+//   - bootstrapSecrets ignores entries that don't match a known generator
+//     Key (it does NOT accept generator .Name as a fallback key).
+//   - Name→Key translation is the CLI layer's responsibility — see
+//     buildRotateAndPruneForceRotateSet in infra_rotate_and_prune.go,
+//     which also enforces single-match + provider_credential constraints
+//     so this contract is unviolable from the operator side.
+//
+// Invariant pinned by this test: a successful force-rotate where Set
+// committed MUST produce a RotationResult (not the false-negative the
+// staging run hit on 2026-05-09).
+func TestBootstrapSecrets_ForceRotate_AppendsRotationResultEvenWhenStoreEmpty(t *testing.T) {
+	withStubGenerator(t, func(_ context.Context, _ string, _ map[string]any) (string, error) {
+		return `{"access_key":"AK_NEW","secret_key":"SK_NEW","created_at":"2026-05-10T01:41:58Z"}`, nil
+	})
+
+	// Empty store — same shape as the staging-dispatch case where the OLD
+	// canonical sub-keys had been wiped before the dispatch (or never
+	// existed under the canonical-key shape because the prior config used
+	// the misconfigured suffixed-key shape). The bug-side surface is:
+	// even with an empty store, forceRotate[gen.Key]=true MUST drive the
+	// rotation appendpath, NOT the "doesn't exist → mint fresh" path that
+	// bypasses the rotations slice append.
+	p := newTrackingProvider(map[string]string{})
+	cfg := &SecretsConfig{
+		Generate: []SecretGen{
+			{Key: "SPACES", Type: "provider_credential", Source: "digitalocean.spaces", Name: "coredump-deploy-key"},
+		},
+	}
+	forceRotate := map[string]bool{"SPACES": true}
+
+	_, rotations, err := bootstrapSecrets(context.Background(), p, cfg, forceRotate)
+	if err != nil {
+		t.Fatalf("bootstrapSecrets: %v", err)
+	}
+
+	// The contract: force-rotate that successfully Set sub-keys MUST
+	// surface a RotationResult. Side effects committed without a
+	// RotationResult is the false-negative the staging run hit.
+	if len(rotations) != 1 {
+		t.Fatalf("expected len(rotations)==1 after successful force-rotate; got %d. Side effects committed (Set calls=%v) but rotations slice is empty — same false-negative as staging run 25616807427.", len(rotations), p.setCalls)
+	}
+	if rotations[0].Key != "SPACES" {
+		t.Errorf("rotations[0].Key = %q, want \"SPACES\"", rotations[0].Key)
+	}
+	if rotations[0].AccessKey != "AK_NEW" {
+		t.Errorf("rotations[0].AccessKey = %q, want \"AK_NEW\"", rotations[0].AccessKey)
+	}
+	if rotations[0].CreatedAt != "2026-05-10T01:41:58Z" {
+		t.Errorf("rotations[0].CreatedAt = %q, want \"2026-05-10T01:41:58Z\"", rotations[0].CreatedAt)
+	}
+	// And the side effects must have happened (proves the rotation ran).
+	if !containsSlice(p.setCalls, "SPACES_access_key") {
+		t.Errorf("Set(SPACES_access_key) must be called; setCalls=%v", p.setCalls)
+	}
+	if !containsSlice(p.setCalls, "SPACES_secret_key") {
+		t.Errorf("Set(SPACES_secret_key) must be called; setCalls=%v", p.setCalls)
+	}
+}

cmd/wfctl/infra_rotate_and_prune.go

@@ -156,6 +156,98 @@ func runInfraRotateAndPruneCmd(args []string) error {
 	return fmt.Errorf("rotate-and-prune: no loaded provider implements EnumeratorAll")
 }
 
+// buildRotateAndPruneForceRotateSet translates the cloud-side resource name
+// passed via `wfctl infra rotate-and-prune --name <NAME>` into the canonical
+// secrets.generate[].Key values that bootstrapSecrets keys its forceRotate
+// map by.
+//
+// Per ADR 0023 + docs/runbooks/spaces-key-prune.md, `--name` is the
+// cloud-side resource Name (e.g. "coredump-deploy-key"), matching the
+// `secrets.generate[].name` field. bootstrapSecrets keys forceRotate by
+// `gen.Key` (e.g. "SPACES"), so the CLI must translate or the force-rotate
+// path is silently skipped — the false-negative that surfaced as staging
+// run 25616807427 ("rotate-and-prune: expected 1 rotation result, got 0"
+// AFTER side effects committed).
+//
+// Match precedence:
+//  1. secrets.generate[].name == name → take that gen's Key
+//  2. secrets.generate[].key == name → fallback for configs that omit Name
+//
+// Returns an error when no entry matches (typo guard, mirrors the
+// buildForceRotateSet validation in infra_bootstrap.go) so the operator
+// gets a fast-fail before bootstrap touches the store.
+//
+// Strict-contract enforcement (per Copilot review on PR 594):
+//   - Exactly ONE matching gen entry. Zero or two-plus matches reject.
+//   - Match MUST be type=provider_credential. Other rotatable types
+//     (random_hex, random_base64, random_alphanumeric) are intentionally
+//     rejected here because rotate-and-prune's downstream invariants
+//     (len(rotations)==1, prune step expects an old credential to revoke)
+//     are coherent only for provider_credential. Operators rotating
+//     non-provider_credential generators must use `wfctl infra bootstrap
+//     --force-rotate` directly.
+//
+// Without these guards a multi-Name config or a non-provider_credential
+// --name target would either rotate multiple keys then fail count check
+// (side effects committed but errored) OR rotate without appending a
+// RotationResult (the false-negative class this PR was opened to fix).
+func buildRotateAndPruneForceRotateSet(name string, cfg *SecretsConfig) (map[string]bool, error) {
+	if name == "" {
+		return nil, fmt.Errorf("--name is required")
+	}
+	if cfg == nil || len(cfg.Generate) == 0 {
+		return nil, fmt.Errorf("config has no secrets.generate entries; nothing to rotate for --name %q", name)
+	}
+	// Match precedence: secrets.generate[].name first (canonical per ADR 0023),
+	// then .key fallback for older configs / unit-test fixtures.
+	var matched []SecretGen
+	for _, gen := range cfg.Generate {
+		if gen.Name == name {
+			matched = append(matched, gen)
+		}
+	}
+	if len(matched) == 0 {
+		for _, gen := range cfg.Generate {
+			if gen.Key == name {
+				matched = append(matched, gen)
+			}
+		}
+	}
+	if len(matched) == 0 {
+		return nil, fmt.Errorf("no secrets.generate entry matches --name %q (matched neither .name nor .key)", name)
+	}
+	if len(matched) > 1 {
+		keys := make([]string, len(matched))
+		for i, g := range matched {
+			keys[i] = g.Key
+		}
+		return nil, fmt.Errorf("--name %q matches multiple secrets.generate entries (keys: %v); rotate-and-prune supports exactly one provider_credential per dispatch — use `wfctl infra bootstrap --force-rotate` for multi-entry rotation", name, keys)
+	}
+	if matched[0].Type != "provider_credential" {
+		return nil, fmt.Errorf("--name %q matches secrets.generate entry of type %q; rotate-and-prune only operates on provider_credential entries (use `wfctl infra bootstrap --force-rotate` for other rotatable types)", name, matched[0].Type)
+	}
+	// Defense-in-depth: ensure the matched Key is unique across all
+	// secrets.generate[] entries. Without this, a config with two gens
+	// sharing the same Key (different Names) would set forceRotate[key]=true
+	// → bootstrapSecrets rotates BOTH → fails len(rotations)==1 after side
+	// effects committed (the same false-negative class this PR was opened
+	// to fix, just via Key collision instead of Name collision).
+	keyCount := 0
+	var keyCollisionNames []string
+	for _, gen := range cfg.Generate {
+		if gen.Key == matched[0].Key {
+			keyCount++
+			if gen.Name != "" {
+				keyCollisionNames = append(keyCollisionNames, gen.Name)
+			}
+		}
+	}
+	if keyCount > 1 {
+		return nil, fmt.Errorf("secrets.generate has %d entries with .key=%q (names: %v); rotate-and-prune requires Key uniqueness so the rotation count invariant holds", keyCount, matched[0].Key, keyCollisionNames)
+	}
+	return map[string]bool{matched[0].Key: true}, nil
+}
+
 // recoveryRecord is persisted to ${WFCTL_STATE_DIR:-$HOME/.wfctl}/last-rotation.json
 // after a successful rotate step, BEFORE the prune step. Used by the
 // `wfctl infra prune --recovery-from-last-rotation` flow (Task 19) to recover
@@ -316,7 +408,18 @@ func runInfraRotateAndPrune(args []string, provider pruneProvider, w io.Writer)
 		fmt.Fprintf(w, "rotate-and-prune: resolve secrets provider: %v\n", err)
 		return 1
 	}
-	forceRotate := map[string]bool{name: true}
+	// Translate the cloud-side resource name (--name; per ADR 0023 +
+	// docs/runbooks/spaces-key-prune.md) into the canonical secrets.generate[].Key
+	// values that bootstrapSecrets keys forceRotate by. Without this translation
+	// forceRotate[gen.Key] is false for every generator and bootstrapSecrets
+	// silently bypasses the force-rotate code path (rotations slice stays empty
+	// even when the underlying generator + Set side effects committed) — the
+	// staging-dispatch false-negative surfaced 2026-05-09 (run 25616807427).
+	forceRotate, err := buildRotateAndPruneForceRotateSet(name, cfg)
+	if err != nil {
+		fmt.Fprintf(w, "rotate-and-prune: %v\n", err)
+		return 1
+	}
 	revoker, closer := resolveCredentialRevoker(ctx, configFile, cfg, forceRotate)
 	if closer != nil {
 		defer closer.Close()