[Bug] CompositeFilesystem root ls/grep escape the workspace under the UNRESTRICTED default backend (list_files(".") enumerates the drive root)

[aiyili]

Summary

CompositeFilesystem routes root-level ls/grep operations to the default backend with an absolute / path, and the UNRESTRICTED resolver passes absolute paths through unchanged — so the operation anchors at the OS root instead of the workspace root. In the default RemoteFilesystemSpec configuration, list_files(".") from the model enumerates the drive root (C:\ top level on Windows), breaking workspace containment.

This is point 4 of the #3245 evaluation ("list_files('.') can enumerate outside the workspace (drive root), since no containment check exists on that path"), which was deliberately tracked separately from PR #3247 (that PR blocks traversal only for opt-in sharedLocalWorkspace users; the default namespaced backend is still UNRESTRICTED).

Framing correction (per evaluation): RemoteFilesystemSpec is itself opt-in (it throws at build time without a distributed store); the harness default is LocalFilesystemSpec (ROOTED), where ls("/") is denied. The accurate scope is configuration-gated host-root disclosure via the UNRESTRICTED backend mode — and the escape is namespace-independent (/ is exempt from prefixing). Ordinary absolute paths passing through the same backend are tracked separately in #3258.

Root cause (verified against main @ ed77530)

• CompositeFilesystem.ls — agentscope-harness/.../CompositeFilesystem.java:199-201: if ("/".equals(path) || ".".equals(path)) { ... defaultBackend.ls(runtimeContext, "/") ... }
• CompositeFilesystem.grep — :280-282: the root branch passes path through verbatim to defaultBackend.grep(...); a "/" path hits the same passthrough.
• LocalFilesystem.resolveUnrestricted — LocalFilesystem.java:681-687: absolute paths are returned unchanged.
• LocalFilesystem.glob is already anchored (resolvePath(".")) — only ls and grep("/") are affected.

Proposed fix

Route the default backend's root operations through the backend's own root anchor: pass a blank path instead of "/". LocalFilesystem.resolvePath already resolves a blank key to the backend cwd (LocalFilesystem.java:598-600) and applyNamespacePrefix leaves blank keys unprefixed (:691-693) — a one-line change per site. Shared-route backends keep their "/" (a virtual store root for them). No change to per-user namespace isolation for ordinary (non-root) paths.

Tests required (per the #3245 follow-up evaluation)

• a .// listing cannot escape the workspace in both spec modes (namespaced default backend and sharedLocalWorkspace=true)
• grep_files(path="/") is contained the same way
• per-user namespace isolation for ordinary paths is unchanged (existing tests pin this and must keep passing)

[aiyili]

Hi, I would like to work on this — it is the follow-up I proposed on #3245 (comment). Plan: pass a blank path (backend-root anchor) instead of "/" in the CompositeFilesystem ls root branch and for the grep default-backend "/" case, leaving shared-route backends and ordinary-path namespace semantics untouched; regression tests for both spec modes plus grep containment. PR coming shortly.

[oss-maintainer]

Issue Evaluation

Category: bug | Status: Confirmed (verified against main @ ed77530a4)

The escape path is real and nothing between the model and the backend guards it:

• CompositeFilesystem.ls rewrites both . and / to a literal / before calling the default backend — agentscope-harness/.../CompositeFilesystem.java:199-201.
• CompositeFilesystem.grep passes the root path through verbatim — CompositeFilesystem.java:280-282. Effect is narrower than for ls: only the literal / escapes, because LocalFilesystem.grep maps null -> . (LocalFilesystem.java:397) and . resolves relative + namespaced.
• LocalFilesystem.resolveUnrestricted returns absolute paths unchanged — LocalFilesystem.java:681-687 — and applyNamespacePrefix exempts / from prefixing (:689-707), so resolvePath(rc, "/") lands on the OS/drive root.
• Reachable from the model: FilesystemTool.listFiles -> CompositeFilesystem.ls (FilesystemTool.java:283-290), with WorkspacePathNormalizer.normalize leaving . and / unchanged.

Three corrections, so the fix scope is agreed before review:

1. Not the harness default. RemoteFilesystemSpec is opt-in and throws at build time without a distributed AgentStateStore (HarnessAgent.java:2433-2441). The built-in default is LocalFilesystemSpec (LocalFsMode.ROOTED, LocalFilesystemSpec.java:61) wrapped in OverlayFilesystem, where ls("/") throws rootAccessDenied instead of escaping. The accurate framing is configuration-gated host-root disclosure. The default that matters here is the backend mode: RemoteFilesystemSpec.java:212 builds the local backend with virtualMode=false -> LocalFsMode.UNRESTRICTED.
2. The namespace is incidental, not causal. / is explicitly exempt from namespace prefixing, so the escape occurs with or without a user namespace.
3. Root ls/grep is not the whole containment gap. In the same configuration, ordinary absolute paths pass through the composite default branch verbatim (CompositeFilesystem.java:226, 232), so read_file("/etc/hostname") reads and write_file("/abs/new") writes on the host; validatePath only blocks .. and is called only by delete/move (AbstractFilesystem.java:178-188). Containing root ls/grep is the right fix for this issue, but it does not by itself restore workspace containment — worth a separate tracked follow-up.

On the proposed fix: passing a blank path instead of / for the default backend's root operations is correct and safe at this HEAD. resolvePath resolves blank to the backend cwd (LocalFilesystem.java:596-600; the issue cites :598-600) and applyNamespacePrefix has a blank guard (:690-692; the issue cites :691-693). Store-backed backends are unaffected (RemoteFilesystem.normalizePath maps blank back to /, RemoteFilesystem.java:720-723), and route backends correctly keep / as their virtual store root. Two points to state explicitly in the PR: (a) with USER isolation, blank resolves to the un-namespaced workspace root, so the root listing shows workspace-root children while ordinary relative paths live under workspace/<uid>/ — contained, but the root view is not the parent of that user's own files (glob already behaves this way, so it is consistent); (b) ls("") is already anchored, so the complete set of escaping root spellings is . and / on ls plus / on grep.

Test gaps to close: CompositeFilesystemTest has no root ls/grep case, and FilesystemToolTest.java:197 only bounds listFiles(".") against a mock. The regression suite named in the issue is the right one, plus a mode-shape assertion alongside LocalFilesystemModeTest, which currently pins that UNRESTRICTED "accepts any absolute path".

No bot PR will be generated: you already opened #3254 (OPEN, MERGEABLE, stacked on #3247) implementing exactly this direction, and #3247's sharedLocalWorkspace is not present at this HEAD. That PR will pick up this review in the regular PR巡检 cycle.

Severity: moderate-high — host filesystem disclosure through ls/grep, gated on an opt-in specification.

---

Automated evaluation by github-manager-bot

[aiyili]

Thanks for the evaluation — accepting all three corrections:

1. Framing fixed: agreed, RemoteFilesystemSpec is opt-in (throws without a distributed store) and the harness default (LocalFilesystemSpec, ROOTED) denies root access; the accurate statement is configuration-gated host-root disclosure via the UNRESTRICTED backend mode (RemoteFilesystemSpec.java:212). I've amended the issue summary accordingly.
2. Namespace incidental: agreed — / is exempt from prefixing, so the escape is namespace-independent.
3. Absolute-path gap: already tracked separately as [Bug] Remote-spec UNRESTRICTED default backend passes arbitrary absolute paths through (read_file("/etc/passwd") reaches the host) #3258 (filed per the fix(harness): contain CompositeFilesystem root ls/grep to the backend root (stacked on #3247) #3254 review's suggestion), including the read_file("/etc/hostname") / write_file("/abs/new") cases and the SANDBOXED/ROOTED direction.

On blank vs "." — flagging a divergence between this evaluation and the #3254 PR review, and how it resolved: the PR review warned that blank bypasses the per-user namespace (cross-tenant root listing under USER isolation) and recommended the namespace-aware "."; the rework landed in #3254 (c04290c) uses "." via a shared rootAnchor helper. One factual note on point (a)'s parenthetical: at this HEAD LocalFilesystem.glob anchors at hasNamespace(rc) ? resolvePath(rc, ".") : cwd (LocalFilesystem.java:421-422) — i.e. the caller's namespace when one is configured, not the bare workspace root — so "." (not blank) is the spelling consistent with glob's actual behavior, and it also makes the root view the parent of the user's own files, addressing the concern in point (a). Both reviews agree on the essential invariant (never pass "/" through to the UNRESTRICTED default backend); the tests in #3254 pin exactly the escaping set named in point (b): ls(".")/ls("/") and grep("/").

[oss-maintainer]

Conceded on point (a) — your correction is right and my parenthetical was wrong.

Re-verified at main @ ed77530a4: LocalFilesystem.glob anchors the root spellings as hasNamespace(rc) ? resolvePath(rc, ".") : cwd (agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystem.java:421-423). So glob is already namespace-aware, and the claim that a blank anchor was "consistent with glob" does not hold — blank bypasses the prefix entirely, because applyNamespacePrefix returns the key unchanged for null/blank (LocalFilesystem.java:690-692) and only prefixes "." into u1/. (:705-706). The namespace-free cwd arm is reached when no factory is configured or the factory yields a null/empty namespace (hasNamespace, :739-745). Correcting the record here rather than letting the wrong aside stand in the thread: "." is the spelling that agrees with glob, and it also makes the root view the parent of the caller's own files, so the cross-tenant ambiguity I described in point (a) is exactly what it removes. That matches what landed in #3254 at c04290c.

Unchanged from the evaluation, and still the part worth keeping in view:

• The invariant is the one both reviews agreed on — never forward "/" to the UNRESTRICTED default backend (RemoteFilesystemSpec.java:212). fix(harness): contain CompositeFilesystem root ls/grep to the backend root (stacked on #3247) #3254 now pins the escaping set named in point (b): ls("."), ls("/"), grep("/").
• The remaining open question on fix(harness): contain CompositeFilesystem root ls/grep to the backend root (stacked on #3247) #3254 is not the anchor choice but where the normalisation lives: rootAnchor is applied to defaultBackend, typed AbstractFilesystem and documented as taking /-rooted paths, so a non-local default backend (e.g. through RoutedSandboxFilesystem) shell-expands "." against the command cwd and returns ./name instead of /name. That is tracked in the 2026-09-22T23:02Z re-review on fix(harness): contain CompositeFilesystem root ls/grep to the backend root (stacked on #3247) #3254, and glob still forwards the raw "/"/null, so the three root-scanning operations have two different normalisation owners today.
• The arbitrary-absolute-path pass-through on the same backend stays tracked as [Bug] Remote-spec UNRESTRICTED default backend passes arbitrary absolute paths through (read_file("/etc/passwd") reaches the host) #3258, as you noted.

No action needed from the bot on this issue beyond this correction. Status unchanged: bug + area/harness, configuration-gated host-root disclosure, community fix in flight on #3254 (stacked on #3247), no bot PR per rule 7.

---

Automated follow-up by github-manager-bot