feat(iac/admin): v1.1 mutation surface + hardening + loadable stub provider + audit-viewer UI#807
Merged
Merged
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
…n-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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…bject 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
…low_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 <noreply@anthropic.com>
…ight + 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 <noreply@anthropic.com>
…U/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 <noreply@anthropic.com>
…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 <noreply@anthropic.com>
…ngs 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 <noreply@anthropic.com>
…pec-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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
- 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 <noreply@anthropic.com>
…ion (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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…etchAndCache,) - 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…wer 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
⏱ Benchmark Results✅ No significant performance regressions detected. benchstat comparison (baseline → PR)
|
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 <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
1 participant
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.
What
infra-admin v1.1 — the write half of the dynamic, proto-driven admin interface (v1 =
#791read surface). Adds Plan/Apply/Destroy/DriftCheck behind a hardened, audited, two-phase contract. PR-1 of 4 (workflow backend+UI). Plan + ADRs: workspace#8.Tasks (13, all TDD + two-stage reviewed)
iac/stubprovider(promotedNoopProvider) · T2plugins/stubproviderbuild-tagged (-tags scenario_stub) loadableiac.provider— fixes the fact v1's scenario stack never booted (no plugin registerediac.provider) · T3wfctlhelpers.DesiredStateHash(resolve+hash promoted for the TOCTOU guard).authz.Enforce→ TOCTOUdesired_hash→ValidateAllowReplaceProtected).#29auth refuse-empty +authz_module+ subject propagation · T8 routes +requireBearerCSRF middleware + per-provider single-flight (TryLock→409) + 3-way audit.actions.html) · T13 ui_test.Security (server-authoritative, ADR-0008)
authn-required → bearer-only CSRF gate → server-side write-tier RBAC (
infra:apply/infra:destroyviaauthz.casbin.Enforceagainst the authenticated subject; clientevidence.granted_permissionsis audit-only, not trusted) → two-phasedesired_hashTOCTOU →allow_replaceper-resource → single-flight → 3-way audit (ok/denied/error). All-raceverified;ViewerCannotApplyproves RBAC is not theater (atomic Enforce-call counter >0).Verification
27 commits; all 6 affected packages pass
go test -race;cmd/serverbuilds tagged + untagged;golangci-lint --new-from-rev=origin/main0 issues. No live cloud touched (stub provider only). Pipeline: design 3 adversarial cycles + plan 3 cycles (all converged, source-verified) → alignment PASS → scope-locked.🤖 Generated with Claude Code