Agent Sandbox Registry

python-sdk

Review Log: Fine-Grained Capabilities, Client/Server Alignment, Base Image

中文

docs/en/archive/fine-grained-capabilities-review.md

Review Log: Fine-Grained Capabilities, Client/Server Alignment, Base Image

Status: archive. Not the current specification.

Reviewed documents: fine-grained-capabilities.md, client-server-alignment.md, base-image-and-server-sdk.md. Reviewed artefact: Draft v1 (frozen copy of the three documents taken before Round 1). Outcome: v2 (reviewed) - every finding below is either resolved in the design docs (section "Changes after review" at the end of each doc), or explicitly deferred with a reason.

0. Method and honesty note

The brief asked for two independent adversarial rounds by fresh subagents with different lenses. The authoring environment exposed no subagent-spawn facility, so the rounds were executed by the author as separate, sequential passes with a deliberately hostile role per lens, each starting from the frozen Draft v1 and an explicit checklist, not from the author's intent. This is weaker than a truly independent reviewer (shared blind spots are possible), so two mitigations were applied:

A human or independent-agent third round is recommended before a GA release (listed in the open questions of the final report).

Severity: H = would ship a vulnerability or a hard incompatibility, M = design gap that would cause bugs or user pain, L = polish.

1. Round 1 - Functional completeness and UX/API ergonomics

1a. Lens: functional completeness ("what scenario cannot be expressed or is ambiguous?")

# Sev Finding Resolution
R1-F1 M The headline scenario ("expose only upload + download, download only from /home/user/out") needed 12 lines of JSON; nothing in the draft made it a one-liner. transfer-only preset and ServerPolicy.only(*endpoint_ids, **rule) are now normative (3.4a): only = "*": {enabled: false} + enabling listed IDs, core endpoints always stay. Tests: test_policy_config.py (presets), test_policy_paths.py::TestTransferOnlyScenario.
R1-F2 M Multi-path endpoints (files_move, archive, search roots) were not specified: is read_only checked against source or destination? Which op class does each path get? Normative table of path roles (3.5): files_move -> src is delete, dst is write; both must pass allow/deny/read-only. files_archive -> root is read, every member re-checked. Tests: test_policy_paths.py::TestDirectoryExpandingEndpoints::test_move_out_of_scope_denied_both_directions, test_policy_pipeline.py::TestScopes::test_move_requires_both_write_and_delete.
R1-F3 H Limits were only enforced via Content-Length. A lying/absent length (chunked) streams unbounded data into files_upload_stream. Request bodies are bounded by the per-endpoint max_request_bytes (default: the transfer cap) applied to the declared Content-Length before the body is read, and max_bytes bounds the decoded payload; chunked Transfer-Encoding is rejected (400) in policy mode. Partial files are removed on abort. Tests: test_http_hardening.py::TestBodyLimit::test_lying_content_length_cannot_exceed_cap, test_policy_paths.py::TestTransferOnlyScenario::test_upload_size_limit. Implementation note: the final implementation does not use a counting reader; the cap is applied to the declared Content-Length before the body is read (the body is read with exactly that length, so a lying length cannot exceed the cap), and Transfer-Encoding is rejected in policy mode (test_transfer_encoding_rejected_in_policy_mode).
R1-F4 M Custom routes (RouteTable.route(...)) and custom commands could not take part: no way to declare scopes, no PathGuard for handlers. RouteInfo.scopes; register(..., scopes=[...], endpoint_id=...); handlers get request.context["policy"] (an EndpointContext with guard(path, op)). Catalogue-less routes default to scope custom:<name>-free (callable by * identities) unless scopes are declared. Test: test_policy_pipeline.py::TestScopes::test_custom_route_scopes.
R1-F5 M capabilities_check appeared in the scope table but had no contract about being an oracle (must not touch the filesystem). Contract: policy-only evaluation, no stat; response never says whether a path exists. Tests: test_policy_pipeline.py::TestDiscovery::test_check_does_not_reveal_existence, test_check_endpoint_matches_enforcement.
R1-F6 M Policy reload / mutability unspecified. Policy is immutable after start; revision is a content hash; operators restart (documented in 4 "Configuration"). Runtime with_endpoint creates a new policy object.
R1-F7 M Per-identity path scopes (token A may read /out, token B may read /in) cannot be expressed; rules are global. Deferred (documented non-goal for v1). Rationale: identity x path matrix doubles the schema; the supported way is separate sandboxes or custom scopes plus separate endpoints. Recorded as open question OQ-2.
R1-F8 L Defaults for unset limits were "today's constants" but not enumerated. Table of legacy constants added (3.6): 10 MiB JSON body, 100 MiB legacy /download, 10 MiB /upload (ABX_MAX_UPLOAD_SIZE), search 200 results, etc.
R1-F9 L files_watch requires process-like long-lived connections; no max duration/concurrency. limits.max_duration_seconds (default 300) + max_concurrency.

1b. Lens: UX / API ergonomics ("what would trip a new user?")

# Sev Finding Resolution
R1-U1 M Typos in policy ("alow", "limit") silently become no-ops and look like security holes. Closed vocabulary; unknown keys are PolicyConfigError with a did-you-mean hint and the JSON path (endpoints.files_list.paths.alow). Test: test_policy_config.py (unknown keys / did-you-mean cases).
R1-U2 M Unknown endpoint IDs in endpoints (e.g. file_download) were silently ignored. An exact selector matching no registered route is an error (globs matching nothing are warnings in check-config).
R1-U3 M Which error should a caller catch? Draft had a flat list. Client exception tree finalised (section 7): everything derives from ServerAPIError; ServerAPIError carries .status_code, .error_type, .error_code, .endpoint and PolicyDeniedError adds .scope; legacy except PermissionDeniedError still catches file-policy denials.
R1-U4 M Pre-check "fail fast" could be surprising when the cache is stale (policy changed, client says 403 locally). Pre-checks happen only after an explicit discover(); server stays authoritative; As implemented: client.forget_discovery() drops the cache and discover() refreshes it (no prechecks=False switch: not calling discover() is the opt-out); the server's own denial is always what the caller finally sees.
R1-U5 L Need a debugging aid: "why was my call denied?" ServerPolicy.explain(...) (server side) and client.check(...) / caps.check_path(...) -> POST /capabilities/check; agent-sandbox-server check-config --explain files_download_stream --path /etc/passwd.
R1-U6 L Size strings (5MB) ambiguous between MB and MiB. Decimal vs binary spelled out: MB=10^6, MiB=2^20; bare ints are bytes; documented and tested.
R1-U7 L Sandbox.server vs the decorator proxy name. Verified no clash (Sandbox has no server attribute today); kept Sandbox.server (port 9000, lazy, cached) + Sandbox.server_at(...).
R1-U8 L Sync users: need _sync twins for every method. Mandatory in alignment doc; completeness test iterates all async methods.
R1-U9 L ServerPolicy.only("download") -- legacy route IDs upload/download vs stream IDs are easy to confuse. only() accepts the alias group token "transfer" meaning {upload, download, files_upload_stream, files_download_stream}.

Round 1 outcome: design updated (sections 3.4a, 3.5 roles table, 3.6, 7, 8); no finding rejected except R1-F7 (deferred).

2. Round 2 - Security bypass and backward compatibility

2a. Lens: security bypass ("assume the policy author is competent and I am hostile")

Evidence from the probe against the unmodified server is marked [probe].

# Sev Finding Resolution
R2-S1 H [probe] files_archive (zip) follows file symlinks: a symlink in the sandbox dir pointing to /etc/passwd is archived with its content. Any path allow-list built on the existing code would be bypassable via the archive endpoint. Pre-existing vulnerability. Archive refuses (skips) symlinks whose target is outside the endpoint boundary or denied; never follows directory symlinks. This is a bug fix applied in all modes (documented in CHANGELOG as security fix). Test: test_policy_paths.py::TestArchiveSymlinkLeakFix.
R2-S2 M [probe] files_search/files_list show names of symlinked entries pointing outside. Names leak topology. In policy mode entries failing the guard are omitted (never descended). Legacy mode unchanged. Test: test_policy_paths.py::TestDirectoryExpandingEndpoints::test_list_omits_denied_and_escaping_entries.
R2-S3 H [probe] A non-numeric Content-Length crashes _read_json_body with an unhandled ValueError (connection dropped, no response); a negative value is parsed as JSON error. A hostile client can use it for desync experiments and the 500 path leaks nothing but is untested. Strict parsing: ^\d+$, else 400; negative -> 400; duplicate differing Content-Length -> 400; Transfer-Encoding present -> 400 on body endpoints. Applies in all modes (pure bug fix; previous behaviour was an abort). Test: test_http_hardening.py::TestContentLength.
R2-S4 M [probe] Duplicate query parameter (?path=/ok&path=/etc/passwd): server takes the first, while an intermediary (gateway/WAF) may take the last -> allow-list decisions made upstream differ. In policy mode duplicated path-bearing params are rejected (400 duplicate_parameter). Legacy mode keeps "first wins". Tests: test_http_hardening.py::TestPolicyModeFraming::test_duplicate_query_param_rejected_in_policy_mode and test_duplicate_query_param_tolerated_when_unconfigured (rejection is policy-mode only).
R2-S5 M [probe] Existence-oracle ordering: in-boundary missing path -> 404 "File not found", out-of-boundary -> 400/403, so a caller can map the filesystem outside its allow-list. In policy mode the boundary/deny check always runs before any stat, and outside-boundary returns the same 403 path_not_allowed regardless of existence. Test: test_policy_paths.py::TestNoExistenceOracle.
R2-S6 L [probe] Relative paths resolve against the server process CWD, not base_dir; with paths.allow that is a foot-gun. Kept for legacy mode (compat) but relative paths are resolved against the first allow root in policy mode (documented). Test: test_policy_paths.py::TestDirectoryExpandingEndpoints::test_relative_paths_resolve_against_allow_root.
R2-S7 H Policy attached to the URL string rather than the matched route would allow encoded-route and trailing-slash discrepancies. Draft already keys by RouteInfo; reviewer verified the router: %73tat -> 404, //files -> 404, trailing slash normalised before matching. Confirmed: policy key = route.name only; regression tests for encoded/doubled/trailing-slash routes; method override headers/params explicitly ignored. Tests: test_http_hardening.py::TestMethodOverride, TestRouteNormalisation.
R2-S8 H TOCTOU / symlink swap: realpath check then open. An attacker with process:run can race a swap. O_NOFOLLOW on the final component; after open compare fstat identity with /proc/self/fd/N resolution and re-run the boundary check on it; symlinks: never additionally rejects any symlink component (lstat walk). Residual risk documented: intermediate-component swap between check and open can't be fully closed in pure-stdlib Python; mitigation is not exposing process/terminal alongside path scoping (lint L1).
R2-S9 M Hardlinks inside the allowed tree to outside inodes are invisible to path checks. Residual risk documented (only creatable via process endpoints; covered by lint L1). files_* endpoints can't create hardlinks.
R2-S10 M Deny globs: case variants, ./, trailing-slash, and matching against the lexical vs resolved path. Deny is evaluated on the resolved path and the normalised lexical path; matching is case-sensitive on POSIX (documented; macOS dev machines note); basename patterns (*.pem) match any component; directory denies are prefix-with-separator matches (/a/b does not deny /a/bc). Tests with each variant.
R2-S11 H Token confusion: draft allowed Authorization: Bearer and X-Access-Token simultaneously; behaviour with two differing X-Access-Token headers undefined. Identities mode: more than one distinct credential header value -> 400 ambiguous_credentials; Bearer wins only when X-Access-Token absent or equal (draft said "Authorization is used" - changed: gateways inject their own X-Access-Token; ordering is Authorization first, documented). Legacy mode untouched (first X-Access-Token).
R2-S12 M Timing side-channel on token lookup with several tokens. Digest-then-compare_digest over all tokens with no early exit (kept); a property of the implementation, not covered by a timing test.
R2-S13 H Secrets exposure: GET /env (system group) returns the whole environment including ABX_SERVER_TOKEN and other identities' token_env; child processes inherit them. In policy-identities mode env_get redacts the server's own secrets (ABX_SERVER_* and every configured token_env name); the child env is scrubbed of the same names only (arbitrary user variables are never touched). env_set refuses those names and the ABX_ prefix. Legacy mode unchanged (compat) but lint recommends policy. Tests: test_policy_pipeline.py::TestSecrets, test_policy_exec.py::TestSecretEnvironment.
R2-S14 M PTY WebSocket (port 9001) has its own auth (?token= in URL: token lands in access logs and gateway logs) and would bypass endpoint policy. PTY server consults the same policy: identity from Authorization: Bearer / X-Access-Token header (new) or the legacy query token; requires scope terminal:use; endpoint pty_connect rule enabled:false closes the socket with 4003. Query token still accepted (compat) but flagged by the discovery warnings. Test: test_policy_pty.py::TestPtyWebSocketAuth.
R2-S15 M Policy file integrity: a writable policy file lets the in-sandbox user relax the policy. agent-sandbox-server check-config (lint L5) warns if the policy file or its directory is writable by the current user (run it as the server's user); documented to place it root-owned 0644 in /etc/agent-sandbox/.
R2-S16 M /proc, /sys, /dev exposure when allow roots are broad (/); /proc/self/environ leaks tokens. Lint L2 on allow root /; hardened preset denies /proc, /sys, /dev, **/.ssh, **/.env*, **/*.pem, **/id_*.
R2-S17 L Very long paths / glob selector CPU (ReDoS). Path length cap 4096; selectors use fnmatch over short IDs; deny patterns are fnmatchcase with no regex from user input.
R2-S18 L Audit log injection through path strings. JSON-encoded lines (escaping by construction); paths truncated to 512 chars.
R2-S19 L Brute-forcing tokens: no failed-auth throttle. Deferred: gateway-fronted deployment; per-IP throttling in a stdlib server is unreliable behind a gateway. Documented; constant-time comparison + high-entropy token guidance in docs.
R2-S20 L Unicode look-alikes / normalisation. POSIX paths are bytes without normalisation, so look-alikes name different files; no confusable bypass. Documented (macOS HFS+ normalisation caveat).

2b. Lens: backward compatibility ("what existing user breaks?")

# Sev Finding Resolution
R2-C1 H Existing tests (test_env_subprocess_boundary.py) wire ThreadingHTTPServer(..., SandboxRequestHandler) and set only auth_token and registry. New attributes (policy, audit) must not be required. Handler reads every new attribute through getattr(self.server, ..., None); None policy = legacy pipeline. Test: existing suite + test_policy_compat.py::test_bare_handler_wiring_still_works.
R2-C2 M RouteInfo is a public dataclass constructed positionally by third parties; inserting a field breaks them. New fields appended after existing ones with defaults; RouteTable.register keyword-only additions.
R2-C3 M GET /capabilities consumers may assert exact keys. Existing keys unchanged (groups remains), additions are new keys; pinned by an exact-superset test.
R2-C4 M Error bodies gain code; clients doing equality on the body dict break. Existing keys/values unchanged, code only added where a stable code exists; documented as additive.
R2-C5 M ABX_SERVER_DISABLED_GROUPS precedence vs policy groups. Superseded: the variable was deleted. One layer (the endpoint rule) decides; ABX_SERVER_ENABLED_GROUPS is the only group variable and can only turn groups on.
R2-C6 M Image() default base: draft tied default to ubuntu; changing to official base would silently change every build. Default unchanged; new Image.official_base(); DockerBuilder emits a warning (not error) when the base isn't official (3 in base-image doc).
R2-C7 M Server must remain stdlib-only and Python 3.10+: no tomllib (3.11), no PyYAML hard dependency. JSON-only parser in the server; YAML accepted only if PyYAML importable; no new dependencies. Verified by the server-only import test (server-sdk/tests/test_server_only_import.py).
R2-C8 M Lockstep versions: new client against old server (and vice-versa). Client treats a missing endpoints key as legacy and never sends new parameters (cwd, env, ...) unless the caller sets them; new endpoints on an old server surface as EndpointNotAvailableError (404 mapped), never a raw HTTP error. Test: client test with a stub legacy server.
R2-C9 L SandboxTemplate strictness: adding server: must not reject current templates; server: content must be validated lazily to keep the client free of server imports. Optional field with dict schema validated by a small structural validator in the client (shared vocabulary constant), not by importing the server package.
R2-C10 L Existing enable_builtin/_enabled_builtins legacy toggles. Superseded: deleted. enable_group / disable_group edit the code layer of the endpoint rules.

Round 2 outcome: two H security items were pre-existing bugs (R2-S1, R2-S3) and are fixed in all modes (not only policy mode) because they are strictly bug fixes - this is the only place "unconfigured behaviour" changes, and in both cases the old behaviour was a leak / crash. All other changes are gated on a policy being configured.

3. Review of the alignment and base-image documents

Both were reviewed in the same two rounds. Material findings:

# Doc Finding Resolution
R-A1 alignment process_output and process_stdin were proposed as server additions without saying how they interact with capture_output limits. Spec added: bounded ring buffer (limits.max_output_bytes, default 1 MiB per process), cursor-based reads.
R-A2 alignment Client ServerClient.files.download vs the envd-backed Sandbox.files.read look interchangeable to users. Documented in the matrix (section 5) with a decision table: envd is the default for code that must work on any sandbox; ServerClient for template-authored policy-scoped access.
R-A3 alignment Streaming methods need cancellation semantics. Async generators close the response on aclose(); sync twins return iterators with close().
R-B1 base-image Decision "warranted" was backed mostly by inference about the gateway; the claim "gateway forwards Authorization" is unverified. Marked as open question OQ-3; the server accepts both headers so either gateway behaviour works.
R-B2 base-image Image() default change (security of existing builds). See R2-C6.
R-B3 base-image Console script agent-sandbox-server must not shadow abx. Name checked: no clash.
R-B4 base-image Reviewer attack: check-config/healthcheck subcommands must not need network or heavy imports (used as Docker HEALTHCHECK). healthcheck uses only urllib.request against 127.0.0.1, 2s timeout, exit code 0/1.

4. Test traceability

Server (server-sdk/tests/test_server/): test_policy_config.py (54: schema, presets, sizes, unknown keys), test_path_guard.py (34: traversal, symlinks, encodings, NUL, long paths, deny globs), test_policy_pipeline.py (38: auth, scopes, command scopes, rate/concurrency, audit, discovery, secrets), test_policy_paths.py (30: the transfer-only scenario, directory-expanding endpoints, read-only, symlinks: never, archive leak fix), test_policy_exec.py (16: command/ language/URL rules, secret env), test_http_hardening.py (18: Content-Length, framing, method override, route normalisation, body cap), test_new_endpoints.py (15: new/extended endpoints), test_policy_pty.py (7), test_policy_compat.py (5), test_cli_main.py (8).

Client (client-sdk/tests/): test_api/server_e2e/test_server_client_e2e.py (33, real in-process SandboxServer on a free port), test_server_client_policy.py (18), test_server_client_models.py (23: error mapping, models, capabilities, legacy stub server), test_server_client_matrix.py (4: route-completeness guard), test_sandbox_server_helpers.py (12), test_api/test_base_image.py (52), test_models/test_template_server_block.py (18).

Findings for which no dedicated test exists: R2-S17 (path-length / glob CPU) is covered only by the long-path case in test_path_guard.py; R-B4 (healthcheck) is covered by test_cli_main.py.

5. Open questions carried forward