feat(craft): sandbox outputs manifest by nmgarza5 · Pull Request #14325 · onyx-dot-app/onyx

nmgarza5 · 2026-08-27T02:00:59Z

Description

Why. The artifact index (PR1, #14319) is only useful if something can fill it, and the reconciler that will (PR4) needs a trustworthy answer to "what is in this session's outputs tree right now". Nothing provides that today: list_directory walks one level with no hashes, and the sandbox agent mutates the filesystem concurrently, so a naive pathname walk can be routed anywhere by a symlink swapped in mid-walk. This PR gives the daemon one call that describes the whole outputs tree, hardened against the agent it shares a filesystem with.

What.

sandbox_daemon/manifest.py: a bounded, fd-relative walk of sessions/{sid}/outputs. Every descent below the anchor is O_NOFOLLOW, file metadata comes from fstat on the very descriptor being hashed, so a swapped path can only fail an open, never route the walk or the hash outside outputs. Symlinks and special files are counted, never followed. Ceilings on entries, depth, per-directory children, per-file hash bytes, and a walk-wide hash budget; the response carries skip counters and a truncated flag so a partial listing is always distinguishable from a complete one.
Served two ways, both returning the same contract model: a signed sidecar HTTP route on kubernetes, and python -m sandbox_daemon.manifest over exec on docker. The docker exec runs the root-owned system interpreter (-E -s, with its own pinned pydantic) against a root-owned /opt copy of the daemon, because the sandbox user owns /workspace and could otherwise swap the code that describes its outputs.
Manager plumbing: SandboxManager.get_outputs_manifest on both backends and the test stub.
Pins the labs BuildKit syntax the Dockerfile's pre-existing COPY --exclude requires, so local builds work without flag overrides.

The manifest, concretely — every regular file and directory under outputs/, recursively, paths outputs-relative and sorted. Files carry size, mtime (fstat on the hashed descriptor), and sha256, null past the hash ceilings; directories carry mtime only. The four trailers are the honesty signals, and the reconciler refuses to act unless truncated is false and skipped_unreadable is zero:

{
  "entries": [
    {"path": "deck.pptx", "is_directory": false, "size": 48211,
     "mtime_ns": 1787795635366373007, "sha256": "5891b5b5..."},
    {"path": "web", "is_directory": true, "size": null,
     "mtime_ns": 1787795635366373007, "sha256": null},
    {"path": "web/package.json", "is_directory": false, "size": 412,
     "mtime_ns": 1787795635366373007, "sha256": "b53a5538..."}
  ],
  "skipped_symlinks": 1,
  "skipped_special": 0,
  "skipped_unreadable": 0,
  "truncated": false
}

Nothing calls get_outputs_manifest yet; the consumer is the reconciler in #14327.

How Has This Been Tested?

Unit tests (test_outputs_manifest.py): files/directories described with sizes and hashes, hash tracks content, symlinks skipped and never followed (including a session directory swapped for a symlink yielding an empty manifest), special files skipped, entry-cap truncation flagged, oversize files listed without a hash, an unreadable directory counted rather than silently omitted, and the full sessions/{sid}/outputs descent.
Live verification in the real image: built the sandbox image from this branch and ran the exact manager exec (docker exec -u 1000:1000 -w /opt ... /usr/local/bin/python3 -E -s -m sandbox_daemon.manifest <sid>) — correct manifest JSON, planted symlink skipped, exit 0. This caught and fixed a real bug: /usr/bin/python3 does not exist in the image and the system interpreter had no pydantic, so the exec path now installs a root-owned pydantic and uses the real interpreter path.
Full CI matrix green (160 checks).

Additional Options

[Optional] Please cherry-pick this PR to the latest release version.
[Optional] Override Linear Check

greptile-apps · 2026-08-27T02:03:56Z

Greptile Summary

The PR adds a bounded, descriptor-relative manifest of sandbox output files and exposes it through Docker exec and the signed Kubernetes sidecar.

Adds recursive output metadata and hashes with truncation and skip counters.
Adds matching Docker and Kubernetes manager APIs.
Adds the root-owned daemon copy and pinned runtime dependency used by Docker exec.
Adds coverage for missing trees, symlinks, special files, limits, hashes, and unreadable directories.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
backend/onyx/server/features/build/sandbox/image/sandbox_daemon/manifest.py	Implements the bounded fd-relative manifest walk and now records directory-scan and child-stat omissions in the unreadable counter.
backend/onyx/server/features/build/sandbox/image/sandbox_daemon/server.py	Adds a signed sidecar endpoint that validates the request and executes the manifest walk off the event loop.
backend/onyx/server/features/build/sandbox/docker/docker_sandbox_manager.py	Adds the Docker exec transport using the root-owned interpreter and daemon module.
backend/onyx/server/features/build/sandbox/kubernetes/sidecar_client.py	Adds the signed HTTP client call and validates the shared manifest response model.
backend/onyx/server/features/build/sandbox/image/Dockerfile	Pins the labs Dockerfile syntax and installs a root-owned daemon copy with its Pydantic dependency.
backend/tests/unit/onyx/server/features/craft/sandbox/test_outputs_manifest.py	Covers manifest contents, hashing, traversal safety, special files, limits, unreadable directories, and session-root descent.

Sequence Diagram

sequenceDiagram
    participant C as Reconciler
    participant M as SandboxManager
    participant D as Docker exec
    participant S as Kubernetes sidecar
    participant W as Manifest walker
    participant F as Outputs filesystem

    C->>M: get_outputs_manifest(sandbox_id, session_id)
    alt Docker backend
        M->>D: python -E -s -m sandbox_daemon.manifest
        D->>W: build_outputs_manifest(session_id)
    else Kubernetes backend
        M->>S: Signed POST /filesystem/outputs-manifest
        S->>W: build_outputs_manifest(session_id)
    end
    W->>F: fd-relative walk with O_NOFOLLOW
    F-->>W: file descriptors and metadata
    W-->>M: entries, skip counters, truncated flag
    M-->>C: OutputsManifestResponse

_{Reviews (3): Last reviewed commit: "refactor(craft): name the hashed-file re..." | Re-trigger Greptile}

github-actions · 2026-08-27T02:04:12Z

Full-stack Preview (frontend + backend)

Status	Preview	Commit	Updated
✅	https://49af94d-onyx.preview.onyxcorp.dev/	`49af94d`	2026-08-27 20:06:07 UTC

nmgarza5 · 2026-08-27T02:07:19Z

@greptile

github-actions · 2026-08-27T02:23:08Z

🖼️ Visual Regression Report

Project	Changed	Added	Removed	Unchanged	Report
`admin`	8	0	0	179	View Report
`exclusive`	0	0	0	10	✅ No changes

One daemon call describes a session's outputs tree: regular files with size, mtime, and sha256, directories, nothing else. Symlinks and special files are counted and never followed. Every descent below the anchor is fd-relative with O_NOFOLLOW. Served over the signed sidecar route on kubernetes and via exec python -m on docker. The docker exec runs the root-owned system interpreter (with its own pydantic) against a root-owned /opt copy of the daemon, so the sandbox user cannot swap the code that describes its outputs. Also pins the labs BuildKit syntax the Dockerfile's COPY --exclude requires, so local builds work without flag overrides.

A directory listing that fails after its open, or a child that vanishes between scandir and stat, previously dropped content silently. Both now increment skipped_unreadable so a partial manifest is distinguishable from a complete one.

Danelegend · 2026-08-27T19:10:30Z

+_MISSING_ERRNOS = frozenset({errno.ENOENT, errno.ENOTDIR, errno.ELOOP})
+
+
+@dataclass


the wire model is the OutputsManifestResponse inside it, the wrapper only exists to carry the hash budget countdown next to it while the recursion runs. dataclass felt like the right signal that it's walk-internal state, not a contract

Danelegend · 2026-08-27T19:21:08Z

+
+
+if __name__ == "__main__":
+    print(build_outputs_manifest(UUID(sys.argv[1])).model_dump_json())


yep, that's the docker transport. docker_sandbox_manager.get_outputs_manifest execs python -m sandbox_daemon.manifest in the container and parses this stdout (k8s uses the sidecar route instead)

Danelegend · 2026-08-27T19:25:00Z

+
+def _hash_regular_file(
+    dir_fd: int, name: str, allowed_bytes: int
+) -> tuple[os.stat_result, str | None, int] | None:


Worth making this a named type?

yeah fair, took it in 49af94d as a _HashedFile NamedTuple, docstring got shorter too

Review asked twice for a named type over the anonymous three-tuple. The NamedTuple carries the meaning the docstring used to spell out.

nmgarza5 · 2026-08-27T20:03:56Z

@greptile

nmgarza5 requested a review from a team as a code owner August 27, 2026 02:01

nmgarza5 added the skip-cubic label Aug 27, 2026

nmgarza5 temporarily deployed to ci-protected August 27, 2026 02:01 — with GitHub Actions Inactive

greptile-apps Bot reviewed Aug 27, 2026

View reviewed changes

Comment thread backend/onyx/server/features/build/sandbox/image/sandbox_daemon/manifest.py

Comment thread backend/onyx/server/features/build/sandbox/image/sandbox_daemon/server.py

nmgarza5 had a problem deploying to ci-protected August 27, 2026 02:04 — with GitHub Actions Error

nmgarza5 temporarily deployed to ci-protected August 27, 2026 02:07 — with GitHub Actions Inactive

nmgarza5 temporarily deployed to ci-protected August 27, 2026 02:08 — with GitHub Actions Inactive

This was referenced Aug 27, 2026

feat(craft): outputs reconciler wired into every terminal branch #14327

Open

feat(craft): outputs artifact classifier #14326

Open

nmgarza5 added 2 commits August 27, 2026 11:45

nmgarza5 force-pushed the nikg/craft-sandbox-manifest branch from 8eb7827 to 3003e2a Compare August 27, 2026 18:56

nmgarza5 temporarily deployed to ci-protected August 27, 2026 18:56 — with GitHub Actions Inactive

nmgarza5 temporarily deployed to ci-protected August 27, 2026 18:58 — with GitHub Actions Inactive

nmgarza5 changed the title ~~feat(craft): sandbox outputs manifest with lstat discipline~~ feat(craft): sandbox outputs manifest Aug 27, 2026

Danelegend approved these changes Aug 27, 2026

View reviewed changes

refactor(craft): name the hashed-file result

49af94d

Review asked twice for a named type over the anonymous three-tuple. The NamedTuple carries the meaning the docstring used to spell out.

nmgarza5 temporarily deployed to ci-protected August 27, 2026 20:03 — with GitHub Actions Inactive

nmgarza5 had a problem deploying to ci-protected August 27, 2026 20:06 — with GitHub Actions Failure

nmgarza5 added this pull request to the merge queue Aug 27, 2026

Merged via the queue into main with commit fc23baf Aug 27, 2026
221 of 224 checks passed

nmgarza5 deleted the nikg/craft-sandbox-manifest branch August 27, 2026 21:18

Navigation Menu

Uh oh!

feat(craft): sandbox outputs manifest - #14325

feat(craft): sandbox outputs manifest#14325
nmgarza5 merged 3 commits into
mainfrom
nikg/craft-sandbox-manifest

nmgarza5 commented Aug 27, 2026 •

edited

Loading

Uh oh!

greptile-apps Bot commented Aug 27, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 27, 2026 •

edited

Loading

Uh oh!

nmgarza5 commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026 •

edited

Loading

Uh oh!

Danelegend Aug 27, 2026

Uh oh!

nmgarza5 Aug 27, 2026

Uh oh!

Danelegend Aug 27, 2026

Uh oh!

nmgarza5 Aug 27, 2026

Uh oh!

Danelegend Aug 27, 2026

Uh oh!

nmgarza5 Aug 27, 2026

Uh oh!

nmgarza5 commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		_MISSING_ERRNOS = frozenset({errno.ENOENT, errno.ENOTDIR, errno.ELOOP})


		@dataclass



		if __name__ == "__main__":
		print(build_outputs_manifest(UUID(sys.argv[1])).model_dump_json())

Uh oh!

Conversation

nmgarza5 commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How Has This Been Tested?

Additional Options

Uh oh!

greptile-apps Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

Uh oh!

github-actions Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nmgarza5 commented Aug 27, 2026

Uh oh!

github-actions Bot commented Aug 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🖼️ Visual Regression Report

Uh oh!

Danelegend Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

nmgarza5 Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Danelegend Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

nmgarza5 Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

Danelegend Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

nmgarza5 Aug 27, 2026

Choose a reason for hiding this comment

Uh oh!

nmgarza5 commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nmgarza5 commented Aug 27, 2026 •

edited

Loading

greptile-apps Bot commented Aug 27, 2026 •

edited

Loading

github-actions Bot commented Aug 27, 2026 •

edited

Loading

github-actions Bot commented Aug 27, 2026 •

edited

Loading