test(sandbox): add per-user isolation regression tests for virtual path contract#3087
Open
64johnlee wants to merge 2 commits into
Open
test(sandbox): add per-user isolation regression tests for virtual path contract#308764johnlee wants to merge 2 commits into
64johnlee wants to merge 2 commits into
Conversation
…th contract (bytedance#3024) The existing virtual-path contract tests verified that LocalSandbox honours /mnt/user-data/... paths for per-thread sandboxes, but none of them exercised user-id scoping: two users sharing the same thread_id must map that virtual path to distinct host directories under users/{user_id}/threads/{thread_id}/user-data/. Add three new tests to the existing suite: - test_per_user_isolation_different_users_same_thread: acquires the same thread_id as alice and then bob; verifies their workspace host paths differ and each contains the correct user_id component. - test_per_user_workspace_maps_to_user_scoped_directory: verifies the workspace virtual path resolves under users/carol/threads/t-carol/. - test_user_a_writes_invisible_to_user_b_on_same_thread_id: writes a file as user-a and confirms it is not readable in user-b's view. These tests answer the question in bytedance#3024 ("Does the new sandbox architecture still need the virtual path contract tests?") and pin down the per-user isolation invariant that bytedance#2873 originally introduced. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds regression coverage to ensure LocalSandboxProvider continues to honor the public /mnt/user-data/* virtual path contract with per-user isolation, preventing refactors from accidentally merging host directories across users sharing the same thread_id.
Changes:
- Adds tests asserting
/mnt/user-data/workspacehost mappings differ byuser_ideven whenthread_idis the same. - Adds a test asserting the resolved workspace path reflects both
user_idandthread_idscoping. - Adds a test intended to ensure user A’s workspace writes are not visible to user B (but currently has an issue noted in comments).
Comment on lines
+434
to
+438
| try: | ||
| content = sandbox_b.read_file("/mnt/user-data/workspace/secret.txt") | ||
| assert "user-a-data" not in content | ||
| except Exception: | ||
| pass # expected: user-b has no such file |
|
|
||
|
|
||
| # ────────────────────────────────────────────────────────────────────────── | ||
| # 6. User-id scoping: per-user path isolation contract (#3024 / #2873) |
Comment on lines
+385
to
+390
| def test_per_user_isolation_different_users_same_thread(provider, isolated_paths, monkeypatch): | ||
| """Two different users acquiring the same thread_id must get distinct host paths.""" | ||
| monkeypatch.setattr("deerflow.runtime.user_context.get_effective_user_id", lambda: "alice") | ||
| provider.acquire("thread-shared") | ||
| alice_path = _get_workspace_local_path(provider, "thread-shared") | ||
|
|
- Rename duplicate section header # 6 → # 7 (User-id scoping) - Fix test_user_a_writes_invisible_to_user_b: use try/except FileNotFoundError/else so AssertionError is no longer swallowed — isolation failures now propagate
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #3024
Problem
The virtual path contract (
/mnt/user-data/{workspace,uploads,outputs}→ per-user/per-thread host directories) had no regression tests covering per-user isolation. Refactors toLocalSandboxProvidercould silently break the contract by merging host directories across users sharing the samethread_id.Fix
Extended
tests/test_local_sandbox_virtual_path_contract.pywith 3 regression tests (new section 7) covering per-user path isolation:test_per_user_isolation_different_users_same_thread— two differentuser_ids acquiring the samethread_idmust get distinct host pathstest_per_user_workspace_maps_to_user_scoped_directory— resolved workspace path must contain bothuser_idandthread_idcomponentstest_user_a_writes_invisible_to_user_b_on_same_thread_id— a file written by user A must not appear in user B's sandbox view of the samethread_id🤖 Generated with Claude Code