fix(hooks): bound OCI hook stdout/stderr capture to avoid OOM#813
Open
Atishyy27 wants to merge 1 commit into
Open
fix(hooks): bound OCI hook stdout/stderr capture to avoid OOM#813Atishyy27 wants to merge 1 commit into
Atishyy27 wants to merge 1 commit into
Conversation
executeHook captured a hook's stdout and stderr into plain bytes.Buffer values, which grow without limit. A hook that streams a large amount of data (accidentally or maliciously) could therefore exhaust urunc's memory, since the captured output is held entirely in RAM only to build an error message. Replace the buffers with limitedBuffer, an io.Writer that retains at most maxHookOutput (1 MiB) per stream and discards the rest, marking the output as truncated. It always reports the full slice as written so the hook process is never blocked or killed with a short-write/EPIPE error. Add unit tests for the under-limit, exact-limit, single-oversized and repeated-write cases. Fixes urunc-dev#797
✅ Deploy Preview for urunc canceled.
|
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.
Problem
Closes #797.
executeHookcaptures a hook's stdout and stderr into plainbytes.Buffervalues:bytes.Buffergrows to hold everything written to it. A prestart/poststart/etc. hook that emits a large amount of data — by accident or malice — is buffered entirely in memory even though the output is only ever used to build an error string. That is an unbounded allocation and a potential OOM.Fix
Introduce
limitedBuffer, anio.Writerthat keeps at mostmaxHookOutput(1 MiB) per stream and discards the rest, flagging the result as truncated:maxHookOutputbytes, appends... [output truncated]when it drops data.len(p), nilfromWrite, soexec's internalio.Copynever fails withErrShortWriteand the hook process is neither blocked nor killed withEPIPE.Tests
TestLimitedBufferCapsOutputcovers: under-limit (kept verbatim), exact-limit (not marked truncated), a single oversized write (capped, full length still reported), and a flood of writes (stays bounded at the cap).Verification
go build,go vetand test compilation pass (cross-compiled tolinux/amd64); thelimitedBufferbehaviour was also run directly (oversized write → n reported = 10, retained = 4; 10 KB flood → retained = 5 with full length reported). CI runs the full suite on Linux.