fix(tools-image): guarantee localhost resolution for images with empty /etc/hosts

[JoyboyBrian]

Summary

Fix guest localhost resolution when an OCI image ships /etc/hosts as an empty placeholder or without canonical loopback mappings.

This PR:

• replaces the existence-only /etc/hosts fallback in pivot-init with a small content-based check;
• preserves existing image-provided entries and appends only missing 127.0.0.1 localhost or ::1 localhost mappings;
• adds an E2B compatibility assertion covering the resulting guest state.

Problem

pivot-init currently initializes /etc/hosts only when the file does not exist:

if [ ! -e /etc/hosts ]; then
    ...
fi

Many OCI images—including ubuntu:22.04 and ubuntu:24.04—ship a zero-byte /etc/hosts because container runtimes normally replace it at startup. AgentENV boots the image as a VM and does not perform that container-runtime mount, so these guests can start without a usable localhost mapping.

For example:

getent -s files hosts localhost

exits with status 2 in an affected guest. A plain getent hosts localhost is not a reliable probe because an upstream DNS resolver may answer for localhost.

Change

At boot, pivot-init now:

1. creates /etc if necessary;
2. repairs a missing trailing newline before appending;
3. checks independently for the canonical IPv4 and IPv6 mappings;
4. appends only the mappings that are missing.

The check is idempotent across repeated boots and pause/resume cycles. Existing /etc/hosts content is retained.

The implementation intentionally remains a small inline bootstrap block. It does not introduce a separate helper, locking, or special symlink replacement policy; pivot-init has a single caller before user init.

Validation

• Added an assertion to scripts/tests/e2e/e2b_python_sdk_compat.py that verifies the sandbox created from the built template contains a loopback-to-localhost mapping.
• Manually verified on an x86_64 Firecracker host with ubuntu:24.04:
  • before: getent -s files hosts localhost exits 2;
  • after: files-source localhost resolution succeeds;
  • repeated boot and pause/resume do not add duplicate entries.

Tools-drive rollout

This PR changes tools-image source only. It does not publish a tools drive or bump [tools].version.

The published AgentENV v0.1.1 release still pins and bundles tools drive 0.1.0, so it does not contain this fix. Rolling the change out requires publishing a new immutable tools-drive version and updating config/deps_manifest.toml.

Existing snapshots remain pinned to the tools_drive_version recorded when they were created and are not modified or invalidated by that future manifest bump. Nodes that may restore historical snapshots must retain the corresponding historical tools-drive versions.

Changing snapshot-based template rebuilds to cold-boot with a newer tools drive, or automatically provisioning missing historical versions, is outside the scope of this focused fix.

[github-actions]

🔍 OpenCodeReview found 1 issue(s) in this PR.

• ✅ Successfully posted inline: 1 comment(s)

[yingdi-shan]

Thanks for your contribution! However, I think a 400+ line addition is overkill for resolving this issue. A few line changes should be enough to handle the empty /etc/hosts case.

[JoyboyBrian]

Hi @yingdi-shan, thanks for the feedback! Agreed, the hardening had grown well beyond the actual bug, so I've slimmed it down to a ~15-line inline block in pivot-init plus a small e2e assertion, and dropped the separate helper script and its test harness.

One note on why it's slightly more than the previous [ ! -e /etc/hosts ] fallback: Docker/K8s manage /etc/hosts at container start, so OCI images routinely ship it present but empty, or without loopback entries. The check therefore has to be content-based rather than existence-based. The remaining lines just append the missing 127.0.0.1/::1 entries and are idempotent across reboots and pause/resume.

[github-actions]

[bug · medium]
These redirections follow symlinks. An image can legitimately ship /etc/hosts as a symlink to a runtime-generated path (which may now be dangling because /run was remounted), and an untrusted image could point it at an unrelated file such as /etc/passwd. In the former case localhost is still not installed; in the latter this root bootstrap corrupts the symlink target. Handle -L /etc/hosts explicitly and replace it with a concrete hosts file (preserving readable existing content where appropriate) before appending, similar to the symlink handling used for resolv.conf above.

[JoyboyBrian]

I believe this can be accepted as is.

On the security half: there is no privilege boundary here. pivot-init runs as PID 1 on the image's own rootfs, and the image author already controls every byte of that filesystem, including /etc/passwd. Redirecting this append through a symlink grants them nothing they did not already have. The resolv.conf handling above replaces its symlink for a functional reason, not a security one, so it is not a precedent that applies here.

On the dangling-symlink half: container images do not ship /etc/hosts as a symlink into /run in practice, since runtimes bind-mount /etc/hosts over whatever the image provides. Even if one did, the shell append follows and creates the link target, so localhost still resolves. The only remaining corner (a link into a directory that no longer exists) fails open with a warning on stderr, matching the pre-existing fallback this PR replaces, so it is not a regression introduced here.

Happy to add a small dangling-symlink guard if the reviewers consider it necessary.

[yingdi-shan]

Thanks! One last question: how should we approach upgrading the tool-images?

Simply upgrading them could break the snapshot, since it may disrupt the mapping between page caching and disk. Until we have a safe way to upgrade tool-images, I think we need to be more cautious about how we update them.

A proper fix is likely complex and will take time, since we may need to support multiple versions of tool-images simultaneously. In the meantime, a simpler workaround would be to update the template build process so that a warm start is guaranteed to be correct.

[JoyboyBrian]

Thanks @yingdi-shan!

I re-checked the current implementation, and it turns out most of the multi-version mechanism is already in place on main:

• Committed snapshots record tools_drive_version in SnapshotRuntimeVersions (src/snapshot/types/version.rs#L11-L16). Fresh sandboxes initialize it from the configured tools version (src/orchestrator/service.rs#L2413-L2428), sandboxes restored from a snapshot inherit the recorded runtime versions (#L376-L381), and template builds probe the actual version from the sandbox before capture (src/snapshot/types/version.rs#L35-L58).
• Snapshot restore uses the version recorded in the snapshot rather than the node's current default (src/sandbox/firecracker/config.rs#L466-L472), and the same applies to paused sandboxes through their persisted config. It resolves that version under {deps_path}/tools/{version}/ and fails explicitly if it is unavailable (#L233-L266); records with no recorded version are refused rather than silently mapped to the current default.
• Published tools-drive versions are intended to be immutable: the local import path rejects conflicting content under an existing version (src/setup/deps.rs#L219-L232), and the publish target refuses to reuse a tag that the registry already resolves (tools-image/Makefile#L48-L66). Registry-side tag immutability remains an operational requirement, as the Makefile itself notes.

Therefore, I do not think this fix requires invalidating existing snapshots. This PR only changes the tools-image source plus an e2e assertion; normal startup still consumes the prebuilt image pinned in config/deps_manifest.toml (currently 0.1.0), so merging alone changes nothing at runtime. The safe rollout is to publish the modified image as a new version (for example 0.1.1), bump [tools].version so fresh sandboxes and template builds that cold-boot from an image pick it up, and keep 0.1.0 available and provisioned on every node that may resume snapshots recorded against it. 0.1.0 must never be overwritten in place.

That said, your caution is justified by two real gaps:

1. Restore does not fetch a missing historical tools version automatically. A node that never installed 0.1.0 cannot resume snapshots recorded against it (setup only provisions the manifest's current version), so multi-node deployments need either fetch-by-recorded-version or an explicit retention/provisioning policy.
2. Snapshot-based template rebuild resumes the base snapshot and therefore inherits its tools version (src/template/runner.rs#L168-L183). If a rebuild is meant to pick up a tools upgrade, one safe upgrade path would be to cold-boot from the committed rootfs with the current tools version and capture a fresh warm snapshot, which matches your idea of guaranteeing warm-start correctness through the template build process.

My recommendation is to keep this PR focused on the source fix, publish 0.1.1 as an immutable release, and update the manifest once that artifact is available. I can address the template cold-rebuild path in a follow-up PR; automatic provisioning of historical versions can remain a separate follow-up. If you would prefer either safeguard to be included in this PR, I am happy to adjust the scope before pushing anything.

[JoyboyBrian]

Hi @yingdi-shan, just following up. I’ve updated the PR description to match the simplified diff and clarify the rollout. AgentENV v0.1.1 still bundles tools drive v0.1.0, so this remains a source-only fix and does not affect existing snapshots.
Would you prefer to merge this and handle the tools-drive rollout separately, or include the cold-rebuild safeguard first? Happy to follow either direction.

[yingdi-shan]

Thanks! I think we can merge this PR once we've prepared for the image upgrade.