feat(iac): engine-side sensitive-output routing through secrets.Provider (v0.27.0)#588
Merged
Merged
Conversation
…ovider Captures the design for v0.27.0 engine layer that routes ResourceOutput fields flagged Sensitive (or declared via SensitiveKeys()) through the configured secrets.Provider before state persistence — keeping plugins platform-agnostic per ADR 0015 / user mandate. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…indings Addresses adversarial-design-review cycle 1 findings: - Critical: drop non-existent audit-secrets recovery; add audit-state-secrets - Critical: drop Restore API (incompatible with write-only GitHub provider); use in-memory hydrated map for same-process hand-off - Critical: explicit resourceName parameter on Route (not from out.Name) - Important: enumerate all 5 state-write call sites + per-site routing policy - Important: SensitiveKeys() stays display-masking-only; routing uses per-call Sensitive map exclusively - Important: Read/Adoption/Refresh paths sanitize-only (no Set), preventing cache pollution - Important: compensating Delete on Set-success/Save-failure (no orphan cloud resources) - Important: MaskSensitiveForDiff prevents drift false-positives Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5-task plan covering: iac/sensitive package, persistResourceWithSecretRouting helper + Apply call-site rewires (sites 1+2), sanitize-only Read paths (sites 3+4+5), audit-state-secrets command + drift masking, docs. Single PR; ~1.6k lines net. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…dings Addresses adversarial-design-review (plan phase) cycle 1 findings: Critical: - Type-system mismatch: helper takes infraStateStore (wfctl-internal), not interfaces.IaCStateStore. Matches existing call-site discipline. - Removed dead currentInfraConfigFile() reference; cfgFile threaded as explicit new parameter through applyWithProviderAndStore + applyPrecomputedPlanWithStore. - Tasks 2 + 3 add Step N.7.bis runtime-launch-validation smoke (build artifact + exercise without/with secrets cfg, capture transcripts). Important: - Task 4 split into Task 4 (audit-state-secrets only) + Task 5 (drift masking enumeration with explicit no-op clause). Plan now has 6 tasks. - Step 2.5.1 adds explicit existing-test-update sub-step for syncInfraOutputSecrets/resolveInfraOutput signature ripple. - Step 2.4 #5 preserves validateOutputProviderID call (regression guard). - Step 2.4 #6 adds pre-flight detection of sensitive-emitting drivers before persistence loop (no partial-apply mid-stream). - Step 2.4 #7 isolates compensation context (fresh 30s timeout). - Task 6 §6.4 validates rollback claim against v0.26.x consumer. - Public API contract section locks exported names from Task 1. - Scope Manifest excludes infra.go:1101 import path with reasoning. Minor: - Documents string-only sensitive value limitation as v0.27.0 constraint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Alignment-check identified ## Task → ### Task normalization required by the writing-plans skill format. Six tasks now match alignment-check's forward + reverse trace and the Scope Manifest counts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Manifest section sha256: f7a8e190ca8bc264252d2609ba856bdd979b3a5eaa3265d0d33f1dc85055c361 Locked at: 2026-05-09T08:11:07Z PR Count: 1 Tasks: 6 Branch: feat/engine-sensitive-output-routing Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…helpers New iac/sensitive package for engine-side routing of ResourceOutput sensitive fields through secrets.Provider. Free functions, no struct. Exported API (locked at v0.27.0): - Route(ctx, provider, resourceName, *out) (sanitized, hydrated, err) - Revoke(ctx, provider, resourceName, mergedKeys) error - IsPlaceholder(v any) bool - MaskSensitiveForDiff(driverKeys, desired, current) (map, map) - Placeholder(resourceName, outputKey) string - PlaceholderPrefix string - SecretKey(resourceName, outputKey) string Placeholder format "secret_ref://<resource>_<key>" is distinct from the existing user-supplied "secret://<key>" config-reference convention. Routing trigger is exclusively out.Sensitive[k]==true; SensitiveKeys() remains a display-masking-only signal per design rev1. 23 unit tests cover happy/error/edge cases per plan §7. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Introduces persistResourceWithSecretRouting helper that funnels both state-write call sites in applyWithProviderAndStore + in-process apply. Routes per-call Sensitive ResourceOutput fields through secrets.Provider into "secret_ref://" placeholders before SaveResource. Threading: - applyWithProviderAndStore + applyPrecomputedPlanWithStore each gain cfgFile string + hydratedOut map[string]string parameters. - applyInfraModules now returns (map[string]string, error) where the map contains routed-secret values for in-process hand-off to syncInfraOutputSecrets. - syncInfraOutputSecrets + resolveInfraOutput accept a hydrated map to rehydrate sensitive.PlaceholderPrefix entries from same-process apply (works on write-only GitHub Actions provider). Failure modes: - Pre-flight detection: if any driver emits Sensitive outputs but no secrets.Provider is configured, hard-fail BEFORE per-resource persistence loop (no partial-apply). - On SaveResource failure post-Set, compensateAfterSaveFailure with a fresh 30s context calls driver.Delete + provider.Delete to prevent orphan cloud resources + routed secrets. Test ripple: every existing call site updated to pass nil/empty for the new parameters. New test file infra_apply_sensitive_routing_test.go covers Route happy path, NoProvider hard-fail, SaveFailure compensation, NoSensitive pass-through, idempotent re-apply, pre-flight detection, read-mode placeholder preservation + new-key drop. Rollback: revert this commit; helper + call sites revert to prior literal Outputs: r.Outputs shape in one diff. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Read paths (adoptExistingResources, runInfraRefreshOutputs) now route state writes through persistResourceWithSecretRouting in persistModeRead. The helper inherits placeholders from prior state (via ListResources) and drops newly-declared sensitive keys — never calls provider.Set from a Read path (prevents cache pollution per design §4.4). Refresh path uses driver.SensitiveKeys() as the per-call Sensitive map source (refresh has no per-call Sensitive declaration; SensitiveKeys is the static driver-declared signal that approximates intent). Site 4 (resourceStateFromLiveOutput) is a builder, not a save site; no change needed there. Sites 3+5 wired through the helper. Rollback: revert this commit; sites revert to direct store.SaveResource. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds wfctl infra audit-state-secrets command per design §4.7. Distinct from existing audit-secrets (which audits the secrets.generate config block for anti-patterns). Audit-state-secrets walks state.Outputs vs. secrets.Provider to detect: - placeholder-without-routed-value findings (state has secret_ref:// placeholder but provider does not have the secret) - legacy plaintext findings (state value matches DefaultSensitiveKeys heuristic AND is plaintext rather than a placeholder) - mistaken secret://... config-reference findings (user-config syntax leaked into a persisted state field) - orphan secret findings (provider has a name matching <res>_<key> pattern; no state resource named <res>) --prune deletes confirmed orphan secrets idempotently (safe to rerun). Exit codes: 0 = no findings; 1 = findings; 2 = audit error. For write-only providers (GitHub Actions Get returns ErrUnsupported), the command emits structured ADVISORY lines for each placeholder it cannot verify, but does not exit non-zero on those alone. Rollback: revert this commit; command is additive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per-the design's drift-masking enumeration task (Task 5 of the engine-sensitive-output-routing plan): grep enumeration of driver.Diff/d.Diff call sites in cmd/wfctl/ + iac/ shows zero production sites that see state.Outputs flowing into Diff. Only iac/conformance/scenario_grpc_roundtrip.go matches and that's a conformance-test tool that synthesizes its own desired/current. This commit ships the documented no-op outcome: a comment block at the top of cmd/wfctl/infra_apply_refresh.go (the closest applicable file) explains the enumeration outcome and how to wire sensitive.MaskSensitiveForDiff if a future call site lands. sensitive.MaskSensitiveForDiff stays exported as the future hook. Rollback: revert this commit; comment is additive. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- DOCUMENTATION.md: new "Sensitive output routing" section explaining Sensitive flag, secret_ref:// placeholder format, failure modes, recovery path, read-path semantics, drift masking, cold-start consumers, and v0.27.0 string-only limitation. - docs/WFCTL.md: command-tree update + new "infra audit-state-secrets" command reference. - CHANGELOG.md: comprehensive Unreleased entry covering Added (engine routing, iac/sensitive package, audit-state-secrets command), Changed (signature ripple for applyWithProviderAndStore + applyPrecomputedPlanWithStore + applyInfraModules + syncInfraOutputSecrets + resolveInfraOutput, sanitize-only routing for adoption + refresh), Migration (greenfield + legacy guidance), Rollback (v0.26.x runbook with validation-deferred note). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds engine-side handling for IaC driver outputs explicitly marked as sensitive, routing them through the configured secrets.Provider and persisting secret_ref://... placeholders to state. This centralizes sensitive-output behavior in the engine/CLI so plugins remain host-agnostic across CI and local runs.
Changes:
- Introduces new
iac/sensitivehelpers (Route,Revoke, placeholder utilities, diff masking) with unit tests. - Rewires wfctl infra apply/adopt/refresh/state-output-secret generation flows to preserve placeholders and pass same-process “hydrated” secret values to consumers.
- Adds
wfctl infra audit-state-secretscommand plus documentation/plan updates describing the routing model and recovery workflows.
Reviewed changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| iac/sensitive/route.go | New core routing + placeholder utilities + diff masking helpers. |
| iac/sensitive/route_test.go | Unit tests for routing/revocation/placeholder/diff masking behavior. |
| cmd/wfctl/infra_apply.go | Funnels state persistence through sensitive routing/sanitize-only helper; adds compensation-on-save-failure. |
| cmd/wfctl/infra.go | Threads apply “hydrated” map to post-apply infra_output secret generation; wires new command. |
| cmd/wfctl/infra_output_secrets.go | Adds hydrated-map support to resolve secret_ref:// placeholders for infra_output secret generation. |
| cmd/wfctl/infra_resolve_state.go | Passes nil hydrated map for offline/preview resolution paths. |
| cmd/wfctl/infra_refresh_outputs.go | Refresh path now sanitize-persists outputs (no provider writes) using read-mode helper. |
| cmd/wfctl/infra_apply_refresh.go | Documents current lack of in-tree Diff call sites needing masking. |
| cmd/wfctl/infra_audit_state_secrets.go | New infra audit-state-secrets implementation (audit + optional prune). |
| cmd/wfctl/infra_audit_state_secrets_test.go | Unit tests for audit findings, prune behavior, and write-only provider advisories. |
| cmd/wfctl/infra_apply_sensitive_routing_test.go | Integration-style tests for persist routing/sanitize-only behavior and compensation. |
| cmd/wfctl/infra_output_secrets_test.go | Updates for new hydrated parameter in syncInfraOutputSecrets. |
| cmd/wfctl/infra_secrets_env_test.go | Updates resolveInfraOutput calls for new hydrated parameter. |
| cmd/wfctl/infra_state_store_test.go | Updates applyInfraModules call signature (returns hydrated map). |
| cmd/wfctl/infra_provider_dispatch_test.go | Updates applyInfraModules call signature. |
| cmd/wfctl/infra_plan_apply_equivalence_test.go | Updates applyInfraModules call signature. |
| cmd/wfctl/infra_apply_v2_test.go | Updates applyWithProviderAndStore signature at test call sites. |
| cmd/wfctl/infra_apply_v2_loader_test.go | Updates applyInfraModules call signature. |
| cmd/wfctl/infra_apply_troubleshoot_test.go | Updates applyWithProviderAndStore signature at test call sites. |
| cmd/wfctl/infra_apply_test.go | Updates applyInfraModules/applyWithProviderAndStore signatures at test call sites. |
| cmd/wfctl/infra_apply_plan_test.go | Updates applyPrecomputedPlanWithStore signature at test call sites. |
| cmd/wfctl/infra_apply_jit_loader_test.go | Updates applyInfraModules call signature. |
| cmd/wfctl/infra_apply_allow_replace_test.go | Updates applyWithProviderAndStore/applyPrecomputedPlanWithStore signatures at test call sites. |
| DOCUMENTATION.md | Adds “Sensitive output routing (v0.27.0+)” section describing placeholders, failure modes, and recovery. |
| docs/WFCTL.md | Documents infra audit-state-secrets and adds it to the command tree diagram. |
| CHANGELOG.md | Adds v0.27.0 entries for sensitive routing, new package/command, signature changes, migration/rollback notes. |
| docs/plans/2026-05-09-engine-sensitive-output-routing.md | Implementation plan document added to repo. |
| docs/plans/2026-05-09-engine-sensitive-output-routing.md.scope-lock | Scope lock metadata for the implementation plan. |
| docs/plans/2026-05-09-engine-sensitive-output-routing-design.md | Design doc capturing goals, non-goals, and approach. |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
Lint fix: - infra_rotate_and_prune.go:121: //nolint:gosec G117 false-positive — access_key is the public DO Spaces identifier (analogous to AWS access key ID); the credential secret_key is stored separately per ADR 0017 split-storage. Copilot review fixes: 1. applyFromPrecomputedPlan now returns the same-process hydrated routed-secret map; --plan path threads it into syncInfraOutputSecrets so rehydration works on write-only providers (GitHub Actions secrets etc.). runHydrated populated for both --plan and live-diff branches. 2. (paired with #1) runInfraApply captures hydrated map from applyFromPrecomputedPlan. 3. adoptExistingResources precomputes priorByName once per pass; new persistResourceWithSecretRoutingCachedPrior helper avoids per-resource ListResources scans (was O(n²) on filesystem-backed stores). 4. audit-state-secrets --prune now refuses noopStateStore (no iac.state configured) — prevents catastrophic deletion when every secret would appear orphan. 5. audit-state-secrets surfaces non-ErrNotFound/non-ErrUnsupported errors from prov.Get as findings (provider error class) instead of swallowing. 6. Orphan-detection collects observed sensitive-key names from state placeholders + DefaultSensitiveKeys() and prefers the longest matching suffix; driver-declared non-default sensitive keys are no longer misclassified as orphans (and --prune won't delete them). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Engine-side routing of
*ResourceOutput.Sensitivefields through the configuredsecrets.Provider, replacing the design's earlier (and rejected) plan for plugin-sideghSecretsProviderconstruction. Plugin-agnostic: works whether wfctl runs from GitHub Actions, GitLab CI, user CLI, or anywhere else.Why
Plan rev6 of the spaces-key-iac-resource design assumed
IaCProvider.Initialize()had asecrets.Providerparameter. Verified at impl time: it doesn't. User chose the architectural fix (engine-side routing throughsecrets.Providerabstraction) over the in-plugin shortcut.What
iac/sensitivepackage (new):Route,Revoke,IsPlaceholder,MaskSensitiveForDiff,Placeholder,PlaceholderPrefix,SecretKey. 23 unit tests.applyWithProviderAndStore,applyPrecomputedPlanWithStore,adoptExistingResources,runInfraRefreshOutputsall funnel throughpersistResourceWithSecretRouting. Apply mode routes; Read mode sanitize-only.Set+SaveResourcepartial failure: isolated 30s context; deletes both cloud-side resource and partially-written secret. Operator never sees orphan resources.applyInfraModulesreturns(map[string]string, error);syncInfraOutputSecretsconsumes hydrated map directly. Works with write-only secret providers.wfctl infra audit-state-secrets [--prune]: detects 4 finding classes (orphan secrets, missing secrets for placeholders, plaintext-in-state, write-only-provider gaps); read-only by default. 8 unit tests.Verification
go test ./...PASSgo test -race ./cmd/wfctl/... ./iac/sensitive/... ./secrets/...PASSgo vet ./...cleangolangci-lint run ./cmd/wfctl/... ./iac/sensitive/...cleanDesign + plan
docs/plans/2026-05-09-engine-sensitive-output-routing-design.md(2 adversarial-design-review cycles to PASS)docs/plans/2026-05-09-engine-sensitive-output-routing.md(2 adversarial cycles to PASS; alignment-check PASS)f7a8e190ca8bc264252d2609ba856bdd979b3a5eaa3265d0d33f1dc85055c361Note
This PR replaces #587 (closed) — same commits, recreated on a fresh branch to avoid force-push per workspace convention. 12 commits ahead of origin/main.
Test plan
🤖 Generated with Claude Code