mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 21:51:30 +00:00
shankar0123
737c329824
Closes Top-10 fix #4 of the 2026-05-02 deployment-target audit re-run (see cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md). Pre-fix, both IIS and WinCertStore's realExecutor invoked PowerShell via exec.CommandContext(ctx, ...) and relied entirely on the caller's ctx to provide a deadline. If the caller forgot to attach one (context.Background() in a deeply-nested path; an operator running an ad-hoc deploy via a CLI that doesn't default-deadline its ctx), a hung WinRM session blocked the deploy worker thread indefinitely. S2 (failure isolation) bar from the audit: "does a hung WinRM take down the deploy worker pool?" — today's answer was "potentially yes" for these two connectors. Post-fix the answer is "no, capped at the configured ExecDeadline (default 60s)". This commit: 1. Adds Config.ExecDeadline (time.Duration, json: "exec_deadline") to both connectors, defaulted to 60 seconds. WinCertStore defaults via the existing applyDefaults helper; IIS defaults inline at New() and inside ValidateConfig (the IIS connector has no shared applyDefaults helper today; out-of-scope to refactor one in for this minor fix). Operators on slow Windows links can override via the JSON config field exec_deadline. 2. Wraps realExecutor.Execute with a fallback context.WithTimeout that fires ONLY when ctx has no deadline of its own. Caller- supplied deadlines always win — the wrapper is a safety net, not a hard cap. defer cancel() guards against goroutine leaks. 3. Tests: - TestIIS_RealExecutor_AttachesDefaultDeadlineWhenCallerHasNone (passes context.Background; asserts the call returns within 500ms with an error). On Linux/macOS runners powershell.exe is missing and exec.Cmd fails fast; on Windows the wrapper's ctx deadline cancels the running PowerShell process. Either path returns well under 500ms. - TestIIS_RealExecutor_RespectsCallerDeadlineWhenSet (10s fallback executor deadline, 50ms caller ctx; asserts caller deadline wins). - TestIIS_RealExecutor_NoDeadlineWiredWhenZero (deadline=0 means no fallback wrapper; caller's tight ctx still bounds). - TestIIS_New_DefaultsExecDeadlineTo60s + TestIIS_New_RespectsExplicitExecDeadline pin the constructor's defaulting behavior (uses winrm mode so the test doesn't need powershell.exe in PATH). - Same five tests in wincertstore_test.go. 4. docs/connectors.md IIS + WinCertStore sections document the new exec_deadline field with: what it is (per-PowerShell- subprocess cap), default (60 seconds), override semantics (caller ctx deadline wins). No change to behavior when the caller already attaches a deadline (the common case in production code paths). Tests using the mock executor (mockExecutor in iis_test.go / wincertstore_test.go) are unaffected — they bypass realExecutor entirely. S2 cross-cutting scorecard rating in cowork/deployment-target-audit-2026-05-02-rerun/findings.json flips from "gap" to "pass" for IIS and WinCertStore (in any future re-audit). Verified locally: - gofmt / go vet / staticcheck clean across both packages. - go test -race -count=1 ./internal/connector/target/iis/... ./internal/connector/target/wincertstore/... green. Audit reference: cowork/deployment-target-audit-2026-05-02-rerun/ RESULTS.md Top-10 fix #4.