Skip to content

fix: harden daemon discovery file trust#65

Draft
Disaster-Terminator wants to merge 1 commit into
mainfrom
fix-mcp-daemon-discovery-vulnerability
Draft

fix: harden daemon discovery file trust#65
Disaster-Terminator wants to merge 1 commit into
mainfrom
fix-mcp-daemon-discovery-vulnerability

Conversation

@Disaster-Terminator

@Disaster-Terminator Disaster-Terminator commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Motivation

  • Discovery metadata read from the state directory was implicitly trusted which allowed an attacker who can write/replace the discovery file to redirect MCP/CLI traffic to an arbitrary endpoint.
  • The change adds POSIX ownership and permission checks to ensure discovery files are only trusted when they are owner-controlled and not accessible/writable by other users.

Description

  • Add POSIX safety checks to discovery reads by introducing assertDiscoveryPathSecure and assertDiscoveryPathSecureSync that verify the discovery file is a regular file, owned by the current user, not group/other-readable or writable, and that the state directory is not group/other-writable (no-op on Windows). (edited src/daemon/discovery.ts)
  • Create discovery state directories with owner-only permissions when writing discovery metadata (mode: 0o700 in writeDaemonDiscovery). (edited src/daemon/discovery.ts)
  • Update discovery tests to preserve secure file permissions for direct writes and add regression tests that reject discovery files readable by other users and discovery in shared writable directories. (edited tests/daemon-discovery.test.ts)
  • Rebuilt distribution artifacts to match source changes.

Testing

  • Ran unit tests: pnpm exec vitest run --maxWorkers=1 tests/daemon-discovery.test.ts tests/mcp-tools.test.ts and all tests passed.
  • Ran static checks: pnpm run typecheck succeeded.
  • Built artifacts: pnpm run build succeeded and updated dist/ and plugins/retinue/dist/ outputs.

Codex Task

Summary by Sourcery

通过对发现文件及其状态目录强制安全的所有权与权限设置,加强守护进程发现元数据的处理。

Bug Fixes:

  • 防止守护进程发现机制信任位于对其他用户可写或可读的文件或目录中的元数据,以避免潜在的流量重定向攻击。

Enhancements:

  • 以仅限所有者访问的权限创建守护进程发现状态目录,确保发现元数据的安全存储。
  • 在发现读取流程中加入基于 POSIX 的安全检查,在使用发现元数据之前验证文件类型、所有权和权限。

Tests:

  • 扩展守护进程发现测试,以覆盖安全权限处理,并验证对其他用户可访问的发现文件和状态目录的拒绝行为。
Original summary in English

Summary by Sourcery

Harden daemon discovery metadata handling by enforcing secure ownership and permissions on discovery files and their state directories.

Bug Fixes:

  • Prevent daemon discovery from trusting metadata stored in files or directories that are writable or readable by other users, avoiding potential traffic redirection attacks.

Enhancements:

  • Create daemon discovery state directories with owner-only permissions to ensure secure storage of discovery metadata.
  • Add POSIX-based security checks to discovery reads to validate file type, ownership, and permissions before using discovery metadata.

Tests:

  • Extend daemon discovery tests to cover secure permission handling and rejection of discovery files and state directories that are accessible by other users.

@sourcery-ai

sourcery-ai Bot commented Jul 11, 2026

Copy link
Copy Markdown

Reviewer's Guide

在守护进程发现文件读取周围添加基于 POSIX 的所有权与权限检查,并确保发现状态目录/文件以安全模式创建,同时附带回归测试和重新构建的 dist 产物。

Sequence diagram for secured daemon discovery read

sequenceDiagram
  actor User
  participant DaemonClient
  participant discovery_module
  participant fs

  User->>DaemonClient: requestDaemonDiscovery(stateDir)
  DaemonClient->>discovery_module: readDaemonDiscovery(stateDir)
  discovery_module->>discovery_module: getDaemonDiscoveryPath(stateDir)
  discovery_module->>discovery_module: assertDiscoveryPathSecure(filePath)
  discovery_module->>fs: lstat(filePath), lstat(dirname(filePath))
  fs-->>discovery_module: fileStat, dirStat
  discovery_module->>discovery_module: assertSecureDiscoveryStats(fileStat, dirStat)
  discovery_module->>fs: readFile(filePath, "utf8")
  fs-->>discovery_module: discoveryJson
  discovery_module->>discovery_module: normalizeDiscovery(parsed)
  discovery_module-->>DaemonClient: DaemonDiscovery
  DaemonClient-->>User: secured daemon endpoint
Loading

File-Level Changes

Change Details Files
使用 POSIX 安全检查和安全的目录/文件模式加强守护进程发现读写逻辑。
  • 在写入发现元数据时,将发现状态目录的创建权限设为仅所有者可访问(模式 0o700)。
  • 引入异步和同步的发现路径安全断言,在 Windows 上为 no-op,在其他系统上会在读取元数据前验证目录/文件类型、所有权以及权限位。
  • 确保在状态目录不是目录、不是当前用户所有、可被其他用户写入,或者发现文件不是常规文件或可被其他用户读/写时,发现读取会抛出明确的错误。
src/daemon/discovery.ts
dist/daemon/discovery.js
plugins/retinue/dist/daemon/discovery.js
扩展守护进程发现测试以覆盖安全权限行为和新的拒绝场景。
  • 更新测试中直接写入发现文件的逻辑,以保持安全文件权限(模式 0o600)。
  • 添加回归测试,拒绝将发现文件设置为对世界或用户组可读的情况(例如 chmod 0o644)。
  • 添加回归测试,拒绝来自可共享写入的状态目录中的发现数据(例如 chmod 0o777)。
tests/daemon-discovery.test.ts
重新生成构建产物以反映发现安全性的变更。
  • 更新 dist 和插件分发中的编译 JS 及 source map,以匹配新的发现实现。
dist/daemon/discovery.js.map
plugins/retinue/dist/daemon/discovery.js.map

Tips and commands

Interacting with Sourcery

  • 触发新评审: 在 pull request 中评论 @sourcery-ai review
  • 继续讨论: 直接回复 Sourcery 的评审评论。
  • 从评审评论生成 GitHub issue: 通过回复某条评审评论,要求 Sourcery 基于该评论创建一个 issue。你也可以在评审评论中回复 @sourcery-ai issue 来从该评论创建 issue。
  • 生成 pull request 标题: 在 pull request 标题中任意位置写上 @sourcery-ai,即可在任何时间生成标题。你也可以在 pull request 中评论 @sourcery-ai title 来(重新)生成标题。
  • 生成 pull request 摘要: 在 pull request 的正文中任意位置写上 @sourcery-ai summary,即可在你希望的位置生成 PR 摘要。你也可以在 pull request 中评论 @sourcery-ai summary 来在任何时间(重新)生成摘要。
  • 生成评审者指南: 在 pull request 中评论 @sourcery-ai guide,即可在任何时间(重新)生成评审者指南。
  • 解决所有 Sourcery 评论: 在 pull request 中评论 @sourcery-ai resolve,以解决所有 Sourcery 评论。如果你已经处理完所有评论且不希望再看到它们,这会很有用。
  • 关闭所有 Sourcery 评审: 在 pull request 中评论 @sourcery-ai dismiss,以关闭所有现有的 Sourcery 评审。如果你希望从一次全新的评审开始,这尤其有用——别忘了评论 @sourcery-ai review 来触发新的评审!

Customizing Your Experience

访问你的 dashboard 来:

  • 启用或禁用评审功能,例如 Sourcery 生成的 pull request 摘要、评审者指南等。
  • 更改评审语言。
  • 添加、移除或编辑自定义评审说明。
  • 调整其他评审设置。

Getting Help

Original review guide in English

Reviewer's Guide

Adds POSIX-based ownership and permission checks around daemon discovery file reads and ensures discovery state directories/files are created with secure modes, with accompanying regression tests and rebuilt dist artifacts.

Sequence diagram for secured daemon discovery read

sequenceDiagram
  actor User
  participant DaemonClient
  participant discovery_module
  participant fs

  User->>DaemonClient: requestDaemonDiscovery(stateDir)
  DaemonClient->>discovery_module: readDaemonDiscovery(stateDir)
  discovery_module->>discovery_module: getDaemonDiscoveryPath(stateDir)
  discovery_module->>discovery_module: assertDiscoveryPathSecure(filePath)
  discovery_module->>fs: lstat(filePath), lstat(dirname(filePath))
  fs-->>discovery_module: fileStat, dirStat
  discovery_module->>discovery_module: assertSecureDiscoveryStats(fileStat, dirStat)
  discovery_module->>fs: readFile(filePath, "utf8")
  fs-->>discovery_module: discoveryJson
  discovery_module->>discovery_module: normalizeDiscovery(parsed)
  discovery_module-->>DaemonClient: DaemonDiscovery
  DaemonClient-->>User: secured daemon endpoint
Loading

File-Level Changes

Change Details Files
Harden daemon discovery read/write logic with POSIX security checks and secure directory/file modes.
  • Set discovery state directory creation to use owner-only permissions (mode 0o700) when writing discovery metadata.
  • Introduce async and sync discovery path security assertions that no-op on Windows and validate directory/file type, ownership, and permission bits before reading metadata.
  • Ensure discovery reads throw explicit errors when the state directory is not a directory, not owned by the current user, writable by others, or when the discovery file is not a regular file or is readable/writable by other users.
src/daemon/discovery.ts
dist/daemon/discovery.js
plugins/retinue/dist/daemon/discovery.js
Extend daemon discovery tests to cover secure permissions behavior and new rejection scenarios.
  • Update direct discovery file writes in tests to preserve secure file permissions (mode 0o600).
  • Add regression test that rejects discovery files made world- or group-readable (e.g., chmod 0o644).
  • Add regression test that rejects discovery from shared writable state directories (e.g., chmod 0o777).
tests/daemon-discovery.test.ts
Regenerate build artifacts to reflect discovery security changes.
  • Update compiled JS and source maps in dist and plugin distributions to match the new discovery implementation.
dist/daemon/discovery.js.map
plugins/retinue/dist/daemon/discovery.js.map

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant