Skip to content

fix(orchestrator): raise envd error-body cap to 512 so mount-failure causes survive (rebase of #3481) - #3729

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/envd-error-body-cap
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/envd-error-body-cap

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Rebased follow-up to #3481 (closed inactive). The problem is still present on main; this is the current-structure fix plus the rationale for the value and for the approach chosen.

Necessity — the failure cause lives only in the truncated body

On a failed envd /init, the error returned to callers and the envd init request failed log both run the response body through envdErrorBody, capped at 100 runes. A volume-mount failure body looks like:

/init returned 400: failed to mount "vol-885b": failed to chroot into "/data1/orchestrator/volumes/team-f079…/vol-885b…": failed to bind mount "…": no such file or directory

That's ~300 chars, and the actual cause (no such file or directory, permission denied, …) is at the end. The 100-rune cut lands mid-path, so the operator sees a truncated path and nothing about why it failed.

Crucially, the cause is not available anywhere else:

  • X-Envd-Failed-Volume carries only the path, which VolumeMountError wraps as failed to mount volume at %q — no errno.
  • So the errno text exists only in the response body that gets truncated.

Capping at 100 therefore throws away the one piece of information needed to diagnose the failure.

Fix

Raise envdErrorBodyRunes from 100 to 512. One constant; the read bound (envdErrorBodyRunes*utf8.UTFMax+1, via io.LimitReader) scales with it, so the bounded-read protection is preserved — only the bound is larger (≈2 KiB, still nowhere near reading an unbounded body). envd error bodies are short server-generated diagnostics, so 512 carries no log-explosion risk, and ~300–450-char worst-case mount paths fit with margin.

Approach considered and rejected — head+tail (tail-preserving) truncation

Since the cause sits at the tail of a Go error chain, a head+tail elision (keep the start and the end, drop the middle) would preserve the cause at any length and would be the theoretically more robust fix. It is not viable here:

  • the body is a forward-only HTTP reader, so reading the tail means reading the whole body — which breaks the bounded-read guarantee TestEnvdCallsReadABoundedBody explicitly asserts (the client must not drain a 64 MiB body);
  • doing it "within a larger read window" would need a new truncation helper, a changed message format (middle …), and churn across the existing test expectations — for marginal gain over a 512-rune head cap that already covers the real worst case.

So the right trade-off is the larger head cap: it keeps the existing format and the bounded-read guarantee, and fixes the actual diagnostic loss with a one-constant change.

Tests

  • existing envd_error_body_test.go updated to reference envdErrorBodyRunes instead of hard-coding 100, so it tracks the cap;
  • added a case pinning that a ~300-char volume-mount-failure body now survives intact (the exact regression).

go build ./..., go vet, and the envd error-body suite pass.

Fixes #3480. Supersedes #3481.

An envd call failure embeds the response body in the returned error and the
init-failure log via envdErrorBody, capped at 100 runes. A volume-mount failure
body carries the full chroot/bind path plus the errno text (~300 chars), so the
100-rune cut falls mid-path and strips the actual cause (no such file or
directory, permission denied, ...), leaving an error that names a truncated
path and nothing else.

Raise envdErrorBodyRunes to 512 so the whole diagnostic survives. The body is
still bounded: envdErrorBody reads through io.LimitReader(envdErrorBodyRunes*
utf8.UTFMax+1) and Truncate still marks a longer body as cut, so the
read-a-bounded-body protection is preserved - only the bound is larger. envd
error bodies are short server-generated diagnostics, not arbitrary user data,
so 512 carries no log-explosion risk.

Rebased onto current main, which since the original attempt (e2b-dev#3481) refactored
the four scattered utils.Truncate(string(body), 100) call sites into the single
envdErrorBody helper; this changes only the cap constant. Existing tests updated
to reference envdErrorBodyRunes instead of hard-coding 100, plus a case pinning
that a ~300-char mount-failure body now survives intact.

Fixes e2b-dev#3480. Supersedes e2b-dev#3481.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(orchestrator): envd error responses truncated to 100 chars, hiding root cause in logs and error chain

2 participants