[Bug] attachment-local: durable-home walk reaches the filesystem root — image attachments fail with EACCES on Android (fix included)

[zexadev]

What happens

With dsh running inside an Android app sandbox (DSH_HOME under /data/user/0/<package>/), attaching an image always fails. The prompt is rejected and the reason is:

EACCES: permission denied, open '/data/user/0'

Sending text is never affected. Only attachments fail, and they fail every time, on every device.

Why

packages/attachment/attachment-local/src/store.ts:

async function ensureDurableHome(path: string): Promise<string> {
  const home = resolve(path)
  if (!durableHomes.has(home)) {
    await ensureDurableDirectory(home, parse(home).root)   // boundary = filesystem root
    durableHomes.add(home)
  }
  return home
}

ensureDurableDirectory walks from home up to that boundary calling syncDirectory(parent) at every level, and syncDirectory opens the directory with O_RDONLY in order to fsync it.

With DSH_HOME = /data/user/0/<pkg>/dsh-home the walk is:

/data/user/0/<pkg>/dsh-home   ours
/data/user/0/<pkg>            ours, opens fine
/data/user/0                  drwxrwx--x system system   <-- EACCES here
/data/user
/

On Android /data/user/0 is drwxrwx--x system system. An ordinary app lands in the "other" bits: execute, so it can traverse, but no read. Opening a directory read-only requires read permission, so this is an unconditional failure rather than a misconfiguration — no ROM or Android version makes it work.

Session persistence is unaffected because it only fsyncs explicit single-level paths that top out at DSH_HOME and never walks upward. That asymmetry is why "send a message" works while "send an image" has never worked.

Reproduced

Redmi 23113RKC6C, Android 16, arm64, published dsh tree run by an Android app that bundles Node 24. Reproduced on 0.1.5-rc.2 and 0.2.0-rc.2; the code is identical in both, and still the same on master at dsh-v0.2.1-alpha.1.

Both attachment entry points reach it — the composer's file input and a paste into the composer.

Not Android-specific in principle

Any environment where some ancestor of DSH_HOME is not readable by the process hits the same wall: a restrictive container mount, a hardened multi-user setup, a DSH_HOME under a directory owned by someone else. Android only makes it unconditional.

Suggested fix

Treat "this process cannot open the ancestor" as "this ancestor is not ours to sync":

async function syncDirectory(path: string): Promise<void> {
  if (process.platform === 'win32') return
  let handle
  try {
    handle = await open(path, constants.O_RDONLY)
  } catch (error) {
    // An ancestor this process may not open is one it cannot have created an entry
    // in either (that needs write+execute), so nothing of ours is pending there.
    const code = (error as NodeJS.ErrnoException)?.code
    if (code === 'EACCES' || code === 'EPERM') return
    throw error
  }
  try {
    await handle.sync()
  } finally {
    await handle.close()
  }
}

This keeps the contract the comment on ensureDurableDirectory describes. Creating an entry in a directory requires write+execute on it, so a directory the process cannot even open read-only is one it cannot have written to; there is no unsynced entry of ours to lose. Every directory the process did create stays inside the sandbox and is still fsynced exactly as before.

If you would rather not loosen syncDirectory for all four of its call sites, the narrower alternative is to stop the ensureDurableHome walk at the first ancestor that cannot be opened instead of at parse(home).root.

Verified

Applied as a build-time patch over the published tree and run on the device above: attachment ingest returns its sha256 reference, the object lands in DSH_HOME/attachments/v1/objects/<xx>/<hash>, the image reaches the provider, and the turn completes. The same build with the stock code reproduces the EACCES every time.

[PerryLink]

机制、边界常量、终止条件我都核过了，你说的都对。但有一条施工信息你没有、而且会直接决定 PR 能不能过，另有一处论证需要收窄。

1. ⭐「走到根」不是疏漏，是被测试逐目录断言的有意设计

这是最要紧的一条。packages/attachment/attachment-local/tests/store.spec.ts:83-97：

