ateom-microvm: replace shell commands with Go by destijl · Pull Request #2147 · agent-substrate/substrate

Greg Castle (destijl) · 2026-10-02T22:29:18Z

cmd/ateom-microvm previously shelled out to mount, umount, and cp via exec.Command. This required bundling a full host shell and userspace distribution (such as debian:13-slim) with coreutils and util-linux.

Replace the exec calls with native Linux kernel system calls using golang.org/x/sys/unix:

Replace overlay, bind, and remount exec commands in overlay_linux.go and systeminfo.go with unix.Mount and unix.Unmount.
Replace cp --sparse=always in ch/merge.go with copySparseFile, which uses existing SEEK_DATA/SEEK_HOLE logic to perform sparse copying in pure Go.

Fixes #2146

Tests pass

…lementations cmd/ateom-microvm previously shelled out to mount, umount, and cp via exec.Command. This required bundling a full host shell and userspace distribution (such as debian:13-slim) with coreutils and util-linux. Replace the exec calls with native Linux kernel system calls using golang.org/x/sys/unix: - Replace overlay, bind, and remount exec commands in overlay_linux.go and systeminfo.go with unix.Mount and unix.Unmount. - Replace cp --sparse=always in ch/merge.go with copySparseFile, which uses existing SEEK_DATA/SEEK_HOLE logic to perform sparse copying in pure Go.

Benjamin Elder (BenTheElder)

I think we shelled out to mount/unmount because kubelet does, but I don't think we need to.

For cp ... my agent found a regression in this change that isn't great.

Also, this doesn't quite fix the issue even though the PR body says "fixes ...". .ko.yaml needs to be updated to drop the base image override.

Benjamin Elder (BenTheElder) · 2026-10-03T00:18:48Z

