Bug
The Files service path confinement guard can reject valid paths under /me or /public when the configured storage root is reached through a symlink/canonicalized prefix. On macOS this shows up with temp/data paths under /var that filepath.EvalSymlinks resolves to /private/var; the lexical root remains /var/..., so safeJoin incorrectly treats the resolved existing prefix as outside the area. The same can happen if an operator configures AGENTBBS_DATA_DIR/files root through a symlink.
Reproduction
On a system where the root path canonicalizes differently (or with a symlinked files root), run:
Before the fix, valid paths such as /me/notes.txt can fail with files: path escapes its area. A smaller reproducer is safeJoin(aliasRoot, "notes.txt") where aliasRoot is a symlink to the real area root.
Expected
- Legitimate paths under a symlinked/canonicalized area root should be accepted.
- Symlink escapes below the area root, e.g.
area/escape -> /etc, should still be rejected.
Proposed fix
Canonicalize the area root before comparing it with the EvalSymlinks result of the longest existing prefix, while keeping the initial lexical containment check. I have a PR with regression tests for both the allowed symlinked-root case and the escaping child-symlink case.
Bug
The Files service path confinement guard can reject valid paths under
/meor/publicwhen the configured storage root is reached through a symlink/canonicalized prefix. On macOS this shows up with temp/data paths under/varthatfilepath.EvalSymlinksresolves to/private/var; the lexical root remains/var/..., sosafeJoinincorrectly treats the resolved existing prefix as outside the area. The same can happen if an operator configuresAGENTBBS_DATA_DIR/files root through a symlink.Reproduction
On a system where the root path canonicalizes differently (or with a symlinked files root), run:
go test ./internal/filesBefore the fix, valid paths such as
/me/notes.txtcan fail withfiles: path escapes its area. A smaller reproducer issafeJoin(aliasRoot, "notes.txt")wherealiasRootis a symlink to the real area root.Expected
area/escape -> /etc, should still be rejected.Proposed fix
Canonicalize the area root before comparing it with the
EvalSymlinksresult of the longest existing prefix, while keeping the initial lexical containment check. I have a PR with regression tests for both the allowed symlinked-root case and the escaping child-symlink case.