mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-28 23:46:21 +00:00
33 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c668716737
|
fix(sandbox): stop E2B reconciliation from reviving warm-pool sandboxes (#5562)
* fix(sandbox): stop E2B reconciliation from reviving warm-pool sandboxes Periodic reconciliation probed every discovered remote sandbox with Sandbox.connect() before the locality check, and the check itself only consulted _sandboxes, not _warm_pool. A sandbox parked by release() was therefore adopted back to active on the first pass, and because the SDK normalizes connect(timeout=None) to its 300s default and the control plane extends a running sandbox's expiry when now+timeout is later, each 60s pass kept pushing the expiry forward — idle warm sandboxes never hit their configured idle_timeout. Treat _sandboxes and _warm_pool ids as locally tracked up front: skip probing them (no timeout-mutating connect), keep them canonical, and route only genuinely remote candidates through the duplicate-reap path. Extend the post-probe adoption recheck to _warm_pool so a release that lands mid-probe cannot be promoted back to active either. Fixes #5550 * fix(sandbox): keep active E2B VMs alive and sweep expired warm entries Address review on #5562: - Reconciliation now refreshes the remote TTL of locally active sandboxes through their cached client (never connect()), restoring the keepalive for turns that outlive idle_timeout without reviving warm-pool VMs. - Warm-pool entries parked longer than idle_timeout are dropped during reconciliation — their VMs are expected to be reaped by the control plane — releasing the ownership lease and the capacity slot they would otherwise pin until reclaim, eviction, or shutdown. - Remove the now-dead thread-local canonical sort; locally tracked ids are skipped unconditionally, so the ordering hint had no effect. * fix(sandbox): preserve active E2B keepalive and shared capacity * fix(sandbox): serialize E2B reconciliation lifecycle transitions * fix(sandbox): fence E2B ownership and timeout lifecycle writes * fix(sandbox): isolate ownership heartbeats from E2B timeout IO --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
492e2ac2cc
|
fix(sandbox): report an exactly-full search result as complete in the remote providers (#5534)
* fix(sandbox): report an exactly-full search result as complete in the remote providers `glob` and `grep` decide `truncated` twice: once for the raw output cap (`parse_remote_search_output`, unchanged) and once for `max_results` after the Python-side filters have run. The second decision returned as soon as `max_results` matches had been collected, which cannot tell a search that held exactly that many from one that held more — a tree holding exactly `max_results` eligible matches came back flagged as cut off, and the tool then told the model the result was incomplete. These providers hold the whole listing (the raw stream is capped at `max(max_results * 4, max_results + 50)` lines and reports its own cut-off), so like AIO's `glob` branches they can look one match past the cap before deciding: `AioSandbox.grep`, plus `glob`/`grep` in E2B, OpenSandbox, Tenki and BoxLite now use the same `len(matches) > max_results` rule. This completes what #5449 started for AIO's `glob`; the local provider's half is #5491. Co-Authored-By: Claude Code <noreply@anthropic.com> * fix(sandbox): let remote grep see one match past the per-file cap E2B and OpenSandbox stopped each file's grep at max(max_results, 50) matches, so a single file holding more than max_results hits — with a raw stream far below its limit — ended the Python loop exactly at the cap and reported the result as complete (#5534 review). Retain one extra match per file so the one-match lookahead can observe the overflow and report truncation. A single-file regression at max_results=50 covers 50 matches (complete) vs 51 (truncated) for both providers. Co-Authored-By: Claude Code <noreply@anthropic.com> --------- Co-authored-by: Claude Code <noreply@anthropic.com> Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
ed986a10ef
|
fix(sandbox): report truncated remote glob and grep results (#5427)
* fix(sandbox): report truncated remote glob and grep results
BoxLite, Tenki, E2B, and OpenSandbox run find/grep in the sandbox, cap
the raw output with `| head`, and then filter those lines in Python:
ignored directories such as node_modules are dropped and grep's glob
scope is applied. They reported truncated only when max_results matches
survived the filter. When the capped lines were mostly filtered out, a
search with real matches past the cap came back short or empty with
truncated=False, and glob_tool/grep_tool rendered it as "No files
matched" / "No matches found". With the default max_results=200 and
1,200 files under node_modules, glob("**/*.py") reported no matches for
a workspace that has src/app.py.
remote_search_command now lets one line past its limit through, and
parse_remote_search_output(..., limit=) returns RemoteSearchOutput(text,
truncated): the first `limit` lines and whether the extra line arrived.
Exactly `limit` lines stays a complete result. Each provider passes the
cap it already computed to both calls and returns that truncated from
glob and grep when fewer than max_results results survive filtering.
The glob and grep tools now describe an empty truncated result as
incomplete instead of reporting no matches, which also covers AIO grep's
forwarded truncated flag. Sandbox.glob/grep document truncated as "the
matches may be incomplete".
* docs(changelog): reference #5427 in the remote search truncation entry
---------
Co-authored-by: Willem Jiang <willem.jiang@gmail.com>
|
||
|
|
3e536944b7
|
fix(sandbox): stop remote grep/glob from reporting failures as no matches (#5380)
* fix(sandbox): stop remote grep/glob from reporting failures as no matches E2B, OpenSandbox, BoxLite and Tenki ran grep/find behind `2>/dev/null | head`, so a missing search root, a missing grep/find binary or an unreadable tree exited 0 with empty stdout and the tools reported "No matches found". Wrap the search in sandbox/remote_search.py, which checks the root first and records the search's own status after head, as remote_list_dir does for list_dir: a missing root raises FileNotFoundError, a failed search raises OSError, and a genuine no-match still returns []. glob's find gains -H for symlinked roots, OpenSandbox's BusyBox fallback keeps the primary grep status, and E2B no longer swallows client errors. Regression tests run each provider's real command in a local POSIX sh. Fixes #5376 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(sandbox): fail remote grep/glob on partial traversal errors grep 2 / find 1 after some results were printed (an unreadable file or subdirectory) were returned as a complete search. Callers have no partial-result channel, and #5376 asks for permission and command failures to raise, so these statuses now raise OSError like any other failure. Only grep 0/1/141 and find 0/141 pass. The error for grep 2 / find 1 says that some files or directories could not be read and asks for a narrower path, so the agent can retry instead of giving up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: Totoro-qaq <279883115+Totoro-qaq@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com> |
||
|
|
d8ed8160c9
|
fix(sandbox): stop list_dir from reporting failures as empty (#5264)
* fix(sandbox): stop list_dir from reporting failures as empty Remote providers swallowed find/client errors as [] and 2>/dev/null missing paths as empty stdout. ls_tool then told the agent the directory was (empty). Raise OSError/FileNotFoundError instead so the tool returns Error. * fix(sandbox): list_dir raises on missing local paths and uses find -H Empty stdout is not a missing path when find's start point is a symlink (E2B /mnt/acp-workspace). Dereference only the start point with find -H. LocalSandbox now raises FileNotFoundError for a non-directory root, matching remote providers. AIO maps a missing result.data to OSError rather than FileNotFoundError. * fix(sandbox): group AIO list_dir find type predicates Without parentheses, find PATH -maxdepth N -type f -o -type d applies -type d without maxdepth and can drop files from the listing. * fix(sandbox): distinguish list_dir command failure from missing path Tenki, Boxlite, and OpenSandbox treated any empty find stdout as FileNotFoundError, so a missing find binary (exit 127) or SDK error looked like a missing directory. Raise OSError when find status is outside (0, 1); keep FileNotFoundError for the find-ran-but-empty case. * fix(sandbox): apply list_dir exit-status contract to AIO and E2B Same gap as Tenki/Boxlite/OpenSandbox: empty find stdout with exit 127 was FileNotFoundError. Raise OSError when the status is outside (0, 1). * fix(sandbox): classify list_dir by find status not head status find | head under sh -lc reports head's exit code, so a missing find binary (127) became FileNotFoundError. Record find's own status after the bounded listing, treat SIGPIPE 141 as truncation success, and add a shell-level regression test. * test(auth): include projects permissions in /me contract pins #5265 added projects:read/write/delete to the registered route set. The /auth/me tests still pinned the pre-projects list, so CI failed after merging main. * fix(sandbox): do not treat missing list_dir marker as success The generated script ended on `rm -f`, so process status was 0/1 even when find's marker never landed. Both codes are in _FIND_OK, and the parser fallback then classified an empty listing as FileNotFoundError — the 127 misclassification this helper was meant to close. Exit with find's status (126 if unknown). A missing marker is now OSError unless the process status is already a non-OK failure. * test(sandbox): emit list_dir status marker in provider fixtures Parser now requires __DF_FIND_STATUS__ and refuses marker-less stdout. Update AIO/Boxlite/E2B stubs and OpenSandbox/Tenki find fakes so listings carry :0 and missing paths carry :1 with matching exit codes. * style(sandbox): format list dir test fixture * style(sandbox): format remote list dir helper * docs(sandbox): keep guidance within the tested size budget --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
e5d23943ce
|
fix(sandbox): stop E2B append from overwriting on read failure (#5261)
* fix(sandbox): stop E2B append from overwriting on read failure E2B has no native append, so write_file(append=True) read-modify-writes. Treat only FileNotFoundException/FileNotFoundError as an empty file; any other pre-read error must abort so a timeout cannot replace the original contents with just the new fragment. * fix(sandbox): distinguish E2B append pre-read refusal in logs A non-not-found pre-read error now logs as a refused overwrite instead of a write failure. Tests pin the successful read-modify-write path, including a bytes pre-image, so dropping `existing` cannot go green. |
||
|
|
e5977320a0
|
feat(sandbox): surface structured mount upload result on E2B sandbox (#4884)
* feat(e2b-sandbox): make mount upload deadline configurable Replace the hardcoded 120-second mount upload deadline with a configurable `mount_upload_deadline_seconds` key read from SandboxConfig (extra=allow). The value is validated: zero and negative inputs are clamped to 1 second. Omitting the key preserves the existing 120-second default. This addresses the follow-up from PR #4842 review: operators with large mounts or slow networks can now size the deadline to their deployment without changing code. * fix(e2b-sandbox): address review feedback on configurable deadline - Remove import-time default capture from _mount_deadline_reason() and _MountUploadBudget.deadline_seconds to prevent silent drift. - Add warning log when mount_upload_deadline_seconds is clamped to 1 (was silent before). - Update AGENTS.md E2B Mount Uploads section: deadline is now configurable, not fixed 120. - Add mount_upload_deadline_seconds to YAML examples in provider docstring and __init__.py. - Add config-path test that exercises SandboxConfig -> _load_config -> _apply_mounts end-to-end. * feat(e2b-sandbox): surface structured mount upload result on sandbox Introduce MountUploadResult dataclass and attach it to E2BSandbox.mount_upload_result after creation. This makes mount truncation observable in code without re-parsing Gateway logs. _apply_mounts() now returns MountUploadResult with truncated, reason, and upload totals. _create_sandbox() captures the result, stores it on the sandbox instance, and records it in a provider-level map so the result survives warm-pool reclaim and reconnect. MountUploadResult.truncated is True only when the upload pass was stopped early by a resource limit (deadline, file count cap, or byte budget). Individual mount failures (missing host path, SDK errors) are logged but do NOT set truncated. Tests cover: success totals, deadline truncation, file-count truncation, byte-budget truncation, non-limit failure not reported as truncation, missing host path not reported as truncation, create→sandbox wiring, and create→release→warm-pool→acquire result preservation. * fix(e2b-sandbox-provider): fix _mount_results lifecycle leak and review findings - Add _forget_mount_result() helper and call it at all terminal sandbox paths: _reuse_in_process_sandbox dead-evict, _reclaim_warm_pool_sandbox reconnect/dead/bootstrap/ownership/shutdown failure branches, _forget_local_sandbox, _kill_and_close. Prevents unbounded dict growth over a long-running Gateway process. - Make MountUploadResult @dataclass(frozen=True) to prevent silent mutation of the shared reference between provider map and sandbox attribute. - Move _mount_results insert under self._lock in _create_sandbox to match the read discipline in _register_connected_sandbox. - Guard _resolve_mount_upload_deadline against None (YAML explicit null) to avoid int(None) TypeError. - Add 5 regression tests covering each bypass path and the frozen invariant. * fix(e2b-sandbox-provider): add _forget_mount_result to _evict_oldest_warm branches Add _forget_mount_result() calls to all four terminal exit paths in the E2B _evict_oldest_warm override (reconnect failure, already-gone, kill failure, kill success). The peer-owned path already cleans up via _forget_local_sandbox. Add test_evict_oldest_warm_cleans_mount_result to pin the kill-success branch. * docs: reduce agent guidance size --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
6b4f803354
|
fix(sandbox): preserve trailing whitespace in filenames from list_dir and glob in remote providers (#4980)
* fix(sandbox): stop stripping filenames when parsing find output in remote providers The list_dir and glob parsers in the e2b, OpenSandbox, AIO, Tenki, and BoxLite providers called .strip() on every line of find output. A filename that legitimately ends (or begins) in whitespace was corrupted, so the listed path never resolved on any follow-up file API call, and the remote providers diverged from LocalSandbox, which preserves such names via pathlib. splitlines() already removes the line terminators, so filter empty lines only and keep each entry verbatim. Same class of bug as the e2b _sync_outputs_to_host fix (#4861), applied to the search parsers. Adds a trailing-space regression test per provider at the seam each suite already uses. * fix(sandbox): split find output on \n only, and rename the tenki test Review follow-ups from willem-bd: - aio_sandbox.list_dir used str.splitlines(), which also breaks records on \v, \f, \x1c-\x1e and \x85 - all legal inside a Linux filename, and all contrary to this PR's own rule that the newline is the only delimiter. find emits \n and nothing else, so split("\n") is the correct parse. - Renamed test_search_preserves_trailing_space_in_filename to test_list_dir_and_glob_preserve_trailing_space_in_filename, matching the sibling tests in test_opensandbox_provider.py and test_boxlite_provider.py. The body covers list_dir and glob; it never touches grep. |
||
|
|
a06a6fed7e
|
feat(harness): deterministic acceptance checklist for subagent delegations (RFC #4651, layer 2) (#5109)
* feat(harness): deterministic acceptance checklist for subagent delegations (RFC #4651, layer 2) PR4 of RFC #4651: check lead-supplied acceptance_criteria in code when a subagent completes, so objectively checkable requirements can never be silently passed by a self-report. - subagents/acceptance_checks.py: deterministic leaf families — file:<path> exists|non-empty and file_written:<path> read through read_current_file_content scoped to the shared thread workspace; the read uses the sandbox-native virtual path form (the local read validator and provider mount tables resolve /mnt/user-data/... paths, not host paths); the scope decision canonicalizes with realpath on the local sandbox so workspace symlinks cannot escape into uploads; a remote provider's "Error: ..." return string is normalized to a failed check (provider-typed via is_local_sandbox); a UnicodeDecodeError marks a binary deliverable as existing and non-empty; out-of-scope paths degrade to UNVERIFIED. tests_passed:<command> anchors to a matching recorded bash execution with status=success and a test-summary shape; matching is shell-structure aware with control-flow attribution (span must end at the last segment with provable execution), negating-option values are ineligible evidence and a target negated anywhere in the command degrades the match, extra flags must be selection-preserving, extra positionals widen only after a path-scoped criterion, truncated commands degrade via command_truncated, the summary shape is read only from output attributable to the matched segment (preceding segments provably silent by invocation form), and pass shapes require a nonzero passed count. Criterion text is neutralized with neutralize_untrusted_tags before storage/rendering. Anything else renders UNVERIFIED, never silently passed. - executor: accumulate bounded bash command/output evidence per streamed chunk (merged by tool_call_id, newest-capped) so subagent summarization compacting earlier messages cannot erase a recorded execution; the recorded status is the actual shell exit status parsed from the output's exit marker (signed codes included; the remote Command exited with code N form is accepted only as the whole trimmed output), falling back to deerflow_tool_meta only when no marker exists. - sandbox providers: e2b/opensandbox/tenki/boxlite append the LocalSandbox-style "Exit Code: N" marker on nonzero exit even with non-empty output; aio propagates the SDK's structured exit_code on both exec paths the same way; local timeouts append Exit Code: 124; and _truncate_bash_output always preserves a trailing exit marker (signed included) inside its budget, with a 32-char floor raising any smaller configured limit, so the actual shell outcome always survives in the output text. - task_tool: run the checklist offloaded (asyncio.to_thread) on the completed branch, failure-isolated; stamp the verdict into result metadata and render the per-criterion section into the model-visible result text. - status contract: additive subagent_acceptance_verdict transport with read-side structural validation. - delegation ledger: entry carries the verdict and renders a compact acceptance segment; gateway strips caller-forged verdicts from both ledger entries and message metadata, like the citation verdict. - blocking-IO anchor pins the offload (teeth proven red->green); leaf read errors catch only OSError/SandboxError so unexpected errors reach the task-tool-level isolation instead of being mislabeled. * fix(harness): close acceptance evidence gaps from review (RFC #4651 PR4) - negating options: overlap with a matched criterion target is now checked by path/nodeid prefix, not exact token equality — excluding a sub-path of the criterion's selection (pytest tests --deselect tests/unit/test_auth.py) degrades to UNVERIFIED instead of holds - output attribution: any redirection token in the matched final segment makes the recorded tail non-attributable (> / >> / 2> are word characters to the parser, so redirection was invisible to the matcher) - silent-source allowlist narrowed from any *activate suffix to the */bin/activate shape - status_contract docstring: restore the shared-fixture sentence and note subagent_acceptance_verdict is deliberately outside the fixture - executor: update_bash_executions publishes [] (stream carried no bash-family calls) instead of collapsing it into None, mirroring update_tool_receipts * fix(harness): close acceptance residual gaps from re-review (RFC #4651 PR4) - tests_passed: add error outcomes to the fail shapes — "4 passed, 1 error" and pytest's "ERROR <nodeid>" short summary no longer satisfy the pass shape when the exit status is swallowed (|| true) or absent; zero-error counts stay clean. - file leaves: bound the deliverable read — a "wc -c" shell size probe answers files above 50k bytes without loading ~2x their size, honoring the host-bash kill switch and falling back to the full read on any non-integer rendering, so verdicts never get less sound. - executor: record the exit marker text as status_marker on harvested bash evidence; the leaf detail now reports the marker actually seen instead of asserting a failure indistinguishable from the command's own trailing text. - extend the blocking-IO anchor to drive the probe branch inside the offload; teeth re-verified red->green. * fix(harness): close acceptance forgery and bound gaps from P2 re-review (RFC #4651 PR4) - file leaves: never read unbounded — size is established first (os.stat on the validated local host path, so the host-bash-disabled configuration needs no shell; a guarded wc -c on remote providers that renders missing/unreadable in its own words). Above the 50k cap the leaf answers from the size alone, at/below it the full read runs, and an unestablishable size degrades to UNVERIFIED instead of an unlimited fallback read. - output attribution: source/. prefixes are never provably silent — a crafted */bin/activate path shape says nothing about what the script prints, so sourced segments can no longer lend a passing summary. - executable identity: an explicitly path-spelled criterion now requires the same normalized executable path; the basename rule stays only for deliberately bare criterion commands. * fix(harness): run acceptance size probe outside subagent-controlled state (RFC #4651 PR4) - remote probe no longer runs in the sandbox's persistent shell: a fresh env -i /bin/sh with absolute-path stat/realpath (poisoned functions, aliases, PATH, exported functions, IFS, locale cannot steer it), plus a marker env routing AIO onto a fresh per-call bash.exec session. - metadata-only: stat never opens content, so a FIFO deliverable cannot block the parent for the provider's idle timeout; non-regular files (fifo/dir/symlink) degrade to UNVERIFIED. - containment canonicalized against the literal mount root: a final-component symlink or a swapped parent directory (root included) cannot redirect the check outside shared storage; unprovable layouts degrade to UNVERIFIED. * fix(harness): canonicalize probe containment against the canonical mount root (RFC #4651 PR4) Literal-root equality made every remote file leaf permanently UNVERIFIED on e2b and Tenki, which realize /mnt/user-data as a symlink to the home dir by default (e2b bootstrap 'sudo ln -sfn', Tenki best-effort symlink). Containment now compares the file's realpath against the mount root's realpath — exactly what the provider's own read path resolves, so probe and read-back stay consistent; final-component symlinks stay rejected by the non-dereferencing stat, and an intermediate dir-link escape under a sane root still lands ESCAPED. The inner script is a module constant and the suite now executes the composed probe for real against on-disk layouts (real dir, symlinked prefix, final symlink, fifo, missing, dir-link escape), which the canned-output stub could not see. * fix(harness): close bare-criterion negation and CDPATH summary channels (RFC #4651 PR4) - matching: a criterion with no positional selection target (bare pytest, make test) stands for the runner's default selection, so ANY negating option (--ignore/--deselect/...) makes the recorded run a different selection — unprovable. The overlap guard only sees consumed criterion tokens, which a bare criterion does not have; scoped criteria keep the unrelated-exclusion behavior. - attribution: cd is no longer blanket-silent — CDPATH makes cd print the resolved (subagent-chosen) destination and the pass shapes match as substrings, so one mkdir 'all tests passed' plus an export minted a pass for any quiet command. A cd argument or CDPATH= value (export or leading assignment) carrying any summary shape makes the segment non-silent; shape-free cd dir wrappers keep matching. - docs: _truncate_bash_output states the effective 32-char floor (the guarantee previously read as an unconditional max_chars bound). * fix(harness): close env-assignment and expansion channels in acceptance matching (RFC #4651 PR4) Self-audit in the shape of the last review rounds — channels the matcher classified as accounted-for that can change what runs, narrow the selection, or lend the summary text: - env assignments are no longer blanket-stripped: only an allowlist of inert display/CI knobs (CI, NO_COLOR, PY_COLORS, ...) may prefix a matched span, and a non-allowlisted assignment in any preceding segment (pure-assignment or export NAME=) is state pollution — PATH redirects the executable, LD_PRELOAD/PYTHONPATH/NODE_OPTIONS inject code, PYTEST_ADDOPTS/GOFLAGS/MAKEFILES inject selection-changing inputs, BASH_ENV runs arbitrary shell startup. All degrade to unprovable. - runtime expansions: any span token carrying /$( )/backticks, any negating-option value carrying an expansion or glob (unknown excluded set), and any extra executed token carrying glob metacharacters (crafted option-looking filenames narrow invisibly) are unprovable. Criterion-side globs stay self-consistent (literal match). - cd: an argument carrying a runtime expansion or glob is non-silent (unknown destination, unknown print); CDPATH= assignments are now handled as state pollution at the match layer, subsuming the value-shape special case. * fix(harness): persistent-shell evidence, exact env sets, option-arity scoping (RFC #4651 PR4) - tests_passed: on a persistent-shell provider (new Sandbox.persistent_shell_sessions capability, set by AioSandbox) every leaf degrades to UNVERIFIED — any earlier call in the shared session could have mutated the state the clean-looking run executed in, and only a fresh controlled session (RFC section 6 verifier) can prove otherwise. The flag is read from the provider registry without acquiring a sandbox. - env assignments: the allowlist is gone — no variable is provably inert across repositories (CI/DEBUG are routinely read by tests). The span's assignment prefix must equal the criterion's exactly (values included, order-insensitive); any assignment or export NAME= in a preceding segment is state pollution. - scoping: positional targets are now read by option arity, so a path embedded in an option (--basetemp=/tmp/p, --junitxml=/tmp/r.xml) never counts as a selection target and an extra positional after such a criterion narrows the default selection it denotes. * fix(harness): stamp shell provenance at harvest, close export/unset and arity gaps (RFC #4651 PR4) * fix(harness): split physical newlines as shell separators in acceptance matching (RFC #4651 PR4) * fix(harness): scope cd wrappers to thread data roots, pin accepted boundaries (RFC #4651 PR4) * fix(harness): preserve criterion connectors, prove file_written readable, fail-closed shell capability (RFC #4651 PR4) * fix(harness): compare only the connector prefix, tolerate trailing criterion semicolons (RFC #4651 PR4) * fix(harness): preserve continuation-line operators, keep ./-spelled executable identity (RFC #4651 PR4) * fix(harness): render criteria single-line so a multiline criterion cannot inject a forged checklist line (RFC #4651 PR4) * fix(harness): reject parent-traversal executable tokens in acceptance matching (RFC #4651 PR4) * fix(harness): reject parent-traversal negated values in acceptance matching (RFC #4651 PR4) |
||
|
|
317577e285
|
fix: enforce custom agent skill allowlists in sandboxes (#5077)
* fix: enforce agent skill allowlists in sandboxes * fix: guard E2B skill projection resets * fix: preserve agent skill isolation across delegation * fix: close sandbox skill isolation bypasses * fix(sandbox): close skill isolation review gaps * fix(sandbox): harden skill isolation lifecycle |
||
|
|
0dd233afc4
|
feat(sandbox): make E2B mount upload deadline configurable (#4876)
* feat(e2b-sandbox): make mount upload deadline configurable Replace the hardcoded 120-second mount upload deadline with a configurable `mount_upload_deadline_seconds` key read from SandboxConfig (extra=allow). The value is validated: zero and negative inputs are clamped to 1 second. Omitting the key preserves the existing 120-second default. This addresses the follow-up from PR #4842 review: operators with large mounts or slow networks can now size the deadline to their deployment without changing code. * fix(e2b-sandbox): address review feedback on configurable deadline - Remove import-time default capture from _mount_deadline_reason() and _MountUploadBudget.deadline_seconds to prevent silent drift. - Add warning log when mount_upload_deadline_seconds is clamped to 1 (was silent before). - Update AGENTS.md E2B Mount Uploads section: deadline is now configurable, not fixed 120. - Add mount_upload_deadline_seconds to YAML examples in provider docstring and __init__.py. - Add config-path test that exercises SandboxConfig -> _load_config -> _apply_mounts end-to-end. * fix(e2b-sandbox-provider): handle non-numeric mount_upload_deadline_seconds Guard _resolve_mount_upload_deadline against None, non-numeric strings, and other invalid values. None returns the default; non-numeric strings like '120s' or 'abc' log a warning and fall back to the 120-second default instead of crashing provider init with TypeError/ValueError. Extend the parametrized clamp test with None, suffix, and alpha cases, and add a warning assertion. Update CONFIGURATION.md with the new mount_upload_deadline_seconds key and its behavior. * fix(sandbox): handle infinite mount deadline |
||
|
|
bb75f8d736
|
feat(sandbox): share sandbox identity derivation and acquire serialization (#4741) (#5089)
* feat(sandbox): share sandbox identity derivation and acquire serialization (#4741) Remote providers (AIO, E2B, BoxLite, Tenki, OpenSandbox) each inlined the same sha256(user:thread)[:16] sandbox-id expression and kept per-scope lock dicts that grew unboundedly until shutdown. This extracts both mechanisms into shared components without changing provider lifecycle, ids, capacity semantics, or public tool behavior: - sandbox/identity.py: keyword-only derive_sandbox_scope_token (byte-pinned compatibility contract) + is_sandbox_scope_token; per-provider golden vectors pin current behavior including BoxLite's raw-None quirk and each provider's private user_id resolution. - sandbox/acquire_serialization.py: AcquireSerializer — per-key lock table with holder/waiter refcount reclamation, bounded dedicated executor (async waits off both the event loop and the default executor), worker-owned cancellation cleanup (no event-loop callback dependency), idempotent close(). - Each provider adopts both components; AIO/E2B key by (user_id, thread_id) with acquire and (E2B) release serialized; BoxLite/Tenki/OpenSandbox key by derived sandbox id and offload the whole sync acquire to the serializer's executor so a cancelled awaiter cannot overlap a retried same-scope body (leaked-remote-VM regression caught in review). - thread_id=None acquires stay unserialized; provider shutdown()/reset() close the serializer; E2B capacity/ledger/reconciliation and AIO ownership/flock machinery untouched. - blocking-IO anchor proves contended OpenSandbox acquire_async stays off the event loop (teeth verified red/green); AGENTS.md documents the shared components. * refactor(sandbox): address review on acquire serialization (#5089) - Replace unreachable checkin branch with an assertion: run() returns False only after abandon(), which the except handler always re-raises; the old _checkin would have double-decremented the refcount. - Document the task.cancelling() == 0 assumption in hold_async. - Drop unused thread_id/user_id kwargs from BoxLite and Tenki _acquire_scope_locked (OpenSandbox still forwards them). * fix(sandbox): preserve request ContextVars in acquire executor bridge (#5089) loop.run_in_executor() does not copy contextvars, unlike the inherited SandboxProvider.acquire_async() which used asyncio.to_thread(). The BoxLite/OpenSandbox/Tenki acquire_async bridges introduced in this PR therefore dropped the request trace id (logged as trace_id=-). Add AcquireSerializer.run_on_executor(), which copies the calling context and runs the callable through ctx.run, and route all three providers through it. Add regression tests binding request_trace_context and verifying the worker thread observes it. |
||
|
|
38440949c6
|
fix(e2b): preserve trailing whitespace in filenames and survive mtime overflow (#4861)
* fix(e2b): preserve trailing whitespace in filenames and survive mtime overflow _sync_outputs_to_host iterated the NUL-delimited find output with entry.strip() on each record. NUL already guarantees record boundaries, so the strip is redundant and harmful: a filename that legitimately ends in whitespace (e.g. "report ") had its trailing space trimmed, pointing host_path at the wrong file and recording a manifest key that can never match — the file was re-downloaded on every release. The same host-write block wrapped only os.utime in the outer except OSError, but os.utime raises OverflowError (not an OSError) when the ns value is out of range (a far-future remote mtime, e.g. `touch -d '99999 years'`). That escaped the loop, skipping the manifest write and forcing a full re-download next release. Wrap os.utime in its own (OSError, OverflowError) so only the timestamp restoration is dropped; the file is still written and the manifest still updated. * test(e2b): rely on monkeypatch cleanup --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
69c9a2022c
|
fix(sandbox): bound aggregate E2B mount upload work (#4842)
* fix(sandbox): bound aggregate E2B mount upload work * fix(sandbox): preserve mount guards on upload failure * fix(sandbox): cover mount preflight with deadline * refactor(sandbox): clarify mount deadline checks * refactor(sanbox): deduplicate mount deadline reason * fix(sandbox): evaluate mount deadline reason lazily |
||
|
|
5b523bc979
|
fix(sandbox): bound E2B mount upload resource use (#4812)
* fix(sandbox): bound E2B mount uploads * fix(sandbox): revalidate E2B mount files |
||
|
|
30a36bd41b
|
fix: accept documented E2B reconciliation config fields (#4772)
* fix: accept documented E2B reconciliation config fields Stop reporting supported reconciliation settings as unknown fields. Keep warnings for invalid keys and add coverage for documented settings. Refs bytedance/deer-flow#4771. * docs: document E2B reconciliation settings |
||
|
|
46fd5c8a00
|
refactor(sandbox): name the E2B ledger meta-field count (#4764)
Admission derived the live-entry count as `HLEN - 3`, where 3 was the number of `meta:*` fields written 35 lines earlier in initialize(). Nothing tied the two together, so adding a fourth meta field would shift the capacity ceiling by one. Name the offset `META_FIELD_COUNT` next to initialize(), and add a guard test asserting a freshly initialized ledger holds exactly those three fields, plus one pinning that a hard_limit of N admits exactly N reservations. References #4575 Co-authored-by: icn5381 <255778606+icn5381@users.noreply.github.com> |
||
|
|
f2e832330e
|
fix(sandbox): enforce disabled skills in filesystem views (#4178)
* fix(sandbox): project enabled skills into sandbox views * fix(skills): keep projection mutations consistent * fix(skills): fail closed on projection errors * fix(skills): isolate per-scope failures during boot projection rebuild rebuild_all_skill_projections() propagated any exception from the public rebuild or from a single user's rebuild straight out of the gateway lifespan startup, uncaught. A single broken user directory (bad permissions, corrupted _skill_states.json, unreadable content) would therefore abort gateway boot for every user, not just that one - _rebuild_*_locked already fails closed internally (clears the view and re-raises), so the boot loop only needed to stop treating that re-raise as fatal. Each scope's rebuild now fails closed independently and boot continues; a scope left empty by a boot failure self-heals on the next sandbox acquire via ensure_skill_projections(). Also patches deerflow.skills.projection.rebuild_all_skill_projections in the memory-flush lifespan test fixture, matching the two sibling fixtures in the same file — this call is now on the lifespan startup path and the fixture's minimal SimpleNamespace config predates it. * test(skills): update authz test for the projection-aware public toggle _persist_shared_skill_state (introduced earlier in this branch) reads the shared extensions_config.json fresh from disk under the projection lock instead of through the cached get_extensions_config() singleton - that's the whole point of the fix (stale worker caches must not clobber another worker's concurrent update). The name no longer exists on the skills router module, so the test's monkeypatch of it started raising AttributeError instead of exercising the endpoint. The mock storage in this test isn't a real LocalSkillStorage instance, so _persist_shared_skill_state's projection-mutation branch is already skipped (nullcontext) and it falls back to a fresh ExtensionsConfig() for the nonexistent tmp config_path - no replacement monkeypatch needed. * fix(sandbox): make skill projection ensure best-effort in acquire acquire() called _ensure_skills_projection() directly, outside any try/except, in both LocalSandboxProvider and AioSandboxProvider. Every other skill-mount setup path in these providers has always caught exceptions and logged a warning rather than failing sandbox acquire outright (e.g. when config.yaml can't be resolved) - these two new call sites broke that contract, so any projection failure (including simply not having a config.yaml, as in CI's test environment) now failed acquire() itself instead of just leaving skill mounts off. _ensure_skills_projection now catches its own exceptions and returns None; both providers' callers already tolerate that (a None projection skips the skill-specific mounts, matching the existing degrade path) after making _append_public_skill_mapping and the custom/legacy mount block in LocalSandboxProvider explicitly None-safe. Caught by running the full suite with config.yaml removed, matching CI's environment - not caught locally because a real config.yaml was present, masking the failure. * fix(sandbox): make E2B skill projection mounts best-effort _skill_projection_mounts called ensure_skill_projections with no guard, unlike Local/AIO's _ensure_skills_projection. A raise propagated out of _apply_mounts before the configured-mounts loop ran, so a skills projection failure dropped the operator's own configured mounts too - only caught by create()'s outer warning, with nothing applied at all. Swallow here and return an empty mount list on failure, matching the Local/AIO pattern: still fail-closed for skills, but no longer widens the blast radius to unrelated configured mounts. Review feedback from PR #4178. * docs(skills): document projection trade-offs flagged in review - _update_tree_digest: note the metadata-only (not content) hashing trade-off and why runtime writes through this codebase are still covered regardless (rebuild-under-lock + rename always changes inode). - LocalSandboxProvider.acquire: note the acquire-time self-heal cost (cheap on a fresh manifest, ~400ms rebuild under lock on stale/drift). - skill_projection_mutation: drop the no-op except-Exception-then-raise; a raise from the mutation already propagates past the yield with the view left cleared, no explicit re-raise needed. - provisioner README: spell out that hostPath skills volumes require the gateway and K8s node to share DEER_FLOW_HOST_BASE_DIR (single-node or shared storage), and that the custom/legacy volumes' hostPath type Directory (not DirectoryOrCreate) makes a violation of that assumption a visible Pod-creation failure instead of a silent empty mount. Review feedback from PR #4178. * fix(skills): lazily repair user projections * fix(skills): close projection review gaps * fix(skills): refresh user projection enable state * fix(skills): close projection review follow-ups * fix(skills): preserve state across projection writes --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
0cc28d2c42
|
fix(sandbox): enforce deployment-wide E2B capacity (#4575)
* docs: design deployment-wide E2B capacity * fix(sandbox): enforce deployment-wide E2B capacity * fix(sandbox): address E2B capacity review findings * fix(sandbox): grace stale E2B capacity inventory --------- Co-authored-by: MiaoRuidx <12540796+MiaoRuidx@users.noreply.github.com> |
||
|
|
904cee4a72
|
fix(sandbox): push read_file ranges into sandbox reads (#3824)
* fix read_file range validation and passthrough * chore: format test_aio_sandbox.py * test: cover e2b sandbox read_file line-range behavior * fix(boxlite): support ranged file reads * fix(sandbox): reject non-positive start lines --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
d455a1815e
|
fix(sandbox): allow grep to search a single file (#4512) | ||
|
|
b22f85c686
|
fix(sandbox): reconcile E2B sandboxes safely (#4443)
* fix(sandbox): reconcile E2B sandboxes safely * fix(sandbox): clear failed E2B adoption intent |
||
|
|
3b77a7401b
|
fix(sandbox): enforce E2B replica capacity limits (#4391)
* fix(sandbox): enforce E2B replica capacity limits (in-process) Add SandboxCapacityExceededError with diagnostic fields. Add overflow_policy (wait/reject/burst), acquire_timeout, and burst_limit config options. Implement atomic capacity reservation with a four-slot model: reserved / active / warm / transitioning. Transitioning slots close the window where active-to-warm or warm-to-active transitions appear to have zero occupied slots, which would let concurrent acquires exceed the configured replica ceiling. Re-route release, reclaim, and evict through transitioning counters. Add shutdown guard: reject waiters, kill VMs created during shutdown. Add 14 tests: policy enforcement, release+acquire race, warm-reclaim race, shutdown-waiter interaction, shutdown-during-create, and concurrent different-thread capacity assertion. Related: #4339 * fix: harden e2b sandbox capacity lifecycle * fix: retain e2b capacity during uncertain eviction * fix: serialize e2b tombstone eviction * fix: retain capacity after uncertain e2b cleanup * fix: track e2b remote operations during shutdown * fix(sandbox): validate E2B capacity config * fix(sandbox): classify capacity errors * fix(sandbox): harden E2B capacity lifecycle * test(sandbox): cover E2B review findings * docs(changelog): note E2B capacity behavior * docs(readme): explain E2B overflow handling * docs(backend): record E2B lifecycle rules * docs(sandbox): clarify destructive E2B reset * fix(sandbox): close E2B capacity race gaps --------- Co-authored-by: Willem Jiang <willem.jiang@gmail.com> |
||
|
|
4dd7cafef1
|
fix(sandbox): serialize E2B release transitions (#4355) | ||
|
|
05e4f4f6d8
|
fix(sandbox): bound E2B output synchronization resources (#4364)
* fix(sandbox): bound E2B output synchronization resources E2B release-time output sync pulled every changed file back from the remote VM with only a per-file size cap and no aggregate bound, so a pathological outputs tree (thousands of files, or many sub-cap files summing to gigabytes, or a slow VM) could make release download unboundedly on a hot path that runs at every agent turn end. Add three aggregate ceilings on top of the per-file cap — total bytes, file count, and a wall-clock deadline — enforced in the sync loop. When a ceiling is hit the pass stops early, logs what it dropped, and defers the rest to the next release. A truncated pass skips stale-manifest pruning so files it never reached are reconciled next time instead of being forgotten and re-downloaded. Closes #4340 * test(sandbox): pin multi-pass convergence of bounded output sync The four truncation tests each exercise a single capped pass. Add a two-pass test that locks in the invariant the design relies on for correctness: already-synced files are skipped before the budget check, so they never consume the cap and the deferred tail drains over successive releases instead of the leading files being re-downloaded every turn. A refactor that let a skipped file consume the cap would pass the single-pass tests but fail this one. |
||
|
|
495e90832c
|
fix(sandbox): scope e2b grep() glob filter to its directory prefix (#4168)
E2BSandbox.grep()'s glob handling reduced a directory-scoped pattern like "src/*.js" down to just "*.js" before passing it to `grep --include=`, dropping the directory-scoping prefix entirely. GNU grep's `--include` matches by basename only, at any depth, so the search silently broadened to every matching-extension file in the sandbox tree instead of just the directory the caller asked for. Keep the basename portion as a coarse `--include=` pre-filter (a superset of the true match set) and post-filter grep's raw hits through path_matches(), the same helper glob() already uses to enforce directory scoping correctly, so grep and glob agree on what a directory-scoped pattern means. |
||
|
|
d2116d861b
|
fix(sandbox): sync same-size E2B output updates (#4329)
* fix(sandbox): detect same-size E2B output updates * test(sandbox): cover E2B output modification times * fix(sandbox): persist E2B output sync versions * test(sandbox): cover E2B output sync manifest * docs(backend): document E2B sync manifest * fix(sandbox): scope E2B sync manifests * test(sandbox): cover E2B sync manifest lifecycle * docs(backend): describe E2B manifest lifecycle * fix(sandbox): fall back from empty E2B sandbox ID * test(sandbox): cover empty E2B sandbox IDs * docs: clarify E2B output sync ownership |
||
|
|
da3feb3863
|
fix(sandbox): fail E2B bootstrap safely (#4325)
* fix(sandbox): fail E2B bootstrap safely * test(sandbox): cover E2B bootstrap cleanup * docs(backend): document E2B bootstrap failure * fix(sandbox): discard E2B bootstrap failures * test(sandbox): cover E2B reconnect bootstrap failures * docs(backend): clarify E2B bootstrap recovery * fix: preserve E2B bootstrap errors * fix: reject falsey E2B bootstrap errors |
||
|
|
271a921baf
|
refactor(sandbox): reuse E2B kill helper during eviction (#4298)
* refactor(sandbox): reuse E2B kill helper during eviction * test(sandbox): preserve close on kill lookup failure |
||
|
|
90d511f3d2
|
refactor(sandbox): consolidate E2B client lifecycle helpers (#4262)
* refactor(sandbox): consolidate E2B client lifecycle * test(sandbox): cover E2B lifecycle cleanup |
||
|
|
dcb2e687d5
|
feat(channels): add GitHub as a webhook-driven channel (#3754)
* feat(channels): add GitHub event-driven agents (#3754) Add a webhook-driven GitHub channel with fail-closed webhook routing, deterministic per-agent PR/issue threads, mention-gated trigger fan-out, GitHub App token injection for sandboxed gh/git commands, and backend/AGENTS.md documentation. * fix(llm-middleware): classify bare IndexError as transient Upstream chat providers occasionally return 200 OK with an empty generations list (observed against Volces "coding" on ark.cn-beijing.volces.com). When that happens, langchain_core.language_models.chat_models.ainvoke raises ``IndexError: list index out of range`` at ``llm_result.generations[0][0].message`` and kills the run. Treat a bare IndexError reaching the middleware as a transient upstream-payload glitch and route it through the existing retry/backoff path instead of failing the whole agent run. The retry budget and backoff schedule are unchanged. Adds three regression tests covering the classifier and both the recover-on-retry and exhausted-retries paths. * fix(runtime): ignore stale LLM fallback markers from prior runs When a run on a thread ends with the LLM-error-handling middleware emitting a `deerflow_error_fallback`-marked AIMessage (e.g. after the IndexError empty-generations classification fix lands), that message is persisted to the thread's checkpoint as part of the messages channel. LangGraph replays the full message history in `stream_mode="values"` chunks, so every subsequent run on the same thread re-streams the stale fallback marker — and the worker's chunk scanner faithfully picks it up, flipping `RunStatus.success` to `RunStatus.error` for runs that themselves had no LLM failure at all. Snapshot the set of pre-existing message ids from the pre-run checkpoint and thread it through `_extract_llm_error_fallback_message` / `_try_extract_from_message` as a filter. Markers on history messages are ignored; markers on fresh messages produced during this run still trip the error path. Falls back to an empty set when the checkpointer is absent or the snapshot can't be captured, preserving the prior behavior on first-run / no-state paths. Adds unit tests for the new filter (helper-level and `_collect_pre_existing_message_ids`) plus an integration test exercising the full `run_agent` path with a stale history checkpointer. * fix(channels): make github channel fire-and-forget to avoid httpx.ReadTimeout on long runs GitHub agent runs (clone -> edit -> test -> push -> PR) routinely exceed the langgraph_sdk default 300s read deadline. The manager's runs.wait call kept an HTTP stream open for the entire run lifetime, so the long run blew up with httpx.ReadTimeout and the outer except branch then released the dedupe key and emitted a false 'internal error' outbound. The GitHub channel's outbound send is log-only by design: agents post to the issue/PR via the gh CLI in the sandbox when they choose to comment or create a PR. There is nothing for the manager to ferry back, so the long-poll was pure overhead. This change adds ChannelRunPolicy.fire_and_forget (default False) and sets it True for the github channel. When fire_and_forget is True, _handle_chat dispatches via client.runs.create (short POST, returns once the run is pending) instead of client.runs.wait, and skips the response-extraction + outbound-publish block. ConflictError on a busy thread still trips the standard THREAD_BUSY_MESSAGE path so behavior on the busy case is preserved for any future non-github fire-and-forget channel. Other (non-github) channels are unchanged: their policy defaults fire_and_forget=False and they continue to dispatch via runs.wait. Adds 6 regression tests in tests/test_channels.py::TestGithubFireAndForget: - Default ChannelRunPolicy.fire_and_forget is False. - The github policy registers fire_and_forget=True. - github inbound calls runs.create, not runs.wait, with the right kwargs. - github inbound publishes no outbound on success. - ConflictError from runs.create still emits THREAD_BUSY_MESSAGE. - Non-github channels (slack) still dispatch via runs.wait. * test(lead-agent): accept user_id kwarg in skill-policy test stubs The two GitHub-channel tests added in #3754 stubbed _load_enabled_skills_for_tool_policy with a lambda that only accepted `available_skills` and `app_config`, but the real function (and its call site in agent.py) also passes `user_id`. This raised TypeError on every run, failing backend-unit-tests. Add `user_id=None` to match the three sibling stubs in the same file. * refactor(gateway): disambiguate context-key set names The two frozensets _INTERNAL_ONLY_CONTEXT_KEYS and _CONTEXT_ONLY_KEYS shared a confusable "CONTEXT_ONLY" token in different orders, and the first broke the _CONTEXT_<X>_KEYS pattern of its sibling _CONTEXT_CONFIGURABLE_KEYS. Rename to make the distinct axes explicit: _CONTEXT_INTERNAL_CALLER_KEYS - WHO: internal callers (scheduler) only _CONTEXT_RUNTIME_ONLY_KEYS - WHERE: runtime context only, never configurable Pure rename, no behavior change. |
||
|
|
09988caf95
|
feat(skills): request-scoped secrets for skills (closes #3861) (#3871)
* feat(sandbox): per-call env injection + platform-secret scrubbing for skills Add an env parameter to Sandbox.execute_command (abstract + local + AIO) so request-scoped secrets can be injected into skill subprocesses, and scrub platform credentials (*KEY*/*SECRET*/*TOKEN*/*PASSWORD*/*CREDENTIAL*) from the inherited environment by default so scoped injection is not security theatre. LocalSandbox always passes an explicit scrubbed env; AioSandbox routes env-bearing commands through bash.exec(env=) on a fresh session and leaves the legacy persistent-shell path unchanged. Part of #3861. BEHAVIOR CHANGE: execute_command no longer inherits the full os.environ; Windows encoding tests updated to assert the scrubbed dict. * feat(skills): parse required-secrets frontmatter declaration Add SecretRequirement and Skill.required_secrets, and parse the required-secrets SKILL.md frontmatter field (a string list or {name, optional} mappings), dropping malformed entries with a warning so one bad declaration does not invalidate the skill. The declared name is both the context.secrets key and the env var injected at activation. Part of #3861. * feat(runtime): request-scoped secret carrier (context.secrets) Add SECRETS_CONTEXT_KEY + extract_request_secrets, centralising the context.secrets carrier contract. The existing context passthrough (build_run_config -> _build_runtime_context) already carries the sub-key to runtime.context without mirroring it into configurable; characterization tests lock that behaviour. Part of #3861. * feat(skills): inject declared secrets at slash-activation into bash env Binding point A: when a skill is slash-activated, SkillActivationMiddleware resolves its declared required-secrets against the request's context.secrets and writes the per-run injection set to runtime.context. The bash tool forwards that set to execute_command(env=). A skill cannot harvest a host platform credential (is_host_platform_secret guard, cf. GHSA-rhgp-j443-p4rf), and injected values are redacted from bash output (mask_secret_values) so an echoed secret never re-enters the prompt/trace. Part of #3861. * test(skills): lock the five secret leak surfaces + add trace redaction helper Regression tests assert the secret value is absent from all five surfaces: prompt (activation message), checkpoint (graph state vs context separation), audit (journal records names only), trace (metadata builder never copies context; never mirrored to configurable), and stdout (mask_secret_values). Add redact_secret_context_keys as a defensive helper for any context serialization. Part of #3861. * docs(backend): document request-scoped secrets for skills Add Request-Scoped Secrets subsection (Skills) + env policy note (Sandbox) and the execute_command(env=) signature change, per the doc-sync policy. Part of #3861. * fix(skills): close gaps found by end-to-end verification of request-scoped secrets Real-gateway e2e + independent review of #3861 surfaced three defects, now fixed: 1. Slash activation never fired in the live chain. InputSanitizationMiddleware wraps user input in BEGIN/END markers before SkillActivationMiddleware sees it, and the original text was only preserved when an upload or IM channel set it. For a plain text message the slash command became undetectable, so no secret was ever resolved. Fix: the sanitizer now setdefaults the pre-wrap text into ORIGINAL_USER_CONTENT_KEY (additive; sanitization behaviour unchanged), so slash activation works for all messages. Pre-existing latent bug surfaced here. 2. The raw request config (with context.secrets) was persisted to runs.kwargs_json and echoed by the run API (RunResponse.kwargs). Fix: redact_config_secrets() strips secret-bearing context keys from the persisted/echoed copy in start_run; the live config that drives the run keeps them. build_run_config now also sets configurable.thread_id on the context path (the checkpointer requires it). 3. Connection-string credentials (DATABASE_URL, REDIS_URL, SENTRY_DSN, GH_PAT, ...) were not scrubbed from the inherited sandbox env. Fix: env_policy adds a *DSN* pattern plus an explicit connection-string denylist (no blanket *URL* — benign service URLs stay readable). Verified end-to-end via a real gateway run (real LLM + skill activation + bash): the secret reaches the sandbox subprocess and appears in NONE of prompt, trace, checkpoint, audit, stdout, runs.kwargs_json, or the run API. Part of #3861. * docs(backend): document the env scrub, persistence redaction, and sanitizer interaction Sync the Request-Scoped Secrets section with the verification-driven fixes: inherited-env scrub (incl. connection-string denylist), run-record/run-API redaction as the 6th sealed leak surface, and the sanitizer preserving original content so slash activation fires. Part of #3861. * fix(skills): inject caller secret over scrubbed host value; drop redundant host-name guard A real-world demo (a skill calling a third-party cloud API with a request-scoped key) exposed that the is_host_platform_secret guard was both wrong and harmful: it refused to inject a caller-supplied secret whenever a same-named variable existed in the Gateway env — which is exactly the #3861 use case (a per-user key overriding a shared platform key). The guard was also redundant: build_sandbox_env already scrubs secret-looking names from the inherited env before injection, so a skill can never read a host credential — it only ever receives the caller's value. Remove the guard; the injected (caller) value simply wins over the scrubbed host value. Verified end-to-end: the agent called the real cloud API successfully with the caller's key, the host's same-named key was scrubbed and never used, and the caller's key leaked to none of the surfaces. Part of #3861. * fix(skills): address review on request-scoped secrets (#3861) Review fixes from PR #3871: - E2BSandbox.execute_command now accepts env/timeout and routes them to commands.run(envs=, timeout=). The bash tool passes env= unconditionally, so the prior signature (command only) raised TypeError on every e2b bash call and broke e2b deployments entirely. env=None stays backward-compatible. - SkillActivationMiddleware clears the active-secret set before resolving each activation, so a later skill in the same run never inherits an earlier skill's injection set (the #3861 contract: a skill only receives what the caller supplied AND that skill declared). - AioSandbox env path uses a dedicated _DEFAULT_HARD_TIMEOUT — bash.exec exposes no idle/no-change timeout, so the prior reuse of the legacy idle constant conflated wall-clock vs idle semantics. The env path also retries on the ErrorObservation signature now, sharing the legacy persistent-shell recovery contract. - mask_secret_values skips values below a minimum length floor so a short declared secret (e.g. "42") cannot shred unrelated bytes (exit codes, timestamps, sizes) of tool output. The secret is still injected into the subprocess; only the output mask skips it. session_id reuse on the env path is intentionally NOT added: a shared session could let request-scoped secrets ride the session env into later commands, which the SDK does not contractually forbid. The fresh-session choice matches the LocalSandbox model (each call is a fresh subprocess); the trade-off (consecutive env-bearing calls do not share cwd/venv/exports) is documented on _execute_with_env. |
||
|
|
e5424cbab9
|
feat(sandbox): add E2B sandbox provider (#3883)
Adds ``deerflow.community.e2b_sandbox.E2BSandboxProvider`` with parity to AioSandboxProvider: metadata-keyed per-thread persistence, server- side idle timeout, warm-pool reclaim with liveness checks, /mnt/user-data bootstrap symlinks, dead-sandbox auto-rebuild, and release-time mirror of agent outputs back to the host artifact directory. Signed-off-by: joey <zchengjoey@gmail.com> |