+func copySparseFile(srcPath, dstPath string) error {
+	s, err := os.Open(srcPath)
+	if err != nil {
+		return err
+	}
+	defer s.Close()
+	si, err := s.Stat()
+	if err != nil {
+		return err
+	}
+	d, err := os.OpenFile(dstPath, os.O_CREATE|os.O_TRUNC|os.O_RDWR, 0o600)
+	if err != nil {
+		return err
+	}
+	defer d.Close()
+	if err := d.Truncate(si.Size()); err != nil {
+		return err
+	}
+	if _, err := copySparseRegions(s, d); err != nil {


🤖 should-fix 🟡 – This isn't quite cp --sparse=always. cp also turns runs of zeros inside data regions into holes. copySparseRegions copies data regions as-is, so any zero-filled extent in the base comes out allocated.

That matters because this copy is the normal path for a locally staged restore (the Nlink > 1 case in MergeDeltaIntoBase, per the comment there), i.e. every pause/resume. Without the hole-punching, a local snapshot can grow denser each cycle. Skipping all-zero blocks in the write loop, leaving them as holes, would keep the old behavior. Measuring the base's allocated size across a few cycles before and after would also settle whether it matters in practice.

Minor, same spot: cp ran under exec.CommandContext, so a multi-GiB copy could be cancelled. This one ignores ctx now.

Suggested fix, as a commit on top of this PR you can cherry-pick: BenTheElder@b0ba826 (branch ateom-microvm-sparse-copy-zeros).

For fresh copies we add a writer that skips all-zero blocks and honors ctx again.
The PR as-is has 1.4-2.8x more allocations than that branch and the branch is 1.4-2.1x faster, plus this makes the snapshot smaller for upload.

I think it's still not exactly drop-in equivilant to all the cp --sparse=always optimizations but it should be at least much closer.

Benjamin Elder (BenTheElder) · 2026-10-03T00:18:48Z

-	ro.Stderr = &roErr
-	if err := reaper.Run(ro); err != nil {
-		return fmt.Errorf("remounting image volume %q read-only: %w (%s)", dst, err, strings.TrimSpace(roErr.String()))
+	if err := unix.Mount("", dst, "", unix.MS_BIND|unix.MS_REMOUNT|unix.MS_RDONLY, ""); err != nil {


🤖 nit 🟢 – mount -o remount,bind,ro (libmount) merges the mount's existing per-mount flags into the remount. A raw MS_BIND|MS_REMOUNT|MS_RDONLY replaces them, so any nosuid/nodev/noexec the bind picked up from its source is silently cleared. The sources here probably carry none, but OR-ing in the current flags from unix.Statfs (ST_NOSUID → MS_NOSUID, and so on) would make this a faithful replacement. Same at line 362 and in systeminfo.go.

I'm too tired t othink about if this is a real problem. Will revisit monday.

Benjamin Elder (BenTheElder) · 2026-10-03T00:18:48Z

 	// Drop any stale mount first (lazy if busy), then ensure clean mountpoint.
-	if err := reaper.Run(exec.Command("umount", dst)); err != nil {
-		_ = reaper.Run(exec.Command("umount", "-l", dst))
+	if err := unix.Unmount(dst, 0); err != nil {


🤖 nit 🟢 – This repeats the unmount helper this PR adds to kata/overlay_linux.go. Exporting that one, e.g. as kata.Unmount, would keep the "lazy if busy" rule in one place.

cp --sparse=always turned runs of zeros inside data regions into holes as well as preserving existing holes. The Go replacement only preserved holes, so a locally staged restore, which always takes the copying merge, could densify the memory image a little more on every pause. Write the fresh copy block by block and skip all-zero blocks; the destination is truncated to full size first, so they read back as zeros. The delta overlay keeps writing every block, since a zero there must overwrite the base. Also honor ctx again, as cp's CommandContext did.

…flags

Greg Castle (destijl) · 2026-10-05T19:39:47Z

Thanks Benjamin Elder (@BenTheElder)! Updated:

Cherry-picked your commit to keep zero-runs sparse (copyNonZeroRegions) and re-enable ctx cancellation.
Dropped the baseImageOverrides for ateom-microvm in .ko.yaml so it builds on distroless:static. I was planning to do this in a follow up PR, but might as well do it here.
Added kata.RemountReadOnly using unix.Statfs to preserve existing mount flags (nosuid, nodev, noexec, etc.) on remount.
Exported kata.Unmount and deduplicated the lazy-unmount logic in systeminfo.go.

Benjamin Elder (BenTheElder)

I think this is looking pretty good, my agent found one new bug.

Benjamin Elder (BenTheElder) · 2026-10-05T20:31:33Z

+	if err := unix.Statfs(dst, &st); err == nil {
+		flags |= uintptr(st.Flags & (unix.ST_NOSUID | unix.ST_NODEV | unix.ST_NOEXEC |
+			unix.ST_NOATIME | unix.ST_NODIRATIME | unix.ST_RELATIME | unix.ST_SYNCHRONOUS | unix.ST_MANDLOCK))


🤖 nit 🟢 – Thanks for adding this. Two small things about the mask:

ST_* and MS_* share bit values for most flags, but not this one: ST_RELATIME is 0x1000, which is MS_BIND, not MS_RELATIME (0x200000). So it re-sets a flag that's already set and never preserves relatime. That's harmless, because a remount with no atime flag keeps the existing atime mode, but it reads like it does something it doesn't.

ST_SYNCHRONOUS and ST_MANDLOCK are superblock flags, which a bind remount ignores.

Masking just ST_NOSUID | ST_NODEV | ST_NOEXEC | ST_NOATIME | ST_NODIRATIME, the per-mount flags libmount carries over, would say exactly what this does. I'd also return the Statfs error rather than silently remount without the flags. If dst can't be statted, the remount is unlikely to succeed anyway.

…mount

wiz-alphabet · 2026-10-05T22:04:54Z

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	-
Sensitive Data	-
Secrets	-
IaC Misconfigurations	-
SAST Findings	-
Software Management Findings	-

Total	-

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio.

Greg Castle (destijl) · 2026-10-05T22:05:37Z

OK updated the mask to strictly ST_NOSUID | ST_NODEV | ST_NOEXEC | ST_NOATIME | ST_NODIRATIME and propagated the unix.Statfs error directly rather than silently falling back. Added tests

… false positive

Benjamin Elder (BenTheElder) · 2026-10-06T01:31:57Z

+		return fmt.Errorf("statfs %q: %w", dst, err)
+	}
+	flags := unix.MS_BIND | unix.MS_REMOUNT | unix.MS_RDONLY |
+		int(st.Flags&(unix.ST_NOSUID|unix.ST_NODEV|unix.ST_NOEXEC|unix.ST_NOATIME|unix.ST_NODIRATIME))


I think we might need to do some explicit mapping between stat and mount flags in the future but it can be a follow-up

Greg Castle (destijl) requested a review from Benjamin Elder (BenTheElder) October 2, 2026 23:10

Benjamin Elder (BenTheElder) reviewed Oct 3, 2026

View reviewed changes

Benjamin Elder (BenTheElder) and others added 2 commits October 5, 2026 19:30

ateom-microvm: drop debian override in .ko.yaml and preserve remount …

abb323d

…flags

Benjamin Elder (BenTheElder) reviewed Oct 5, 2026

View reviewed changes

Greg Castle (destijl) added 2 commits October 5, 2026 21:37

ateom-microvm: check statfs error and mask only per-mount flags on re…

4ef6c24

…mount

ateom-microvm: add unit and root tests for RemountReadOnly and Unmount

75a7f5f

ateom-microvm: inline uintptr conversion to avoid SAST unsafe pointer…

d432bc2

… false positive

Benjamin Elder (BenTheElder) reviewed Oct 6, 2026

View reviewed changes

Benjamin Elder (BenTheElder) approved these changes Oct 6, 2026

View reviewed changes

Benjamin Elder (BenTheElder) added this pull request to the merge queue Oct 6, 2026

Benjamin Elder (BenTheElder) added kind/cleanup Small fixes that are not bugs, for example a typo in a code comment area/security Security related issue/pr area/microVM labels Oct 6, 2026

Merged via the queue into agent-substrate:main with commit e65c5d1 Oct 6, 2026
15 checks passed

Navigation Menu

Repository navigation

ateom-microvm: replace shell commands with Go - #2147

ateom-microvm: replace shell commands with Go#2147
Benjamin Elder (BenTheElder) merged 6 commits into
agent-substrate:mainfrom
destijl:remove-microvm-shell-exec

Greg Castle (destijl) commented Oct 2, 2026

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Uh oh!

Greg Castle (destijl) commented Oct 5, 2026

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 5, 2026

Uh oh!

wiz-alphabet Bot commented Oct 5, 2026 •

edited

Loading

Uh oh!

Greg Castle (destijl) commented Oct 5, 2026

Uh oh!

Benjamin Elder (BenTheElder) Oct 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Greg Castle (destijl) commented Oct 2, 2026

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 3, 2026

Choose a reason for hiding this comment

Uh oh!

Greg Castle (destijl) commented Oct 5, 2026

Uh oh!

Benjamin Elder (BenTheElder) left a comment

Choose a reason for hiding this comment

Uh oh!

Benjamin Elder (BenTheElder) Oct 5, 2026

Choose a reason for hiding this comment

Uh oh!

wiz-alphabet Bot commented Oct 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Wiz Scan Summary

Uh oh!

Greg Castle (destijl) commented Oct 5, 2026

Uh oh!

Benjamin Elder (BenTheElder) Oct 6, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

wiz-alphabet Bot commented Oct 5, 2026 •

edited

Loading