* 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>
* feat(lark): sidecar credential broker for sandbox lark-cli (Pattern B)
Removes the plaintext Lark credential mounts (appSecret + OAuth tokens)
from the sandbox container. A long-running broker sidecar owns lark-cli
and the per-user config/data dirs and serves the command surface over
Pod loopback; the sandbox gets only a forwarding shim on PATH, so the
raw credential files never exist in the sandbox filesystem.
- lark_broker.py: stdlib-only loopback broker (argv passthrough with
shell=False, server-injected credential env, bounded I/O) + shim
script constant + install-shim mode.
- docker/lark-cli-broker: init(install-shim) + serve image.
- provisioner: LARK_CLI_BROKER_IMAGE + provision_lark_cli_broker →
shim init container + lark-cli-broker sidecar (config/data mounted
sidecar-only); credentials dropped from the sandbox container;
/api/capabilities reports lark_cli_broker_image. Broker supersedes
the Pattern A init-container binary when both are configured.
- gateway: lark_cli_env_overlay(broker=True) omits config/data env;
sandbox_lark_broker_active() TTL-cached mode resolver; broker added
to sandbox_runtime_mode / readiness and the settings UI.
Opt-in and off by default (empty LARK_CLI_BROKER_IMAGE ⇒ no change).
Closes#4338
* fix(lark): address Pattern B broker review findings (#4501)
Follow-up to the sidecar credential broker addressing the PR #4501 review:
- shim: split the on-PATH lark-cli into a /bin/sh launcher + Python shim body
so broker mode fails loudly (exit 127, actionable message) instead of ENOEXEC
when the sandbox image ships no python3; interpreter pinnable via
DEERFLOW_LARK_BROKER_PYTHON. Launcher bakes in the shim's absolute path since
$0 is the bare command name when run off PATH.
- broker: drop the dead cwd payload field (broker can't see the sandbox FS) and
document the command-surface-only / no-file-IO limitation.
- broker: return a structured 500 JSON on unexpected exec errors so the shim
gets a meaningful message, not an opaque transport failure; set a handler
socket timeout to bound slow/stuck connections.
- broker: add an opt-in DEERFLOW_LARK_BROKER_DENY_SUBCOMMANDS denylist that
refuses secret-dumping subcommands before spawning the binary, forwarded from
the provisioner sidecar.
- gateway: tighten the per-bash-call broker probe timeout (1.5s) and cache
negatives longer (300s) so non-broker remote-provisioner users don't pay a
latency hit; guard the mode cache with a lock; drop the dead
_probe_provisioner_lark_cli_init_image wrapper.
- docs: remove the broken design-doc link from the broker README.
Adds tests for launcher python resolution, cwd omission, denylist enforcement,
500-on-error, hot-path probe timeout + negative caching, and provisioner
denylist-env wiring.
* feat: add lark cli integration
* fix: polish lark integration actions
* feat: support lark incremental permissions
* fix: detect lark authorization completion
* fix: harden lark integration install
* feat: expand lark auth scopes and reuse host auth in sandbox
Default lark auth to least-privilege (recommend=false, base sign-in only)
and expose the full set of lark-cli --domain business domains as native
--domain grants instead of a 4-domain read-only mapping. Resolve the
skill pack from the latest larksuite/cli GitHub release at install time
with content-hash integrity, and surface version/runtime drift in status.
Share the per-user lark-cli config/data profile between the Gateway
Settings auth flow and agent conversations by mounting the integration
dirs into the AIO sandbox and injecting the matching env for lark-cli
commands, with an allowlisted extra_mounts path in the provisioner/K8s
backend and traversal guards on integration paths.
* style: fix lint issues from ruff and prettier
Sort imports in the provisioner PVC test and re-wrap two long i18n
description strings to satisfy backend ruff and frontend prettier CI.
* fix(lark): address managed integration review feedback
* fix(frontend): stabilize integrations settings e2e
* test(sandbox): isolate remote backend legacy visibility check
* test: fix backend unit failures after merge
* Harden Lark integration review fixes
* Format Lark integration E2E test
* fix(lark): harden sandbox credential exposure and status disclosure
Address willem_bd's security review on PR #3971:
- Mount the per-user lark-cli config dir (long-lived appSecret) read-only
into the AIO sandbox; only the refreshable-token data dir stays writable.
- Redact host filesystem paths (install_path, cli.path) from
GET /lark/status and the config/auth complete responses for non-admin
callers, fail-closed on any auth error.
- Document the npm postinstall trade-off (--ignore-scripts is not viable
because @larksuite/cli fetches its platform binary in postinstall).
- Document the sandbox credential trust boundary in AGENTS.md and README,
pointing at the sidecar-broker follow-up (#4338).
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
* fix: V-001 security vulnerability
Automated security fix generated by OrbisAI Security
* fix(sandbox): wire provisioner API key through backend client and config
RemoteSandboxBackend now accepts an api_key parameter and sends it as
X-API-Key on all five provisioner HTTP calls (list, create, destroy,
is_alive, discover). AioSandboxProvider reads provisioner_api_key from
SandboxConfig and forwards it at construction time. SandboxConfig
formally declares the field; config.example.yaml documents it under
Option 4; docker-compose-dev.yaml threads PROVISIONER_API_KEY into both
the provisioner and gateway containers so a single .env entry covers
both sides.
Tests: monkeypatch PROVISIONER_API_KEY and send X-API-Key headers in the
five parametrized threading tests (previously 401-failing); new
test_auth_middleware asserts /health is open, /api/* rejects no-header
and wrong-key with 401, and accepts the correct key.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(provisioner): correct fail-closed auth docs and fix test mock signatures
The first round of PR #4116 fixes left four blocking issues:
- Mock signatures in test_remote_sandbox_backend.py (17) and
test_aio_sandbox_provider.py (2) didn't accept the new headers= kwarg,
causing TypeError on every provisioner HTTP call test.
- sandbox_config.py and config.example.yaml described the auth as
optional ("leave unset to disable") but the middleware is fail-closed:
an unset PROVISIONER_API_KEY causes 401 on every /api/* request.
- .env.example had no PROVISIONER_API_KEY entry, leaving users with an
empty value and silent 401s.
- No test covered the PROVISIONER_API_KEY="" fail-closed path.
Fix all four: add headers=None to all mock signatures, correct the
field description and example comment to state that both sides must
have the same key set, add PROVISIONER_API_KEY to .env.example with
generation guidance, add test_auth_middleware_unset_key, and add a
logger.warning on auth rejection for observability.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---------
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
* fix(provisioner): gate legacy skills mount by user visibility
* fix(test): aio sandbox provider
* fix: use shared legacy skill visibility helper for sandbox mounts
* Fix stale AIO sandbox cache reuse
* Address AIO sandbox review feedback
* Distinguish sandbox health check failures
* Keep local discovery recoverable when the runtime check fails
LocalContainerBackend.discover() shares _is_container_running, which now
raises on transient daemon errors instead of returning False. Discovery has
no exception handling in _discover_or_create_with_lock(_async), so a brief
Docker hiccup turned a recoverable "could not verify, create instead" into a
hard acquire failure. Catch the check failure inside discover() and return
None so an unverifiable container is simply not adopted, restoring the
pre-change fall-through while keeping raise-on-unknown semantics protecting
the destroy path.
Reported by fancy-agent on PR #3494.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Narrow the not-found match in container inspect error handling
A bare "not found" substring also matches transient failures like "command
not found" or "context not found", which would misclassify a check error as
"container definitely gone" and bypass the raise-on-unknown contract. Keep
Docker's specific "No such object"/"No such container" phrases, and only
trust a generic "not found" (Apple Container) when the message names the
inspected container or refers to a container/object.
Reported by WillemJiang on PR #3494.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
