4 Commits

Author SHA1 Message Date
Storm
b9a75b869d
fix(sandbox): attest lark broker mode per Pod and quarantine uncertain shell outcomes (#6436)
* fix(sandbox): attest lark broker mode per Pod and quarantine uncertain shell outcomes

Why this change:
Two fail-open paths in the AIO sandbox lifecycle. First, the lark broker
capability probe collapsed "provisioner unreachable" into "broker not
configured": one 1.5s timeout cached a negative for 300s, so new sandboxes
were created without the broker sidecar while the per-user credential
directories were still bind-mounted into the container, and existing Pods
kept being reused without their actual mode ever being read. Second, an
implicit-shell command that timed out or returned an unknown status only
marked the current client object; warm-pool reclaim and rediscovery built a
fresh client that re-entered a container whose shell state could not be
proven clean, and nothing about that ambiguity survived a Gateway restart.

What changed:
- The capability cache stores confirmed observations only, keyed per
  provisioner endpoint/auth identity; an unknown answer (timeout, malformed
  payload) raises instead of selecting credential placement, while a
  confirmed broker requirement is retained through outages. One in-flight
  probe per identity shares success and failure, and a Pod-attested mode
  contradicting the cache forces one shared refresh.
- Execution uses the mode attested for the actual sandbox:
  SandboxInfo.lark_cli_broker flows from the provisioner's atomic Pod
  observation into the client, so a probe outage delays drift detection but
  never denies reuse of an attested runtime, and never trusts an unattested
  one.
- The provisioner reports each Pod's actual broker mode, generation (Pod
  UID) and shell capacity from one read, rejects lark requests conflicting
  with its configuration (409/503 carrying a capability-refresh marker the
  Gateway uses to drop the stale cache entry), rejects conflicting or
  Terminating Pods on coordination creates while reporting Terminating as
  data to status/get/list, and answers Pod presence independently of
  Service readiness.
- A persistent quarantine (DEER_FLOW_HOME/sandbox-quarantine, keyed by
  scope/sandbox/generation) fences containers after uncertain implicit
  shell outcomes across client rebuilds and restarts; in-process reuse,
  warm reclaim, discovery, create responses and orphan reconciliation all
  pass one policy gate, and replacement runs under the existing ownership
  and teardown fences. A fenced sandbox still tracked after a failed
  release-recycle is destroyed inline on the next acquire.
- Quarantine records retire only after confirmed absence, retrying partial
  cleanup for the quarantined generation and releasing remembered ports
  under the same fences; local stop failures now propagate instead of being
  logged away, and remote destroy errors surface to callers.

Problem solved:
A provisioner blip could silently strip broker isolation from new lark
sandboxes for minutes at a time, and ambiguous shell outcomes let corrupted
default shells persist across warm reuse, rediscovery and restarts. Docs
updated: README, aio_sandbox and skills AGENTS.md.

* docs(changelog): record sandbox attestation and quarantine fix (#6436)

* fix(sandbox): preserve active leases and fail closed on unattested broker modes

Concurrent uploads could recycle a healthy AIO runtime while another holder
was creating or cleaning up a command session. Older provisioner responses
could also trigger destructive replacement before failing admission.

- Distinguish pending session creation from ambiguous outcomes and reserve
  idle teardown until execution and upload holders finish scope cleanup.
- Reject stale fork lookups and reconcile cancelled acquires without
  releasing a sandbox still borrowed by another holder.
- Default Lark broker mode to unverified and preserve unattested Pods until
  the provisioner is upgraded.
- Recognize Docker 29 absence diagnostics while matching resource names exactly.
- Preserve Gateway upload errors and cover admission and lifecycle regressions.

* fix(sandbox): scope the unverified-broker refuse path to attested providers

The fail-closed raise in _lark_cli_env_from_runtime treated every Sandbox
without a lark_cli_broker attribute as unverified, but only AioSandbox
carries the attestation — E2B, OpenSandbox, Tenki, and Boxlite have no
broker sidecar, no credential mounts, and no provisioner to upgrade, so
every lark-cli command on those providers failed with an unactionable
error.

Declare the tri-state on the Sandbox base class (defaulting to False,
the only mode brokerless providers have) so the None fail-closed path
stays scoped to AioSandbox's deliberate unverified default.

- Add lark_cli_broker: bool | None = False to the Sandbox ABC, mirroring
  the persistent_shell_sessions tri-state pattern
- Add a regression test pinning the non-broker overlay for a provider
  without the attribute

* fix(sandbox): close review gaps in broker attestation and teardown lifecycle

Address the latest #6436 review round:

- The provisioner's post-build broker-mode 409 now carries the
  capability-refresh marker, so an incompatible-mode conflict surfaced by
  idempotent re-create, attestation drift or post-create verification drops
  the Gateway's cached capability observation instead of repeating the same
  409 for the full negative-cache TTL.
- AioSandboxProvider.destroy() returns the _destroy_tracked outcome: False
  when teardown is deferred because an execution/upload holder or a peer
  teardown is still active, so callers can no longer mistake a deferred
  destroy for a completed one.
- A live replacement under the same logical ID proves the fenced old
  generations are absent; retire_replaced_generations prunes only those
  stale records so they cannot strand future creates, while the live
  generation's fence and unversioned crash fences are never touched.
- SandboxInfo.from_dict keeps a missing lark_cli_broker attestation
  unverified (None) instead of silently rehydrating as attested non-broker.
- The restricted-network inspect path shares the strict
  _is_no_such_network_error parser, closing the substring loophole for
  context-name collisions and wrong-target errors.

* docs(sandbox): condense lease notes to keep the AGENTS.md chain under the size gate

The three lease notes added in 26c307f3 pushed the effective guidance chain
to 98,312 bytes — 8 over the 98,304 hard limit enforced by agent-guidance
(AG002). Fold them into two lines carrying the same rules.

---------

Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-10-10 07:56:09 +08:00
Hyeonsang Cho
8e86729aa0
fix(gateway): confine artifact PUT to /mnt/user-data/outputs after path resolution (#5321)
* fix(gateway): confine artifact PUT to /mnt/user-data/outputs after path resolution

The outputs-only guard on PUT /api/threads/{id}/artifacts/{path} was a
string-prefix check on the raw path. A percent-encoded `..`
(`outputs/%2e%2e/uploads/x.txt`) survives nginx's variable proxy_pass
untouched, is decoded by Starlette, passes the prefix check, and the
resolver only confines the result to `user-data/` -- so an owner could
overwrite a sibling upload or workspace file in their own thread.

Collapse dot segments before the prefix check, and re-check the resolved
host path against the resolved outputs root so a symlink planted inside
`outputs/` cannot redirect the write either. The normalized virtual path
is what the response echoes and what non-mounted sandboxes receive.

* refactor(gateway): share the outputs-confinement rule with channel attachments

Review follow-up on #5321: the "only under /mnt/user-data/outputs" rule was
implemented independently by the artifact editor and by IM-channel
attachment delivery, and the two copies had already drifted.

Move it into app/gateway/path_utils.py as normalize_outputs_virtual_path
(collapse `..` before the prefix check) and resolve_outputs_confined_path
(re-check the resolved host path against the resolved outputs root, which
also catches a symlink planted inside outputs/). PUT /artifacts and
ChannelManager._resolve_attachments both call the helper; artifact_archive
keeps its stricter ZIP-member rules layered on top.

Tests that previously stubbed resolve_thread_virtual_path for the editor now
stub resolve_outputs_confined_path, and the channel attachment tests patch
path_utils.get_paths, which the helper binds at import like the other
consumers. The confinement itself is pinned by tests/test_gateway_path_utils.py.
2026-09-10 15:01:56 +08:00
PeaceMaker-best
137a3cb60d
fix(authz): recheck policy before sandbox reuse (#5006)
* fix(authz): recheck policy before sandbox reuse

* fix(authz): avoid duplicate async sandbox checks

* fix(authz): scope sandbox decision across middleware

* fix(authz): construct async providers on the event loop

* test(authz): avoid cold imports under Blockbuster

---------

Co-authored-by: 嗜鵼 <hy2010hy2010@qq.com>
Co-authored-by: PeaceMaker-best <221849497+PeaceMaker-best@users.noreply.github.com>
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
2026-08-30 15:29:51 +08:00
hataa
cc6a2657e7
feat(authz): enforce sandbox:execute authorization at sandbox acquisition (#4063 Phase 3) (#4911)
Sandbox is an execution environment, not a named resource: multiple tools
(bash, read_file, write_file, glob, grep, ...) depend on it, all funneled
through ensure_sandbox_initialized / ensure_sandbox_initialized_async. Gate
the single acquisition entry point (single source of truth) instead of
maintaining a sandbox-tool-name set in middleware:

- authorize_sandbox_execution helper (authz/sandbox_authz.py) checks
  authorize("sandbox", "execute", target="*") — a binary judgment
  (can this role use the sandbox at all); RBAC allow:"*"/true permits,
  allow:[]/false denies.
- lazy path: ensure_sandbox_initialized (+ async) calls the gate before
  provider.acquire.
- eager path: SandboxMiddleware.before_agent / abefore_agent call the gate
  before _acquire_sandbox.
- deny raises SandboxAuthorizationError (SandboxError subclass) which
  propagates through tool execution as a friendly ToolMessage (RFC §9:
  'not a crash').
- authorization.enabled: false is a no-op everywhere; provider errors
  follow fail_closed (deny) / fail_open (allow).

12 tests in tests/test_sandbox_authorization.py cover disabled/allow/deny/
deny-via-bool/no-policy-unrestricted/provider-error-fail-closed/open/
internal-caller + ensure_sandbox_initialized deny (never acquires) and
allow (acquires) integration paths.
2026-08-24 16:32:06 +08:00