Skip to content

fix(sandbox): sandbox=true 下 skill manifest 不可读导致 skill show 报 not found - #1036

Open
xiaoxueSunn wants to merge 1 commit into
deepcoldy:masterfrom
xiaoxueSunn:fix/dsh-skill-manifest-sandbox-readable
Open

fix(sandbox): sandbox=true 下 skill manifest 不可读导致 skill show 报 not found#1036
xiaoxueSunn wants to merge 1 commit into
deepcoldy:masterfrom
xiaoxueSunn:fix/dsh-skill-manifest-sandbox-readable

Conversation

@xiaoxueSunn

Copy link
Copy Markdown
Contributor

问题

sandbox=true + skillInjection=prompt 时,任务内执行 botmux skill show <name> 对所有用户 Skill 统一返回:

skill manifest not found for session <session-id>

但 manifest 文件在沙箱外用同一 session id 能读到,且执行时该文件确实已存在(reporter 提供:session 66a95046-...,manifest 于 16:02:58 已生成,16:07:30 任务内连续 5 次 skill show 全部 not found)。因此这里的 "not found" 实际是沙箱不可读,而非文件不存在。

根因

两处叠加:

  1. worker.ts 构造 FsPolicy 时没有把当前 session 的 skill manifest 加入只读白名单。manifest 由 manifest-store.ts 写到 <config.session.dataDir>/skill-manifests/<sessionId>.json,但沙箱从未 re-expose 它 → 沙箱内读取 EACCES。
  2. readSessionSkillManifest() 只用 existsSync 判断,读失败一律 catch → 返回 null,把权限错误吞成了"不存在",最终显示为 not found,掩盖了沙箱配置缺陷。

改动(对应 reporter 的方案,最小范围)

不扩大对 Botmux 配置 / 凭证 / 其他任务数据的访问面(验收 #8)。

改动文件:src/worker.tssrc/core/skills/manifest-store.tssrc/core/skills/cli-session-command.ts + 两个测试,共 +117 / -9。

验证

  • 新增回归:manifest 缺失 → null;损坏 → 抛 SkillManifestParseError;present-but-unreadable(chmod 000)→ 抛 SkillManifestReadError(root 下自动跳过);CLI 层损坏 vs 缺失的消息与退出码区分。
  • 全绿:skill-manifest-store / skill-cli-commands / session-skill-runtime / session-skill-injection / session-skill-manifest-resolution / skill-resource-reader / plugin-cli-generation / fs-policy(70)。
  • tsc --noEmit 全量无报错。

仍未验证

剩余检查 需要的环境 / 原因 影响范围
Linux bwrap 下 sandbox=true 真机端到端:skill show 读到完整 Skill 正文;~/.dsh/manifest 只读可读、其他 session manifest 被拒 需 Linux + bwrap;本地 macOS 无法建立 sandbox 命名空间 阻塞发布,不阻塞合并
fs-policy-bwrap.e2e / sandbox-mask-manifest 的 bwrap 用例 同上(本地 macOS 全部 skip,非本 PR 引入) 不阻塞

对现有任务

沙箱策略在任务创建时冻结,本修复只对新任务生效;已创建任务需新建任务或重新生成沙箱策略。修复上线后,reporter 临时热更新的 sandboxPaths.readOnly: .../skill-manifests(会放大到全目录元数据)应撤销。

🤖 Generated with Claude Code

… works under sandbox=true

sandbox=true + skillInjection=prompt made `botmux skill show <name>` return
"skill manifest not found" for every user skill: the file sandbox never
exposed `<dataDir>/skill-manifests/<sessionId>.json`, so the in-sandbox read
EACCES'd and readSessionSkillManifest — which only distinguished existsSync —
collapsed the permission error to null, surfaced as not-found. User-skill
bodies were unreadable inside the sandbox; the manifest was fine outside it.

Fix (matches the reporter's requested approach, minimal scope):
- worker.ts: add ONLY this session's own manifest file to mandatoryReadOnlyPaths,
  canonicalized so a symlinked HOME (/home/u -> /data00/home/u) still matches.
  Scoped to <sessionId>.json (not the whole dir) so a task can't read other
  sessions' manifests, and built from config.session.dataDir (the exact path the
  store writes) rather than the possibly-unset SESSION_DATA_DIR.
- manifest-store.ts: readSessionSkillManifest now returns null ONLY on ENOENT;
  a permission/read error throws SkillManifestReadError and corrupt JSON throws
  SkillManifestParseError, so a sandbox misconfig no longer masquerades as
  not-found.
- cli-session-command.ts: map those errors to distinct, diagnosable messages
  (exit 1) while a genuinely-absent manifest stays not-found (exit 2).

Adds regression tests: absent -> null, corrupt -> throws, unreadable -> throws;
and CLI-level corrupt vs absent messaging.

Co-Authored-By: Claude <noreply@anthropic.com>
@xiaoxueSunn
xiaoxueSunn requested a review from deepcoldy as a code owner August 27, 2026 09:16
@deepcoldy

Copy link
Copy Markdown
Owner

感谢这个 PR,问题定位和最小化改动的思路都很清楚,安全边界(只放行本 session 单个 <sessionId>.json、只读、canonical() 处理 symlink HOME、用 config.session.dataDir 而非可能未设的 SESSION_DATA_DIR)我逐条实测过,都是正确的。下面是自动评审跑出来的几点,主要集中在「这个修复能否达成 PR 标题所声称的效果」上,供参考。

1)🔴 主要问题:manifest 放行了,但 skill 正文(body)仍未放行 → skill show 依然拿不到内容

我在 Linux + bwrap 真机上实测了改动后的有效策略:

/…/data/skill-manifests/<self>.json      -> readOnly   ✅ 本 PR 修好
/…/data/skill-manifests/<other>.json     -> none       ✅ 隔离正确
~/.botmux/skills/store/deploy/SKILL.md   -> none       ❌ 仍然不可读
~/.botmux/skills/store                   -> none       ❌

原因是 skill 正文住在 ~/.botmux/skills/store/<name>/registry-paths.ts,线上 manifest 里 rootDir 实测就是这个路径),而:

  • prepareSkillDelivery(adapter, manifest, 'prompt') 直接 return { readonlyRoots: [] } —— 正是本 issue 的 skillInjection=prompt 场景;
  • worker.tsreadonlyRoots 只承接 cfg.skillReadonlyRoots,所以是空的;
  • baseline 里有 ro(~/.botmux/bin)ro(~/.botmux/claude-plugin),但没有 ~/.botmux/skills

bwrap 端到端实测(manifest 放行、store 不放行):

manifest read: OK, skills = deploy
realpath(rootDir) FAILED: ENOENT

readSkillEntrypointreadSkillResource 里第一行 realpathSync(skill.rootDir) 不在 try 内,会把裸 ENOENT 抛到 runSkillSessionCommand 的外层 catch → exit 1 + 一条 realpath ENOENT 报错

所以效果是:报错信息从 skill manifest not found 变成了另一条(确实更可诊断),但 skill show <name> 仍然读不到 Skill 正文——而 prompt 模式的目录块恰恰是这样告诉模型的:<read>botmux skill show <name></read>skill list 是真的修好了(只读 manifest 字段),这是实打实的收益。

补充两个「碰巧能work」的例外,麻烦对一下你复现环境属于哪种:

  • workingDir 覆盖到 ~(此时 body 实测 readWrite);
  • claude-code + delivery=auto(走 prepareClaudeSkillPlugin 把正文 cpSyncruntime-skills/<sid>/claude-plugin,该目录进了 readonlyRoots → 可读)。

除此之外(其余 20+ 个没声明 skillDelivery 的适配器,或任意适配器显式 prompt),正文都仍不可读。

