From 14e1dd9f4e52bd5e70fb4c99d236b5ff833201f8 Mon Sep 17 00:00:00 2001 From: murashit Date: Wed, 27 May 2026 10:39:59 +0900 Subject: [PATCH] Fix vault-relative file link resolution --- src/features/chat/chat-message-renderer.ts | 13 +- src/features/chat/markdown-file-links.ts | 34 ++++- .../chat/chat-message-renderer.test.ts | 133 +++++++++++++++++- .../features/chat/markdown-file-links.test.ts | 55 ++++++-- 4 files changed, 209 insertions(+), 26 deletions(-) diff --git a/src/features/chat/chat-message-renderer.ts b/src/features/chat/chat-message-renderer.ts index 69cfbcf7..69801219 100644 --- a/src/features/chat/chat-message-renderer.ts +++ b/src/features/chat/chat-message-renderer.ts @@ -1,4 +1,4 @@ -import { MarkdownRenderer, type App, type Component } from "obsidian"; +import { MarkdownRenderer, Notice, type App, type Component } from "obsidian"; import type { DisplayItem } from "./display/types"; import { copyTextWithNotice } from "../../shared/ui/clipboard"; @@ -6,7 +6,7 @@ import { renderTextWithWikiLinks as renderInlineWikiLinks } from "../../shared/u import { messageRenderBlocks, notifyMessageContentRendered, syncMessageRenderBlocks } from "./ui/message-stream"; import { bottomScrollTop, captureScrollAnchor, isNearScrollBottom, restoreScrollAnchor } from "./ui/scroll"; import type { ChatTurnDiffViewState } from "./ui/turn-diff"; -import { markdownFileLinkTarget } from "./markdown-file-links"; +import { isAbsoluteFileHref, vaultFileLinkTarget, vaultRelativeFileLinkTarget } from "./markdown-file-links"; import { isRollbackCandidateItem, rollbackCandidateFromItems } from "./rollback"; import type { ChatState } from "./chat-state"; @@ -137,7 +137,12 @@ export class ChatMessageRenderer { link.addClass("codex-panel__wikilink"); link.onclick = (event) => { event.preventDefault(); - const target = link.getAttribute("data-href") ?? link.getAttribute("href") ?? link.textContent; + const href = link.getAttribute("data-href") ?? link.getAttribute("href") ?? link.textContent; + const target = vaultRelativeFileLinkTarget(this.options.vaultPath, this.options.app.vault.configDir, href) ?? href; + if (target === href && isAbsoluteFileHref(href)) { + new Notice("Cannot open files outside the vault."); + return; + } if (target.trim().length > 0) { void this.options.app.workspace.openLinkText(target, sourcePath, false); } @@ -148,7 +153,7 @@ export class ChatMessageRenderer { private bindRenderedMarkdownFileLinks(parent: HTMLElement, sourcePath: string): void { parent.querySelectorAll("a[href]:not(.internal-link)").forEach((link) => { const href = link.getAttribute("href") ?? ""; - const target = markdownFileLinkTarget(this.options.app, this.options.vaultPath, href); + const target = vaultFileLinkTarget(this.options.app, this.options.vaultPath, href); if (!target) return; link.addClass("codex-panel__filelink"); diff --git a/src/features/chat/markdown-file-links.ts b/src/features/chat/markdown-file-links.ts index 1d636038..1ad76c68 100644 --- a/src/features/chat/markdown-file-links.ts +++ b/src/features/chat/markdown-file-links.ts @@ -1,18 +1,37 @@ import { TFile, type App } from "obsidian"; -export function markdownFileLinkTarget(app: App, vaultPath: string, href: string): string | null { - const parsed = parseMarkdownFileHref(href); +export function vaultFileLinkTarget(app: App, vaultPath: string, href: string): string | null { + const relativePath = vaultRelativeFileHref(vaultPath, app.vault.configDir, href); + if (!relativePath) return null; + + const abstractFile = app.vault.getAbstractFileByPath(relativePath.path); + return abstractFile instanceof TFile ? `${relativePath.path}${relativePath.subpath}` : null; +} + +export function vaultRelativeFileLinkTarget(vaultPath: string, configDir: string, href: string): string | null { + const relativePath = vaultRelativeFileHref(vaultPath, configDir, href); + return relativePath ? `${relativePath.path}${relativePath.subpath}` : null; +} + +export function isAbsoluteFileHref(href: string): boolean { + const parsed = parseFileHref(href); + return parsed ? isAbsolutePath(normalizeFilePath(parsed.path)) : false; +} + +function vaultRelativeFileHref(vaultPath: string, configDir: string, href: string): { path: string; subpath: string } | null { + const parsed = parseFileHref(href); if (!parsed) return null; const relativePath = vaultRelativePath(vaultPath, parsed.path); if (!relativePath) return null; const normalized = normalizeFilePath(relativePath); - const abstractFile = app.vault.getAbstractFileByPath(normalized); - return abstractFile instanceof TFile ? `${normalized}${parsed.subpath}` : null; + if (isVaultConfigPath(normalized, configDir)) return null; + + return { path: normalized, subpath: parsed.subpath }; } -function parseMarkdownFileHref(href: string): { path: string; subpath: string } | null { +function parseFileHref(href: string): { path: string; subpath: string } | null { const trimmed = href.trim(); if (!trimmed || isExternalHref(trimmed)) return null; @@ -63,3 +82,8 @@ function isAbsolutePath(path: string): boolean { function isWindowsAbsolutePath(path: string): boolean { return /^[a-z]:[\\/]/i.test(path); } + +function isVaultConfigPath(path: string, configDir: string): boolean { + const normalizedConfigDir = normalizeFilePath(configDir); + return path === normalizedConfigDir || path.startsWith(`${normalizedConfigDir}/`); +} diff --git a/tests/features/chat/chat-message-renderer.test.ts b/tests/features/chat/chat-message-renderer.test.ts index e9c1eef2..d908c748 100644 --- a/tests/features/chat/chat-message-renderer.test.ts +++ b/tests/features/chat/chat-message-renderer.test.ts @@ -1,14 +1,117 @@ // @vitest-environment jsdom -import { describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { TFile } from "obsidian"; import { ChatMessageRenderer } from "../../../src/features/chat/chat-message-renderer"; import { createChatState } from "../../../src/features/chat/chat-state"; import { installObsidianDomShims } from "./ui/dom-test-helpers"; +import { notices } from "../../mocks/obsidian"; installObsidianDomShims(); describe("ChatMessageRenderer scroll pinning", () => { + beforeEach(() => { + notices.length = 0; + }); + + it("normalizes rendered internal links that point at absolute vault paths", () => { + const openLinkText = vi.fn(); + const renderer = chatMessageRenderer(createChatState(), openLinkText, "/Users/showhey/Vault", ["docs/Guide.md"]); + const parent = document.createElement("div"); + const link = parent.createEl("a", { + cls: "internal-link", + text: "Guide.md", + attr: { + "data-href": "/Users/showhey/Vault/docs/Guide.md", + href: "/Users/showhey/Vault/docs/Guide.md", + }, + }); + + bindRenderedWikiLinks(renderer, parent, "Inbox.md"); + link.click(); + + expect(openLinkText).toHaveBeenCalledWith("docs/Guide.md", "Inbox.md", false); + }); + + it("normalizes rendered internal links for missing files inside the vault", () => { + const openLinkText = vi.fn(); + const renderer = chatMessageRenderer(createChatState(), openLinkText, "/Users/showhey/Vault"); + const parent = document.createElement("div"); + const link = parent.createEl("a", { + cls: "internal-link", + text: "Missing.md", + attr: { + "data-href": "/Users/showhey/Vault/docs/Missing.md", + href: "/Users/showhey/Vault/docs/Missing.md", + }, + }); + + bindRenderedWikiLinks(renderer, parent, "Inbox.md"); + link.click(); + + expect(openLinkText).toHaveBeenCalledWith("docs/Missing.md", "Inbox.md", false); + }); + + it("keeps rendered internal links unchanged when they are not vault file paths", () => { + const openLinkText = vi.fn(); + const renderer = chatMessageRenderer(createChatState(), openLinkText); + const parent = document.createElement("div"); + const link = parent.createEl("a", { + cls: "internal-link", + text: "Project", + attr: { + "data-href": "Project", + href: "Project", + }, + }); + + bindRenderedWikiLinks(renderer, parent, "Inbox.md"); + link.click(); + + expect(openLinkText).toHaveBeenCalledWith("Project", "Inbox.md", false); + }); + + it("does not open rendered internal links for absolute paths outside the vault", () => { + const openLinkText = vi.fn(); + const renderer = chatMessageRenderer(createChatState(), openLinkText, "/Users/showhey/Vault"); + const parent = document.createElement("div"); + const link = parent.createEl("a", { + cls: "internal-link", + text: "README.md", + attr: { + "data-href": "/Users/showhey/Repos/codex-panel/README.md", + href: "/Users/showhey/Repos/codex-panel/README.md", + }, + }); + + bindRenderedWikiLinks(renderer, parent, "Inbox.md"); + link.click(); + + expect(openLinkText).not.toHaveBeenCalled(); + expect(notices).toEqual(["Cannot open files outside the vault."]); + }); + + it("does not open rendered internal links for vault config paths", () => { + const openLinkText = vi.fn(); + const renderer = chatMessageRenderer(createChatState(), openLinkText, "/Users/showhey/Vault"); + const parent = document.createElement("div"); + const link = parent.createEl("a", { + cls: "internal-link", + text: "main.js", + attr: { + "data-href": "/Users/showhey/Vault/vault-config/plugins/foo/main.js", + href: "/Users/showhey/Vault/vault-config/plugins/foo/main.js", + }, + }); + + bindRenderedWikiLinks(renderer, parent, "Inbox.md"); + link.click(); + + expect(openLinkText).not.toHaveBeenCalled(); + expect(notices).toEqual(["Cannot open files outside the vault."]); + }); + it("pins to the scroll container bottom without aligning the last message element", async () => { const state = createChatState(); state.activeThreadId = "thread"; @@ -84,17 +187,27 @@ describe("ChatMessageRenderer scroll pinning", () => { }); }); -function chatMessageRenderer(state = createChatState()): ChatMessageRenderer { +function chatMessageRenderer( + state = createChatState(), + openLinkText = vi.fn(), + vaultPath = "/vault", + vaultFiles: string[] = [], +): ChatMessageRenderer { + const files = new Map(vaultFiles.map((path) => [path, tFile(path)])); return new ChatMessageRenderer({ app: { workspace: { getActiveFile: vi.fn(() => null), - openLinkText: vi.fn(), + openLinkText, + }, + vault: { + configDir: "vault-config", + getAbstractFileByPath: (path: string) => files.get(path) ?? null, }, } as never, owner: {} as never, state, - vaultPath: "/vault", + vaultPath, blockSignatures: new Map(), consumeScrollIntent: () => "auto", loadOlderTurns: vi.fn(), @@ -106,6 +219,18 @@ function chatMessageRenderer(state = createChatState()): ChatMessageRenderer { }); } +function bindRenderedWikiLinks(renderer: ChatMessageRenderer, parent: HTMLElement, sourcePath: string): void { + (renderer as unknown as { bindRenderedWikiLinks: (parent: HTMLElement, sourcePath: string) => void }).bindRenderedWikiLinks( + parent, + sourcePath, + ); +} + +function tFile(path: string): TFile { + const basename = path.split("/").pop()?.replace(/\.md$/, "") ?? path; + return Object.assign(new TFile(), { path, basename }); +} + async function settleMessageRender(element: HTMLElement): Promise { await Promise.resolve(); await new Promise((resolve) => { diff --git a/tests/features/chat/markdown-file-links.test.ts b/tests/features/chat/markdown-file-links.test.ts index 6457cff2..317b57b2 100644 --- a/tests/features/chat/markdown-file-links.test.ts +++ b/tests/features/chat/markdown-file-links.test.ts @@ -1,49 +1,77 @@ import { describe, expect, it } from "vitest"; import { TFile, type App } from "obsidian"; -import { markdownFileLinkTarget } from "../../../src/features/chat/markdown-file-links"; +import { isAbsoluteFileHref, vaultFileLinkTarget, vaultRelativeFileLinkTarget } from "../../../src/features/chat/markdown-file-links"; describe("markdown file links", () => { it("resolves absolute vault paths", () => { const app = appFixture(["docs/Guide.md"]); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/docs/Guide.md")).toBe("docs/Guide.md"); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/docs/Guide.md")).toBe("docs/Guide.md"); + }); + + it("normalizes absolute vault paths without requiring an existing file", () => { + expect(vaultRelativeFileLinkTarget("/Users/showhey/Vault", "vault-config", "/Users/showhey/Vault/docs/Missing.md")).toBe( + "docs/Missing.md", + ); + expect(vaultRelativeFileLinkTarget("/Users/showhey/Vault", "vault-config", "/Users/showhey/Vault/docs/Missing.md#Heading")).toBe( + "docs/Missing.md#Heading", + ); + }); + + it("does not normalize vault config paths as openable vault links", () => { + expect( + vaultRelativeFileLinkTarget("/Users/showhey/Vault", "vault-config", "/Users/showhey/Vault/vault-config/plugins/foo/main.js"), + ).toBeNull(); + }); + + it("requires an existing file for vault file links", () => { + const app = appFixture([]); + + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/docs/Missing.md")).toBeNull(); }); it("strips Codex line suffixes from absolute vault paths", () => { const app = appFixture(["src/main.ts"]); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/src/main.ts:12")).toBe("src/main.ts"); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/src/main.ts:12:4")).toBe("src/main.ts"); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/src/main.ts:12")).toBe("src/main.ts"); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/src/main.ts:12:4")).toBe("src/main.ts"); }); it("keeps markdown fragments on vault file links", () => { const app = appFixture(["docs/foo.md", "src/main.ts"]); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/docs/foo.md#Heading")).toBe("docs/foo.md#Heading"); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/src/main.ts:12#L12")).toBe("src/main.ts#L12"); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/docs/foo.md#Heading")).toBe("docs/foo.md#Heading"); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Vault/src/main.ts:12#L12")).toBe("src/main.ts#L12"); }); it("resolves relative markdown links when the file exists", () => { const app = appFixture(["docs/foo.md"]); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "docs/foo.md")).toBe("docs/foo.md"); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "docs/foo.md")).toBe("docs/foo.md"); }); it("resolves Windows absolute vault paths", () => { const app = appFixture(["docs/Guide.md", "src/main.ts"]); - expect(markdownFileLinkTarget(app, "C:\\Users\\showhey\\Vault", "C:\\Users\\showhey\\Vault\\docs\\Guide.md")).toBe("docs/Guide.md"); - expect(markdownFileLinkTarget(app, "C:/Users/showhey/Vault", "C:/Users/showhey/Vault/src/main.ts:12")).toBe("src/main.ts"); + expect(vaultFileLinkTarget(app, "C:\\Users\\showhey\\Vault", "C:\\Users\\showhey\\Vault\\docs\\Guide.md")).toBe("docs/Guide.md"); + expect(vaultFileLinkTarget(app, "C:/Users/showhey/Vault", "C:/Users/showhey/Vault/src/main.ts:12")).toBe("src/main.ts"); }); it("leaves external and non-vault links untouched", () => { const app = appFixture(["docs/foo.md"]); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "https://example.com/docs/foo.md")).toBeNull(); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "mailto:test@example.com")).toBeNull(); - expect(markdownFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Other/docs/foo.md")).toBeNull(); - expect(markdownFileLinkTarget(app, "C:/Users/showhey/Vault", "C:/Users/showhey/Other/docs/foo.md")).toBeNull(); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "https://example.com/docs/foo.md")).toBeNull(); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "mailto:test@example.com")).toBeNull(); + expect(vaultFileLinkTarget(app, "/Users/showhey/Vault", "/Users/showhey/Other/docs/foo.md")).toBeNull(); + expect(vaultFileLinkTarget(app, "C:/Users/showhey/Vault", "C:/Users/showhey/Other/docs/foo.md")).toBeNull(); + }); + + it("identifies filesystem absolute hrefs", () => { + expect(isAbsoluteFileHref("/Users/showhey/Vault/docs/Guide.md")).toBe(true); + expect(isAbsoluteFileHref("C:\\Users\\showhey\\Vault\\docs\\Guide.md")).toBe(true); + expect(isAbsoluteFileHref("docs/Guide.md")).toBe(false); + expect(isAbsoluteFileHref("https://example.com/docs/Guide.md")).toBe(false); }); }); @@ -51,6 +79,7 @@ function appFixture(paths: string[]): App { const files = new Map(paths.map((path) => [path, tFile(path)])); return { vault: { + configDir: "vault-config", getAbstractFileByPath: (path: string) => files.get(path) ?? null, }, } as unknown as App;