Skip to content

feat(components/execd): modify bash runtime by pty - #1

Closed
Pangjiping wants to merge 4 commits into
mainfrom
feat/run_in_session
Closed

feat(components/execd): modify bash runtime by pty#1
Pangjiping wants to merge 4 commits into
mainfrom
feat/run_in_session

Conversation

@Pangjiping

@Pangjiping Pangjiping commented Jan 17, 2026

Copy link
Copy Markdown
Owner

Summary

  • What is changing and why?

Testing

  • Not run (explain why)
  • Unit tests
  • Integration tests
  • e2e / manual verification

Breaking Changes

  • None
  • Yes (describe impact and migration path)

Checklist

  • Linked Issue or clearly described motivation
  • Added/updated docs (if needed)
  • Added/updated tests (if needed)
  • Security impact considered
  • Backward compatibility considered

@Pangjiping Pangjiping closed this Jan 18, 2026
Pangjiping pushed a commit that referenced this pull request Jun 8, 2026
@asiudgufgbukbsa @claude
fix(sdks): kill near-expiry sandboxes and auto-scale acquireMinRemain…
4e4f12c 
…ingTtl default

Addresses three bot reviews on PR opensandbox-group#986 head (commit 3f7dddf).

### #1 + #2 — Resource leak: skipped near-expiry sandboxes are now killed

Previously, when ``acquireMinRemainingTtl`` was positive, both
``tryTakeIdle`` and ``reapExpiredIdle`` silently dropped near-expiry idle
entries from store membership but never told the pool, so the live
sandboxes kept running on the server (consuming quota / cost) until their
own TTL elapsed — temporarily exceeding the intended pool size.

Redesigned the store API to surface those IDs to callers:

* New ``TakeIdleResult`` value type carrying ``sandbox_id`` plus a
  ``discarded_alive_sandbox_ids`` list. ``tryTakeIdle(name, minTtl)`` now
  returns it. Already-expired entries (server has reaped them) are
  intentionally excluded — a kill round-trip would be wasted.
* ``reapExpiredIdle(name, now, minTtl)`` now returns the alive evicted IDs
  (same exclusion rule).
* Both Lua scripts now return arrays so the discarded-alive list survives
  the round-trip; the empty-pool fast-path still returns ``nil`` so clients
  can distinguish "nothing to do" cleanly.

Wired into the pool:

* ``SandboxPool.acquire`` (Kotlin) and ``SandboxPoolSync/Async.acquire``
  (Python) call ``_kill_discarded_alive`` on the returned list. Failures
  are logged and swallowed — the primary acquire outcome must not depend
  on a janitor failure.
* ``PoolReconciler.reapExpiredIdle`` routes the returned list through the
  existing ``onDiscardSandbox`` callback, which already triggers a
  best-effort kill (same path used by ``shrinkExcessIdle``).

### #3 — Default no longer breaks existing users with short ``idleTimeout``

A 60s default would fail validation (``acquireMinRemainingTtl <
idleTimeout``) for any pool configured with ``idleTimeout <= 60s``,
silently breaking user code on upgrade. Builder field is now nullable:
``null`` resolves at ``build()`` time to ``min(60s, idleTimeout / 2)``.
This is always strictly less than ``idleTimeout``, so no existing pool
construction breaks. Pass ``Duration.ZERO`` to opt out, or any explicit
positive value to override.

The strict ``< idleTimeout`` validation is retained for explicit user
values so misconfigurations (e.g. ``acquireMinRemainingTtl ==
idleTimeout``) are still caught.

### Tests

* Kotlin ``:sandbox:test`` and ``:sandbox-pool-redis:test`` (with real
  Redis 7 via ``OPENSANDBOX_TEST_REDIS_URL``) — 14/14 in the Redis suite.
* Python ``pytest`` — 197 passed.
* New cases verify, in both languages and both stores: alive entries below
  threshold are surfaced; fully-expired entries are silently dropped;
  ``reapExpiredIdle`` excludes already-expired from the alive list; the
  default scales to ``min(60s, idleTimeout/2)``; explicit ``ZERO`` opts
  out; explicit values still get validated against ``idleTimeout``.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Pangjiping added a commit that referenced this pull request Jun 17, 2026
