Просмотр исходного кода

fix(subagent-acp): require search permission on the child cwd

statSync().isDirectory() is true for a mode-600 directory, but a
subprocess cwd needs X_OK — spawn would fail EACCES, deferring a
self-contained configuration error past the load/start boundary the
resolution owns. Probe accessSync(X_OK) alongside the type check so
"not an accessible directory" is literally what the guard verifies.
Yichen Jiang 2 месяцев назад
Родитель
Сommit
fcbbd22af7

+ 12 - 5
packages/subagent/subagent-acp/src/index.ts

@@ -7,7 +7,7 @@
  * @module @deepseek-ai/dsh-subagent-acp
  */
 
-import { statSync } from 'node:fs'
+import { accessSync, constants, statSync } from 'node:fs'
 import { isAbsolute, resolve } from 'node:path'
 import type { Context } from 'cordis'
 import z from 'schemastery'
@@ -77,13 +77,20 @@ function assertPositiveFinite(name: string, value: number): void {
 /** The shape after schemastery applied the defaults (cwd has none). */
 type ResolvedConfig = Required<Omit<Config, 'cwd'>> & Pick<Config, 'cwd'>
 
-/** Whether `path` names an existing, accessible directory. */
+/**
+ * Whether `path` names an existing directory the harness can ENTER. The
+ * search-permission probe matters: `statSync().isDirectory()` is true for a
+ * mode-600 directory, but a subprocess cwd needs `X_OK` or spawn fails EACCES.
+ */
 function isDirectory(path: string): boolean {
   try {
-    return statSync(path).isDirectory()
+    if (!statSync(path).isDirectory()) return false
+    accessSync(path, constants.X_OK)
+    return true
   } catch {
-    // statSync throws only filesystem access errors here (ENOENT/EACCES/ENOTDIR/…),
-    // and every one of them means the path cannot serve as the child's cwd.
+    // statSync/accessSync throw only filesystem access errors here
+    // (ENOENT/EACCES/ENOTDIR/…), and every one of them means the path cannot
+    // serve as the child's cwd.
     return false
   }
 }

+ 24 - 1
packages/subagent/subagent-acp/tests/subagent-acp.spec.ts

@@ -1,7 +1,7 @@
 import { describe, expect, it } from 'vitest'
 import { Context } from 'cordis'
 import Loader from '@cordisjs/plugin-loader'
-import { existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
+import { chmodSync, existsSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join, resolve } from 'node:path'
 import { fileURLToPath } from 'node:url'
@@ -215,6 +215,29 @@ describe('cwd resolution', () => {
     await ctx.fiber.dispose()
   })
 
+  it('rejects a config cwd directory without search permission at load', async () => {
+    // statSync().isDirectory() is true for a mode-600 directory, but a
+    // subprocess cwd needs SEARCH permission — spawn would fail EACCES.
+    const tmp = mkdtempSync(join(tmpdir(), 'acp-noexec-'))
+    chmodSync(tmp, 0o600)
+    try {
+      const ctx = new Context()
+      await ctx.plugin(SubagentService)
+      await expect(ctx.plugin(acp, {
+        providerName: 'acp',
+        command: 'true',
+        args: [],
+        cwd: tmp,
+        permission: 'reject',
+        env: {},
+      })).rejects.toThrow('not an accessible directory')
+      await ctx.fiber.dispose()
+    } finally {
+      chmodSync(tmp, 0o700)
+      rmSync(tmp, { recursive: true, force: true })
+    }
+  })
+
   it('rejects a config cwd that is not an accessible directory at load', async () => {
     const ctx = new Context()
     await ctx.plugin(SubagentService)