SiYuan MCP asset.upload Reads Arbitrary Absolute File Paths (Workspace Boundary Bypass)GHSA-p23f-cm6q-2qp8

Security Advisory — SiYuan MCP asset.upload Reads Arbitrary Absolute File Paths (Workspace Boundary Bypass)

Summary

SiYuan exposes a native MCP tool asset → asset.upload. Its files argument is documented as a comma-separated list of absolute file paths. The handler (kernel/mcp/tools/asset.go:195) only normalizes each entry with filepath.Abs(...) — it performs no workspace boundary check (IsSubPath) and no sensitive-path check (IsSensitivePath). The downstream model.InsertLocalAssets (kernel/model/upload.go:97) then os.Opens each path and copies its bytes into the workspace assets/ directory.

Every other AI-plane file primitive in SiYuan is workspace-constrained:

• file / unzip tools resolve paths via resolvePath (workspace-relative, escape-proof).

asset.upload is the only AI tool that accepts arbitrary absolute paths with zero boundary validation. An attacker who can steer the Agent (prompt injection) can induce it to upload sensitive files — e.g. /Users/victim/.ssh/id_rsa, ~/.aws/credentials, /etc/passwd — into the workspace, from where they are reachable via notes / export / sync.

Relationship to Prior Advisories

• CVE-2026-66012 (GHSA-cvhv-7xhj-xjp8) remediated the MCP endpoint by requiring CheckAdminRole on /mcp and adding refuseToAccess for conf/conf.json in the file tool. However, the asset.upload tool was never given a workspace boundary check — it still accepts and reads any absolute path. This is a residual boundary omission from that remediation: the admin gate restricts who may call MCP, but does not constrain which files asset.upload may read. We report it as the unpatched half of the MCP file-surface hardening.

Affected Version

Statically confirmed on the latest release v3.8.0 (tag v3.8.0, commit 251596fc0):

• kernel/mcp/tools/asset.go:195 assetUpload: abs, _ := filepath.Abs(strings.TrimSpace(f)) — normalization only.
• kernel/model/upload.go:97 InsertLocalAssets: iterates the path list and os.Open(assetAbsPath) → writeAssetFile(writePath, ...) into assetsDirPath; the IsSubPath(assetsDirPath, assetAbsPath) check at line 130 is only a dedup guard, not a boundary limit. No workspace/external restriction is applied.

Component

• kernel/mcp/tools/asset.go:195 — assetUpload (files arg → filepath.Abs only).
• kernel/model/upload.go:97 — InsertLocalAssets (os.Open + copy to assets/).
• Contrast (safe): kernel/mcp/tools/unzip.go:52 unzipHandler uses resolvePath (workspace-relative, escape-proof).

Attack Vector

AI Agent / prompt injection. Victim lets the Agent process attacker-controlled web/note content → agent is induced to call asset.upload with /Users/victim/.ssh/id_rsa (or similar) as a files entry. The user sees a category-level confirmation ("upload asset") that does not display the specific source path, so the approval is effectively blind to the actual file being read. Once approved, the file is copied into the workspace and exfiltrated via notes/export/sync. No admin role is required beyond the existing Agent/MCP configuration.

Proof of Concept

// Agent tool call (asset.upload), attacker-influenced files argument:
{
  "id": "<someBlockID>",
  "files": "/Users/victim/.ssh/id_rsa,/Users/victim/.aws/credentials,/etc/passwd"
}

Handler flow:

1. filepath.Abs("/Users/victim/.ssh/id_rsa") → /Users/victim/.ssh/id_rsa (no IsSubPath/IsSensitivePath rejection).
2. InsertLocalAssets → os.Open → bytes copied into <workspace>/data/assets/....
3. The private key is now readable via the workspace file API / export / sync.

Impact

Confidentiality breach: arbitrary local file read (including SSH keys, cloud credentials, OS secrets) through the AI plane. Integrity/availability not directly impacted. Severity is moderated by the required user approval step, but the approval dialog does not reveal the real source path, so the user cannot make an informed decision.

Scope

Reachable only when the Agent/MCP surface is configured and the user approves the upload action. The boundary check is absent regardless of config — any approved upload reads outside the workspace.

Remediation

1. Add boundary enforcement in assetUpload: reject any entry where !util.IsSubPath(util.WorkspaceDir, abs) or util.IsSensitivePath(abs) (same checks used elsewhere).
2. Show the real source path in the confirmation dialog so the user can make an informed decision (currently the LocalWrite gate is action-category based and hides the specific path).
3. Mirror the resolvePath workspace-relative constraint already applied to the file / unzip tools.

Note: patch authored against v3.8.0 source; not compiled into a full SiYuan release build. Provided for the maintainer to validate in CI.

Appendix: Suggested Patch (F10)

diff --git a/kernel/mcp/tools/asset.go b/kernel/mcp/tools/asset.go
index aaa..bbb 100644
--- a/kernel/mcp/tools/asset.go
+++ b/kernel/mcp/tools/asset.go
@@ -195,8 +195,16 @@ func assetUpload(args map[string]any) (CallToolResult, error) {
 	fileList := strings.Split(filesStr, ",")
 	for i, f := range fileList {
 		abs, err := filepath.Abs(strings.TrimSpace(f))
-		if err != nil {
+		if err != nil {
 			return CallToolResult{}, err
 		}
+		// 边界校验：仅允许工作区内的资产，拒绝任意绝对路径越界读
+		if !util.IsSubPath(util.WorkspaceDir, abs) || util.IsSensitivePath(abs) {
+			ret.Code = -1
+			ret.Msg = fmt.Sprintf("asset path %s is outside the workspace or sensitive", abs)
+			return CallToolResult{}, fmt.Errorf("asset path outside workspace: %s", abs)
+		}
 		fileList[i] = abs
 	}
 	succMap, err := model.InsertLocalAssets(id, fileList, true)