Skip to content

Commit a7d8b6a

Browse files
authored
fix: resolve symlinks in safeRemoveDirSync to fix cleanup with symlinked paths (#13894)
Fixes rendering with `embed-resources: true` when the input path goes through a symlinked directory.
1 parent 7c6cd4e commit a7d8b6a

File tree

7 files changed

+132
-1
lines changed

7 files changed

+132
-1
lines changed

news/changelog-1.9.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -134,4 +134,5 @@ All changes included in 1.9:
134134
- ([#13656](https://github.com/quarto-dev/quarto-cli/issues/13656)): Fix R code cells with empty `lang: ""` option producing invalid markdown class attributes.
135135
- ([#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)
136136
- ([#13856](https://github.com/quarto-dev/quarto-cli/issues/13856)): Add code annotation support for Typst and Observable.js code blocks. (author: @mcanouil)
137+
- ([#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.
137138
- ([#13907](https://github.com/quarto-dev/quarto-cli/issues/13907)): Ignore AI assistant configuration files (`CLAUDE.md`, `AGENTS.md`) when scanning for project input files and in extension templates, similar to how `README.md` is handled.

src/deno_ral/fs.ts

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { fromFileUrl } from "./path.ts";
88
import { resolve, SEP as SEPARATOR } from "./path.ts";
99
import { copySync } from "fs/copy";
1010
import { existsSync } from "fs/exists";
11+
import { originalRealPathSync } from "./original-real-path.ts";
1112

1213
export { ensureDir, ensureDirSync } from "fs/ensure-dir";
1314
export { existsSync } from "fs/exists";
@@ -132,7 +133,26 @@ export function safeRemoveDirSync(
132133
path: string,
133134
boundary: string,
134135
) {
135-
if (path === boundary || !isSubdir(boundary, path)) {
136+
// Resolve symlinks to ensure consistent path comparison.
137+
// This is needed because external tools (like knitr) may resolve symlinks
138+
// while project.dir preserves them.
139+
//
140+
// We use the original Deno.realPathSync (saved before monkey-patching)
141+
// because the monkey-patch replaces it with normalizePath which doesn't
142+
// resolve symlinks.
143+
//
144+
// Note: The UNC path bug that motivated the monkey-patch was fixed in
145+
// Deno v1.16 (see denoland/deno#12243), so this is safe on all platforms.
146+
let resolvedPath = path;
147+
let resolvedBoundary = boundary;
148+
try {
149+
resolvedPath = originalRealPathSync(path);
150+
resolvedBoundary = originalRealPathSync(boundary);
151+
} catch {
152+
// If resolution fails (e.g., path doesn't exist), use original paths
153+
}
154+
155+
if (resolvedPath === resolvedBoundary || !isSubdir(resolvedBoundary, resolvedPath)) {
136156
throw new UnsafeRemovalError(
137157
`Refusing to remove directory ${path} that isn't a subdirectory of ${boundary}`,
138158
);

src/deno_ral/original-real-path.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
/*
2+
* original-real-path.ts
3+
*
4+
* Copyright (C) 2020-2025 Posit Software, PBC
5+
*
6+
* Saves original Deno.realPathSync before monkey-patching.
7+
* This module MUST be imported in quarto.ts BEFORE monkey-patch.ts.
8+
*
9+
* DO NOT add any imports to this file - it must remain dependency-free
10+
* to avoid circular import issues.
11+
*/
12+
export const originalRealPathSync: typeof Deno.realPathSync = Deno.realPathSync;

src/quarto.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44
* Copyright (C) 2020-2022 Posit Software, PBC
55
*/
66

7+
// Must be FIRST to save original Deno.realPathSync before monkey-patching
8+
import "./deno_ral/original-real-path.ts";
79
import "./core/deno/monkey-patch.ts";
810

911
import {
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
format:
3+
html:
4+
embed-resources: true
5+
---
6+
7+
# Test Document
8+
9+
This tests rendering via symlinked directory with embed-resources.
10+
11+
```{r}
12+
plot(1:10)
13+
```
Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,54 @@
1+
/*
2+
* render-symlink-embed-resources.test.ts
3+
*
4+
* Regression test for https://github.com/quarto-dev/quarto-cli/issues/13890
5+
*
6+
* When rendering a qmd via a symlinked path with embed-resources: true,
7+
* the cleanup should not fail due to path mismatch between symlink and real path.
8+
*
9+
* Copyright (C) 2020-2025 Posit Software, PBC
10+
*/
11+
12+
import { testQuartoCmd } from "../../test.ts";
13+
import { noErrors, fileExists } from "../../verify.ts";
14+
import { docs } from "../../utils.ts";
15+
import { join, dirname, resolve } from "../../../src/deno_ral/path.ts";
16+
import { existsSync } from "../../../src/deno_ral/fs.ts";
17+
import { isWindows } from "../../../src/deno_ral/platform.ts";
18+
19+
const testDir = docs("render/symlink-embed-resources");
20+
const testFile = "test.qmd";
21+
const symlinkDir = join(dirname(testDir), "symlink-embed-resources-link");
22+
23+
testQuartoCmd(
24+
"render",
25+
[join(symlinkDir, testFile)],
26+
[
27+
noErrors,
28+
fileExists(join(symlinkDir, "test.html")),
29+
],
30+
{
31+
ignore: isWindows,
32+
setup: async () => {
33+
if (existsSync(symlinkDir)) {
34+
await Deno.remove(symlinkDir);
35+
}
36+
// Use absolute paths for symlink to ensure correct resolution
37+
Deno.symlinkSync(resolve(testDir), resolve(symlinkDir));
38+
},
39+
teardown: async () => {
40+
const htmlViaSymlink = join(symlinkDir, "test.html");
41+
if (existsSync(htmlViaSymlink)) {
42+
await Deno.remove(htmlViaSymlink);
43+
}
44+
if (existsSync(symlinkDir)) {
45+
await Deno.remove(symlinkDir);
46+
}
47+
const realHtml = join(testDir, "test.html");
48+
if (existsSync(realHtml)) {
49+
await Deno.remove(realHtml);
50+
}
51+
},
52+
},
53+
"Render via symlink with embed-resources (issue #13890)",
54+
);

tests/unit/ral/safe-remove-dir.test.ts

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import { assert, assertThrows } from "testing/asserts";
1111
import { createTempContext } from "../../../src/core/temp.ts";
1212
import { ensureDirSync, safeRemoveDirSync } from "../../../src/deno_ral/fs.ts";
1313
import { join } from "../../../src/deno_ral/path.ts";
14+
import { isWindows } from "../../../src/deno_ral/platform.ts";
1415

1516
unitTest("safeRemoveDirSync", async () => {
1617

@@ -30,3 +31,31 @@ unitTest("safeRemoveDirSync", async () => {
3031

3132
temp.cleanup();
3233
});
34+
35+
unitTest("safeRemoveDirSync with symlinks", async () => {
36+
const temp = createTempContext();
37+
38+
const realDir = temp.createDir();
39+
const projectRoot = join(realDir, "project-root");
40+
const subDir = join(projectRoot, "test_files");
41+
ensureDirSync(projectRoot);
42+
ensureDirSync(subDir);
43+
44+
const symlinkPath = join(realDir, "project-symlink");
45+
Deno.symlinkSync(projectRoot, symlinkPath);
46+
47+
// Test: path via symlink, boundary via real path - should succeed
48+
const pathViaSymlink = join(symlinkPath, "test_files");
49+
safeRemoveDirSync(pathViaSymlink, projectRoot);
50+
51+
// Recreate for next test
52+
ensureDirSync(subDir);
53+
54+
// Test: path via real path, boundary via symlink - should succeed
55+
safeRemoveDirSync(subDir, symlinkPath);
56+
57+
// Cleanup symlink
58+
Deno.removeSync(symlinkPath);
59+
60+
temp.cleanup();
61+
}, { ignore: isWindows });

0 commit comments

Comments
 (0)