diff --git a/news/changelog-1.9.md b/news/changelog-1.9.md index 9c327bc5e1d..a78022004f2 100644 --- a/news/changelog-1.9.md +++ b/news/changelog-1.9.md @@ -128,3 +128,4 @@ All changes included in 1.9: - ([#13575](https://github.com/quarto-dev/quarto-cli/pull/13575)): Improve CPU architecture detection/reporting in macOS to allow quarto to run in virtualized environments such as OpenAI's `codex`. - ([#13656](https://github.com/quarto-dev/quarto-cli/issues/13656)): Fix R code cells with empty `lang: ""` option producing invalid markdown class attributes. - ([#13832](https://github.com/quarto-dev/quarto-cli/pull/13832)): Fix `license.text` metadata not being accessible when using an inline license (`license: "text"`), and populate it with the license name for CC licenses instead of empty string. (author: @mcanouil) +- ([#13890](https://github.com/quarto-dev/quarto-cli/issues/13890)): Fix render failure when using `embed-resources: true` with input path through a symlinked directory. The cleanup now resolves symlinks before comparing paths. diff --git a/src/deno_ral/fs.ts b/src/deno_ral/fs.ts index e48e1c68ca9..05bc526b4f1 100644 --- a/src/deno_ral/fs.ts +++ b/src/deno_ral/fs.ts @@ -8,6 +8,7 @@ import { fromFileUrl } from "./path.ts"; import { resolve, SEP as SEPARATOR } from "./path.ts"; import { copySync } from "fs/copy"; import { existsSync } from "fs/exists"; +import { originalRealPathSync } from "./original-real-path.ts"; export { ensureDir, ensureDirSync } from "fs/ensure-dir"; export { existsSync } from "fs/exists"; @@ -132,7 +133,26 @@ export function safeRemoveDirSync( path: string, boundary: string, ) { - if (path === boundary || !isSubdir(boundary, path)) { + // Resolve symlinks to ensure consistent path comparison. + // This is needed because external tools (like knitr) may resolve symlinks + // while project.dir preserves them. + // + // We use the original Deno.realPathSync (saved before monkey-patching) + // because the monkey-patch replaces it with normalizePath which doesn't + // resolve symlinks. + // + // Note: The UNC path bug that motivated the monkey-patch was fixed in + // Deno v1.16 (see denoland/deno#12243), so this is safe on all platforms. + let resolvedPath = path; + let resolvedBoundary = boundary; + try { + resolvedPath = originalRealPathSync(path); + resolvedBoundary = originalRealPathSync(boundary); + } catch { + // If resolution fails (e.g., path doesn't exist), use original paths + } + + if (resolvedPath === resolvedBoundary || !isSubdir(resolvedBoundary, resolvedPath)) { throw new UnsafeRemovalError( `Refusing to remove directory ${path} that isn't a subdirectory of ${boundary}`, ); diff --git a/src/deno_ral/original-real-path.ts b/src/deno_ral/original-real-path.ts new file mode 100644 index 00000000000..e2c315ca642 --- /dev/null +++ b/src/deno_ral/original-real-path.ts @@ -0,0 +1,12 @@ +/* + * original-real-path.ts + * + * Copyright (C) 2020-2025 Posit Software, PBC + * + * Saves original Deno.realPathSync before monkey-patching. + * This module MUST be imported in quarto.ts BEFORE monkey-patch.ts. + * + * DO NOT add any imports to this file - it must remain dependency-free + * to avoid circular import issues. + */ +export const originalRealPathSync: typeof Deno.realPathSync = Deno.realPathSync; diff --git a/src/quarto.ts b/src/quarto.ts index f421bf2ed6f..616bf8928be 100644 --- a/src/quarto.ts +++ b/src/quarto.ts @@ -4,6 +4,8 @@ * Copyright (C) 2020-2022 Posit Software, PBC */ +// Must be FIRST to save original Deno.realPathSync before monkey-patching +import "./deno_ral/original-real-path.ts"; import "./core/deno/monkey-patch.ts"; import { diff --git a/tests/docs/render/symlink-embed-resources/test.qmd b/tests/docs/render/symlink-embed-resources/test.qmd new file mode 100644 index 00000000000..8948f0ddec0 --- /dev/null +++ b/tests/docs/render/symlink-embed-resources/test.qmd @@ -0,0 +1,13 @@ +--- +format: + html: + embed-resources: true +--- + +# Test Document + +This tests rendering via symlinked directory with embed-resources. + +```{r} +plot(1:10) +``` diff --git a/tests/smoke/render/render-symlink-embed-resources.test.ts b/tests/smoke/render/render-symlink-embed-resources.test.ts new file mode 100644 index 00000000000..f9395cb4f4a --- /dev/null +++ b/tests/smoke/render/render-symlink-embed-resources.test.ts @@ -0,0 +1,54 @@ +/* + * render-symlink-embed-resources.test.ts + * + * Regression test for https://github.com/quarto-dev/quarto-cli/issues/13890 + * + * When rendering a qmd via a symlinked path with embed-resources: true, + * the cleanup should not fail due to path mismatch between symlink and real path. + * + * Copyright (C) 2020-2025 Posit Software, PBC + */ + +import { testQuartoCmd } from "../../test.ts"; +import { noErrors, fileExists } from "../../verify.ts"; +import { docs } from "../../utils.ts"; +import { join, dirname, resolve } from "../../../src/deno_ral/path.ts"; +import { existsSync } from "../../../src/deno_ral/fs.ts"; +import { isWindows } from "../../../src/deno_ral/platform.ts"; + +const testDir = docs("render/symlink-embed-resources"); +const testFile = "test.qmd"; +const symlinkDir = join(dirname(testDir), "symlink-embed-resources-link"); + +testQuartoCmd( + "render", + [join(symlinkDir, testFile)], + [ + noErrors, + fileExists(join(symlinkDir, "test.html")), + ], + { + ignore: isWindows, + setup: async () => { + if (existsSync(symlinkDir)) { + await Deno.remove(symlinkDir); + } + // Use absolute paths for symlink to ensure correct resolution + Deno.symlinkSync(resolve(testDir), resolve(symlinkDir)); + }, + teardown: async () => { + const htmlViaSymlink = join(symlinkDir, "test.html"); + if (existsSync(htmlViaSymlink)) { + await Deno.remove(htmlViaSymlink); + } + if (existsSync(symlinkDir)) { + await Deno.remove(symlinkDir); + } + const realHtml = join(testDir, "test.html"); + if (existsSync(realHtml)) { + await Deno.remove(realHtml); + } + }, + }, + "Render via symlink with embed-resources (issue #13890)", +); diff --git a/tests/unit/ral/safe-remove-dir.test.ts b/tests/unit/ral/safe-remove-dir.test.ts index 984a7945825..e08300299fe 100644 --- a/tests/unit/ral/safe-remove-dir.test.ts +++ b/tests/unit/ral/safe-remove-dir.test.ts @@ -11,6 +11,7 @@ import { assert, assertThrows } from "testing/asserts"; import { createTempContext } from "../../../src/core/temp.ts"; import { ensureDirSync, safeRemoveDirSync } from "../../../src/deno_ral/fs.ts"; import { join } from "../../../src/deno_ral/path.ts"; +import { isWindows } from "../../../src/deno_ral/platform.ts"; unitTest("safeRemoveDirSync", async () => { @@ -30,3 +31,31 @@ unitTest("safeRemoveDirSync", async () => { temp.cleanup(); }); + +unitTest("safeRemoveDirSync with symlinks", async () => { + const temp = createTempContext(); + + const realDir = temp.createDir(); + const projectRoot = join(realDir, "project-root"); + const subDir = join(projectRoot, "test_files"); + ensureDirSync(projectRoot); + ensureDirSync(subDir); + + const symlinkPath = join(realDir, "project-symlink"); + Deno.symlinkSync(projectRoot, symlinkPath); + + // Test: path via symlink, boundary via real path - should succeed + const pathViaSymlink = join(symlinkPath, "test_files"); + safeRemoveDirSync(pathViaSymlink, projectRoot); + + // Recreate for next test + ensureDirSync(subDir); + + // Test: path via real path, boundary via symlink - should succeed + safeRemoveDirSync(subDir, symlinkPath); + + // Cleanup symlink + Deno.removeSync(symlinkPath); + + temp.cleanup(); +}, { ignore: isWindows });