it.skipIf(process.platform === 'win32')('syncs every object ancestor up to the durable boundary before returning', async () => {
  // Each process first proves DSH_HOME durable all the way to the filesystem root;
  // existence alone cannot vouch for a concurrent creator's fsync.
  expect(fsControl.syncedDirectories).toEqual([ ...parentChainToRoot(base), ... ])

parentChainToRoot（:67-76）用 parse(level).root 作为终点，一路收集到根；断言用的收集器精确到每个被 open(O_RDONLY) 的目录（:36-39）。

⇒ 任何「不要走到根」的改动必须同步改这条期望数组，否则 CI 立即红。 而且注释里写了理由（「existence alone cannot vouch for a concurrent creator's fsync」），所以你的修复说明必须回应这个跨进程并发契约，否则会被当成削弱语义驳回。

2. 只有一条调用点会越过 DSH_HOME（所以你的"窄改法"确实更安全）

syncDirectory 一共 3 个调用点：

• store.ts:164 —— ensureDurableDirectory 向上走，会越过 DSH_HOME
• store.ts:294 —— publishImmutableAlias，止于 root
• store.ts:379 —— publishStagedObject，止于 root

ensureDurableHome（:176-183）的边界就是 parse(home).root，而 :318-319 传入的其实是 dirname(dirname(resolve(root)))（= DSH_HOME）——是 ensureDurableHome 又把它走到了 /。所以「不让四个调用点统一放宽」的担心可以放下：真正的越界只有 :164 这一条。

3. 你的修复论证需要收窄一处

你写的是：

Creating an entry in a directory requires write+execute on it, so a directory the process cannot even open read-only is one it cannot have written to.

后半句不成立。 open(O_RDONLY) 需要目录的 read 位；创建目录项需要 write+execute 位 —— 三者相互独立。存在 --wx（0333、0733）这类「可写但不可读」的目录：进程确实可能在里面创建了条目，却打不开它 ⇒ 这个判据会把一个真的需要 fsync 的祖先误判成「不是我们的」而跳过，静默破坏 ensureDurableDirectory 自己声明的 durability 契约。

Android 的 /data/user/0 是 0711（other 只有 x，无 r 无 w），不落在这个反例里，所以对你这个场景修复完全成立。但注释里要写明前提：仅当该目录对本进程无写权限时，「打不开」才等价于「没有条目要 sync」。这样它才是一条通用正确的规则，而不是一个恰好够用的补丁。

另外一并吞 EPERM 是对的 —— 部分 FUSE/容器挂载返回 EPERM 而非 EACCES。

4. 两处小澄清

• 这条链路走的是 ATTACHMENT_WRITE_FAILED（store.ts:346 / :386 把非 AttachmentError 包装成它），不是 ATTACHMENT_NOT_FOUND（store.ts:443 / file-store.ts:171，那是读缺失对象的错误码）。别让人顺着后者去找。
• Windows 已在 store.ts:134 短路（if (process.platform === 'win32') return），测试也是 it.skipIf(process.platform === 'win32') ⇒ 这是纯 POSIX 语义问题，改动不影响 Windows 行为。
• durableHomes（:23、:178-181）是进程内记忆化，所以修复后不会退化成「每个附件都重走一遍到根的遍历」，只是打不开的层变成 no-op。

5. 会话侧为什么不受影响 —— 你的不对称解释成立

packages/storage/storage-json/src/atomic.ts:35 → await fsyncDirectory(dirname(path))，:44-51 的 fsyncDirectory 只 open(path, 'r') 直接父目录一层；packages/session/session-persistence-jsonl/src/index.ts:1317-1322 同为单层。所以确实只有附件这条会向上走。

建议

采纳你的 syncDirectory try/catch（EACCES/EPERM → return）作为最小改动，但：

1. 注释写清前提（见第 3 节）；
2. PR 必须同时更新 store.spec.ts:83-97 的期望数组，并补一个「祖先不可读则不抛错」的 POSIX 用例 —— 否则即使代码对，CI 也会挡住你。

如果你倾向更保守的窄方案（让 ensureDurableHome 停在第一个打不开的祖先），那需要一并修改 store.spec.ts:93-95 那条注释所声明的跨进程并发契约，论证成本反而更高。

[zexadev]

You're right on point 3, and it's the one that matters.

The permission inference was too wide. open(O_RDONLY) needs r; creating an entry needs w+x. They are independent, so a 0333/0733 ancestor is writable and unopenable at once, and blanket-skipping it would silently drop a sync this process really does owe. Narrowed to the condition that actually licenses the skip:

async function syncDirectory(path: string): Promise<void> {
  if (process.platform === 'win32') return
  let handle
  try {
    handle = await open(path, constants.O_RDONLY)
  } catch (error) {
    const code = (error as NodeJS.ErrnoException)?.code
    // Unopenable alone does not mean nothing is pending: `r` and `w`+`x` are independent,
    // so a 0333 ancestor is writable and unreadable at once. The skip is licensed only
    // when this process also may not write there — then it cannot have created an entry.
    if ((code === 'EACCES' || code === 'EPERM') && !(await writable(path))) return
    throw error
  }
  try {
    await handle.sync()
  } finally {
    await handle.close()
  }
}

async function writable(path: string): Promise<boolean> {
  try { await access(path, constants.W_OK); return true } catch { return false }
}

On Android /data/user/0 is drwxrwx--x and an app lands in the other bits — execute only, no write — so the walk still passes it, while a writable-but-unreadable ancestor keeps today's behaviour (throw) instead of quietly weakening the contract.

On the test: I don't think the expectation array has to change for this version of the fix. tests/store.spec.ts:36-39 mocks open as a spy that records the path and then delegates to the real node:fs/promises open, and the suite runs under mkdtemp(tmpdir()), whose whole ancestor chain is readable by the test user. The walk still visits every level up to the root and every level is still recorded — recorded before the delegation, so even a denied level would appear — and toEqual([...parentChainToRoot(base), …]) holds unchanged. What changes is only the behaviour on a level that denies open, which no existing case exercises; the POSIX case you suggest is purely additive. That is from reading the spec, not from running your suite. One construction note if you add it: it has to skip under uid 0, because root bypasses the permission check and the directory would open fine.

The narrower alternative — stopping ensureDurableHome at the first unopenable ancestor — is the one that would have to rewrite that expectation and the cross-process argument in its comment, so I agree it is the worse trade.

Two corrections to me, both yours: three call sites, not four (store.ts:164, :294, :379), and only :164 can cross DSH_HOME; and the code on this path is ATTACHMENT_WRITE_FAILED, not ATTACHMENT_NOT_FOUND.

On where this can land: I can't open the PR — CONTRIBUTING says external pull requests aren't accepted at the moment and issues are closed, which is why this is a discussion. Everything needed is here if a maintainer wants to take it.

For the record, 0.2.1-alpha.2 still ships all three Android blockers unchanged: the root-bounded walk here, node-addon-require-builtin required unconditionally in app-boot (#9248, in both lib/index.js and lib/worker/profile-resolution-bootstrap.js), and koffi pinned to exactly 3.1.1 (#9249).