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

src/core/skills/cli-session-command.ts

@@ -1,4 +1,4 @@
-import { readSessionSkillManifest } from './manifest-store.js';
+import { readSessionSkillManifest, SkillManifestReadError, SkillManifestParseError } from './manifest-store.js';
 import { listSkillResources, readSkillEntrypoint, readSkillResource } from './resource-reader.js';
 import { builtinSkillContent, builtinSkillEntries } from '../../skills/injection-mode.js';
 import { whiteboardEnabled } from '../../services/whiteboard-store.js';
@@ -37,7 +37,21 @@ export function runSkillSessionCommand(
   }
   const sessionId = sessionIdFromEnv(env);
   if (!sessionId) return { code: 2, stdout: '', stderr: 'missing BOTMUX_SESSION_ID\n' };
-  const manifest = readSessionSkillManifest(sessionId);
+  let manifest;
+  try {
+    manifest = readSessionSkillManifest(sessionId);
+  } catch (err) {
+    // Present-but-unreadable (sandbox/policy) or corrupt manifest — a real
+    // fault, NOT "not found". Distinct message + exit 1 so the sandbox misconfig
+    // is diagnosable instead of masquerading as an absent manifest (exit 2).
+    if (err instanceof SkillManifestReadError) {
+      return { code: 1, stdout: '', stderr: `skill manifest exists but is unreadable for session ${sessionId} (${(err.cause as any)?.code ?? 'read error'}) — check the file sandbox read-only policy: ${err.path}\n` };
+    }
+    if (err instanceof SkillManifestParseError) {
+      return { code: 1, stdout: '', stderr: `skill manifest is corrupt for session ${sessionId}: ${err.path}\n` };
+    }
+    throw err;
+  }
   if (!manifest) {
     // No user-skill manifest — still surface built-ins for discovery.
     if (sub === 'list') {

src/core/skills/manifest-store.ts

@@ -1,9 +1,27 @@
-import { existsSync, mkdirSync, readFileSync, rmSync } from 'node:fs';
+import { mkdirSync, readFileSync, rmSync } from 'node:fs';
 import { join } from 'node:path';
 import { config } from '../../config.js';
 import { atomicWriteFileSync } from '../../utils/atomic-write.js';
 import type { SessionSkillManifest } from './types.js';
 
+/** Manifest file exists but could not be READ (permission denied, I/O error).
+ *  Distinct from "absent" so callers can diagnose a sandbox/policy misconfig
+ *  instead of reporting a misleading "not found". */
+export class SkillManifestReadError extends Error {
+  constructor(public readonly path: string, public readonly cause: any) {
+    super(`skill manifest unreadable (${cause?.code ?? 'read error'}): ${path}`);
+    this.name = 'SkillManifestReadError';
+  }
+}
+
+/** Manifest file was read but is not valid JSON. */
+export class SkillManifestParseError extends Error {
+  constructor(public readonly path: string, public readonly cause: any) {
+    super(`skill manifest is corrupt (invalid JSON): ${path}`);
+    this.name = 'SkillManifestParseError';
+  }
+}
+
 function manifestDir(): string {
   return join(config.session.dataDir, 'skill-manifests');
 }
@@ -19,11 +37,24 @@ export function writeSessionSkillManifest(manifest: SessionSkillManifest): void
 
 export function readSessionSkillManifest(sessionId: string): SessionSkillManifest | null {
   const file = manifestPath(sessionId);
-  if (!existsSync(file)) return null;
+  let raw: string;
+  try {
+    raw = readFileSync(file, 'utf-8');
+  } catch (err: any) {
+    // ENOENT is the only "genuinely absent" case → null (callers report
+    // not-found). A permission error (EACCES/EPERM — e.g. the file sandbox
+    // never exposed this session's manifest) or any other read failure is a
+    // real fault: surface it so `skill show` can tell "no manifest" apart from
+    // "manifest unreadable", instead of masking a sandbox misconfig as
+    // not-found (the historical failure mode).
+    if (err?.code === 'ENOENT') return null;
+    throw new SkillManifestReadError(file, err);
+  }
   try {
-    return JSON.parse(readFileSync(file, 'utf-8')) as SessionSkillManifest;
-  } catch {
-    return null;
+    return JSON.parse(raw) as SessionSkillManifest;
+  } catch (err: any) {
+    // File is present and readable but corrupt — a distinct, diagnosable fault.
+    throw new SkillManifestParseError(file, err);
   }
 }
 

src/worker.ts

@@ -12903,6 +12903,20 @@ async function spawnCli(
         config.session.dataDir,
       ).map(canonical));
     }
+    // The sandboxed CLI reads its OWN session's skill manifest via
+    // `botmux skill show <name>` (manifest-store.ts writes it to
+    // `<config.session.dataDir>/skill-manifests/<sessionId>.json`). Without an
+    // explicit carve-out the file sandbox never exposes it, so the read EACCES's
+    // and `skill show` maps that to "manifest not found" — user-skill bodies are
+    // unreadable under sandbox=true + skillInjection=prompt. Expose ONLY this
+    // session's own manifest file, read-only, canonicalized so a symlinked HOME
+    // (`/home/u` → `/data00/home/u`) still matches. Scoping to the single
+    // `<sessionId>.json` (not the whole skill-manifests dir) keeps a task from
+    // reading other sessions' manifests. Built from config.session.dataDir — the
+    // exact path the store writes to, not the possibly-unset SESSION_DATA_DIR.
+    mandatoryReadOnlyPaths.push(
+      canonical(join(config.session.dataDir, 'skill-manifests', `${cfg.sessionId}.json`)),
+    );
     if (process.platform === 'darwin') {
       const gatewaySocketRoot = canonical(
         sessionMcpGatewayHost ? dirname(sessionMcpGatewayHost.socketDir) : tmpdir(),

test/skill-cli-commands.test.ts

@@ -62,4 +62,18 @@ describe('botmux skill session command', () => {
   it('refuses to run without a session id', () => {
     expect(runSkillSessionCommand(['list'], {}).stderr).toContain('missing BOTMUX_SESSION_ID');
   });
+
+  it('reports a corrupt manifest as unreadable, not not-found', () => {
+    write(join(dataDir, 'skill-manifests', 'broken.json'), '{ nope');
+    const res = runSkillSessionCommand(['show', 'deploy'], { BOTMUX_SESSION_ID: 'broken' });
+    expect(res.code).toBe(1);
+    expect(res.stderr).toContain('corrupt');
+    expect(res.stderr).not.toContain('not found');
+  });
+
+  it('still reports a genuinely absent manifest as not found', () => {
+    const res = runSkillSessionCommand(['show', 'deploy'], { BOTMUX_SESSION_ID: 'absent' });
+    expect(res.code).toBe(2);
+    expect(res.stderr).toContain('manifest not found');
+  });
 });

test/skill-manifest-store.test.ts

@@ -1,9 +1,14 @@
 import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
-import { mkdtempSync, rmSync } from 'node:fs';
+import { mkdtempSync, rmSync, writeFileSync, mkdirSync, chmodSync } from 'node:fs';
 import { join } from 'node:path';
 import { tmpdir } from 'node:os';
 
-import { readSessionSkillManifest, writeSessionSkillManifest } from '../src/core/skills/manifest-store.js';
+import {
+  readSessionSkillManifest,
+  writeSessionSkillManifest,
+  SkillManifestReadError,
+  SkillManifestParseError,
+} from '../src/core/skills/manifest-store.js';
 import type { SessionSkillManifest } from '../src/core/skills/types.js';
 
 describe('session skill manifest store', () => {
@@ -34,4 +39,34 @@ describe('session skill manifest store', () => {
 
     expect(readSessionSkillManifest('s1')).toEqual(manifest);
   });
+
+  it('returns null only when the manifest is genuinely absent (ENOENT)', () => {
+    expect(readSessionSkillManifest('never-written')).toBeNull();
+  });
+
+  it('throws a corrupt-JSON error instead of masking it as not-found', () => {
+    mkdirSync(join(dataDir, 'skill-manifests'), { recursive: true });
+    writeFileSync(join(dataDir, 'skill-manifests', 'bad.json'), '{ not valid json');
+    expect(() => readSessionSkillManifest('bad')).toThrow(SkillManifestParseError);
+  });
+
+  it('throws a read error (not null) when the manifest is present but unreadable', () => {
+    // A permission-denied read (the sandbox never exposed this session's
+    // manifest) must NOT collapse to "not found" — regression for the
+    // sandbox=true skill-body-unreadable bug.
+    mkdirSync(join(dataDir, 'skill-manifests'), { recursive: true });
+    const file = join(dataDir, 'skill-manifests', 'locked.json');
+    writeFileSync(file, '{}');
+    chmodSync(file, 0o000);
+    try {
+      // root ignores mode bits — skip the assertion there rather than false-fail.
+      let readable = true;
+      try { readSessionSkillManifest('locked'); } catch { readable = false; }
+      if (typeof process.getuid === 'function' && process.getuid() === 0) return;
+      expect(readable).toBe(false);
+      expect(() => readSessionSkillManifest('locked')).toThrow(SkillManifestReadError);
+    } finally {
+      chmodSync(file, 0o600);
+    }
+  });
 });