需要说明:prepareSkillDelivery 在 master 上逐字相同,这个缺口是既有债,不是本 PR 引入的。只是它正好落在本 PR 声称要修的目标上,也正是 PR「仍未验证」表格里列的那一项——我这边有 Linux+bwrap,所以帮你把这一格填上了。建议要么补一条 skills/store(或每个 rootDir)的只读放行,要么把 PR 标题/描述收窄成「修 manifest 可读性 + 错误分诊」,不宣称 skill show 已修复。

2)🟡 错误分类那一半,在 Linux bwrap 上基本不会触发

PR 的根因叙述是「沙箱内读取 EACCES」。实测 bwrap 下未挂载的路径给的是 ENOENT,不是 EACCES

{"p":".../other.json","existsSync":false,"readFileSync":"ENOENT"}
{"p":"/root/.botmux/skills/store","existsSync":false,"readFileSync":"ENOENT"}

--unshare-useruid=0,mode bits 直接被忽略。所以新代码的 if (err?.code === 'ENOENT') return null 会把这种情况照样收敛成 null → 依旧显示 not found。这半边对 macOS Seatbelt(deny read → EPERM/EACCES)和真实权限问题仍然有价值,但它不是线上(Linux daemon)那条报障路径的诊断改善。

另外 chmod 000 那条测试 getuid()===0 时直接 return 跳过,而 daemon 实际就是 root 跑的——这条断言在真实部署环境里不生效。

3)🟡 真正的修复(worker 那处 carve-out)零测试覆盖

反向变异实测:把 mandatoryReadOnlyPaths.push(canonical(join(...'skill-manifests', ...))) 整块删掉后,
fs-policy / skill-manifest-store / skill-cli-commands / session-skill-runtime / plugin-cli-generation / session-skill-injection / skill-resource-reader / read-isolation8 个套件 126 个测试全部照绿

反过来把 manifest-store.ts 还原成 master 的 catch → null,则有 2 条测试立刻变红。也就是说:有牙的测试测的是次要那半,主要那半(策略放行 + 跨 session 隔离)没有任何测试。建议补一条 buildFsPolicy 级断言:本 session manifest → readOnly、别的 session manifest → 不可达。

4)ℹ️ 一个次要观察:file bind 会钉住 inode

bwrap 实测,单文件 --ro-bind 期间宿主做 tmp+rename(atomicWriteFileSync 就是这个形态):

in-sandbox before: {"gen":1}
[host] after rename, host sees: {"gen":2,"NEW":true}
in-sandbox after:  {"gen":1}

沙箱会一直看到旧 inode。当前每次 fresh/resumed spawn 都会重建沙箱,所以不构成本 PR 的正确性问题(且「旧但自洽」比「读到半截」安全);只是 session 存续期间 manifest 被改写/删除时,沙箱内看到的是陈旧内容,留个记录。

已验证通过的部分

  • 单文件放行的隔离性:bwrap 实测本 session manifest 可读、其他 session ENOENT 不可见、写入被拒(Read-only file system)✅
  • 时序正确:manifest 在 prepareCliPluginGenerationAndGateway(worker.ts:12111)写入,早于策略构建(12805),能过 existence-filter ✅
  • tsc --noEmit 干净;你列出的 7 个套件 91/91 绿 ✅
  • 另外帮你把「仍未验证」里的 bwrap 用例跑了:fs-policy-bwrap.e2e + sandbox-mask-manifest 在真 Linux+bwrap 上 20/20 通过
  • 未扩大对凭证 / 其他 session 数据的访问面,实测确认 ✅

综合建议:方向正确、安全边界写得很稳,主要是第 1 点让 PR 目前达不到标题声称的效果(补一处正文放行,或收窄描述),第 3 点建议给主修复补一条有牙的断言。另外分支落后 master 205 个 commit(merge-tree 无冲突,仅供参考)。

以上是自动评审的初步意见,可能有误判,最终以维护者审阅为准

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants