Skip to content

test(runtime): cover MicroSandbox mount parity - #1453

Merged
skevetter merged 5 commits into
mainfrom
codex/microsandbox-mount-parity
Oct 10, 2026
Merged

skevetter merged 5 commits into
mainfrom
codex/microsandbox-mount-parity

Conversation

@skevetter

@skevetter skevetter commented Oct 10, 2026 •

Copy link
Copy Markdown
Contributor

MicroSandbox's parity gate lacked real VM coverage for extra mounts and alternate workspace permission policies. Add ten shared scenarios across the built-in driver and external provider v0.1.6: writable/read-only binds, named-volume and tmpfs lifetime through stop/start and recreation, and strict/relaxed/off stat virtualization with private host permissions.

The tests verify guest and host contents and metadata, including read-only write rejection with a subsequent successful SSH read. Host-created permission fixtures appear after setup so recursive chown cannot mask fallback ownership. Their host owner must differ from the guest fallback, including when CI runs as root. Named volumes use unique test-owned names and are removed after workspace cleanup. A dedicated KVM CI matrix installs checksum-pinned MicroSandbox v0.7.7 with bounded suite/job deadlines; documentation records the new coverage and remaining parity work.

Validation:

  • Uncached race tests and vet for e2e/tests/up and e2e/framework; Ginkgo dry run selects all ten new specs.
  • Module verification and strict CLI lint (zero issues). All 13 repository hooks passed; applicable Go hooks were rechecked after the ownership and cleanup corrections.
  • Full fresh committed local CodeRabbit reviewed all six changed files with zero findings.
  • Current-head CI passed all 74 implementation jobs, including actual VM lifecycle (2/2), images (6/6), and mounts (10/10) against built-in and external v0.1.6. Fresh Greptile returned 5/5 with no issues, and full CodeRabbit reviewed all six files without actionable findings. Its docstring warning was independently dispositioned for private test helpers/test entry points. All five commit signatures are valid; no review threads remain. Local MicroSandbox v0.7.2 is below the supported parity minimum.

The actual VM run exposed an unquoted YAML off enum in both manifests. The built-in manifest now quotes the policy, with an actual-parser regression that failed before the fix. External provider PR #16 published the same correction in v0.1.6, whose native release digests, manifest checksums, downloaded host bytes, and real host parser/download/version smoke were verified. All lifecycle, image, and mount suites now pin that actual release. This change keeps the built-in provider and preserves resource/networking policies. Remaining parity coverage must pass before provider cutover.

The next VM run passed lifecycle (2/2), images (6/6), and eight mount cases. Both off cases exposed a test assumption: MicroSandbox v0.7.7 caches attributes for five seconds, and bind mounts retain that default. The test now polls the same host owner and mode 600 for at most 15 seconds after host chmod, with a cancellable context and immediate failure on SSH errors. Strict/relaxed checks remain immediate; spec, suite, and job deadlines are unchanged. The final-head VM run passed both off cases and all ten mount specs.

@netlify

netlify Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for devsydev ready!

Name Link
🔨 Latest commit 6961476
🔍 Latest deploy log https://app.netlify.com/projects/devsydev/deploys/6ac9c395f2df7500084731fa
😎 Deploy Preview https://deploy-preview-1453--devsydev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review in Change Stack →Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: adf15e35-83e3-4bf6-8ba0-878c2b4abce8



📥 Commits

Reviewing files that changed from the base of the PR and between d68e5d3 and 6961476.




📒 Files selected for processing (6)
  • .github/workflows/pr-ci.yml
  • e2e/tests/up/provider_microsandbox.go
  • e2e/tests/up/provider_microsandbox_mounts.go
  • providers/microsandbox/provider.yaml
  • providers/providers_test.go
  • sites/docs-devsy-sh/content/docs/developing-providers/runtime-protocol.mdx



Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.





📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The change adds MicroSandbox mount-parity tests for built-in and external providers, updates the stat-virtualization option declaration, and adds the tests to CI. The documentation describes the scenarios and updates the stated external provider release.

Changes

MicroSandbox mount parity

Layer / File(s) Summary
Stat virtualization policy contract
providers/microsandbox/provider.yaml, providers/providers_test.go
The off enum value is quoted as a string. A test checks that the parsed option has the values strict, relaxed, and off in order.
Mount parity scenarios
e2e/tests/up/provider_microsandbox.go, e2e/tests/up/provider_microsandbox_mounts.go
The external provider reference changes to v0.1.6. Tests for both providers check writable and read-only bind mounts, named-volume persistence, tmpfs reset, and strict, relaxed, and off stat-virtualization policies.
CI and parity documentation
.github/workflows/pr-ci.yml, sites/docs-devsy-sh/content/docs/developing-providers/runtime-protocol.mdx
The CI matrix runs the mount suite with MicroSandbox enabled. The documentation describes the suite, updates the external provider release, and revises the parity-gate coverage text.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Other





Merge Risk: ⚪ Minimal · up to 69614

No actionable issue remains from this review; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 69614

The changes primarily expand verification of existing mount behavior and preserve the strict default. No introduced security flaw was established. Remaining uncertainty concerns the external provider release and interrupted resource cleanup, with exposure bounded to the CI execution environment and its credentials.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • observed — The mount job inherits the existing Linux root test invocation and GitHub token forwarding. Its requires-secret=false setting skips GitHub App token generation but does not make execution credential-free. This root-and-token pattern predates the new matrix entry.

Trust Boundaries and Controls

  • observed — The read-only bind scenario checks both write rejection and a subsequent successful SSH read, then verifies unchanged host contents. This distinguishes mount enforcement from an unrelated connectivity failure.
  • inferred — The new scenarios exercise an existing guest-to-host boundary using test-owned resources rather than adding production reachability. The principal unresolved trust question is the behavior of the newly selected external provider inside the already privileged CI environment.



Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (3 skipped: 3 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely identifies the main change: adding runtime tests for MicroSandbox mount parity.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 3 files. (3 skipped: 3 unsupported.)


  • Fix all pre-merge checks with AI
✨ Finishing Touches
✨ Simplify code
  • Commit to this branch
  • Create a new PR





  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@netlify

netlify Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for images-devsy-sh canceled.

Name Link
🔨 Latest commit 6961476
🔍 Latest deploy log https://app.netlify.com/projects/images-devsy-sh/deploys/6ac9c3955490440008c4282d

@skevetter

Copy link
Copy Markdown
Contributor Author

@greptileai review

@greptile-apps

greptile-apps Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Adds ten shared MicroSandbox mount checks across the built-in provider and external v0.1.6.

  • MicroSandbox mount checks run against both providers.
  • The built-in provider keeps off as a policy choice.

No actionable issues found.

Reviews (1) · Last reviewed commit: "test(runtime): respect guest attribute c..." · Reviewed by Greptile

@skevetter

Copy link
Copy Markdown
Contributor Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Oct 10, 2026 •

Copy link
Copy Markdown
✅ Action performed

Full review finished.

@skevetter

Copy link
Copy Markdown
Contributor Author

Final-head review disposition for 6961476cdb26f13917992f244539b8255f3dc4a3:

Full CodeRabbit run adf15e35-83e3-4bf6-8ba0-878c2b4abce8 selected all six PR files and completed without actionable or inline findings. I inspected the entire security architecture and merge-risk sections: no retained architecture concerns. The external v0.1.6 release and all 18 actual VM scenarios have been verified; the existing privileged CI/token pattern is unchanged.

The docstring coverage warning does not justify a source change: the newly introduced functions are private, descriptively named E2E fixture/assertion/command helpers and a Go test entry point, rather than exported production API. Three functions are also marked unsupported by the analyzer. Existing comments retain the non-obvious cache, ownership, connectivity, and cleanup invariants. Strict Go lint and all repository pre-commit hooks passed. Adding comments that repeat these function names would not improve the code.

All 74 implementation jobs passed, including lifecycle 2/2, image 6/6, and mount 10/10 against built-in/external v0.1.6. Fresh Greptile is 5/5 with no findings; full local review covers all six files; all five commits have valid GitHub signatures. No review threads remain.

@skevetter
skevetter marked this pull request as ready for review October 10, 2026 05:54
@skevetter
skevetter merged commit 74810dd into main Oct 10, 2026
96 checks passed
@skevetter
skevetter deleted the codex/microsandbox-mount-parity branch October 10, 2026 05:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant