🔒 fix(provider): bound exec stdout, set WaitDelay, capture stderr - #81
Merged
Conversation
CallExec had three containment gaps that CallHTTP, twenty lines above in the same file, does not (audit 2026-08-18; REL-01 is byte-identical across three audits because no test measured it): - REL-01 (REQ-AUD2-S01-01/02/03): child stdout went into an unlimited bytes.Buffer. It now flows through a boundedCapture that retains at most MaxResponseBytes+1 and applies the SAME readBounded verdict as the HTTP body read — over-limit is an error with nil bytes, at-limit is legitimate traffic. No second limit constant: MaxResponseBytes stays the single response bound. - REL-02 (REQ-AUD2-S01-04/05): cmd.WaitDelay = opts.Timeout. Killing the child does not close a stdout pipe its background grandchildren still hold, so cmd.Run blocked past the operator's deadline; the call now returns in ~2x Timeout and the resulting error classifies as unavailable. - REL-07 (REQ-AUD2-S01-06): stderr is captured into its OWN bounded buffer and a truncated excerpt is folded into the returned error with %w, so a failing provider explains itself instead of yielding a bare "exit status 1". Per judgment call (c) it is never merged into the fact bytes. Behaviour change flagged by judgment call (a): a failed CallExec now returns nil bytes alongside the error, matching CallHTTP. Every caller was checked — cmd/assent/provider_host.go:206 wraps it as a CallFunc and ResolveFacts ignores raw when err != nil (REQ-AUD2-S01-07: any call error -> unavailable). Mutation-proved (all observed red, then reverted): removing the readBounded verdict; removing the retention cap while keeping the verdict (black-box tests stay GREEN — hence the white-box transport_internal_test.go); removing cmd.WaitDelay (test blocks the full 60s); removing the cmd.Stderr wiring; and merging stderr into stdout.
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.
AUD2-S01 — exec transport trio (REL-01 / REL-02 / REL-07)
Integration of
lane/aud2-s01(review headc7db55e, verdict APPROVE, no P0/P1, no merge conditions), rebased ontoorigin/mainwith the CHANGELOG regenerated.What changes (
internal/provider/transport.go)readBounded/MaxResponseBytespath via a newboundedCapturewriter. Over-limit is an error with no bytes, identical toCallHTTP's body semantics, so nothing can parse a truncated document and a runaway provider cannot grow the runner's heap without limit.cmd.WaitDelay = opts.Timeout, so a provider that forks a grandchild inheriting stdout cannot holdcmd.Runopen past the operator's deadline. The resultingexec.ErrWaitDelayclassifies as unavailable like any other call error.errors.Is/As). It is never concatenated into the returned fact bytes.CallExecnow returnsnilbytes on error instead of the partial buffer;ResolveFactsCheckedalready discardsrawon the error branch, so this is strictly more fail-closed.Verification on the rebased head
task check— exit 0go test -race ./internal/provider/...— passTestExecWaitDelayBoundsOrphanedStdout(wall-clock assertion) —-race -count=5, 5/5 green, ~4.5 s/iterationMaxResponseBytesstill declared exactly once ininternal/provider, unchanged at8 << 20CHANGELOG.mdBranch note: pushed as
lane/aud2-s01-integratebecause the rebase + CHANGELOG amend rewrites SHAs already published onlane/aud2-s01, and force-push is not authorized (AGENTS.md hard rule 2).lane/aud2-s01stays atc7db55eas the review record.Last of AUD2's four fix lanes.