@Pangjiping @claude
fix(execd): address 7 more review issues (OSEP-0013)
8d2767f 
Security:
- #1 P1: Hide upper root from namespace with --tmpfs to prevent
  cross-session data access
- #2 P1: Use per-run UUID marker instead of fixed string to prevent
  exit code spoofing
- #3 P1: SDK profile/mode fields now Optional (None = server default),
  prevents SDK defaults from overriding server configuration

Correctness:
- #6: Close seccomp memfd after cmd.Start() to prevent fd leak
- #10: Call Validate() on create and run request bodies
- #11: Skip --tmpfs /tmp when workspace is /tmp to avoid mount override
- #12: Return HTTP 201 for session creation per OpenAPI spec

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pangjiping pushed a commit that referenced this pull request Jun 18, 2026
Debugging unit test error (#1)
b957034 
* Plan egress DNS test fix

* Fix egress DNS proxy unit test

* Validate egress DNS test fix

* Restore egress build artifact

* Remove committed egress binary

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Pangjiping added a commit that referenced this pull request Jun 22, 2026
@Pangjiping @claude
fix(execd): address 7 more review issues (OSEP-0013)
08a411e 
Security:
- #1 P1: Hide upper root from namespace with --tmpfs to prevent
  cross-session data access
- #2 P1: Use per-run UUID marker instead of fixed string to prevent
  exit code spoofing
- #3 P1: SDK profile/mode fields now Optional (None = server default),
  prevents SDK defaults from overriding server configuration

Correctness:
- #6: Close seccomp memfd after cmd.Start() to prevent fd leak
- #10: Call Validate() on create and run request bodies
- #11: Skip --tmpfs /tmp when workspace is /tmp to avoid mount override
- #12: Return HTTP 201 for session creation per OpenAPI spec

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Pangjiping added a commit that referenced this pull request Jun 23, 2026
@Pangjiping @claude
feat(execd): add isolation package with bwrap support (opensandbox-gr…
ed3aa1c 
…oup#1008)

* feat(execd): add isolation package with bwrap support (OSEP-0013 Phase 1)

- Add pkg/isolation/ package: Isolator interface, bwrap argv builder,
  startup probe, upper directory management, seccomp loading
- Switch bwrap distribution from //go:embed to Dockerfile static build
  (musl-gcc) and init container injection alongside execd
- Add isolation flags (upper root, max bytes, diff max bytes, allowed
  writable) with env var overrides
- Add smoke test: Docker build, extract binaries, verify static link,
  bwrap namespace test, execd probe
- Add smoke_bwrap.sh to CI workflow (ubuntu-latest only)
- Defer diff/commit to Phase 2 (stub returning 503)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(execd): implement isolated session API (OSEP-0013 Phase 2)

- Add isolated session model types and /v1/isolated/* router (17 endpoints)
- Implement session lifecycle: Create/Get/Run/Delete with bwrap+bash
- SSE streaming via basicController writeSingleEvent (context-aware)
- MergedView overlay filesystem with whiteout support (20 unit tests)
- Filesystem proxy endpoints (10 handlers via MergedView)
- Idle GC: background goroutine scavenges sessions past idle_timeout
- Bwrap integration tests (43 tests, linux+bwrap build tag)
- Isolated session unit tests (15 tests, stub isolator)
- CI job bwrap-smoke: meson build bwrap v0.11.2 + sudo go test
- Windows stubs for cross-platform compilation
- Fix: cmd.Wait() zombie cleanup, context cancellation propagation,
  setpriv skip when uid=0, correct v0.11.x overlay syntax

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(execd): Phase 3 — seccomp, telemetry, OpenAPI spec, coverage tests (OSEP-0013)

Seccomp BPF:
- pkg/isolation/seccomp_gen.go — BPF generator (elastic/go-seccomp-bpf, pure Go)
- Default-allow denylist: 40+ dangerous syscalls blocked (mount, ptrace, etc.)
- BPF passed to bwrap via memfd + ExtraFiles fd

Telemetry:
- execd.isolation.session.count (gauge)
- execd.isolation.run.duration (histogram, ms)
- execd.isolation.upper.usage_bytes (gauge)
- IsolationStatsProvider pattern for gauge callbacks

OpenAPI Spec:
- specs/execd-api.yaml: 17 endpoints, 10 schemas, ServiceUnavailable response

Integration tests (64 total in bwrap_test/):
- 5 seccomp tests (filter active, normal syscalls, ptrace block, mount block, persist)
- 3 ExtraWritable tests (write, read-write roundtrip, multiple sessions)
- 11 gap-coverage tests (stderr, recovery, cancellation, network iso, 100x stress, delete-recreate, bash builtins, large file, subprocess cleanup, buffer size, workspace isolation)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* docs(spec): add /v1/isolated API to OpenAPI spec (OSEP-0013)

17 endpoints, 10 new schemas, ServiceUnavailable response.
FS proxy endpoints reuse same schemas as /files/* and /directories/*.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(execd): PR review fixes, TOML config, API simplification, VFS interface (OSEP-0013)

Review fixes:
- Fix session lifecycle: done channel prevents stdin close on normal return
- Fix overlay mode: pass upper/work dirs to bwrap WrapOptions
- Fix end marker mid-line concatenation with strings.Index
- Extract scanUntilMarker() to reduce cognitive complexity
- Add runMu serialization, upperID tracking, validateExtraWritable()
- Delete dead seccomp.go (replaced by seccomp_gen.go + memfd)
- Fix MergedView.ReadDir upper-takes-precedence via entryMap
- Fix UpperManager.Collect delete-on-success only

TOML config (replaces 4 --isolation-* flags):
- New isolation.Config + LoadConfig() with go-toml/v2
- Seccomp override: [seccomp].deny fully replaces built-in denylist
- configs/isolation.example.toml with documented defaults
- Single --isolation-config flag + EXECD_ISOLATION_CONFIG env

API simplification:
- Flatten CreateIsolatedSessionRequest (remove isolation wrapper)
- Delete dead PersistSpec, ArtifactURLs, IsolationSpec types
- Wire up envs param with shell-escaped export prepend
- Remove unused cwd from IsolatedRunRequest
- Update OpenAPI spec to match

VFS interface:
- New pkg/vfs.FS interface for filesystem abstraction
- MergedView satisfies vfs.FS (compile-time check)
- Isolated file handlers use vfs.FS instead of concrete MergedView
- TODO for FilesystemController migration

Tests:
- 6 end-to-end workflow tests (Run↔File API interop)
- 2 seccomp config override e2e tests
- 8 config loading tests
- Document overlayfs API→Run limitation in MergedView

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* feat(sdks,server): 5-language SDK, server bwrap distribution, E2E tests (OSEP-0013)

Python/Go/JS/Kotlin/C# SDKs:
- IsolationService + IsolationSession handle pattern
  (sandbox.isolation.create() → session.run/get/delete/files)
- Models, service interfaces, adapters for all 5 languages

Server:
- bwrap binary distribution (Docker cache + K8s init container)
- bootstrap.execd.isolation=enable extension grants CAP_SYS_ADMIN +
  apparmor/seccomp=unconfined for bwrap namespace operations
- K8s: seccompProfile + appArmorProfile on container securityContext

Execd:
- Probe diagnostic message in capabilities API response
- bwrap path /opt/opensandbox/bwrap (alongside execd)
- Shared parseUploadForm between filesystem and isolated handlers

E2E tests (Python/Go/JS/Java/C#):
- capabilities, lifecycle, echo, PID isolation, env injection,
  state persistence, /tmp isolation (strict), SSE handlers,
  overlay mode, file operations via run

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): address PR review — env defaults, prefix bypass, timeout, EOF (OSEP-0013)

1. P1: Apply deny-blacklist env defaults when EnvPassthroughMode is
   empty — prevents leaking execd secrets (*_TOKEN, *_SECRET) into
   isolated sessions

2. P1: Use path-component comparison for extra_writable allowlist
   validation — rejects sibling paths like /workspace/cache-escape
   when only /workspace/cache is allowed

3. P2: Honor timeout_seconds in Run handler — wrap context with
   WithTimeout when value is positive

4. P2: Detect EOF without end marker — return error instead of
   exit code 0 when bash process dies before outputting the marker

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): address 7 more review issues (OSEP-0013)

Security:
- #1 P1: Hide upper root from namespace with --tmpfs to prevent
  cross-session data access
- #2 P1: Use per-run UUID marker instead of fixed string to prevent
  exit code spoofing
- #3 P1: SDK profile/mode fields now Optional (None = server default),
  prevents SDK defaults from overriding server configuration

Correctness:
- #6: Close seccomp memfd after cmd.Start() to prevent fd leak
- #10: Call Validate() on create and run request bodies
- #11: Skip --tmpfs /tmp when workspace is /tmp to avoid mount override
- #12: Return HTTP 201 for session creation per OpenAPI spec

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd,sdks): symlink guard, env scoping, GC safety, 404, SDK defaults, spec (OSEP-0013)

Security:
- Reject symlink targets in MergedView write paths (WriteFile, WriteFileReader)

Correctness:
- Scope per-run envs in subshell so they don't leak across runs
- GC skips sessions with active runs (TryLock on runMu)
- Return 404 (not 500) when running in a nonexistent session
- Kotlin/C# SDK: profile and mode default to null (server decides)
- OpenAPI spec: add message field to CapabilitiesResponse

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): version parsing, overlay probe, upper limit enforcement (OSEP-0013)

- Fix bwrap version parsing: read stdout instead of stderr
- Implement probeOverlayMount() to test overlay capability at startup
- Enforce UpperMaxBytes in Allocate() with ErrUpperLimitExceeded
- Add tests for limit exceeded and no-limit-when-zero scenarios

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): death-watch, startup check, memfd cleanup, mode validation, probe ns (OSEP-0013)

- Add doneCh death-watch goroutine to detect dead bwrap processes
- GC collects dead sessions; Run returns clear error for dead sessions
- Add 100ms startup check to detect immediate bwrap failures
- Close ExtraFiles (seccomp memfds) on Wrap error path
- Validate workspace mode in CreateIsolatedSessionRequest.Validate()
- Add --unshare-pid/uts/ipc to probe smoke test
- Extract magic strings to constants (session status, workspace mode, profile, env mode)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): profile validation, SSE error struct, workspace auto-create, UID defaults (OSEP-0013)

- Reject unknown isolation profiles instead of silently defaulting to strict
- Use --clear-groups instead of --init-groups for arbitrary UIDs without passwd entries
- Populate SSE Error field with structured ErrorOutput on isolated run failures
- Auto-create workspace directory if it doesn't exist
- Default merged-view uid/gid to process owner instead of root

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): align isolated filesystem API with normal filesystem contract (OSEP-0013)

- Search: return []FileInfo (not []string), accept path query as root, skip directories
- Rename/Replace/Mkdir: read from JSON body to match normal API contract
- Chmod: parse mode as octal, copy-up from lower before modifying
- Download: add Range header and offset/limit line-based reading support
- FileInfo: include type, owner mode (octal format), matching normal buildFileInfo
- Add ListDirectory handler and route (GET /directories/list)
- MergedView: reject symlinks on upper read paths (Stat/Open/ReadFile)
- MergedView: create whiteout on Remove/Rename for lower-only files
- Fix upper root hiding: hide entire isolation root, not just current session dir
- Update vfs.FS Search signature to accept root parameter

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd,server,tests): safePath fix, tmpfs/emptyDir for overlay, comprehensive e2e tests (OSEP-0013)

- Fix MergedView.safePath to strip workspace prefix from absolute paths,
  preventing double-join (e.g. /tmp/tmp/file.txt)
- Docker provider: add tmpfs mount at isolation upper root for overlay support
- K8s providers: add emptyDir volume + volumeMount for isolation upper root
- Extract ISOLATION_UPPER_MOUNT_PATH constant to avoid magic strings
- Add 37 e2e tests covering rw/ro/overlay modes for both session and filesystem API
- Overlay tests skip gracefully when environment lacks overlayfs support

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): overlay probe on upper root, whiteout-aware reads, capabilities merge (OSEP-0013)

- Probe overlay mount on upper root (tmpfs) instead of /tmp (overlay2),
  fixing false negative in Docker environments
- Capabilities endpoint merges probe result with bwrap capabilities,
  fixing commit_supported/diff_supported always returning false
- MergedView Stat/Open/ReadFile respect whiteout markers, returning
  404 for files that were deleted via the API
- Add diagnostic logging to overlay probe for debugging

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* test: add filesystem API + RO + overlay e2e tests for all SDKs (OSEP-0013)

Add comprehensive isolated session e2e tests covering rw/ro/overlay modes
and full filesystem API for Go, JavaScript, Java/Kotlin, and C# SDKs.

Each SDK now has ~35 tests covering:
- Core session operations (capabilities, lifecycle, run, PID, envs, state, /tmp)
- RW mode filesystem API (upload, download, info, search, mkdir, delete, move,
  chmod, replace, list directory, host visibility)
- RO mode (read existing, write denied, API read/search/list)
- Overlay mode (CoW isolation, host file reading, filesystem API through upper
  layer, whiteout on delete, merged search/list)

Overlay tests skip gracefully when overlayfs is not available.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): test expectation, timeout kill, exit code, 400 errors, chown mkdir (OSEP-0013)

- Fix bwrap_test.go: update --init-groups to --clear-groups in assertions
- Timeout cancellation sends SIGINT to process group instead of closing
  stdin, preserving the persistent bash session for reuse
- SSE error events extract numeric exit code as EValue (e.g. "42" not
  "command exited with code 42") with EName="ExitError"
- Create session returns 400 for validation errors (allowlist, profile)
- MkdirAll chowns created directories to session uid/gid

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(sdks): use auto-generated API clients for isolated filesystem endpoints (OSEP-0013)

Replace URL prefix composition hack with proper generated API calls for
isolated filesystem operations in Python, JS, and Kotlin SDKs. Fix spec
to align isolated endpoints with normal filesystem contract, regenerate
all SDK API clients, and fix JS e2e test expectations.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd,server): default env denylist, K8s seccomp/apparmor, overlay RemoveAll whiteout (OSEP-0013)

- Apply strictEnvBlacklist by default when env_passthrough is omitted
- Serialize seccompProfile and appArmorProfile in K8s security context
- Create whiteout markers in RemoveAll for lower-only overlay paths

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd,sdks,server): address 13 review issues — security, correctness, compat (OSEP-0013)

- Reject symlinks in all upper path components, not just final element
- Fix BatchSandbox volumes variable shadowing (P1)
- Don't advertise unimplemented diff/commit as supported
- Add verbose query param to isolated replace spec
- Wait for stdout scanner goroutine on run cancellation
- Use UpperManager.Remove (not Release) to reclaim disk on delete
- Guard isolated file endpoints when runner is nil (return 503)
- Preserve original file permissions during replace operations
- Return not-found error for missing directories in ReadDir
- Propagate endpoint headers in Go SDK isolated file client
- Validate env_passthrough.mode in create request (400 not 500)
- Separate timed/streaming fetch in JS isolated adapter
- Make ExecdStack.isolation optional for backward compatibility

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(sdk/kotlin): pass verbose param to isolatedReplaceContent (OSEP-0013)

Generated API now requires verbose parameter after spec update.
Pass verbose=false for replaceContents, verbose=true for detailed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): update bwrap integration test for Search(root, pattern) signature (OSEP-0013)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: fix ruff lint and Kotlin spotless formatting (OSEP-0013)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: fix ruff I001 import sorting in filesystem_model_converter (OSEP-0013)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: gofmt isolated_session.go (OSEP-0013)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: fix golangci-lint gocognit/nilerr issues (OSEP-0013)

- Extract unsetBlacklistedEnv() helper to reduce bwrapEnvSegment complexity
- Add nolint:nilerr for WalkDir callbacks (intentional skip-on-error)
- Add nolint:gocognit for Search (3-pass walk is inherently complex)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* style: move nolint:nilerr to return line for golangci-lint (OSEP-0013)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

* fix(execd): update unit tests for whiteout behavior and temp workspace paths (OSEP-0013)

- TestMergedView_Remove_LowerOnly: expect whiteout creation, not error
- Runtime tests: use t.TempDir() instead of /workspace for CI compat

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@Pangjiping