Potential fix for code scanning alert no. 20: Uncontrolled data used in path expression - #29

…re + follow-up TODOs

bootstrapStateBackend now dispatches by iac.state.backend:
- spaces → bootstrapStateBackendSpaces (DO endpoint URL constructed here)
- s3/gcs/azure → clear not-yet-implemented errors with contribution guidance
- filesystem/memory/postgres/"" → no-op

bootstrapStateBackendSpaces: exports WFCTL_STATE_BUCKET (generic) +
SPACES_BUCKET (backend-specific, no cloud-provider prefix). bootstrapDOSpacesBucket
now accepts caller-supplied endpoint; creds error message genericised; log
message updated to "created spaces state bucket".

resolveStateStore: wires module.PostgresIaCStateStore for backend=postgres;
s3/gcs/azure return clear errors instead of silent noop.

Misc:
- TODO(#28) added to deploy.go hardcoded credential checks
- TODO(#29) added to providerCredentialSubKeys map
- infra import --provider help text no longer lists specific providers
- All bootstrapDOSpacesBucketFn test fakes updated for new endpoint param
- CHANGELOG updated

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

…troy/status/drift direct paths + bootstrap URL export (#471)

* feat(wfctl): persist ResourceState after provider.Apply — fixes BMW state loss

Refactors applyWithProvider → applyWithProviderAndStore(ctx, provider,
providerType, specs, current, store). After provider.Apply returns success,
each ResourceOutput is written to the store; save failures log a loud warning
but do not abort the apply. Delete actions trigger store.DeleteResource after
the Apply call.

Also introduces resolveStateStore(cfgFile) which reads the iac.state module
config and instantiates the correct backend (filesystem or Spaces), rewriting
loadCurrentState to delegate to it so that remote backends are supported
beyond the previous filesystem-only path.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(wfctl): direct-path infra destroy + status + drift for infra.* modules

Adds destroyInfraModules, statusInfraModules, and driftInfraModules that load
state from the configured backend, group records by IaCProvider, and dispatch
to provider.Destroy/Status/DetectDrift respectively.

runInfraDestroy, runInfraStatus, and runInfraDrift now dispatch to the direct
path when hasInfraModules() is true, falling back to the legacy pipeline path
for platform.* configs.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(wfctl): bootstrap exports Spaces bucket URL back to config

After bootstrapStateBackend creates or confirms a DO Spaces bucket,
prints 'export DO_SPACES_BUCKET=<name>' to stdout for CI capture via
$GITHUB_ENV, and calls writeBucketBackToConfig to bake the resolved
bucket name into the on-disk YAML so downstream wfctl commands can read
state without the env var being set again.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* chore: add v0.18.6 CHANGELOG entry

* fix(lint): resolve 8 gocritic/nilerr issues in infra apply/destroy/status/drift

- Extract provider-loop bodies into closures (applyGroup, destroyGroup,
  statusGroup, driftGroup) to fix deferInLoop in all four files
- Switch range-value loops to index loops for rangeValCopy (plan.Actions
  136B, ResourceState 208B) in infra_apply, infra_destroy, infra_status_drift
- Split combined err+len guard in resolveStateStore to satisfy nilerr;
  add //nolint:nilerr on the intentional err-swallow (noop on missing config)

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(wfctl): provider-agnostic bootstrap dispatch + postgres state store + follow-up TODOs

bootstrapStateBackend now dispatches by iac.state.backend:
- spaces → bootstrapStateBackendSpaces (DO endpoint URL constructed here)
- s3/gcs/azure → clear not-yet-implemented errors with contribution guidance
- filesystem/memory/postgres/"" → no-op

bootstrapStateBackendSpaces: exports WFCTL_STATE_BUCKET (generic) +
SPACES_BUCKET (backend-specific, no cloud-provider prefix). bootstrapDOSpacesBucket
now accepts caller-supplied endpoint; creds error message genericised; log
message updated to "created spaces state bucket".

resolveStateStore: wires module.PostgresIaCStateStore for backend=postgres;
s3/gcs/azure return clear errors instead of silent noop.

Misc:
- TODO(#28) added to deploy.go hardcoded credential checks
- TODO(#29) added to providerCredentialSubKeys map
- infra import --provider help text no longer lists specific providers
- All bootstrapDOSpacesBucketFn test fakes updated for new endpoint param
- CHANGELOG updated

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* refactor: BootstrapStateBackend → IaCProvider interface dispatch

Move all cloud-provider-specific bucket-creation logic out of wfctl core
and behind the IaCProvider plugin interface:

- interfaces/iac_provider.go: add BootstrapStateBackend(ctx, cfg)
  (*BootstrapResult, error) to IaCProvider; add BootstrapResult struct
  (Bucket, Region, Endpoint, EnvVars)
- infra_bootstrap.go: remove all DO/AWS SDK imports and S3-specific code;
  bootstrapStateBackend now finds the first iac.provider module, calls
  resolveIaCProvider, then delegates to provider.BootstrapStateBackend;
  self-contained backends (filesystem/memory/postgres/"") remain no-ops
- deploy_providers.go: wire BootstrapStateBackend on remoteIaCProvider
  via InvokeService + anyToStruct, consistent with rest of bridge
- All IaCProvider test stubs: add BootstrapStateBackend(nil, nil) stub
- Delete infra_bootstrap_bucket_test.go: DO-specific S3 tests move to
  DO plugin (follow-up #31, workflow-plugin-digitalocean v0.7.4)
- Rewrite infra_bootstrap_env_test.go + infra_bootstrap_url_export_test.go:
  replace bootstrapDOSpacesBucketFn overrides with resolveIaCProvider
  overrides using envBootstrapProvider / urlBootstrapProvider fakes
- Add infra_bootstrap_provider_test.go: TestBootstrapStateBackend_DispatchesToProvider
  (full round-trip), SelfContainedBackendsSkipped, NoProviderModuleErrors

Addresses review comment #3132352899 (intel352): DO-specific bootstrap
should not live in wfctl core.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>

…ovider + audit-viewer UI (#807)

* feat(iac/stubprovider): promote NoopProvider to a loadable package

Adds iac/stubprovider with an in-process interfaces.IaCProvider that:
- Plan: produces create/update/delete actions by comparing desired vs current
- ResourceDriver: returns a stub driver whose Create/Update/Delete succeed
- Destroy: returns all refs as Destroyed
- DetectDrift: returns Drifted:false for every ref

iactest.NoopProvider is left intact (no import cycle; stubprovider imports
only interfaces).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(iac/admin/ui): mutation panels (plan/apply/destroy/drift)

Adds v1.1 mutation panel to resource.html + resource.js:
- Bearer token input (sessionStorage, password field)
- Plan button → POST /api/infra-admin/plan → renders action diff table
  with allow_replace checkboxes on replace rows (selectable, not free-text)
- Apply button (guarded by confirm checkbox, disabled until Plan runs) →
  POST /api/infra-admin/apply with {plan_id, desired_hash, allow_replace[]}
- Destroy button (guarded by confirm checkbox) →
  POST /api/infra-admin/destroy with resource ref + confirm_hash
- Check Drift button → POST /api/infra-admin/drift → renders drift table
All mutation fetches send Authorization: Bearer <token>.
CSRF gated by requireBearer middleware (401 on missing header).
All existing embed + asset tests pass.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(plugins/stubprovider): build-tagged loadable stub iac.provider

- plugins/stubprovider/plugin.go: EnginePlugin registering 'iac.provider'
  factory; stubModule.Init validates provider=stub, logs demo-only warning,
  registers stubprovider.Provider via ProvidesServices so infra.admin can
  resolve it via app.GetService(<providerModuleName>, &iacProvider)
- plugins/all/all.go: adds scenarioExtras var; DefaultPlugins() appends it
- plugins/all/extras_stub.go (//go:build scenario_stub): init() registers stub
- plugins/all/extras_base_test.go (!scenario_stub): asserts stub absent
- plugins/all/extras_stub_test.go (scenario_stub): asserts stub present

Untagged builds: stub absent (TestDefaultPlugins_BaseExcludesStub PASS)
Tagged builds:   stub present (TestDefaultPlugins_ContainsStub PASS)
Tagged cmd/server: builds clean

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* refactor(wfctlhelpers): promote DesiredStateHash (resolve+hash) for in-process TOCTOU

Adds wfctlhelpers.DesiredStateHash(cfg, desired, current, env) which:
- Builds syncedOutputs from current state (mirrors buildSyncedOutputsFromState)
- Applies plan-time JIT substitution via jitsubst.TryResolveSpec to collapse
  ${MODULE.id} refs to current ProviderIDs before hashing
- Sorts specs by name + SHA-256 hashes the canonical JSON

cmd/wfctl/infra_apply.go desiredStateHash() becomes a thin wrapper calling
the promoted function with pre-resolved specs (nil cfg + nil current).

Tests:
- TestDesiredStateHash_Determinism: same inputs → same hash
- TestDesiredStateHash_ModuleRefCollapses: ${vpc1.id} → ProviderID before hash
- TestDesiredStateHash_ChangesOnFieldChange: different config → different hash
- TestDesiredStateHash_EmptySpecsIsStable: sha256("[]") not empty string
- TestDesiredStateHash_SortOrderIndependent: order of input specs irrelevant

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* test(iac/admin/proto): apply code-review nits F1/F2/F3

F1: capture Marshal error in all 4 error-discriminator sub-tests (was
    `_, _` discarding it, masking root cause on failure).
F2: correct TestMutationOutputs_DiscardUnknown comment + add Input-direction
    test (AdminApplyInput with unknown field) — the more security-relevant
    direction: server must not reject valid client requests for unknown fields.
F3: assert typed fields are empty on all 4 error-response round-trips
    (PlanId/DesiredHash/Actions, Applied/Errors, Destroyed/Errors, Drift).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(iac/admin/ui): T11 spec-review I-1 + S-1 + S-2

I-1 (Important): add confirm_hash guard to handleDestroy — mirror Apply's
  !PLAN_STATE.desiredHash early-return so Destroy cannot send an empty
  confirm_hash without a prior Plan run (TOCTOU discipline per design §3.2).
S-1: clear PLAN_STATE.actions alongside planId/desiredHash after Apply
  (stale actions were left in memory after a successful apply).
S-2: remove redundant bearer-token change listener (bearer() already
  persists to sessionStorage on each mutation call).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(module/infra_admin): #29 require auth_module + authz_module + subject propagation

- InfraAdminConfig: +AllowUnauthenticated bool + AuthzModule string fields
- Enforcer interface: variadic Enforce(sub,obj,act string,extra ...string) (plan C-NEW-1)
- InfraAdmin.authz field: resolved at Init when authz_module configured
- Init: returns error when auth_module=="" && !allow_unauthenticated
- Init: logs pinned warning 'infra.admin: mutation routes DISABLED (no auth_module); reads only'
- Init: resolves authz_module as Enforcer via GetService
- Dependencies()+RequiresServices(): include authz_module when non-empty
- subjectFromRequest(r): extracts claims["sub"] from authClaimsContextKey context
- standardCfg() updated to AllowUnauthenticated:true for test isolation

Tests added:
  TestInfraAdmin_Init_AuthModuleRequired
  TestInfraAdmin_Init_AllowUnauthenticatedNoError (pins warning literal)
  TestInfraAdmin_Init_AuthzModuleResolved
  TestInfraAdmin_Init_AuthzModuleListedInDependencies
  TestInfraAdmin_SubjectFromRequest

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(iac/admin/ui): code-review minor F1+F2 (T11 post-review)

F1: add resource-not-loaded guard at top of handleDestroy — if
  RESOURCE_STATE.type is empty (load() failed), show error + return
  early before building a malformed ref with empty type.
F2: disable mutation buttons during in-flight fetch (finally restores):
  - btn-plan: disabled true/false around postMutation
  - btn-apply: disabled true during fetch, restored to confirm-checkbox
    state in finally (preserves the checkbox gate logic)
  - btn-destroy: same pattern as apply (confirm-checkbox gate)
  - btn-drift: disabled true/false around postMutation
  Prevents overlapping requests from rapid clicks; server single-flight
  mutex (T8) remains the authoritative guard.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(iac/admin/handler): PlanResource + DriftCheckResource

- plan_resource.go: PlanResource(ctx, store, providers, cfg, desiredSpecs, in)
  - authz default-deny via authzError
  - selects first provider (single-provider v1.1 model)
  - loads current state from store if non-nil
  - filters by resource_filter / app_context
  - computes desired_hash via handlerDesiredHash (inlined to break wfctlhelpers→module→handler cycle)
  - calls provider.Plan then maps actions to AdminPlanAction
  - returns plan_id, desired_hash, actions, plan_json
- drift_check.go: DriftCheckResource(ctx, providers, in)
  - authz default-deny
  - calls provider.DetectDrift; maps DriftResult→AdminDriftResult
- handlerDesiredHash: inlined wfctlhelpers.DesiredStateHash logic
  (avoids iac/admin/handler → wfctlhelpers → module → iac/admin/handler cycle)
- All plan/drift tests PASS; lint 0 issues

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(iac/admin/handler): ApplyResource + DestroyResource (TOCTOU + allow_replace + RBAC)

- apply_resource.go: ApplyResource(ctx, store, providers, authz, subject, cfg, desiredSpecs, in)
  Gates: (1) authzError default-deny, (2) server-side Enforcer.Enforce(sub,'infra:apply','allow')
  NOT trusting client's evidence.granted_permissions, (3) TOCTOU hash recompute+compare,
  (4) handlerValidateAllowReplaceProtected, (5) handlerApplyPlan (create/update/delete via ResourceDriver)
- destroy_resource.go: DestroyResource(ctx, providers, authz, subject, in)
  Gates: (1) default-deny, (2) Enforcer.Enforce(sub,'infra:destroy','allow')
  Calls provider.Destroy directly.
- Enforcer interface defined in handler package (avoids redefining in module; same variadic shape)
- handlerValidateAllowReplaceProtected: inlined from wfctlhelpers (avoids import cycle)
- handlerApplyPlan: simplified apply loop via ResourceDriver (Create/Update/Delete)
- redactCredentials: minimal URL-userinfo scrubber for Output.error
- All 55 handler tests PASS; lint 0 issues

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* feat(module/infra_admin): mutation routes + requireBearer + single-flight + 3-way audit + write RBAC

T8 additions to module/infra_admin.go:
- InfraAdmin struct: +wfCfg, +desiredSpecs, +providerMu fields
- NewInfraAdmin: initialize providerMu map
- populateProviderTypes extended: stores wfCfg + extracts desiredSpecs from infra.*
  modules via config.ResolvedModule (preserves Protected flag + per-env overrides)
- Init provider loop: creates per-provider *sync.Mutex in providerMu
- Start: registers 4 mutation routes (plan/apply/destroy/drift) only when m.auth!=nil
  with [auth, secHdrs, requireBearer] middleware stack
- requireBearerAuthMiddleware: rejects non-Bearer requests with 401 (CSRF gate)
- tryLockProvider: TryLock per-provider mutex; writes 409 on contention
- handlePlanResource/ApplyResource/DestroyResource/DriftCheckResource: dispatch to
  T6/T7 handlers passing m.authz, m.subjectFromRequest(r), m.wfCfg, m.desiredSpecs
- auditResultFor: 3-way classification (ok/denied/error) via error-string markers
- Helper funcs: isInfraModuleType, infraSpecFromResolved, cloneAnyMap

Tests added:
  TestInfraAdmin_MutationRoutesRegistered (4 routes present with auth)
  TestInfraAdmin_MutationRouteAbsentWithoutAuth (absent without auth)
  TestInfraAdmin_MutationRequiresBearerToken (401 without Bearer)
  TestInfraAdmin_AuditResultFor3Way (3-way classification)
All 32 module tests PASS; lint 0 issues

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* test(module/infra_admin): security regression suite (CSRF/spoof/TOCTOU/single-flight/audit)

Named security regression tests per T9:
- TestInfraAdmin_MutationRequiresBearer: all 4 mutation routes reject non-Bearer (CSRF gate)
- TestInfraAdmin_ApplyRejectsStalePlanHash: stale desired_hash → error before cloud op (TOCTOU)
- TestInfraAdmin_ConcurrentApplyReturns409: goroutine holding mutex → 409 on concurrent apply
  (MUST be concurrent per plan-review M-2 — sequential variant would falsely pass)
- TestInfraAdmin_ViewerCannotApply: server-side Enforcer denies infra:apply regardless of
  client body evidence.granted_permissions (proves RBAC is real not theater)
- TestInfraAdmin_AuditDistinguishesDeniedFromError: 3-way audit correctly classifies
  authz/stale-hash denials vs provider errors

All 38 module tests PASS; lint 0 issues

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* test(module/infra_admin): mutation integration (apply->state->audit) via real engine path

T10: manually wired infra.admin + auth stub + stubprovider.New() + audit log:
- TestMutationIntegration_Apply: POST /plan → desired_hash → POST /apply →
  applied[] no errors + audit entry {action:apply, result:ok}
- TestMutationIntegration_Destroy: POST /destroy with 2 refs → destroyed[2]
  + audit entry {action:destroy, result:ok}

Note: state store write (assertion 1) is omitted because the admin handler
delegates cloud ops to provider.ResourceDriver (stub in-process) without
persisting to state — state persistence requires the full wfctlhelpers
engine path which is out of scope for the handler library.
Both tests PASS; lint 0 issues

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(iac/stubprovider): code-review + spec-review fixes (OBS-1 + findings 1-3)

- TestStub_DetectDrift_NotDrifted: add r.Class==DriftClassInSync assertion (OBS-1 / F1)
- TestStub_InterfaceConformance: replace vacuous var_ with nil-return assertion (F2)
- TestStub_Plan_UpdateAction: desired+current → 'update' action (F3)
- TestStub_Plan_DeleteAction: current-only → 'delete' action (F3)

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(wfctlhelpers): rename DesiredStateHash env param from _ to env (spec-review)

Spec requires named env parameter. _ suppressed it and masked the intended
extension point for buildResolvedSecretsFromState. Now named + documented:
callers requiring full ${secret.*} parity must extend via env+cfg.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* test(iac/admin/ui): serve mutation panels + audit-viewer assets

T13: extends ui_test.go with 6 new tests:
- TestAssetFS_AllExpectedFilesEmbedded: adds actions.html + actions.js
  to the pinned expected-file list (catches embed glob regressions)
- TestResourceHTML_MutationPanelMarkup: pins 12 key markup IDs in
  resource.html (mutations section, bearer-token, btn-plan, btn-drift,
  plan-result, plan-actions-table, apply-confirm, btn-apply,
  destroy-confirm, btn-destroy, drift-result, mutation-error)
- TestResourceJS_MutationPanelEndpoints: pins 4 endpoint refs, Bearer
  header, PLAN_STATE, allow-replace-cb in resource.js
- TestActionsHTML_AuditViewerMarkup: pins 8 markup IDs + 3 result
  filter option values (ok/denied/error — selectable only, no free-text)
- TestActionsJS_AuditEndpoint: pins audit endpoint, Authorization/Bearer,
  parseNdjson, renderEntries, 3 CSS classes, setInterval, sessionStorage
- TestAssetPrefix_FilesAccessibleViaSubFS: verifies actions.html/js +
  resource.html/js are reachable after fs.Sub("ui_dist") — the same
  path http.FileServer uses in module/infra_admin.go

All 9 sub-tests (AllExpectedFilesEmbedded) + 6 top-level tests pass.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(iac/admin/handler): T6 spec-review fixes (FAIL-14 + FAIL-16)

FAIL-14: export handler.DesiredHash wrapping handlerDesiredHash so
iac/wfctlhelpers/desired_hash_test.go can assert both implementations
produce identical digests — TestDesiredStateHash_MatchesHandlerInlined
covers empty/create/module-ref-collapsed cases; no import cycle
(handler does not import wfctlhelpers).

FAIL-16: TestPlanResource_WithCurrentState now pins action types by
resource name: vpc1 (existing) → 'update', db1 (new) → 'create'.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(iac/admin/handler): T7 spec-review fixes (IMPORTANT-1 + IMPORTANT-2)

IMPORTANT-1 — DestroyResource confirm_hash TOCTOU gate:
- Added Gate 3: hashDestroyRefs(refs) = SHA-256 of sorted [{name,type}] JSON;
  empty or mismatched confirm_hash → error before any destroy operation
- Exported HashDestroyRefs so UI/tests can compute the expected value
- TestDestroyResource_MismatchedConfirmHash: empty + wrong hash both rejected;
  updated TestDestroyResource_HappyPath to echo correct confirm_hash

IMPORTANT-2 — replace-protected gate test:
- TestApplyResource_ReplaceWithoutAuthorization: replacePlanProvider always
  returns replace action on protected:true resource; empty allow_replace →
  rejected at Gate 4 (handlerValidateAllowReplaceProtected), no apply

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(iac/admin/ui): T12 spec-review — setInterval uses fetchAndCache + dead code removed

- setInterval now calls fetchAndCache (not fetchAudit) so auto-refresh
  keeps lastEntries current; filter re-renders after auto-refresh
  correctly show fresh data instead of pre-refresh cache.
- Removed dead fetchAudit function (duplicate of fetchAndCache without
  cache assignment) and unused origFetch variable.
- Removed the removeEventListener/re-addEventListener hack that worked
  around the now-removed duplication.
- Added lastEntries declaration at module top with explanatory comment.

All 15 ui_test.go tests pass (TestActionsJS_AuditEndpoint still pins
setInterval + sessionStorage + all required strings).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(module): adapt tests for T7 confirm_hash gate + T12 4th contribution

- TestInfraAdmin_Start_Fires3ContributionPipelines: updated to expect 4
  contributions after T12 added register-infra-admin-actions (audit-viewer)
- TestMutationIntegration_Destroy: compute handler.HashDestroyRefs and echo
  as confirm_hash to satisfy T7's TOCTOU gate on DestroyResource

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(module/infra_admin): infraSpecFromResolved: add DependsOn extraction (T8 spec-review)

Matches the CLI's resourceSpecFromResolvedModule (cmd/wfctl/infra.go:468) which
calls extractDependsOn(cfg). Without it, any config with depends_on produces
a different TOCTOU hash in the module vs the CLI — exactly the divergence
T3/plan-review-I-2 was designed to prevent.

extractModuleDependsOn is inlined (cmd/wfctl is package main, not importable).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(T10+handler): C-1/C-2/I-1/I-2/I-3 spec-review fixes

handler: handlerApplyPlan now accepts + calls store.SaveResource after
successful create/update — satisfies T10 spec assertion (1) that state
store gains the resource.

T10 integration test rewrites:
- C-1: recordingStateStore captures SaveResource; assert 'db1' in store post-apply
- C-2: WorkflowConfig adds infra.database 'db1' → plan produces create action
  → applied[] non-empty; assert len(applied) >= 1
- I-1: assert plan_id and desired_hash both non-empty after plan step
- I-2: wire integrationEnforcer (always-allow) via AuthzModule so Enforce is
  exercised end-to-end in the happy-path
- I-3: confirm_hash already present from prior T7 fix; also fixed section
  cache refresh bug (withConfigSectionApp.section must be updated when
  services map changes after construction)

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* test(iac/admin/ui): code-review nits — io.ReadAll + pin setInterval(fetchAndCache,)

- readAsset: replace manual 4096-byte read loop with io.ReadAll (idiomatic
  Go; eliminates the manual buf+loop pattern).
- TestActionsJS_AuditEndpoint: pin setInterval(fetchAndCache, not just
  setInterval, so the T12 regression bug (setInterval(fetchAudit,...))
  would be caught automatically by CI rather than only by spec-review.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix: code-reviewer minor findings (T6/T7/T9)

T7 code-reviewer F1: hashDestroyRefs marshal-error sentinel changed from ""
to "hash-error-N-refs" so an empty client confirm_hash never accidentally
satisfies Gate 3 on a (unreachable) Marshal failure.

T6 code-reviewer F1: TestDesiredStateHash_MatchesHandlerInlined adds
delete-branch case (desired=nil, current=[old-vpc]) to parity test.

T9 code-reviewer F1: TestInfraAdmin_ViewerCannotApply now also drives
/destroy (not just /apply) — catches a future regression that skips
RBAC on destroy route only.

T9/T4 code-reviewer F3: _ = app.RegisterService replaced with explicit
error check in TestInfraAdmin_ViewerCannotApply and
TestInfraAdmin_Init_AuthzModuleResolved.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(T10): add Enforce call counter to integrationEnforcer (spec-reviewer note)

integrationEnforcer now tracks calls with atomic.Int64; apply test
asserts enforcer.calls > 0 after apply, proving Enforce was actually
invoked end-to-end (not just that m.authz != nil at wiring time).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix: code-reviewer minor findings (T3/T8)

T3 F1: DesiredStateHash marshal-error sentinel changed from "" to
"hash-error" — unambiguous, won't match any valid hex digest.
T3 F2: add _ = cfg // reserved comment to match _ = env pattern.

T8 F1: add //nolint:errcheck comment to all four mutation handler
adapters explaining the proto tag-100 error-encoding convention,
so future readers don't add redundant error checks.
T8 F2: log.Warn at Start when len(ProviderModules) > 1 — operators
with multi-provider configs learn about the single-flight limitation
without reading source.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(T10): code-reviewer minor findings F1/F2/F3

F1: mustRegister helper replaces _ = app.RegisterService(...) in
integration test setup — all 4 registrations now propagate errors.
F2: stateRows, _ → stateRows, err := store.ListResources with fatal on err.
F3: applyPayload/destroyPayload json.Marshal now use mustMarshal(t, ...).

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

* fix(wfctlhelpers): DesiredStateHash must NOT resolve env/secret refs

Using os.LookupEnv in TryResolveSpec caused plan-hash != apply-hash:
secret-gen vars (random_hex etc.) resolve to fresh values each call, so
the plan-time hash diverged from the apply-time hash.

Fix: replace os.LookupEnv with a no-op env resolver so ${ENV_VAR} and
${secret.*} placeholders are preserved verbatim. Only ${MODULE.field}
refs whose source is in current state (syncedOutputs) are collapsed —
those are stable ProviderIDs that don't change between plan and apply.

Env drift is tracked separately via plan.InputSnapshot/InputDriftReport,
not the TOCTOU hash.

Fixes: TestParseInfraResourceSpecs_Preserves{SecretGen,RequiredSecret,
SecretEntries}VarsInUserData in cmd/wfctl/infra_plan_env_vars_preserve_test.go

Also fixes the same bug in the inlined handler.DesiredHash copy
(iac/admin/handler/plan_resource.go:handlerDesiredHash).
Drops now-unused os import from both files.

Co-Authored-By: Claude Sonnet 4.6 <[email protected]>

---------

Co-authored-by: Claude Sonnet 4.6 <[email protected]>