From dcc081db0df56f93f015258548d92d0d2aa2d77d Mon Sep 17 00:00:00 2001 From: Anders Kaseorg Date: Thu, 4 May 2023 02:01:20 -0700 Subject: [PATCH] =?UTF-8?q?Don=E2=80=99t=20expand=20globs=20via=20symbolic?= =?UTF-8?q?=20links=20(#14627)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: fisker --- changelog_unreleased/cli/14627.md | 6 + docs/cli.md | 2 + src/cli/expand-patterns.js | 11 +- src/cli/utils.js | 17 ++ tests/integration/__tests__/patterns-dirs.js | 165 ++++++++++++++++++ .../cli/patterns-symlinks/.gitignore | 2 + 6 files changed, 200 insertions(+), 3 deletions(-) create mode 100644 changelog_unreleased/cli/14627.md create mode 100644 tests/integration/cli/patterns-symlinks/.gitignore diff --git a/changelog_unreleased/cli/14627.md b/changelog_unreleased/cli/14627.md new file mode 100644 index 000000000..87dc8a740 --- /dev/null +++ b/changelog_unreleased/cli/14627.md @@ -0,0 +1,6 @@ +#### Don’t expand globs via symbolic links (#14627 by @andersk) + +Prettier no longer follows symbolic links while expanding command line +arguments. This avoids problems in many scenarios such as symlinks +outside the source tree, symlinks to ignored files, and cycles of +symlinks. diff --git a/docs/cli.md b/docs/cli.md index 52615324c..bfe8fd803 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -45,6 +45,8 @@ Given a list of paths/patterns, the Prettier CLI first treats every entry in it Prettier CLI will ignore files located in `node_modules` directory. To opt out from this behavior, use `--with-node-modules` flag. +Prettier CLI will not follow symbolic links when expanding arguments. + To escape special characters in globs, one of the two escaping syntaxes can be used: `prettier "\[my-dir]/*.js"` or `prettier "[[]my-dir]/*.js"`. Both match all JS files in a directory named `[my-dir]`, however the latter syntax is preferable as the former doesn’t work on Windows, where backslashes are treated as path separators. ## `--check` diff --git a/src/cli/expand-patterns.js b/src/cli/expand-patterns.js index 823640037..8d162da99 100644 --- a/src/cli/expand-patterns.js +++ b/src/cli/expand-patterns.js @@ -1,5 +1,5 @@ import path from "node:path"; -import { statSafe, normalizeToPosix } from "./utils.js"; +import { lstatSafe, normalizeToPosix } from "./utils.js"; import { fastGlob } from "./prettier-internal.js"; /** @typedef {import('./context').Context} Context */ @@ -49,6 +49,7 @@ async function* expandPatternsInternal(context) { const globOptions = { dot: true, ignore: silentlyIgnoredDirs.map((dir) => "**/" + dir), + followSymbolicLinks: false, }; let supportedFilesGlob; @@ -64,9 +65,13 @@ async function* expandPatternsInternal(context) { continue; } - const stat = await statSafe(absolutePath); + const stat = await lstatSafe(absolutePath); if (stat) { - if (stat.isFile()) { + if (stat.isSymbolicLink()) { + yield { + error: `Explicitly specified pattern "${pattern}" is a symbolic link.`, + }; + } else if (stat.isFile()) { entries.push({ type: "file", glob: escapePathForGlob(fixWindowsSlashes(pattern)), diff --git a/src/cli/utils.js b/src/cli/utils.js index 199b5f316..778bf63aa 100644 --- a/src/cli/utils.js +++ b/src/cli/utils.js @@ -66,6 +66,22 @@ async function statSafe(filePath) { } } +/** + * Get stats of a given path without following symbolic links. + * @param {string} filePath The path to target file. + * @returns {Promise} The stats. + */ +async function lstatSafe(filePath) { + try { + return await fs.lstat(filePath); + } catch (error) { + /* c8 ignore next 3 */ + if (error.code !== "ENOENT") { + throw error; + } + } +} + /** * @param {string} value * @returns {boolean} @@ -96,6 +112,7 @@ export { pick, createHash, statSafe, + lstatSafe, isJson, normalizeToPosix, }; diff --git a/tests/integration/__tests__/patterns-dirs.js b/tests/integration/__tests__/patterns-dirs.js index 5f74bc591..2687f0471 100644 --- a/tests/integration/__tests__/patterns-dirs.js +++ b/tests/integration/__tests__/patterns-dirs.js @@ -152,6 +152,171 @@ if (path.sep === "/") { }); } +function isSymlinkSupported() { + if (process.platform !== "win32") { + return true; + } + + const target = path.join( + __dirname, + "../cli/patterns-symlinks/test-symlink-feature-detect" + ); + fs.rmSync(target, { force: true, recursive: true }); + fs.mkdirSync(target); + const symlink = path.join(target, "symlink"); + try { + fs.symlinkSync(target, symlink); + } catch { + return false; + } + return fs.lstatSync(symlink).isSymbolicLink(); +} + +(isSymlinkSupported() ? describe : describe.skip)("Ignore symlinks", () => { + const base = path.join(__dirname, "../cli/patterns-symlinks"); + const directoryA = path.join(base, "test-a"); + const directoryB = path.join(base, "test-b"); + const clean = () => { + fs.rmSync(directoryA, { force: true, recursive: true }); + fs.rmSync(directoryB, { force: true, recursive: true }); + }; + beforeAll(() => { + clean(); + fs.mkdirSync(directoryA); + fs.mkdirSync(directoryB); + fs.writeFileSync(path.join(directoryA, "a.js"), "x"); + fs.writeFileSync(path.join(directoryB, "b.js"), "x"); + fs.symlinkSync(directoryA, path.join(directoryA, "symlink-to-directory-a")); + fs.symlinkSync(directoryB, path.join(directoryA, "symlink-to-directory-b")); + fs.symlinkSync( + path.join(directoryA, "a.js"), + path.join(directoryA, "symlink-to-file-a") + ); + fs.symlinkSync( + path.join(directoryB, "b.js"), + path.join(directoryA, "symlink-to-file-b") + ); + }); + afterAll(clean); + + test("file struct", async () => { + const getFileStruct = async (directory) => + (await fs.promises.readdir(directory, { withFileTypes: true })) + .map((dirent) => ({ + name: dirent.name, + isSymbolicLink: dirent.isSymbolicLink(), + })) + .sort((a, b) => a.name.localeCompare(b.name)); + + expect(await getFileStruct(directoryA)).toMatchInlineSnapshot(` + [ + { + "isSymbolicLink": false, + "name": "a.js", + }, + { + "isSymbolicLink": true, + "name": "symlink-to-directory-a", + }, + { + "isSymbolicLink": true, + "name": "symlink-to-directory-b", + }, + { + "isSymbolicLink": true, + "name": "symlink-to-file-a", + }, + { + "isSymbolicLink": true, + "name": "symlink-to-file-b", + }, + ] + `); + expect(await getFileStruct(directoryB)).toMatchInlineSnapshot(` + [ + { + "isSymbolicLink": false, + "name": "b.js", + }, + ] + `); + }); + + testPatterns("", ["test-a/*"], { stdout: "test-a/a.js" }, base); + testPatterns( + "", + ["test-a/symlink-to-directory-a"], + { + status: 2, + stdout: "", + stderr: + '[error] Explicitly specified pattern "test-a/symlink-to-directory-a" is a symbolic link.', + }, + base + ); + testPatterns( + "", + ["test-a/symlink-to-directory-b"], + { + status: 2, + stdout: "", + stderr: + '[error] Explicitly specified pattern "test-a/symlink-to-directory-b" is a symbolic link.', + }, + base + ); + testPatterns( + "", + ["test-a/symlink-to-file-a"], + { + status: 2, + stdout: "", + stderr: + '[error] Explicitly specified pattern "test-a/symlink-to-file-a" is a symbolic link.', + }, + base + ); + testPatterns( + "", + ["test-a/symlink-to-file-b"], + { + status: 2, + stdout: "", + stderr: + '[error] Explicitly specified pattern "test-a/symlink-to-file-b" is a symbolic link.', + }, + base + ); + testPatterns( + "", + ["test-a/symlink-*"], + { + status: 2, + stdout: "", + stderr: + '[error] No files matching the pattern were found: "test-a/symlink-*".', + }, + base + ); + testPatterns( + "", + ["test-a/*", "test-a/symlink-to-file-b"], + { + status: 2, + stdout: "test-a/a.js", + stderr: + '[error] Explicitly specified pattern "test-a/symlink-to-file-b" is a symbolic link.', + }, + base + ); + testPatterns( + "", + ["test-a/symlink-to-directory-b/*"], + { stdout: "test-a/symlink-to-directory-b/b.js" }, + base + ); +}); + function testPatterns( namePrefix, cliArgs, diff --git a/tests/integration/cli/patterns-symlinks/.gitignore b/tests/integration/cli/patterns-symlinks/.gitignore new file mode 100644 index 000000000..e53c48ff5 --- /dev/null +++ b/tests/integration/cli/patterns-symlinks/.gitignore @@ -0,0 +1,2 @@ +# dynamically created by tests +test-*