fix(coding): virtualize the workspace root - #5927
Closed
serrrfirat wants to merge 7 commits into
Closed
serrrfirat wants to merge 7 commits into
serrrfirat wants to merge 7 commits into
Conversation
Contributor
🔎 IronLoop Review StatusHead: Current reviewers:
Reviewer summaries
Recent activity
Available commands
Run metadataAdmission: webhook accepted the request and IronLoop persisted reviewer state before this projection. |
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5927
July 10, 2026 10:46
Destroyed
github-actions
Bot
added
size: M
📝 WalkthroughSummary by CodeRabbit
WalkthroughCoding filesystem operations now handle virtual and empty mount roots consistently. Local process execution and local-development capability wiring derive scoped mounts, validate trusted host paths, and prevent non-owner workspace escapes. ChangesMount-scoped filesystem execution
Estimated code review effort: 5 (Critical) | ~100 minutes Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for handling empty mount roots that are not yet materialized on the filesystem (e.g., in libSQL backends). It replaces direct filesystem stat and list operations with stat_or_empty_mount_root and list_dir_or_empty_mount_root helpers, ensuring that reading an unmaterialized mount root returns an empty result rather than a NotFound error. Additionally, tests have been added to verify this behavior. There are no review comments, and I have no feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![ironloopai[bot]](https://avatars.githubusercontent.com/in/4116490?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
❌ IronLoop Review: reviewer
Review at a glance
| Verdict | Blocking | Notes | Inline | Head |
|---|---|---|---|---|
| ❌ Changes requested | 1 | 0 | 1 | c2d258c9e11d |
Head: c2d258c9e11df3b78c2d4137f289ecc89cc697b5
Next: Fix the blocking findings, push the PR branch, then re-run this reviewer.
Run details
Status: Current
Needs human: no
Needs validation: no
Summary
Found one permission regression in the new empty mount-root handling for grep.
Findings
Blocking: 1 / Notes: 0
Blocking findings
1. ❌ [MEDIUM] Preserve list permission for empty grep roots
Location: crates/ironclaw_first_party_extensions/src/coding/grep_tool.rs:48-52
stat_or_empty_mount_root now returns Ok(None) for an unmaterialized mount root, but grep only enforces ListDir when the stat result is Some(Directory). A caller with read: true and list: false can therefore grep / or /workspace against an empty/uncreated workspace and receive a successful empty result instead of the authorization failure enforced for materialized directories. Treat None as a logical directory for this permission check and add a regression test for the empty-root case.
Developer follow-up
After fixing this feedback:
- Push the fix to this PR branch.
- Re-run this reviewer with
@ironloopai review --agent reviewerif you only changed this reviewer's findings. - Re-run all reviewers with
@ironloopai reviewwhen the fix may affect multiple areas. - Use
@ironloopai statusto check queued/running/completed/failed/superseded state while reviewers run.
| )); | ||
| } | ||
| if root_stat.file_type == FileType::Directory | ||
| if root_stat |
Contributor
There was a problem hiding this comment.
With the new empty-root path, root_stat can be None, which represents a logical directory. This skips the ListDir permission check below, so read: true, list: false callers can successfully grep an unmaterialized workspace root. Please treat None as a directory for the list-permission check.
Contributor
Coverage ratchetReborn integration-tier coverageLine coverage (Reborn crates): 85.1% — 286846 / 337087 lines Per-crate breakdown (63 crates, lowest-covered first)
This table itself is informational and never gates the PR on its own — not the percentage, not the per-crate holes, not the 0-coverage callout. A separate coverage ratchet (dry-run until enforce=true; see tests/integration/coverage-floor.toml) can fail the build on specific configured floors. Exemptions (3 entry/entries excluded from the accounting above)
|
|
🚅 Deployed to the ironclaw-pr-5927 environment in ironclaw-ci-preview
|
17 of 30 tasks
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5927
July 10, 2026 11:28
Destroyed
github-actions
Bot
added
size: XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_host_runtime/tests/first_party_builtin_tools.rs (1)
3433-3453: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a decomposition issue for this test file.
crates/ironclaw_host_runtime/tests/first_party_builtin_tools.rsis 9,522 lines, far past the repo’s 3,000-line budget for touched files.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_host_runtime/tests/first_party_builtin_tools.rs` around lines 3433 - 3453, Decompose the oversized first_party_builtin_tools test file to satisfy the 3,000-line touched-file budget. Move the shell-related tests, including builtin_shell_rejects_scoped_mount_workdir_without_trusted_mount_source and their shared helpers/imports, into a focused test module or file, updating module references and test organization as needed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/ironclaw_capabilities/src/host.rs`:
- Around line 99-115: Empty scoped mount views currently override
execution-context mounts in both spawn paths. Update the mount selection logic
around the spawn handling near `effective_dispatch_mounts()` call sites to treat
`Some(MountView)` with an empty `mounts` collection like `None`, falling back to
`request.context.mounts.clone()`; apply the same empty-check consistently in
both paths.
In `@crates/ironclaw_host_runtime/src/first_party_tools/shell.rs`:
- Around line 223-238: Preserve diagnostics in the workdir authorization path by
binding and logging the errors before converting them. Update the `scoped_path`
and `resolve_with_grant` calls to use error bindings (such as `|err|`), log each
underlying error with appropriate security-gate context, then return the
existing `FirstPartyCapabilityError` mappings and permission checks unchanged.
In `@crates/ironclaw_host_runtime/src/process_port.rs`:
- Around line 528-604: Static command validation in
validate_raw_mount_source_paths and validate_relative_mount_source_paths can be
bypassed by shell expansions before execute_local_command runs sh -c. Reject
mount-sensitive shell expansions (such as command substitution and variable
expansion) or add execution-time path containment checks after expansion,
ensuring all resolved paths remain within the scoped root and violations fail
with disallowed_mount_source_path.
In `@crates/ironclaw_reborn_composition/src/runtime/local_dev.rs`:
- Around line 220-228: Update host_api_agent_loop_error to use fixed
host-authored text for safe_summary, preserving the raw error only in detail. In
the non-owner branch of local_dev workspace mount selection, replace hardcoded
MountPermissions::read_write() with the configured workspace grant permissions
used by workspace_mounts_for_gate in approval.rs.
In `@crates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rs`:
- Around line 242-268: Extract the repeated LocalDevLoopCapabilityPortFactory
construction from the three shell tests into a shared build_shell_factory
helper, accepting runtime, local_runtime, fallback_user_id, and capability I/O
dependencies such as policy, mounts, input_resolver, and result_writer as
needed. Replace each inline factory literal, including
local_dev_yolo_shell_translates_workspace_workdir_without_scoped_mounts and the
two referenced tests, with the helper while preserving their test-specific
values and mirroring the invoke_write_file_tool extraction pattern.
---
Outside diff comments:
In `@crates/ironclaw_host_runtime/tests/first_party_builtin_tools.rs`:
- Around line 3433-3453: Decompose the oversized first_party_builtin_tools test
file to satisfy the 3,000-line touched-file budget. Move the shell-related
tests, including
builtin_shell_rejects_scoped_mount_workdir_without_trusted_mount_source and
their shared helpers/imports, into a focused test module or file, updating
module references and test organization as needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 5f38000f-e10a-4aad-87a6-72ce8d253897
📒 Files selected for processing (12)
crates/ironclaw_capabilities/src/host.rscrates/ironclaw_host_runtime/src/first_party_tools/shell.rscrates/ironclaw_host_runtime/src/process_aliases.rscrates/ironclaw_host_runtime/src/process_port.rscrates/ironclaw_host_runtime/tests/first_party_builtin_tools.rscrates/ironclaw_reborn_composition/src/factory.rscrates/ironclaw_reborn_composition/src/local_dev_mounts.rscrates/ironclaw_reborn_composition/src/runtime.rscrates/ironclaw_reborn_composition/src/runtime/approval.rscrates/ironclaw_reborn_composition/src/runtime/local_dev.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/tests.rs
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5927
July 10, 2026 14:18
Destroyed
Collaborator
Author
|
Addressed the linked review findings in e358ebc. Fixed:
Local verification:
Not changed:
|
railway-app
Bot
temporarily deployed
to
ironclaw-ci-preview / ironclaw-pr-5927
July 10, 2026 14:29
Destroyed
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/ironclaw_host_runtime/src/process_port.rs (1)
380-395: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject tilde expansion in scoped shell commands.
validate_mount_sensitive_shell_expansionsalready blocks$()/backticks and non-$PWDenv vars, but it never scans~/~user.execute_local_commandstill goes throughsh -c, and inInheritedmodeHOMEstays on the host, so tilde expansion can jump outside the mounted workspace after the scan. Add tilde handling here or move the containment check to exec-time path resolution.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/ironclaw_host_runtime/src/process_port.rs` around lines 380 - 395, The scoped command validation misses shell tilde expansions, allowing host paths through inherited HOME. Update validate_mount_sensitive_shell_expansions, or the exec-time path resolution it relies on, to detect and reject both ~ and ~user expansions using the same mount-source containment checks, while preserving existing $PWD handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/ironclaw_host_runtime/src/process_port.rs`:
- Around line 380-395: The scoped command validation misses shell tilde
expansions, allowing host paths through inherited HOME. Update
validate_mount_sensitive_shell_expansions, or the exec-time path resolution it
relies on, to detect and reject both ~ and ~user expansions using the same
mount-source containment checks, while preserving existing $PWD handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9e36633b-681f-4774-b8d1-acfc3d448868
📒 Files selected for processing (2)
crates/ironclaw_host_runtime/src/process_port.rscrates/ironclaw_reborn_composition/src/runtime/local_dev/shell_tests.rs
serrrfirat
added a commit
that referenced
this pull request
Aug 3, 2026
The standalone /fs/* browser mount (scoped_browse_mount_view) maps the
Workspace alias to a shared /projects/workspace target, so a hosted user
could list other users' workspace artifacts through the WebUI. Memory is
already caller-scoped server-side; Workspace was not.
Confine Workspace reads to the caller's own subtree
(tenants/{tenant}/users/{user}) in the WebUI /fs/{list,stat,content}
handlers when the effective scoped-projection flag is true (deployment
flag OR non-operator caller). The flag reuses the same effective value
the session endpoint reports. Operator fallback with scoped projection
off keeps the raw shared workspace root so local/single-user workspaces
stay visible.
The handlers prepend the per-caller prefix before forwarding to the
product layer and strip it from any path the product layer echoes back,
so the browser keeps round-tripping mount-relative paths and never sees
the storage ownership prefix. A cross-user path from a scoped caller
becomes tenants/<caller>/users/<caller>/tenants/.../other and 404s
inside the caller's own subtree.
Complementary to #5927, which scopes agent tool/shell workspace writes
to the same per-user subtree. This PR scopes the browser read surface.
Also addresses CodeRabbit follow-up on 03c5084:
- resolveMemoryDirectory now seeds the walk at the mount root instead of
relativePath, which doubled any non-root memory subdirectory.
- userScopedPrefix is reduced to a boolean hasCallerIdentity predicate so
no future caller can re-prepend a tenant/user path string.
- Regression test for memory subdirectory navigation.
Server-side contract tests cover: scoped workspace list prefixes the
caller subtree; non-operator is scoped even when the state flag is off;
operator fallback with the flag off keeps the raw root; scoped stat
echoes the mount-relative path.
14 of 30 tasks
serrrfirat
added a commit
that referenced
this pull request
Aug 3, 2026
The standalone /fs/* browser mount (scoped_browse_mount_view) maps the
Workspace alias to a shared /projects/workspace target, so a hosted user
could list other users' workspace artifacts through the WebUI. Memory is
already caller-scoped server-side; Workspace was not.
Confine Workspace reads to the caller's own subtree
(tenants/{tenant}/users/{user}) in the WebUI /fs/{list,stat,content}
handlers when the effective scoped-projection flag is true (deployment
flag OR non-operator caller). The flag reuses the same effective value
the session endpoint reports. Operator fallback with scoped projection
off keeps the raw shared workspace root so local/single-user workspaces
stay visible.
The handlers prepend the per-caller prefix before forwarding to the
product layer and strip it from any path the product layer echoes back,
so the browser keeps round-tripping mount-relative paths and never sees
the storage ownership prefix. A cross-user path from a scoped caller
becomes tenants/<caller>/users/<caller>/tenants/.../other and 404s
inside the caller's own subtree.
Complementary to #5927, which scopes agent tool/shell workspace writes
to the same per-user subtree. This PR scopes the browser read surface.
Also addresses CodeRabbit follow-up on 03c5084:
- resolveMemoryDirectory now seeds the walk at the mount root instead of
relativePath, which doubled any non-root memory subdirectory.
- userScopedPrefix is reduced to a boolean hasCallerIdentity predicate so
no future caller can re-prepend a tenant/user path string.
- Regression test for memory subdirectory navigation.
Server-side contract tests cover: scoped workspace list prefixes the
caller subtree; non-operator is scoped even when the state flag is off;
operator fallback with the flag off keeps the raw root; scoped stat
echoes the mount-relative path.
pull Bot
pushed a commit
to Stars1233/ironclaw
that referenced
this pull request
Aug 4, 2026
* fix(webui): scope workspace and memory views Scope WebUI Workspace and Memory presentation to the authenticated tenant/user instead of exposing shared storage paths. Fail closed for signed/non-operator sessions and for missing scoped roots, while preserving local operator fallback. Collapse internal memory wrapper directories and hide sidecar files from the user-facing view. * fix(webui): consume mount-relative /fs/* paths and gate test-only imports Address review feedback on nearai#7062 (reopened from nearai#5831): - The /fs/{list,stat,content} handlers bind the authenticated caller and return mount-relative, server-scoped paths (covered by the existing webui_filesystem_memory_mount_is_scoped_to_authenticated_user composed test). The frontend now consumes that contract directly: it no longer builds tenants/{tenant}/users/{user} prefixes for /fs/* requests. currentUser is retained only as a fail-closed identity gate when requireScopedWorkspace is true; missing identity renders empty listings and short-circuits file reads instead of requesting a raw mount root. - Default requireScopedWorkspace to true at the exported entry points (listWorkspace, readWorkspaceFile) and internal resolvers so an omitted option cannot silently enable raw-root behavior. - Keep memory collapse-walk over the caller's own mount-relative tree so the internal agents/<agent>/projects/_none wrapper directories stay hidden; all requests remain mount-relative. - Gate the three test-support-only imports in composition runtime.rs so default-feature clippy no longer reports them unused. - Update workspace-api tests to mock the server-scoped, mount-relative contract: hosted calls expect mount-relative paths, requireScopedWorkspace is explicit, and assert.rejects records/constrains rejections instead of asserting inside the fetch handler. * fix(webui): scope workspace browser to caller subtree server-side The standalone /fs/* browser mount (scoped_browse_mount_view) maps the Workspace alias to a shared /projects/workspace target, so a hosted user could list other users' workspace artifacts through the WebUI. Memory is already caller-scoped server-side; Workspace was not. Confine Workspace reads to the caller's own subtree (tenants/{tenant}/users/{user}) in the WebUI /fs/{list,stat,content} handlers when the effective scoped-projection flag is true (deployment flag OR non-operator caller). The flag reuses the same effective value the session endpoint reports. Operator fallback with scoped projection off keeps the raw shared workspace root so local/single-user workspaces stay visible. The handlers prepend the per-caller prefix before forwarding to the product layer and strip it from any path the product layer echoes back, so the browser keeps round-tripping mount-relative paths and never sees the storage ownership prefix. A cross-user path from a scoped caller becomes tenants/<caller>/users/<caller>/tenants/.../other and 404s inside the caller's own subtree. Complementary to nearai#5927, which scopes agent tool/shell workspace writes to the same per-user subtree. This PR scopes the browser read surface. Also addresses CodeRabbit follow-up on 03c5084: - resolveMemoryDirectory now seeds the walk at the mount root instead of relativePath, which doubled any non-root memory subdirectory. - userScopedPrefix is reduced to a boolean hasCallerIdentity predicate so no future caller can re-prepend a tenant/user path string. - Regression test for memory subdirectory navigation. Server-side contract tests cover: scoped workspace list prefixes the caller subtree; non-operator is scoped even when the state flag is off; operator fallback with the flag off keeps the raw root; scoped stat echoes the mount-relative path. * style: apply rustfmt to workspace scoping handlers and tests * fix(webui): reject parent traversal in scoped workspace projection Address CodeRabbit follow-up on the workspace-scoping handlers: - workspace_served_path now rejects any path containing a '..' segment before prepending the caller prefix, so a request like '../other-user/secret' can never become 'tenants/{tenant}/users/{user}/../other-user'. The product layer rejects traversal too, but this keeps the escape attempt from being dispatched at all. - Add a read_fs_file scoped-projection regression test mirroring the list/stat coverage: asserts the download command receives the caller-subtree-prefixed path. - Add a parent-traversal rejection test asserting a 400 and that the product layer is never reached. Rebased onto current main (resolved webui_v2_serve.rs conflict from nearai#6780/nearai#7050/nearai#7033). * fix(webui): wire per-caller workspace writes + address review 4847764083 Hosted non-admin users saw an empty workspace because the browser read the per-user subtree (tenants/{tenant}/users/{user}) while agent tool/attachment writes still landed in the shared /projects/workspace root. Wire the agent read-write workspace filesystem to a per-caller resolver so writes land in the caller's own subtree, mirroring memory scoping. The WebUI browser reads the same subtree, so a hosted user sees, reads, and writes only their own artifacts. Composition (runtime_mounts.rs): - Add scoped_workspace_mount_view(scope, permissions) mapping WORKSPACE_ALIAS to /projects/workspace/tenants/{tenant}/users/{user}. - read_write_workspace_filesystem now uses ScopedFilesystem::new with that resolver (per-call, per-caller) instead of the shared fixed view, so each authenticated caller keys its own subtree and the shared root is never exposed for writes. WebUI handlers (review 4847764083): - Extract workspace_scoped_projection_required(state, capabilities) and call it from both get_session and workspace_projection_for so /session and /fs/* never drift on the policy. - workspace_projection_for now fails closed (returns Err) when scoped projection is required but the caller identity cannot key a subtree, instead of returning None (which served the shared root). Validated newtypes make empty identity unreachable in practice; the guard is defense in depth. - strip_workspace_prefix strips only an exact prefix or prefix/ boundary, not sibling prefixes like tenants/.../users/alice2. - webui_serve.rs doc comment: remove presentation-layer-only wording; the flag controls /fs/* workspace path selection, not only UI display. Contract tests (webui_v2_handlers_contract.rs): - Stub FS_LIST_VIEW now echoes entry paths under the served (prefixed) root, so the handler's prefix stripping is observable. - Add browse_fs_dir_strips_prefixed_entry_paths_under_scoped_projection asserting response entries are mount-relative. - Existing scoped/traversal/raw-fallback/stat/read tests preserved. * fix(composition): scope agent workspace writes per caller on hosted profiles PR nearai#7062 scoped the WebUI workspace browser (and, in 0707e57, the WebUI attachment handle) to /projects/workspace/tenants/{tenant}/users/{user} on hosted profiles, but the agent still WROTE to the shared root: a hosted user saw an empty workspace because the browser read one subtree while every agent write landed in another. The read and write sides were deciding independently. Introduce ONE composition-owned decision --- DeploymentConfig::workspace_scoped_per_caller (true for every profile except Standalone/StandaloneUnrestricted) --- carried to the runtime as runtime_mounts::WorkspaceMountPolicy, and route every lane through it: - Grant minting: RefreshingLoopCapabilityPortFactory::create_capability_port now resolves the workspace MountView per run from the same ResourceScope the skill mounts already key off, so every `mounts = "workspace"` capability (write_file, read_file, apply_patch, list_dir, glob, grep, shell, http.save, attach_workspace_file_to_reply, ...) resolves paths inside the caller's own subtree. Previously it used the composition-wide ambient view. - Approval lease terms: PolicyApprovalLeaseTermsProvider mints workspace mounts from the gate's own ResourceScope, so approving one user's write no longer leases the shared root. Scoping failure denies rather than falling back. - Channel-inbound attachment lander: production channel_host_source built a fixed shared view while its test-support twin used the scoped resolver. Both now call one owner, runtime_mounts::read_write_workspace_filesystem, and the project_filesystem reader is built over the same handle so reader and lander address the same subtree. - WebUI attachment handle: read_write_workspace_filesystem scoped UNCONDITIONALLY after 0707e57, which relocated standalone uploads into tenants/local/users/... away from the raw root the standalone agent and browser use. It now follows the same policy bit. Standalone keeps the ambient shared view including the raw host-home aliases local coding profiles depend on; those are never scoped. The CLI's profile_requires_scoped_workspace_projection now delegates to RebornCompositionProfile::workspace_scoped_per_caller, so the browser projection and the write lanes cannot drift. Regression tests (each verified failing before the fix): - workspace_scoping_tests: a hosted-profile turn writing /workspace/note.txt through the production capability port lands under the caller's subtree, two callers writing the same relative path do not collide, and nothing lands in the shared root; the standalone counterpart still lands at the shared root. Fails pre-fix with the file absent from the caller subtree. - runtime::approval: per-caller lease terms key the gate's own subtree and two callers get different targets. Fails pre-fix against the shared view. - runtime_mounts: the shared read-write handle owner resolves different roots per policy branch, pinning the WebUI/channel/C-ATTACH lanes together. - serve.rs: the browser projection flag agrees with the composition write policy for every profile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(composition): close workspace read/write mismatch on multi-user standalone Follow-up to the per-caller workspace scoping commit, addressing two gaps the deeper lane trace surfaced. 1. The scoping decision had TWO derivations, so a host override was silently dropped. `production_build_assembly` re-derived it from the profile (`context.profile.workspace_scoped_per_caller()`) instead of reading the deployment the host assembled. The build context now carries the resolved boolean, so the deployment is the only source. Caught by the new e2e test below, which wrote to the shared root despite the override being set. 2. Profile alone was the wrong gate. The WebUI browser confines EVERY non-operator caller's Workspace reads to `tenants/{tenant}/users/{user}` (`workspace_scoped_projection_required` = state flag OR not-operator), so a standalone-composed deployment with a multi-user authenticator --- exactly what `serve` builds when SSO is on --- still read a per-user subtree while the agent wrote shared. That is the reported bug, alive outside the hosted profiles. `RebornHostBindings::with_workspace_scoped_per_caller` lets the assembling host raise the deployment's decision (raise-only; a hosted profile stays scoped). `serve` now computes the value ONCE as `profile default || SSO on` and feeds it to both `build_reborn_runtime` and `WebuiServeConfig::with_workspace_requires_scoped_projection`, so the browser and the write lanes can no longer disagree in any configuration the binary builds. Lanes verified against the trace findings, no change needed: - Grant view and execution mounts already come from ONE resolved value: `create_capability_port` resolves the per-caller view and passes it to both `visible_capability_request` (grant.constraints.mounts) and `with_execution_mounts` (context.mounts), so the authorization obligation subset check (`next.is_subset_of(&context.mounts)`) compares a view against itself. Proven end to end by the write_file dispatch test, which completes with verdict Success and lands bytes in the caller subtree --- a helper-only test would have missed the obligation wrapper. - Sandbox process lane: `resolve_grant` strips the source virtual root and joins the remainder as subdirectories, creating read-write targets on demand, so a nested `/projects/workspace/tenants/{t}/users/{u}` grant binds the caller's own directory inside the trusted source. Newly pinned by `per_caller_workspace_grant_binds_the_callers_subdirectory`. The default per-`RebornSandboxScopeKey` workdir bind (no `/workspace` in the view) is untouched. - `MountScopedRootFilesystem` for the ScopedVirtual backend is built from the same request mounts, so its containment re-check follows automatically. - WASM has no filesystem write host-call; nothing to change. Regression tests: - `agent_workspace_writes_are_visible_to_their_owner_and_hidden_from_other_users` (webui_v2_e2e) --- two-user WebUI over a standalone-composed runtime with scoping raised: user A's agent writes a workspace file, A lists it through `/fs/list`, and user B neither lists nor reads it. Fails before the fix (user A's listing 404s because the write landed in the shared root). - `per_caller_workspace_grant_binds_the_callers_subdirectory` (host runtime sandbox mounts). - Standalone integration suites still assert the FLAT workspace root and pass unchanged (group_journeys, group_multiuser, group_approvals, group_memory, integration_attach, integration_tool_call, webui_v2_product_api). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(composition): gate test-only workspace view accessor and single-source the mount policy The `Fast deterministic checks` gate failed on `crates/ironclaw_reborn_composition/src/factory/test_support.rs`: `workspace_mounts_for_test` carried an `.expect()` without an item-level `#[cfg(any(test, feature = "test-support"))]`, so `check_no_panics.py` classified it as production code. The parent module gate is not what the scanner reads. Gate the accessor like every sibling in the file, and drop the duplicated `#[cfg]` attribute that landed on `workspace_mount_policy_for_test`. `Code Style` was only a rollup of this same failure. Review follow-ups on the same scoping change: - `WorkspaceMountPolicy::resolve` is now the single constructor for the per-caller/shared decision. `host_access_assembly` and `production_backend_assembly` each re-implemented the branch, so a deployment could have drifted into a scoped view on one write lane and an ambient shared view on another. - Annotate the `.ok()?` in `read_write_workspace_filesystem`'s `Shared` branch per `.claude/rules/error-handling.md`; the mount view is built from compile-time constants with no caller input. - Use `tokio::fs::canonicalize` in the sandbox-mount test so the async test no longer blocks a Tokio worker. Behavior is unchanged; the existing scoping tests (`workspace_scoping_tests`, `webui_v2_e2e`) cover both policy branches through the production seam. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(composition): give a fresh caller an empty workspace instead of a hard error Production report (hosted volume container, profile local-dev + SSO, so workspace scoping is active): `builtin.list_dir` and `builtin.glob` on /workspace failed `operation_failed` for the caller `reborn-cli`. Grants were correct --- /workspace mapped to /projects/workspace/tenants/reborn-cli/users/reborn-cli --- but that directory had never been created, so the agent was unusable for every new user. Root cause: a shared deployment's /projects/workspace is created once at composition (`build_host_access`). Per-caller scoping introduced a root with no equivalent step. The local backend returns `NotFound`, and the coding tools map `NotFound` to `operation_error`, so the very first read of an untouched workspace was a hard failure rather than an empty listing. Fix: `runtime_mounts::ensure_caller_workspace_root` creates the caller's own workspace root, called once per run from `RefreshingLoopCapabilityPortFactory::create_capability_port` --- the single seam that already resolves the caller's scope for grant minting, so the path comes from their own mount view and never from tool input. Creating was preferred over softening `NotFound`: - it fixes reads and writes together, where mapping NotFound to empty would have had to be repeated in list_dir, glob, grep and the sensitive-path pre-check, each an opportunity to drift; - error mapping is left untouched everywhere else, so `NotFound` is not weakened for any other path (the scoping brief's memory-safety constraint); - it mirrors the sandbox lane, which already `create_dir_all`s its per-scope workspace in `prepare_workspace`. It is a no-op for shared deployments, whose root composition already created, and failure is logged and swallowed --- a run must not die because a directory the first write would create anyway could not be pre-created. Also fixes the `reborn_struct_test_support_ratchet` failure that c84c9c5 introduced: gating `workspace_mounts_for_test` for `check_no_panics.py` made it a `#[cfg]`-gated member on a production struct, which is what that ratchet counts. Converting it to a gated free function satisfies both gates. Verified the ratchet fails at c84c9c5 with this change stashed. Regression test `fresh_caller_reads_an_empty_workspace_then_writes_into_it`: a caller whose subtree was never created lists an empty workspace, globs and greps cleanly, then writes --- including into a nested path whose parents do not exist --- and reads the file back from their own subtree. Fails before the fix with exactly the production error (`RecoverableFailure { error_kind: OperationFailed }`) on the first list_dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: read a never-written workspace mount root as empty at every lane's own seam Relocates the fresh-caller fix (544cebb) from a per-run eager create_dir_all in create_capability_port to the read paths themselves, per review: the eager create covered only the agent lane (a fresh user opening the WebUI Workspace tab before any run still 404'd), was best-effort with a swallowed failure, and paid a per-run cost to fix a read problem. The rule, applied once per lane at the seam that knows the mount root: an authorized caller's mount ROOT exists by definition --- it is the namespace their grant names, not a path they chose --- so a backend that has nothing under it yet reads as an EMPTY directory. Anything deeper keeps reporting NotFound. - Coding tools (the agent lane, which resolves grant mounts against the raw HostWorkspace filesystem): `ResolvedPath::is_mount_root` + `list_dir_empty_if_missing_root`; list_dir's sensitive-stat guard, glob's walk, and grep's root stat tolerate exactly the missing root. The dead `deny_sensitive_existing_path` helper is deleted. - `ScopedFilesystem` (WebUI browse/attachment handles, channel lander): list/stat resolve root-aware and read a missing mount root as empty. - `MountScopedRootFilesystem` (ScopedVirtual backend): same rule. - WebUI browse handler: a scoped caller's own projection ROOT (their `tenants/{t}/users/{u}` subtree under the shared workspace mount) lists as empty on NotFound --- it is authorization-derived, not user input. Deeper paths keep their 404. Regression coverage: `fresh_caller_reads_an_empty_workspace_then_writes_into_it` (agent lane, drives the production capability port; fails before this change), and the two-user webui_v2_e2e test's fresh-user assertion is tightened from `OK || NOT_FOUND` to `OK` + zero entries --- the NOT_FOUND arm it used to tolerate was this bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): scope workspace writes whenever serve can mint a non-operator caller Review finding on 56dc411: gating the write-side raise on `sso_enabled` was provably too narrow. `serve` installs the admin-API token minter on every start, that minter stamps `operator = false` by design, and the no-SSO branch wires a `SessionAuthenticator` so those bearers validate. The WebUI browser confines every non-operator caller's Workspace reads to their own subtree, so on Standalone + SSO off, a `POST /admin/users` user got scoped reads over a shared write root: the exact empty-workspace mismatch this branch exists to close, still open through a different door. `serve` now raises `with_workspace_scoped_per_caller_services(true)` unconditionally, and the browser flag is READ BACK from the deployment the runtime actually received (`runtime_input.config()`) instead of being recomputed locally --- the raise is a request, composition owns the answer, and the two sides can no longer drift (this also closes the silent-no-op gap where a raise on a `services: None` input vanished while the local recomputation proceeded). The now-unused `profile_requires_scoped_workspace_projection` / `workspace_scoped_per_caller` helpers are deleted and `composition_profile` returns to private. Regression tests: `serve_scopes_workspace_writes_even_on_standalone_without_sso` pins the raise + read-back pair on the Standalone profile; `workspace_scoping_default_follows_deployment_profile` keeps pinning the per-profile defaults that the raise starts from. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review comments on the fresh-workspace read path - A slash-only ?path= names the same projected workspace root as an empty path; the fresh-caller empty-listing fallback now keys on the trimmed path, and the two-user e2e pins path=/ returning OK + zero entries. - Mirror the parent-traversal rejection test across stat_fs_path and read_fs_file: all three /fs routes share workspace_served_path's `..` guard, so all three are pinned to 400 without reaching the product layer. - The workspace-scoping test helper now fails loudly on result-store errors instead of collapsing them into a missing output, and the fresh-caller test asserts glob and grep return empty file sets rather than bare success. Review comments 3711525396, 3707056909, 3711525388 on PR nearai#7062. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: integration-tier coverage for per-caller workspace scoping + gate residue exemptions The merge-queue run for this branch (actions/runs/30905070584) failed the changed-line coverage gate at 74.77%: the per-caller scoping and never-written-root behavior was pinned only by #[cfg(test)] unit modules, which the integration-tier lcov excludes. Close the gap with real coverage at the tiers the gate measures, and exempt the residue the lcov structurally cannot observe. New coverage: - tests/integration/group_multiuser/scenario_scoped_workspace_isolation.rs over the new RebornIntegrationGroup::multiuser_scoped_workspace(): a runtime built with the same scoping raise `serve` applies, capability grants confined to the caller's own tenants/{tenant}/users/{user} subtree via the production resolver (new test-support export scoped_workspace_mount_view_for_test). Drives through real turns: a fresh caller's list_dir/glob/grep read an EMPTY workspace instead of erroring; a GATED write is approved (per-caller lease) and lands on disk in the caller's own subtree with parents created, never at the shared root; the written subtree lists/greps back; a missing SUB-path stays a hard error. - HostRuntimeHarnessOptions::with_workspace_scoped_per_caller() — the harness seam applying RebornRuntimeInput::with_workspace_scoped_per_caller_services, so the integration tier can compose a scoped deployment at all. - filesystem_contract: never_written_mount_root_reads_as_empty_but_subpaths_stay_not_found pins ScopedFilesystem's list/list_bounded/stat mount-root rule at its owning seam (a tests/ binary, visible to the gate's lcov). - profile_acceptance: workspace_scoping_default_per_profile pins the per-profile default the serve raise starts from. Exemptions (tests/integration/changed-coverage-exemptions.toml, tracked in issue nearai#7142): the create_capability_port async-fn body (llvm-cov attributes it to the signature line — 114 hits — and emits no body DA records, the nearai#6963 class), the serve command body + webui_serve builder (unit-tier-only), the ScopedVirtual-only MountScopedRootFilesystem arm, defensive error arms, and a handful of match-arm construction counters whose driving tests assert the behavior. Verified against the queue run's own merged lcov unioned with a local llvm-cov run of the new binaries: changed-line coverage 98.74% (gate exit 0) without base-lcov subtraction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: address review comments on the scoped-workspace coverage - filesystem_contract: pin the WRITE half of the never-written-root rule at the owning tier — the first write materializes the root and parents, the root then lists the entry, and the bytes read back (review 3712779771). - group_constructors: state the SINGLE-CALLER constraint on multiuser_scoped_workspace loudly — the grant view is resolved once for the canonical subject while dispatch resolves owners per run, so a second .with_actor_id thread would hold the wrong subtree grant; cross-actor isolation is pinned where grants are per-caller by construction (review 3712779794). - harness options: cite the symbol the harness actually calls (RebornRuntimeInput::with_workspace_scoped_per_caller_services), not the host-bindings raise (review 3712779803). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(coding): reject document paths in apply_patch --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
l3ocifer
pushed a commit
to l3ocifer/frick-ironclaw
that referenced
this pull request
Sep 3, 2026
* fix(webui): scope workspace and memory views Scope WebUI Workspace and Memory presentation to the authenticated tenant/user instead of exposing shared storage paths. Fail closed for signed/non-operator sessions and for missing scoped roots, while preserving local operator fallback. Collapse internal memory wrapper directories and hide sidecar files from the user-facing view. * fix(webui): consume mount-relative /fs/* paths and gate test-only imports Address review feedback on nearai#7062 (reopened from nearai#5831): - The /fs/{list,stat,content} handlers bind the authenticated caller and return mount-relative, server-scoped paths (covered by the existing webui_filesystem_memory_mount_is_scoped_to_authenticated_user composed test). The frontend now consumes that contract directly: it no longer builds tenants/{tenant}/users/{user} prefixes for /fs/* requests. currentUser is retained only as a fail-closed identity gate when requireScopedWorkspace is true; missing identity renders empty listings and short-circuits file reads instead of requesting a raw mount root. - Default requireScopedWorkspace to true at the exported entry points (listWorkspace, readWorkspaceFile) and internal resolvers so an omitted option cannot silently enable raw-root behavior. - Keep memory collapse-walk over the caller's own mount-relative tree so the internal agents/<agent>/projects/_none wrapper directories stay hidden; all requests remain mount-relative. - Gate the three test-support-only imports in composition runtime.rs so default-feature clippy no longer reports them unused. - Update workspace-api tests to mock the server-scoped, mount-relative contract: hosted calls expect mount-relative paths, requireScopedWorkspace is explicit, and assert.rejects records/constrains rejections instead of asserting inside the fetch handler. * fix(webui): scope workspace browser to caller subtree server-side The standalone /fs/* browser mount (scoped_browse_mount_view) maps the Workspace alias to a shared /projects/workspace target, so a hosted user could list other users' workspace artifacts through the WebUI. Memory is already caller-scoped server-side; Workspace was not. Confine Workspace reads to the caller's own subtree (tenants/{tenant}/users/{user}) in the WebUI /fs/{list,stat,content} handlers when the effective scoped-projection flag is true (deployment flag OR non-operator caller). The flag reuses the same effective value the session endpoint reports. Operator fallback with scoped projection off keeps the raw shared workspace root so local/single-user workspaces stay visible. The handlers prepend the per-caller prefix before forwarding to the product layer and strip it from any path the product layer echoes back, so the browser keeps round-tripping mount-relative paths and never sees the storage ownership prefix. A cross-user path from a scoped caller becomes tenants/<caller>/users/<caller>/tenants/.../other and 404s inside the caller's own subtree. Complementary to nearai#5927, which scopes agent tool/shell workspace writes to the same per-user subtree. This PR scopes the browser read surface. Also addresses CodeRabbit follow-up on 03c5084: - resolveMemoryDirectory now seeds the walk at the mount root instead of relativePath, which doubled any non-root memory subdirectory. - userScopedPrefix is reduced to a boolean hasCallerIdentity predicate so no future caller can re-prepend a tenant/user path string. - Regression test for memory subdirectory navigation. Server-side contract tests cover: scoped workspace list prefixes the caller subtree; non-operator is scoped even when the state flag is off; operator fallback with the flag off keeps the raw root; scoped stat echoes the mount-relative path. * style: apply rustfmt to workspace scoping handlers and tests * fix(webui): reject parent traversal in scoped workspace projection Address CodeRabbit follow-up on the workspace-scoping handlers: - workspace_served_path now rejects any path containing a '..' segment before prepending the caller prefix, so a request like '../other-user/secret' can never become 'tenants/{tenant}/users/{user}/../other-user'. The product layer rejects traversal too, but this keeps the escape attempt from being dispatched at all. - Add a read_fs_file scoped-projection regression test mirroring the list/stat coverage: asserts the download command receives the caller-subtree-prefixed path. - Add a parent-traversal rejection test asserting a 400 and that the product layer is never reached. Rebased onto current main (resolved webui_v2_serve.rs conflict from nearai#6780/nearai#7050/nearai#7033). * fix(webui): wire per-caller workspace writes + address review 4847764083 Hosted non-admin users saw an empty workspace because the browser read the per-user subtree (tenants/{tenant}/users/{user}) while agent tool/attachment writes still landed in the shared /projects/workspace root. Wire the agent read-write workspace filesystem to a per-caller resolver so writes land in the caller's own subtree, mirroring memory scoping. The WebUI browser reads the same subtree, so a hosted user sees, reads, and writes only their own artifacts. Composition (runtime_mounts.rs): - Add scoped_workspace_mount_view(scope, permissions) mapping WORKSPACE_ALIAS to /projects/workspace/tenants/{tenant}/users/{user}. - read_write_workspace_filesystem now uses ScopedFilesystem::new with that resolver (per-call, per-caller) instead of the shared fixed view, so each authenticated caller keys its own subtree and the shared root is never exposed for writes. WebUI handlers (review 4847764083): - Extract workspace_scoped_projection_required(state, capabilities) and call it from both get_session and workspace_projection_for so /session and /fs/* never drift on the policy. - workspace_projection_for now fails closed (returns Err) when scoped projection is required but the caller identity cannot key a subtree, instead of returning None (which served the shared root). Validated newtypes make empty identity unreachable in practice; the guard is defense in depth. - strip_workspace_prefix strips only an exact prefix or prefix/ boundary, not sibling prefixes like tenants/.../users/alice2. - webui_serve.rs doc comment: remove presentation-layer-only wording; the flag controls /fs/* workspace path selection, not only UI display. Contract tests (webui_v2_handlers_contract.rs): - Stub FS_LIST_VIEW now echoes entry paths under the served (prefixed) root, so the handler's prefix stripping is observable. - Add browse_fs_dir_strips_prefixed_entry_paths_under_scoped_projection asserting response entries are mount-relative. - Existing scoped/traversal/raw-fallback/stat/read tests preserved. * fix(composition): scope agent workspace writes per caller on hosted profiles PR nearai#7062 scoped the WebUI workspace browser (and, in 0707e57, the WebUI attachment handle) to /projects/workspace/tenants/{tenant}/users/{user} on hosted profiles, but the agent still WROTE to the shared root: a hosted user saw an empty workspace because the browser read one subtree while every agent write landed in another. The read and write sides were deciding independently. Introduce ONE composition-owned decision --- DeploymentConfig::workspace_scoped_per_caller (true for every profile except Standalone/StandaloneUnrestricted) --- carried to the runtime as runtime_mounts::WorkspaceMountPolicy, and route every lane through it: - Grant minting: RefreshingLoopCapabilityPortFactory::create_capability_port now resolves the workspace MountView per run from the same ResourceScope the skill mounts already key off, so every `mounts = "workspace"` capability (write_file, read_file, apply_patch, list_dir, glob, grep, shell, http.save, attach_workspace_file_to_reply, ...) resolves paths inside the caller's own subtree. Previously it used the composition-wide ambient view. - Approval lease terms: PolicyApprovalLeaseTermsProvider mints workspace mounts from the gate's own ResourceScope, so approving one user's write no longer leases the shared root. Scoping failure denies rather than falling back. - Channel-inbound attachment lander: production channel_host_source built a fixed shared view while its test-support twin used the scoped resolver. Both now call one owner, runtime_mounts::read_write_workspace_filesystem, and the project_filesystem reader is built over the same handle so reader and lander address the same subtree. - WebUI attachment handle: read_write_workspace_filesystem scoped UNCONDITIONALLY after 0707e57, which relocated standalone uploads into tenants/local/users/... away from the raw root the standalone agent and browser use. It now follows the same policy bit. Standalone keeps the ambient shared view including the raw host-home aliases local coding profiles depend on; those are never scoped. The CLI's profile_requires_scoped_workspace_projection now delegates to RebornCompositionProfile::workspace_scoped_per_caller, so the browser projection and the write lanes cannot drift. Regression tests (each verified failing before the fix): - workspace_scoping_tests: a hosted-profile turn writing /workspace/note.txt through the production capability port lands under the caller's subtree, two callers writing the same relative path do not collide, and nothing lands in the shared root; the standalone counterpart still lands at the shared root. Fails pre-fix with the file absent from the caller subtree. - runtime::approval: per-caller lease terms key the gate's own subtree and two callers get different targets. Fails pre-fix against the shared view. - runtime_mounts: the shared read-write handle owner resolves different roots per policy branch, pinning the WebUI/channel/C-ATTACH lanes together. - serve.rs: the browser projection flag agrees with the composition write policy for every profile. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(composition): close workspace read/write mismatch on multi-user standalone Follow-up to the per-caller workspace scoping commit, addressing two gaps the deeper lane trace surfaced. 1. The scoping decision had TWO derivations, so a host override was silently dropped. `production_build_assembly` re-derived it from the profile (`context.profile.workspace_scoped_per_caller()`) instead of reading the deployment the host assembled. The build context now carries the resolved boolean, so the deployment is the only source. Caught by the new e2e test below, which wrote to the shared root despite the override being set. 2. Profile alone was the wrong gate. The WebUI browser confines EVERY non-operator caller's Workspace reads to `tenants/{tenant}/users/{user}` (`workspace_scoped_projection_required` = state flag OR not-operator), so a standalone-composed deployment with a multi-user authenticator --- exactly what `serve` builds when SSO is on --- still read a per-user subtree while the agent wrote shared. That is the reported bug, alive outside the hosted profiles. `RebornHostBindings::with_workspace_scoped_per_caller` lets the assembling host raise the deployment's decision (raise-only; a hosted profile stays scoped). `serve` now computes the value ONCE as `profile default || SSO on` and feeds it to both `build_reborn_runtime` and `WebuiServeConfig::with_workspace_requires_scoped_projection`, so the browser and the write lanes can no longer disagree in any configuration the binary builds. Lanes verified against the trace findings, no change needed: - Grant view and execution mounts already come from ONE resolved value: `create_capability_port` resolves the per-caller view and passes it to both `visible_capability_request` (grant.constraints.mounts) and `with_execution_mounts` (context.mounts), so the authorization obligation subset check (`next.is_subset_of(&context.mounts)`) compares a view against itself. Proven end to end by the write_file dispatch test, which completes with verdict Success and lands bytes in the caller subtree --- a helper-only test would have missed the obligation wrapper. - Sandbox process lane: `resolve_grant` strips the source virtual root and joins the remainder as subdirectories, creating read-write targets on demand, so a nested `/projects/workspace/tenants/{t}/users/{u}` grant binds the caller's own directory inside the trusted source. Newly pinned by `per_caller_workspace_grant_binds_the_callers_subdirectory`. The default per-`RebornSandboxScopeKey` workdir bind (no `/workspace` in the view) is untouched. - `MountScopedRootFilesystem` for the ScopedVirtual backend is built from the same request mounts, so its containment re-check follows automatically. - WASM has no filesystem write host-call; nothing to change. Regression tests: - `agent_workspace_writes_are_visible_to_their_owner_and_hidden_from_other_users` (webui_v2_e2e) --- two-user WebUI over a standalone-composed runtime with scoping raised: user A's agent writes a workspace file, A lists it through `/fs/list`, and user B neither lists nor reads it. Fails before the fix (user A's listing 404s because the write landed in the shared root). - `per_caller_workspace_grant_binds_the_callers_subdirectory` (host runtime sandbox mounts). - Standalone integration suites still assert the FLAT workspace root and pass unchanged (group_journeys, group_multiuser, group_approvals, group_memory, integration_attach, integration_tool_call, webui_v2_product_api). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(composition): gate test-only workspace view accessor and single-source the mount policy The `Fast deterministic checks` gate failed on `crates/ironclaw_reborn_composition/src/factory/test_support.rs`: `workspace_mounts_for_test` carried an `.expect()` without an item-level `#[cfg(any(test, feature = "test-support"))]`, so `check_no_panics.py` classified it as production code. The parent module gate is not what the scanner reads. Gate the accessor like every sibling in the file, and drop the duplicated `#[cfg]` attribute that landed on `workspace_mount_policy_for_test`. `Code Style` was only a rollup of this same failure. Review follow-ups on the same scoping change: - `WorkspaceMountPolicy::resolve` is now the single constructor for the per-caller/shared decision. `host_access_assembly` and `production_backend_assembly` each re-implemented the branch, so a deployment could have drifted into a scoped view on one write lane and an ambient shared view on another. - Annotate the `.ok()?` in `read_write_workspace_filesystem`'s `Shared` branch per `.claude/rules/error-handling.md`; the mount view is built from compile-time constants with no caller input. - Use `tokio::fs::canonicalize` in the sandbox-mount test so the async test no longer blocks a Tokio worker. Behavior is unchanged; the existing scoping tests (`workspace_scoping_tests`, `webui_v2_e2e`) cover both policy branches through the production seam. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(composition): give a fresh caller an empty workspace instead of a hard error Production report (hosted volume container, profile local-dev + SSO, so workspace scoping is active): `builtin.list_dir` and `builtin.glob` on /workspace failed `operation_failed` for the caller `reborn-cli`. Grants were correct --- /workspace mapped to /projects/workspace/tenants/reborn-cli/users/reborn-cli --- but that directory had never been created, so the agent was unusable for every new user. Root cause: a shared deployment's /projects/workspace is created once at composition (`build_host_access`). Per-caller scoping introduced a root with no equivalent step. The local backend returns `NotFound`, and the coding tools map `NotFound` to `operation_error`, so the very first read of an untouched workspace was a hard failure rather than an empty listing. Fix: `runtime_mounts::ensure_caller_workspace_root` creates the caller's own workspace root, called once per run from `RefreshingLoopCapabilityPortFactory::create_capability_port` --- the single seam that already resolves the caller's scope for grant minting, so the path comes from their own mount view and never from tool input. Creating was preferred over softening `NotFound`: - it fixes reads and writes together, where mapping NotFound to empty would have had to be repeated in list_dir, glob, grep and the sensitive-path pre-check, each an opportunity to drift; - error mapping is left untouched everywhere else, so `NotFound` is not weakened for any other path (the scoping brief's memory-safety constraint); - it mirrors the sandbox lane, which already `create_dir_all`s its per-scope workspace in `prepare_workspace`. It is a no-op for shared deployments, whose root composition already created, and failure is logged and swallowed --- a run must not die because a directory the first write would create anyway could not be pre-created. Also fixes the `reborn_struct_test_support_ratchet` failure that c84c9c5 introduced: gating `workspace_mounts_for_test` for `check_no_panics.py` made it a `#[cfg]`-gated member on a production struct, which is what that ratchet counts. Converting it to a gated free function satisfies both gates. Verified the ratchet fails at c84c9c5 with this change stashed. Regression test `fresh_caller_reads_an_empty_workspace_then_writes_into_it`: a caller whose subtree was never created lists an empty workspace, globs and greps cleanly, then writes --- including into a nested path whose parents do not exist --- and reads the file back from their own subtree. Fails before the fix with exactly the production error (`RecoverableFailure { error_kind: OperationFailed }`) on the first list_dir. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: read a never-written workspace mount root as empty at every lane's own seam Relocates the fresh-caller fix (544cebb) from a per-run eager create_dir_all in create_capability_port to the read paths themselves, per review: the eager create covered only the agent lane (a fresh user opening the WebUI Workspace tab before any run still 404'd), was best-effort with a swallowed failure, and paid a per-run cost to fix a read problem. The rule, applied once per lane at the seam that knows the mount root: an authorized caller's mount ROOT exists by definition --- it is the namespace their grant names, not a path they chose --- so a backend that has nothing under it yet reads as an EMPTY directory. Anything deeper keeps reporting NotFound. - Coding tools (the agent lane, which resolves grant mounts against the raw HostWorkspace filesystem): `ResolvedPath::is_mount_root` + `list_dir_empty_if_missing_root`; list_dir's sensitive-stat guard, glob's walk, and grep's root stat tolerate exactly the missing root. The dead `deny_sensitive_existing_path` helper is deleted. - `ScopedFilesystem` (WebUI browse/attachment handles, channel lander): list/stat resolve root-aware and read a missing mount root as empty. - `MountScopedRootFilesystem` (ScopedVirtual backend): same rule. - WebUI browse handler: a scoped caller's own projection ROOT (their `tenants/{t}/users/{u}` subtree under the shared workspace mount) lists as empty on NotFound --- it is authorization-derived, not user input. Deeper paths keep their 404. Regression coverage: `fresh_caller_reads_an_empty_workspace_then_writes_into_it` (agent lane, drives the production capability port; fails before this change), and the two-user webui_v2_e2e test's fresh-user assertion is tightened from `OK || NOT_FOUND` to `OK` + zero entries --- the NOT_FOUND arm it used to tolerate was this bug. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(cli): scope workspace writes whenever serve can mint a non-operator caller Review finding on 56dc411: gating the write-side raise on `sso_enabled` was provably too narrow. `serve` installs the admin-API token minter on every start, that minter stamps `operator = false` by design, and the no-SSO branch wires a `SessionAuthenticator` so those bearers validate. The WebUI browser confines every non-operator caller's Workspace reads to their own subtree, so on Standalone + SSO off, a `POST /admin/users` user got scoped reads over a shared write root: the exact empty-workspace mismatch this branch exists to close, still open through a different door. `serve` now raises `with_workspace_scoped_per_caller_services(true)` unconditionally, and the browser flag is READ BACK from the deployment the runtime actually received (`runtime_input.config()`) instead of being recomputed locally --- the raise is a request, composition owns the answer, and the two sides can no longer drift (this also closes the silent-no-op gap where a raise on a `services: None` input vanished while the local recomputation proceeded). The now-unused `profile_requires_scoped_workspace_projection` / `workspace_scoped_per_caller` helpers are deleted and `composition_profile` returns to private. Regression tests: `serve_scopes_workspace_writes_even_on_standalone_without_sso` pins the raise + read-back pair on the Standalone profile; `workspace_scoping_default_follows_deployment_profile` keeps pinning the per-profile defaults that the raise starts from. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: address review comments on the fresh-workspace read path - A slash-only ?path= names the same projected workspace root as an empty path; the fresh-caller empty-listing fallback now keys on the trimmed path, and the two-user e2e pins path=/ returning OK + zero entries. - Mirror the parent-traversal rejection test across stat_fs_path and read_fs_file: all three /fs routes share workspace_served_path's `..` guard, so all three are pinned to 400 without reaching the product layer. - The workspace-scoping test helper now fails loudly on result-store errors instead of collapsing them into a missing output, and the fresh-caller test asserts glob and grep return empty file sets rather than bare success. Review comments 3711525396, 3707056909, 3711525388 on PR nearai#7062. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: integration-tier coverage for per-caller workspace scoping + gate residue exemptions The merge-queue run for this branch (actions/runs/30905070584) failed the changed-line coverage gate at 74.77%: the per-caller scoping and never-written-root behavior was pinned only by #[cfg(test)] unit modules, which the integration-tier lcov excludes. Close the gap with real coverage at the tiers the gate measures, and exempt the residue the lcov structurally cannot observe. New coverage: - tests/integration/group_multiuser/scenario_scoped_workspace_isolation.rs over the new RebornIntegrationGroup::multiuser_scoped_workspace(): a runtime built with the same scoping raise `serve` applies, capability grants confined to the caller's own tenants/{tenant}/users/{user} subtree via the production resolver (new test-support export scoped_workspace_mount_view_for_test). Drives through real turns: a fresh caller's list_dir/glob/grep read an EMPTY workspace instead of erroring; a GATED write is approved (per-caller lease) and lands on disk in the caller's own subtree with parents created, never at the shared root; the written subtree lists/greps back; a missing SUB-path stays a hard error. - HostRuntimeHarnessOptions::with_workspace_scoped_per_caller() — the harness seam applying RebornRuntimeInput::with_workspace_scoped_per_caller_services, so the integration tier can compose a scoped deployment at all. - filesystem_contract: never_written_mount_root_reads_as_empty_but_subpaths_stay_not_found pins ScopedFilesystem's list/list_bounded/stat mount-root rule at its owning seam (a tests/ binary, visible to the gate's lcov). - profile_acceptance: workspace_scoping_default_per_profile pins the per-profile default the serve raise starts from. Exemptions (tests/integration/changed-coverage-exemptions.toml, tracked in issue nearai#7142): the create_capability_port async-fn body (llvm-cov attributes it to the signature line — 114 hits — and emits no body DA records, the nearai#6963 class), the serve command body + webui_serve builder (unit-tier-only), the ScopedVirtual-only MountScopedRootFilesystem arm, defensive error arms, and a handful of match-arm construction counters whose driving tests assert the behavior. Verified against the queue run's own merged lcov unioned with a local llvm-cov run of the new binaries: changed-line coverage 98.74% (gate exit 0) without base-lcov subtraction. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: address review comments on the scoped-workspace coverage - filesystem_contract: pin the WRITE half of the never-written-root rule at the owning tier — the first write materializes the root and parents, the root then lists the entry, and the bytes read back (review 3712779771). - group_constructors: state the SINGLE-CALLER constraint on multiuser_scoped_workspace loudly — the grant view is resolved once for the canonical subject while dispatch resolves owners per run, so a second .with_actor_id thread would hold the wrong subtree grant; cross-actor isolation is pinned where grants are per-caller by construction (review 3712779794). - harness options: cite the symbol the harness actually calls (RebornRuntimeInput::with_workspace_scoped_per_caller_services), not the host-bindings raise (review 3712779803). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(coding): reject document paths in apply_patch --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This branch was successfully deployed
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
1 participant
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.
Summary
/to the workspace mount.list_dir,glob, andgrep./shell access for scoped workspaces.Relationship to #5831
#5831 now only changes the WebUI Workspace/Memory presentation. This PR owns the tool/host execution behavior that could affect agents and benchmark scores: coding tools, shell workdir mapping, and scoped-workspace shell escape guards.
Current Behavior
list_dir,glob, andgrepcan treat logical/as the workspace root without requiring the physical workspace directory to already exist./workspace, but attempts to use raw shared workspace paths,..escapes to sibling users, or raw/are rejected at the host boundary.Validation
cargo fmt --all --checkcargo test -p ironclaw_host_runtime builtin_coding_ --test first_party_builtin_tools --features test-support,libsql -- --nocapturecargo test -p ironclaw_host_runtime local_host_process_port_rejects_raw_root_for_scoped_workspace --lib --features test-support,libsql -- --nocapturecargo clippy -p ironclaw_first_party_extensions -p ironclaw_host_runtime --all-targets --features test-support,libsql -- -D warningsgit diff --check