diff --git a/news/changelog-1.9.md b/news/changelog-1.9.md index 9c327bc5e1d..181ab8e51a5 100644 --- a/news/changelog-1.9.md +++ b/news/changelog-1.9.md @@ -11,6 +11,7 @@ All changes included in 1.9: - ([#13633](https://github.com/quarto-dev/quarto-cli/issues/13633)): Fix detection and auto-installation of babel language packages from newer error format that doesn't explicitly mention `.ldf` filename. - ([#13694](https://github.com/quarto-dev/quarto-cli/issues/13694)): Fix `notebook-view.url` being ignored - external notebook links now properly use specified URLs instead of local preview files. - ([#13732](https://github.com/quarto-dev/quarto-cli/issues/13732)): Fix automatic font package installation for fonts with spaces in their names (e.g., "Noto Emoji", "DejaVu Sans"). Font file search patterns now match both with and without spaces. +- ([#13892](https://github.com/quarto-dev/quarto-cli/issues/13892)): Fix `output-dir: ./` deleting entire project directory. Path comparison now uses `resolve()` to correctly handle all current-directory variations (`.`, `./`, `.\`), and output directory cleanup uses `safeRemoveDirSync` for boundary protection. ## Dependencies diff --git a/src/command/render/project.ts b/src/command/render/project.ts index a80d65f6831..cf16d99b5da 100644 --- a/src/command/render/project.ts +++ b/src/command/render/project.ts @@ -59,7 +59,6 @@ import { resourceFilesFromRenderedFile } from "./resources.ts"; import { inputFilesDir } from "../../core/render.ts"; import { removeIfEmptyDir, - removeIfExists, safeRemoveIfExists, } from "../../core/path.ts"; import { handlerForScript } from "../../core/run/run.ts"; @@ -280,15 +279,17 @@ export async function renderProject( (projectRenderConfig.options.flags?.clean == true) && (projType.cleanOutputDir === true)) ) { - // output dir + // output dir - use safeRemoveDirSync for boundary protection (#13892) const realProjectDir = normalizePath(context.dir); if (existsSync(projOutputDir)) { const realOutputDir = normalizePath(projOutputDir); - if ( - (realOutputDir !== realProjectDir) && - realOutputDir.startsWith(realProjectDir) - ) { - removeIfExists(realOutputDir); + try { + safeRemoveDirSync(realOutputDir, realProjectDir); + } catch (e) { + if (!(e instanceof UnsafeRemovalError)) { + throw e; + } + // Silently skip if output dir equals or is outside project dir } } // remove index diff --git a/src/project/project-context.ts b/src/project/project-context.ts index a863a0d839a..c758ba1faf3 100644 --- a/src/project/project-context.ts +++ b/src/project/project-context.ts @@ -10,6 +10,7 @@ import { isAbsolute, join, relative, + resolve, SEP, } from "../deno_ral/path.ts"; @@ -300,11 +301,17 @@ export async function projectContext( projectConfig.project[kProjectOutputDir] = type.outputDir; } - // if the output-dir is "." that's equivalent to no output dir so make that - // conversion now (this allows code downstream to just check for no output dir - // rather than that as well as ".") - if (projectConfig.project[kProjectOutputDir] === ".") { - delete projectConfig.project[kProjectOutputDir]; + // if the output-dir resolves to the project directory, that's equivalent to + // no output dir so make that conversion now (this allows code downstream to + // just check for no output dir rather than checking for ".", "./", etc.) + // Fixes issue #13892: output-dir: ./ would delete the entire project + const outputDir = projectConfig.project[kProjectOutputDir]; + if (outputDir) { + const resolvedOutputDir = resolve(dir, outputDir); + const resolvedDir = resolve(dir); + if (resolvedOutputDir === resolvedDir) { + delete projectConfig.project[kProjectOutputDir]; + } } // if the output-dir is absolute then make it project dir relative diff --git a/tests/docs/project/output-dir-dot/.gitignore b/tests/docs/project/output-dir-dot/.gitignore new file mode 100644 index 00000000000..ad293093b07 --- /dev/null +++ b/tests/docs/project/output-dir-dot/.gitignore @@ -0,0 +1,2 @@ +/.quarto/ +**/*.quarto_ipynb diff --git a/tests/docs/project/output-dir-dot/_quarto.yml b/tests/docs/project/output-dir-dot/_quarto.yml new file mode 100644 index 00000000000..8e3786777f7 --- /dev/null +++ b/tests/docs/project/output-dir-dot/_quarto.yml @@ -0,0 +1,3 @@ +project: + type: website + output-dir: . diff --git a/tests/docs/project/output-dir-dot/marker.txt b/tests/docs/project/output-dir-dot/marker.txt new file mode 100644 index 00000000000..3c9f25d2452 --- /dev/null +++ b/tests/docs/project/output-dir-dot/marker.txt @@ -0,0 +1 @@ +This file should NOT be deleted when rendering with output-dir: . diff --git a/tests/docs/project/output-dir-dot/test.qmd b/tests/docs/project/output-dir-dot/test.qmd new file mode 100644 index 00000000000..422d9802635 --- /dev/null +++ b/tests/docs/project/output-dir-dot/test.qmd @@ -0,0 +1,8 @@ +--- +title: "Output Dir Dot Test" +format: html +--- + +# Test Document + +This document tests that `output-dir: .` works correctly. diff --git a/tests/docs/project/output-dir-parent-ref/.gitignore b/tests/docs/project/output-dir-parent-ref/.gitignore new file mode 100644 index 00000000000..ad293093b07 --- /dev/null +++ b/tests/docs/project/output-dir-parent-ref/.gitignore @@ -0,0 +1,2 @@ +/.quarto/ +**/*.quarto_ipynb diff --git a/tests/docs/project/output-dir-parent-ref/_quarto.yml b/tests/docs/project/output-dir-parent-ref/_quarto.yml new file mode 100644 index 00000000000..0e21ddcd51c --- /dev/null +++ b/tests/docs/project/output-dir-parent-ref/_quarto.yml @@ -0,0 +1,3 @@ +project: + type: website + output-dir: ../output-dir-parent-ref diff --git a/tests/docs/project/output-dir-parent-ref/marker.txt b/tests/docs/project/output-dir-parent-ref/marker.txt new file mode 100644 index 00000000000..17fe8fbdf9e --- /dev/null +++ b/tests/docs/project/output-dir-parent-ref/marker.txt @@ -0,0 +1,2 @@ +This file should NOT be deleted when rendering with output-dir: ../output-dir-parent-ref +This pattern references the project directory via parent traversal. diff --git a/tests/docs/project/output-dir-parent-ref/test.qmd b/tests/docs/project/output-dir-parent-ref/test.qmd new file mode 100644 index 00000000000..7e72e48f4fe --- /dev/null +++ b/tests/docs/project/output-dir-parent-ref/test.qmd @@ -0,0 +1,8 @@ +--- +title: "Output Dir Parent Ref Test" +format: html +--- + +# Test Document + +This document tests that `output-dir: ../output-dir-parent-ref` (referencing the project directory via parent traversal) does not delete the project directory. diff --git a/tests/docs/project/output-dir-safety/.gitignore b/tests/docs/project/output-dir-safety/.gitignore new file mode 100644 index 00000000000..ad293093b07 --- /dev/null +++ b/tests/docs/project/output-dir-safety/.gitignore @@ -0,0 +1,2 @@ +/.quarto/ +**/*.quarto_ipynb diff --git a/tests/docs/project/output-dir-safety/_quarto.yml b/tests/docs/project/output-dir-safety/_quarto.yml new file mode 100644 index 00000000000..34d46b48543 --- /dev/null +++ b/tests/docs/project/output-dir-safety/_quarto.yml @@ -0,0 +1,3 @@ +project: + type: website + output-dir: ./ diff --git a/tests/docs/project/output-dir-safety/marker.txt b/tests/docs/project/output-dir-safety/marker.txt new file mode 100644 index 00000000000..f3426db9d35 --- /dev/null +++ b/tests/docs/project/output-dir-safety/marker.txt @@ -0,0 +1,2 @@ +This file should NOT be deleted when rendering with output-dir: ./ +If this file is missing after render, the bug is present. diff --git a/tests/docs/project/output-dir-safety/test.qmd b/tests/docs/project/output-dir-safety/test.qmd new file mode 100644 index 00000000000..edd27e1cd16 --- /dev/null +++ b/tests/docs/project/output-dir-safety/test.qmd @@ -0,0 +1,8 @@ +--- +title: "Output Dir Safety Test" +format: html +--- + +# Test Document + +This document tests that `output-dir: ./` does not delete the project directory. diff --git a/tests/docs/project/output-dir-subdir/.gitignore b/tests/docs/project/output-dir-subdir/.gitignore new file mode 100644 index 00000000000..ad293093b07 --- /dev/null +++ b/tests/docs/project/output-dir-subdir/.gitignore @@ -0,0 +1,2 @@ +/.quarto/ +**/*.quarto_ipynb diff --git a/tests/docs/project/output-dir-subdir/_quarto.yml b/tests/docs/project/output-dir-subdir/_quarto.yml new file mode 100644 index 00000000000..4d2666fed92 --- /dev/null +++ b/tests/docs/project/output-dir-subdir/_quarto.yml @@ -0,0 +1,3 @@ +project: + type: default + output-dir: _output diff --git a/tests/docs/project/output-dir-subdir/marker.txt b/tests/docs/project/output-dir-subdir/marker.txt new file mode 100644 index 00000000000..df358a488b4 --- /dev/null +++ b/tests/docs/project/output-dir-subdir/marker.txt @@ -0,0 +1 @@ +This file should NOT be deleted when rendering with output-dir: _output diff --git a/tests/docs/project/output-dir-subdir/test.qmd b/tests/docs/project/output-dir-subdir/test.qmd new file mode 100644 index 00000000000..c0a6c26be1f --- /dev/null +++ b/tests/docs/project/output-dir-subdir/test.qmd @@ -0,0 +1,8 @@ +--- +title: "Output Dir Subdir Test" +format: html +--- + +# Test Document + +This document tests that `output-dir: _output` creates output in subdirectory. diff --git a/tests/smoke/project/project-output-dir-safety.test.ts b/tests/smoke/project/project-output-dir-safety.test.ts new file mode 100644 index 00000000000..0e94e973b32 --- /dev/null +++ b/tests/smoke/project/project-output-dir-safety.test.ts @@ -0,0 +1,86 @@ +/* + * project-output-dir-safety.test.ts + * + * Test for issue #13892: output-dir configurations should not delete project files + * + * Copyright (C) 2020-2025 Posit Software, PBC + */ +import { docs } from "../../utils.ts"; + +import { join } from "../../../src/deno_ral/path.ts"; +import { existsSync } from "../../../src/deno_ral/fs.ts"; +import { testQuartoCmd } from "../../test.ts"; +import { fileExists, noErrors } from "../../verify.ts"; + +// Helper to clean up website output files from a directory +// Used when output-dir points to the project directory itself +async function cleanWebsiteOutput(dir: string) { + const filesToRemove = ["index.html", "test.html", "search.json"]; + const dirsToRemove = ["site_libs", ".quarto"]; + + for (const file of filesToRemove) { + const path = join(dir, file); + if (existsSync(path)) { + await Deno.remove(path); + } + } + for (const subdir of dirsToRemove) { + const path = join(dir, subdir); + if (existsSync(path)) { + await Deno.remove(path, { recursive: true }); + } + } +} + +// Helper to create output-dir safety tests +function testOutputDirSafety( + name: string, + outputDir: string | null, // null means output is in project dir (website type) +) { + const testDir = docs(`project/output-dir-${name}`); + const dir = join(Deno.cwd(), testDir); + const outputPath = outputDir ? join(dir, outputDir) : dir; + + testQuartoCmd( + "render", + [testDir], + [ + noErrors, + fileExists(join(dir, "marker.txt")), // Project file must survive + fileExists(join(outputPath, "test.html")), // Output created correctly + ], + { + teardown: async () => { + // Clean up rendered output + if (outputDir) { + // Subdirectory case - remove the whole output dir + if (existsSync(outputPath)) { + await Deno.remove(outputPath, { recursive: true }); + } + // Clean up .quarto directory + const quartoDir = join(dir, ".quarto"); + if (existsSync(quartoDir)) { + await Deno.remove(quartoDir, { recursive: true }); + } + } else { + // In-place website case - clean up website output files only + await cleanWebsiteOutput(dir); + } + }, + }, + ); +} + +// Test 1: output-dir: ./ (the bug case from #13892) +testOutputDirSafety("safety", null); + +// Test 2: output-dir: . (without trailing slash) +testOutputDirSafety("dot", null); + +// Test 3: output-dir: _output (normal subdirectory case) +testOutputDirSafety("subdir", "_output"); + +// Test 4: output-dir: ../dirname (parent traversal back to project dir) +// This tests the case where output-dir references the project directory +// via parent traversal (e.g., project in "quarto-proj", output-dir: "../quarto-proj") +testOutputDirSafety("parent-ref", null); diff --git a/tests/unit/path.test.ts b/tests/unit/path.test.ts index 4352579b70c..e0af8aaf776 100644 --- a/tests/unit/path.test.ts +++ b/tests/unit/path.test.ts @@ -7,7 +7,8 @@ import { unitTest } from "../test.ts"; import { assert } from "testing/asserts"; -import { join } from "../../src/deno_ral/path.ts"; +import { join, resolve } from "../../src/deno_ral/path.ts"; +import { isWindows } from "../../src/deno_ral/platform.ts"; import { dirAndStem, removeIfEmptyDir, @@ -153,3 +154,45 @@ unitTest("path - resolvePathGlobs", async () => { ); }); }); + +// Test for issue #13892: output-dir: ./ should resolve to same path as . +// This validates the fix approach using resolve() for path comparison +// deno-lint-ignore require-await +unitTest("path - output-dir equivalence with resolve()", async () => { + const testDir = Deno.makeTempDirSync({ prefix: "quarto-outputdir-test" }); + + // All variations of "current directory" should resolve to the same path + // Note: ".\" is Windows-only (backslash separator) + const variations = [".", "./", "././", "./."]; + if (isWindows) { + variations.push(".\\"); + } + for (const variation of variations) { + const resolved = resolve(testDir, variation); + const resolvedDir = resolve(testDir); + assert( + resolved === resolvedDir, + `output-dir "${variation}" should resolve to project dir, got ${resolved} vs ${resolvedDir}`, + ); + } + + // Parent traversal back to project dir should also be equivalent + // e.g., project in "quarto-proj", output-dir: "../quarto-proj" + const dirName = testDir.split(/[/\\]/).pop()!; + const parentRef = `../${dirName}`; + const resolvedParentRef = resolve(testDir, parentRef); + assert( + resolvedParentRef === resolve(testDir), + `output-dir "${parentRef}" should resolve to project dir, got ${resolvedParentRef} vs ${resolve(testDir)}`, + ); + + // Actual subdirectories should NOT be equivalent + const subdir = "output"; + const resolvedSubdir = resolve(testDir, subdir); + assert( + resolvedSubdir !== resolve(testDir), + `output-dir "${subdir}" should NOT resolve to project dir`, + ); + + Deno.removeSync(testDir, { recursive: true }); +});