fix(sandbox): widen boxlite/aio_sandbox tenant hash and verify identity on reclaim - #4171
Conversation
github-actions
Bot
added
area:docs
willem-bd
left a comment
willem-bd
left a comment
There was a problem hiding this comment.
Reviewed against PR head (1bf7d77) and the current codebase; ran both suites against the PR's code (73 passed) and probed the residual-collision path empirically (probe since removed, working tree verified clean).
Verdict: approve. The 64-bit widening closes the real production bug (32-bit ~77k-pair birthday bound), and the identity check is a sound reclaim-path backstop that correctly handles the release/acquire inter-check race (re-checked immediately before promotion). _thread_key ordering is consistent between acquire and release; reset() (Boxlite) correctly preserves identity; AIO has no reset() so no parity gap.
The substantive inline comment (on both providers) documents a residual persistent-collision leak that is production-moot (needs a real 64-bit collision) but currently untested - the main actionable is extending the forced-collision test through A.re-acquire-while-B-active so the boundary is documented and regression-safe. The rest are minor nits.
area:docslabel looks wrong - this is a security code fix (2 provider files + 2 test files + doc), not docs-only. Same mislabel seen on PR #4025; should be the sandbox/security area.
764fe3f to
32a4106
Compare
There was a problem hiding this comment.
Pull request overview
This PR hardens the BoxLite and AIO sandbox warm-pool reclaim logic against deterministic-ID collisions by widening the truncated SHA-256 tenant hash (8 → 16 hex chars) and introducing explicit identity tracking/checks so a warm entry can’t be silently reclaimed by the wrong (user_id, thread_id).
Changes:
- Widen deterministic sandbox IDs in both
BoxliteProviderandAioSandboxProviderfrom 32-bit to 64-bit truncation. - Add per-sandbox identity tracking for both active and warm-pool entries, and enforce identity checks during reclaim/promote.
- Expand unit tests (and a few helper provider constructors in tests) to cover the widened-ID behavior and identity/collision handling.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| backend/packages/harness/deerflow/community/boxlite/provider.py | Widen BoxLite deterministic IDs; add warm/active identity tracking and collision detection during warm reclaim and creation. |
| backend/packages/harness/deerflow/community/aio_sandbox/aio_sandbox_provider.py | Widen AIO deterministic IDs; add warm/active identity tracking and collision checks/handling across reclaim/discover/create paths. |
| backend/tests/test_boxlite_provider.py | Update existing warm-pool tests for new reclaim signature and add tenant-isolation/collision coverage. |
| backend/tests/test_aio_sandbox_provider.py | Seed identity maps in test providers and add tenant-isolation/collision coverage. |
| backend/tests/test_sandbox_orphan_reconciliation.py | Initialize new identity maps in reconciliation test provider setup. |
| backend/tests/blocking_io/test_sandbox_release.py | Initialize new AIO identity maps in blocking-IO release test provider setup. |
| backend/tests/blocking_io/test_aio_sandbox_get.py | Initialize new AIO identity maps in blocking-IO get test provider setup. |
32a4106 to
12fbd25
Compare
…ty on reclaim (bytedance#4171) * fix(sandbox): prevent truncated tenant ID reuse * fix(sandbox): handle late same-tenant box registration
…ty on reclaim (bytedance#4171) * fix(sandbox): prevent truncated tenant ID reuse * fix(sandbox): handle late same-tenant box registration
…ty on reclaim (bytedance#4171) * fix(sandbox): prevent truncated tenant ID reuse * fix(sandbox): handle late same-tenant box registration
Summary
(user_id, thread_id)identity for active and warm entriesOlder 8-character containers are not reused after rollout and remain eligible for normal orphan cleanup. Startup-adopted warm entries have unknown identity until their first successful reclaim; subsequent reclaims enforce the recorded identity.
Tests